| Message ID | 20210521055116.1053587-14-hch@lst.de |
|---|---|
| State | New |
| Headers | show |
| Series | [01/26] block: refactor device number setup in __device_add_disk | expand |
On 5/21/21 7:51 AM, Christoph Hellwig wrote: > Convert the dm driver to use the blk_alloc_disk and blk_cleanup_disk > helpers to simplify gendisk and request_queue allocation. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/md/dm.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/md/dm.c b/drivers/md/dm.c > index ca2aedd8ee7d..3c7c2d257018 100644 > --- a/drivers/md/dm.c > +++ b/drivers/md/dm.c > @@ -1801,13 +1801,13 @@ static void cleanup_mapped_device(struct mapped_device *md) > md->disk->private_data = NULL; > spin_unlock(&_minor_lock); > del_gendisk(md->disk); > - put_disk(md->disk); > } > > - if (md->queue) { > + if (md->queue) > dm_queue_destroy_keyslot_manager(md->queue); > - blk_cleanup_queue(md->queue); > - } > + > + if (md->disk) > + blk_cleanup_disk(md->disk); > > cleanup_srcu_struct(&md->io_barrier); > Can't these conditionals be merged into a single 'if (md->disk)'? Eg like: if (md->disk) { spin_lock(&_minor_lock); md->disk->private_data = NULL; spin_unlock(&_minor_lock); del_gendisk(md->disk); dm_queue_destroy_keyslot_manager(md->queue); blk_cleanup_disk(md->queue); } We're now always allocating 'md->disk' and 'md->queue' together, so how can we end up in a situation where one is set without the other? > @@ -1869,13 +1869,10 @@ static struct mapped_device *alloc_dev(int minor) > * established. If request-based table is loaded: blk-mq will > * override accordingly. > */ > - md->queue = blk_alloc_queue(numa_node_id); > - if (!md->queue) > - goto bad; > - > - md->disk = alloc_disk_node(1, md->numa_node_id); > + md->disk = blk_alloc_disk(md->numa_node_id); > if (!md->disk) > goto bad; > + md->queue = md->disk->queue; > > init_waitqueue_head(&md->wait); > INIT_WORK(&md->work, dm_wq_work); > @@ -1888,6 +1885,7 @@ static struct mapped_device *alloc_dev(int minor) > > md->disk->major = _major; > md->disk->first_minor = minor; > + md->disk->minors = 1; > md->disk->fops = &dm_blk_dops; > md->disk->queue = md->queue; > md->disk->private_data = md; > Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
On Sun, May 23, 2021 at 10:10:34AM +0200, Hannes Reinecke wrote: > Can't these conditionals be merged into a single 'if (md->disk)'? > Eg like: > > if (md->disk) { > spin_lock(&_minor_lock); > md->disk->private_data = NULL; > spin_unlock(&_minor_lock); > del_gendisk(md->disk); > dm_queue_destroy_keyslot_manager(md->queue); > blk_cleanup_disk(md->queue); > } > > We're now always allocating 'md->disk' and 'md->queue' together, > so how can we end up in a situation where one is set without the other? I guess we could do that, not sure it is worth the churn, though.
diff --git a/drivers/md/dm.c b/drivers/md/dm.c index ca2aedd8ee7d..3c7c2d257018 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1801,13 +1801,13 @@ static void cleanup_mapped_device(struct mapped_device *md) md->disk->private_data = NULL; spin_unlock(&_minor_lock); del_gendisk(md->disk); - put_disk(md->disk); } - if (md->queue) { + if (md->queue) dm_queue_destroy_keyslot_manager(md->queue); - blk_cleanup_queue(md->queue); - } + + if (md->disk) + blk_cleanup_disk(md->disk); cleanup_srcu_struct(&md->io_barrier); @@ -1869,13 +1869,10 @@ static struct mapped_device *alloc_dev(int minor) * established. If request-based table is loaded: blk-mq will * override accordingly. */ - md->queue = blk_alloc_queue(numa_node_id); - if (!md->queue) - goto bad; - - md->disk = alloc_disk_node(1, md->numa_node_id); + md->disk = blk_alloc_disk(md->numa_node_id); if (!md->disk) goto bad; + md->queue = md->disk->queue; init_waitqueue_head(&md->wait); INIT_WORK(&md->work, dm_wq_work); @@ -1888,6 +1885,7 @@ static struct mapped_device *alloc_dev(int minor) md->disk->major = _major; md->disk->first_minor = minor; + md->disk->minors = 1; md->disk->fops = &dm_blk_dops; md->disk->queue = md->queue; md->disk->private_data = md;
Convert the dm driver to use the blk_alloc_disk and blk_cleanup_disk helpers to simplify gendisk and request_queue allocation. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/md/dm.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)