| Message ID | 9df1970109b84fc170530a5ffa5c07e4480e445f.1503953204.git.shli@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/28/2017 02:49 PM, Shaohua Li wrote: > Commit 2984c86(nullb: factor disk parameters) has a typo. The > nullb_device allocation/free is done outside of null_add_dev. The commit > accidentally frees the nullb_device in error code path. Is the code in nullb_device_power_store() correct after this? Also, looks like the second else if in there should be using 'dev' instead of to_nullb_device(item)->power.
On Mon, Aug 28, 2017 at 02:55:59PM -0600, Jens Axboe wrote: > On 08/28/2017 02:49 PM, Shaohua Li wrote: > > Commit 2984c86(nullb: factor disk parameters) has a typo. The > > nullb_device allocation/free is done outside of null_add_dev. The commit > > accidentally frees the nullb_device in error code path. > > Is the code in nullb_device_power_store() correct after this? Yes, for runtime disk configuration, we allocate the device in nullb_group_make_item and free the device in nullb_device_release. the nullb_device_power_store only does null_add_dev/null_del_dev, not allocate/free dev. > Also, looks like the second else if in there should be using > 'dev' instead of to_nullb_device(item)->power. Ah, yes, it should be 'dev->power'. dev == to_nullb_device(item). Do you want me to send a separate patch to do the cleanup? Thanks, Shaohua
On 08/28/2017 03:04 PM, Shaohua Li wrote: > On Mon, Aug 28, 2017 at 02:55:59PM -0600, Jens Axboe wrote: >> On 08/28/2017 02:49 PM, Shaohua Li wrote: >>> Commit 2984c86(nullb: factor disk parameters) has a typo. The >>> nullb_device allocation/free is done outside of null_add_dev. The commit >>> accidentally frees the nullb_device in error code path. >> >> Is the code in nullb_device_power_store() correct after this? > > Yes, for runtime disk configuration, we allocate the device in > nullb_group_make_item and free the device in nullb_device_release. the > nullb_device_power_store only does null_add_dev/null_del_dev, not allocate/free > dev. OK good, just wanted to confirm. >> Also, looks like the second else if in there should be using >> 'dev' instead of to_nullb_device(item)->power. > > Ah, yes, it should be 'dev->power'. dev == to_nullb_device(item). Do you want > me to send a separate patch to do the cleanup? No worries, I'll just do it myself.
diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c index 4d328e3..18f2cb3 100644 --- a/drivers/block/null_blk.c +++ b/drivers/block/null_blk.c @@ -1909,7 +1909,6 @@ static int null_add_dev(struct nullb_device *dev) out_free_nullb: kfree(nullb); out: - null_free_dev(dev); return rv; }
Commit 2984c86(nullb: factor disk parameters) has a typo. The nullb_device allocation/free is done outside of null_add_dev. The commit accidentally frees the nullb_device in error code path. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Shaohua Li <shli@fb.com> --- drivers/block/null_blk.c | 1 - 1 file changed, 1 deletion(-)