[14/26] md: convert to blk_alloc_disk/blk_cleanup_disk

Message ID 20210521055116.1053587-15-hch@lst.de
State New
Headers show
Series
  • [01/26] block: refactor device number setup in __device_add_disk
Related show

Commit Message

Christoph Hellwig May 21, 2021, 5:51 a.m.
Convert the md 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/md.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

Comments

Hannes Reinecke May 23, 2021, 8:12 a.m. | #1
On 5/21/21 7:51 AM, Christoph Hellwig wrote:
> Convert the md 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/md.c | 25 +++++++++----------------

>   1 file changed, 9 insertions(+), 16 deletions(-)

> 

> diff --git a/drivers/md/md.c b/drivers/md/md.c

> index 49f897fbb89b..d806be8cc210 100644

> --- a/drivers/md/md.c

> +++ b/drivers/md/md.c

> @@ -5598,12 +5598,10 @@ static void md_free(struct kobject *ko)

>   	if (mddev->sysfs_level)

>   		sysfs_put(mddev->sysfs_level);

>   

> -	if (mddev->gendisk)

> +	if (mddev->gendisk) {

>   		del_gendisk(mddev->gendisk);

> -	if (mddev->queue)

> -		blk_cleanup_queue(mddev->queue);

> -	if (mddev->gendisk)

> -		put_disk(mddev->gendisk);

> +		blk_cleanup_disk(mddev->gendisk);

> +	}

>   	percpu_ref_exit(&mddev->writes_pending);

>   

>   	bioset_exit(&mddev->bio_set);

> @@ -5711,20 +5709,13 @@ static int md_alloc(dev_t dev, char *name)

>   		goto abort;

>   

>   	error = -ENOMEM;

> -	mddev->queue = blk_alloc_queue(NUMA_NO_NODE);

> -	if (!mddev->queue)

> +	disk = blk_alloc_disk(NUMA_NO_NODE);

> +	if (!disk)

>   		goto abort;

>   

> -	blk_set_stacking_limits(&mddev->queue->limits);

> -

> -	disk = alloc_disk(1 << shift);

> -	if (!disk) {

> -		blk_cleanup_queue(mddev->queue);

> -		mddev->queue = NULL;

> -		goto abort;

> -	}

>   	disk->major = MAJOR(mddev->unit);

>   	disk->first_minor = unit << shift;

> +	disk->minors = 1 << shift;

>   	if (name)

>   		strcpy(disk->disk_name, name);

>   	else if (partitioned)

> @@ -5733,7 +5724,9 @@ static int md_alloc(dev_t dev, char *name)

>   		sprintf(disk->disk_name, "md%d", unit);

>   	disk->fops = &md_fops;

>   	disk->private_data = mddev;

> -	disk->queue = mddev->queue;

> +

> +	mddev->queue = disk->queue;

> +	blk_set_stacking_limits(&mddev->queue->limits);

>   	blk_queue_write_cache(mddev->queue, true, true);

>   	/* Allow extended partitions.  This makes the

>   	 * 'mdp' device redundant, but we can't really

> 

Wouldn't it make sense to introduce a helper 'blk_queue_from_disk()' or 
somesuch to avoid having to keep an explicit 'queue' pointer?


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
Christoph Hellwig May 24, 2021, 7:26 a.m. | #2
On Sun, May 23, 2021 at 10:12:49AM +0200, Hannes Reinecke wrote:
>> +	blk_set_stacking_limits(&mddev->queue->limits);

>>   	blk_queue_write_cache(mddev->queue, true, true);

>>   	/* Allow extended partitions.  This makes the

>>   	 * 'mdp' device redundant, but we can't really

>>

> Wouldn't it make sense to introduce a helper 'blk_queue_from_disk()' or 

> somesuch to avoid having to keep an explicit 'queue' pointer?


My rought plan is that a few series from now bio based drivers will
never directly deal with the request_queue at all.
Hannes Reinecke May 24, 2021, 8:27 a.m. | #3
On 5/24/21 9:26 AM, Christoph Hellwig wrote:
> On Sun, May 23, 2021 at 10:12:49AM +0200, Hannes Reinecke wrote:

>>> +	blk_set_stacking_limits(&mddev->queue->limits);

>>>    	blk_queue_write_cache(mddev->queue, true, true);

>>>    	/* Allow extended partitions.  This makes the

>>>    	 * 'mdp' device redundant, but we can't really

>>>

>> Wouldn't it make sense to introduce a helper 'blk_queue_from_disk()' or

>> somesuch to avoid having to keep an explicit 'queue' pointer?

> 

> My rought plan is that a few series from now bio based drivers will

> never directly deal with the request_queue at all.

> 

Go for it.

Reviewed-by: Hannes Reinecke <hare@suse.de>


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

Patch

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 49f897fbb89b..d806be8cc210 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5598,12 +5598,10 @@  static void md_free(struct kobject *ko)
 	if (mddev->sysfs_level)
 		sysfs_put(mddev->sysfs_level);
 
-	if (mddev->gendisk)
+	if (mddev->gendisk) {
 		del_gendisk(mddev->gendisk);
-	if (mddev->queue)
-		blk_cleanup_queue(mddev->queue);
-	if (mddev->gendisk)
-		put_disk(mddev->gendisk);
+		blk_cleanup_disk(mddev->gendisk);
+	}
 	percpu_ref_exit(&mddev->writes_pending);
 
 	bioset_exit(&mddev->bio_set);
@@ -5711,20 +5709,13 @@  static int md_alloc(dev_t dev, char *name)
 		goto abort;
 
 	error = -ENOMEM;
-	mddev->queue = blk_alloc_queue(NUMA_NO_NODE);
-	if (!mddev->queue)
+	disk = blk_alloc_disk(NUMA_NO_NODE);
+	if (!disk)
 		goto abort;
 
-	blk_set_stacking_limits(&mddev->queue->limits);
-
-	disk = alloc_disk(1 << shift);
-	if (!disk) {
-		blk_cleanup_queue(mddev->queue);
-		mddev->queue = NULL;
-		goto abort;
-	}
 	disk->major = MAJOR(mddev->unit);
 	disk->first_minor = unit << shift;
+	disk->minors = 1 << shift;
 	if (name)
 		strcpy(disk->disk_name, name);
 	else if (partitioned)
@@ -5733,7 +5724,9 @@  static int md_alloc(dev_t dev, char *name)
 		sprintf(disk->disk_name, "md%d", unit);
 	disk->fops = &md_fops;
 	disk->private_data = mddev;
-	disk->queue = mddev->queue;
+
+	mddev->queue = disk->queue;
+	blk_set_stacking_limits(&mddev->queue->limits);
 	blk_queue_write_cache(mddev->queue, true, true);
 	/* Allow extended partitions.  This makes the
 	 * 'mdp' device redundant, but we can't really