On 5/21/21 1:51 PM, Christoph Hellwig wrote:
> Convert the bcache driver to use the blk_alloc_disk and blk_cleanup_disk
> helpers to simplify gendisk and request_queue allocation.
> 
> Signed-off-by: Christoph Hellwig <h...@lst.de>
> ---
>  drivers/md/bcache/super.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index bea8c4429ae8..185246a0d855 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -890,13 +890,9 @@ static void bcache_device_free(struct bcache_device *d)
>               if (disk_added)
>                       del_gendisk(disk);
>  
> -             if (disk->queue)
> -                     blk_cleanup_queue(disk->queue);
> -
> +             blk_cleanup_disk(disk);
>               ida_simple_remove(&bcache_device_idx,
>                                 first_minor_to_idx(disk->first_minor));
> -             if (disk_added)
> -                     put_disk(disk);

The  above 2 lines are added on purpose to prevent an refcount
underflow. It is from commit 86da9f736740 ("bcache: fix refcount
underflow in bcache_device_free()").

Maybe add a parameter to blk_cleanup_disk() or checking (disk->flags &
GENHD_FL_UP) inside blk_cleanup_disk() ?

Coly Li


>       }
>  
>       bioset_exit(&d->bio_split);
> @@ -946,7 +942,7 @@ static int bcache_device_init(struct bcache_device *d, 
> unsigned int block_size,
>                       BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
>               goto err;
>  
> -     d->disk = alloc_disk(BCACHE_MINORS);
> +     d->disk = blk_alloc_disk(NUMA_NO_NODE);
>       if (!d->disk)
>               goto err;
>  
> @@ -955,14 +951,11 @@ static int bcache_device_init(struct bcache_device *d, 
> unsigned int block_size,
>  
>       d->disk->major          = bcache_major;
>       d->disk->first_minor    = idx_to_first_minor(idx);
> +     d->disk->minors         = BCACHE_MINORS;
>       d->disk->fops           = ops;
>       d->disk->private_data   = d;
>  
> -     q = blk_alloc_queue(NUMA_NO_NODE);
> -     if (!q)
> -             return -ENOMEM;
> -
> -     d->disk->queue                  = q;
> +     q = d->disk->queue;
>       q->limits.max_hw_sectors        = UINT_MAX;
>       q->limits.max_sectors           = UINT_MAX;
>       q->limits.max_segment_size      = UINT_MAX;
> 

The rested looks fine to me.

Thanks.

Coly Li

Reply via email to