Re: [PATCH 7/8] block: don't handle bio based drivers in blk_revalidate_disk_zones

["Javier González" <javier@javigon.com>]

On 03.12.2019 10:39, Christoph Hellwig wrote:
>bio based drivers only need to update q->nr_zones.  Do that manually
>instead of overloading blk_revalidate_disk_zones to keep that function
>simpler for the next round of changes that will rely even more on the
>request based functionality.
>
>Signed-off-by: Christoph Hellwig <hch@lst.de>
>---
> block/blk-zoned.c             | 16 +++++-----------
> drivers/block/null_blk_main.c | 12 +++++++++---
> drivers/md/dm-table.c         | 12 +++++++-----
> include/linux/blkdev.h        |  5 -----
> 4 files changed, 21 insertions(+), 24 deletions(-)
>
>diff --git a/block/blk-zoned.c b/block/blk-zoned.c
>index 0131f9e14bd1..51d427659ce7 100644
>--- a/block/blk-zoned.c
>+++ b/block/blk-zoned.c
>@@ -419,8 +419,9 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx,
>  *
>  * Helper function for low-level device drivers to (re) allocate and initialize
>  * a disk request queue zone bitmaps. This functions should normally be called
>- * within the disk ->revalidate method. For BIO based queues, no zone bitmap
>- * is allocated.
>+ * within the disk ->revalidate method for blk-mq based drivers.  For BIO based
>+ * drivers only q->nr_zones needs to be updated so that the sysfs exposed value
>+ * is correct.
>  */
> int blk_revalidate_disk_zones(struct gendisk *disk)
> {
>@@ -433,15 +434,8 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
>
> 	if (WARN_ON_ONCE(!blk_queue_is_zoned(q)))
> 		return -EIO;
>-
>-	/*
>-	 * BIO based queues do not use a scheduler so only q->nr_zones
>-	 * needs to be updated so that the sysfs exposed value is correct.
>-	 */
>-	if (!queue_is_mq(q)) {
>-		q->nr_zones = args.nr_zones;
>-		return 0;
>-	}
>+	if (WARN_ON_ONCE(!queue_is_mq(q)))
>+		return -EIO;
>
> 	/*
> 	 * Ensure that all memory allocations in this context are done as
>diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
>index dd6026289fbf..068cd0ae6e2c 100644
>--- a/drivers/block/null_blk_main.c
>+++ b/drivers/block/null_blk_main.c
>@@ -1576,11 +1576,17 @@ static int null_gendisk_register(struct nullb *nullb)
> 	disk->queue		= nullb->q;
> 	strncpy(disk->disk_name, nullb->disk_name, DISK_NAME_LEN);
>
>+#ifdef CONFIG_BLK_DEV_ZONED
> 	if (nullb->dev->zoned) {
>-		ret = blk_revalidate_disk_zones(disk);
>-		if (ret)
>-			return ret;
>+		if (queue_is_mq(nullb->q)) {
>+			ret = blk_revalidate_disk_zones(disk);
>+			if (ret)
>+				return ret;
>+		} else {
>+			nullb->q->nr_zones = blkdev_nr_zones(disk);
>+		}
> 	}
>+#endif
>
> 	add_disk(disk);
> 	return 0;
>diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
>index 2ae0c1913766..0a2cc197f62b 100644
>--- a/drivers/md/dm-table.c
>+++ b/drivers/md/dm-table.c
>@@ -1954,12 +1954,14 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
> 	/*
> 	 * For a zoned target, the number of zones should be updated for the
> 	 * correct value to be exposed in sysfs queue/nr_zones. For a BIO based
>-	 * target, this is all that is needed. For a request based target, the
>-	 * queue zone bitmaps must also be updated.
>-	 * Use blk_revalidate_disk_zones() to handle this.
>+	 * target, this is all that is needed.
> 	 */
>-	if (blk_queue_is_zoned(q))
>-		blk_revalidate_disk_zones(t->md->disk);
>+#ifdef CONFIG_BLK_DEV_ZONED
>+	if (blk_queue_is_zoned(q)) {
>+		WARN_ON_ONCE(queue_is_mq(q));
>+		q->nr_zones = blkdev_nr_zones(t->md->disk);
>+	}
>+#endif
>
> 	/* Allow reads to exceed readahead limits */
> 	q->backing_dev_info->io_pages = limits->max_sectors >> (PAGE_SHIFT - 9);
>diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>index 503c4d4c5884..47eb22a3b7f9 100644
>--- a/include/linux/blkdev.h
>+++ b/include/linux/blkdev.h
>@@ -375,11 +375,6 @@ static inline unsigned int blkdev_nr_zones(struct gendisk *disk)
> 	return 0;
> }
>
>-static inline int blk_revalidate_disk_zones(struct gendisk *disk)
>-{
>-	return 0;
>-}
>-
> static inline int blkdev_report_zones_ioctl(struct block_device *bdev,
> 					    fmode_t mode, unsigned int cmd,
> 					    unsigned long arg)
>-- 
>2.20.1
>

Looks good to me.

Reviewed-by: Javier González <javier@javigon.com>