On 2020/4/8 6:02 下午, Wu Bo wrote:
> From: Wu Bo <wubo40@xxxxxxxxxx>
>
> Cleanup resources when device init failed on bcache_device_init function
>
> Signed-off-by: Wu Bo <wubo40@xxxxxxxxxx>
Hi Wu Bo,
Do you test this patch ? I am not sure whether it may work properly,
because all the resources will be released in bcache_device_stop() from
register_bdev(). Your patch makes them being released twice.
If bcache_device_init() fails, cached_dev_init() will fail, then the
caller register_bdev() will fail and jump to err label to call
bcache_device_stop().
If this is a cached device, in bcache_device_stop(),
closure_queue(&d->cl) will call the callback routine cached_dev_flush().
Then in cached_dev_flush() the continue_at() callback cached_dev_free()
will be called. In cached_dev_free() you will see these resources are
freed properly.
Coly Li
> ---
> drivers/md/bcache/super.c | 27 +++++++++++++++++++--------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index d98354f..b4e3d0e 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -823,6 +823,7 @@ static int bcache_device_init(struct bcache_device
> *d, unsigned int block_size,
> SIZE_MAX / sizeof(atomic_t));
> size_t n;
> int idx;
> + int rtn = -ENOMEM;
>
> if (!d->stripe_size)
> d->stripe_size = 1 << 31;
> @@ -843,20 +844,22 @@ static int bcache_device_init(struct bcache_device
> *d, unsigned int block_size,
> n = BITS_TO_LONGS(d->nr_stripes) * sizeof(unsigned long);
> d->full_dirty_stripes = kvzalloc(n, GFP_KERNEL);
> if (!d->full_dirty_stripes)
> - return -ENOMEM;
> + goto out_free_sectors_dirty;
>
> idx = ida_simple_get(&bcache_device_idx, 0,
> BCACHE_DEVICE_IDX_MAX, GFP_KERNEL);
> - if (idx < 0)
> - return idx;
> + if (idx < 0) {
> + rtn = idx;
> + goto out_free_dirty_stripes;
> + }
>
> if (bioset_init(&d->bio_split, 4, offsetof(struct bbio, bio),
> BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
> - goto err;
> + goto out_free_idx;
>
> d->disk = alloc_disk(BCACHE_MINORS);
> if (!d->disk)
> - goto err;
> + goto out_bioset_exit;
>
> set_capacity(d->disk, sectors);
> snprintf(d->disk->disk_name, DISK_NAME_LEN, "bcache%i", idx);
> @@ -868,7 +871,7 @@ static int bcache_device_init(struct bcache_device
> *d, unsigned int block_size,
>
> q = blk_alloc_queue(make_request_fn, NUMA_NO_NODE);
> if (!q)
> - return -ENOMEM;
> + goto out_free_disk;
>
> d->disk->queue = q;
> q->queuedata = d;
> @@ -890,9 +893,17 @@ static int bcache_device_init(struct bcache_device
> *d, unsigned int block_size,
>
> return 0;
>
> -err:
> +out_free_disk:
> + put_disk(d->disk);
> +out_bioset_exit:
> + bioset_exit(&d->bio_split);
> +out_free_idx:
> ida_simple_remove(&bcache_device_idx, idx);
> - return -ENOMEM;
> +out_free_dirty_stripes:
> + kvfree(d->full_dirty_stripes);
> +out_free_sectors_dirty:
> + kvfree(d->stripe_sectors_dirty);
> + return rtn;
>
> }
