[PATCH 0/3] queue limits cleanup

[PATCH 0/3] queue limits cleanup

[Damien Le Moal]

One fix and 2 cleanups to go on top of Christoph's queue limits work.

Damien Le Moal (3):
  virtio_blk: Do not use disk_set_max_open/active_zones()
  block: Rename disk_set_zoned()
  block: Rename disk_set_max_open/active_zones()

 block/blk-settings.c       | 10 ++++------
 drivers/block/virtio_blk.c |  4 ++--
 drivers/nvme/host/zns.c    |  6 +++---
 drivers/scsi/sd.c          |  2 +-
 drivers/scsi/sd_zbc.c      |  6 +++---
 include/linux/blkdev.h     | 30 ++++++++++++++++--------------
 6 files changed, 29 insertions(+), 29 deletions(-)

-- 
2.44.0

[PATCH 1/3] virtio_blk: Do not use disk_set_max_open/active_zones()

[Damien Le Moal]

In virtblk_read_zoned_limits(), setting a zoned block device maximum
number of open and active zones using the functions
disk_set_max_open_zones() and disk_set_max_active_zones() is incorrect
as setting the limits for the request queue is now done atomically when
the gendisk is created (with blk_mq_alloc_disk()). The value set by the
disk_set_max_open/active_zones() functions will be overwritten.
Fix this by setting the maximum number of open and active zones directly
in the queue_limits structure passed to virtblk_read_zoned_limits().

Fixes: 8b837256560c ("virtio_blk: pass queue_limits to blk_mq_alloc_disk")
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/block/virtio_blk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index d8b55874cd59..aa9f86507719 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -732,12 +732,12 @@ static int virtblk_read_zoned_limits(struct virtio_blk *vblk,
 
 	virtio_cread(vdev, struct virtio_blk_config,
 		     zoned.max_open_zones, &v);
-	disk_set_max_open_zones(vblk->disk, v);
+	lim->max_open_zones = v;
 	dev_dbg(&vdev->dev, "max open zones = %u\n", v);
 
 	virtio_cread(vdev, struct virtio_blk_config,
 		     zoned.max_active_zones, &v);
-	disk_set_max_active_zones(vblk->disk, v);
+	lim->max_active_zones = v;
 	dev_dbg(&vdev->dev, "max active zones = %u\n", v);
 
 	virtio_cread(vdev, struct virtio_blk_config,
-- 
2.44.0

[PATCH 2/3] block: Rename disk_set_zoned()

[Damien Le Moal]

The disk_set_zoned() function operates on the zoned request queue limit
of a block device and does not change anything to the gendisk of the
device. To reflect this behavior and to be consistent with other request
queue limit setting functions, rename disk_set_zoned() to
blk_queue_zoned().

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 block/blk-settings.c    | 10 ++++------
 drivers/nvme/host/zns.c |  2 +-
 drivers/scsi/sd.c       |  2 +-
 include/linux/blkdev.h  |  4 ++--
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 13865a9f8972..a5102b1cd006 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -1108,13 +1108,11 @@ bool blk_queue_can_use_dma_map_merging(struct request_queue *q,
 EXPORT_SYMBOL_GPL(blk_queue_can_use_dma_map_merging);
 
 /**
- * disk_set_zoned - inidicate a zoned device
- * @disk:	gendisk to configure
+ * blk_queue_zoned - indicate a zoned device
+ * @q:		the request queue for the device
  */
-void disk_set_zoned(struct gendisk *disk)
+void blk_queue_zoned(struct request_queue *q)
 {
-	struct request_queue *q = disk->queue;
-
 	WARN_ON_ONCE(!IS_ENABLED(CONFIG_BLK_DEV_ZONED));
 
 	/*
@@ -1124,7 +1122,7 @@ void disk_set_zoned(struct gendisk *disk)
 	q->limits.zoned = true;
 	blk_queue_zone_write_granularity(q, queue_logical_block_size(q));
 }
-EXPORT_SYMBOL_GPL(disk_set_zoned);
+EXPORT_SYMBOL_GPL(blk_queue_zoned);
 
 int bdev_alignment_offset(struct block_device *bdev)
 {
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 499bbb0eee8d..96d9206efc5e 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -109,7 +109,7 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
 		goto free_data;
 	}
 
-	disk_set_zoned(ns->disk);
+	blk_queue_zoned(q);
 	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
 	disk_set_max_open_zones(ns->disk, le32_to_cpu(id->mor) + 1);
 	disk_set_max_active_zones(ns->disk, le32_to_cpu(id->mar) + 1);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 0833b3e6aa6e..477451a36038 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3141,7 +3141,7 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)
 		/*
 		 * Host-managed.
 		 */
-		disk_set_zoned(sdkp->disk);
+		blk_queue_zoned(q);
 
 		/*
 		 * Per ZBC and ZAC specifications, writes in sequential write
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 285e82723d64..1081c2d9e6bd 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -321,8 +321,6 @@ struct queue_limits {
 typedef int (*report_zones_cb)(struct blk_zone *zone, unsigned int idx,
 			       void *data);
 
-void disk_set_zoned(struct gendisk *disk);
-
 #define BLK_ALL_ZONES  ((unsigned int)-1)
 int blkdev_report_zones(struct block_device *bdev, sector_t sector,
 		unsigned int nr_zones, report_zones_cb cb, void *data);
@@ -610,6 +608,8 @@ static inline enum rpm_status queue_rpm_status(struct request_queue *q)
 }
 #endif
 
+void blk_queue_zoned(struct request_queue *q);
+
 static inline bool blk_queue_is_zoned(struct request_queue *q)
 {
 	return IS_ENABLED(CONFIG_BLK_DEV_ZONED) && q->limits.zoned;
-- 
2.44.0

[PATCH 3/3] block: Rename disk_set_max_open/active_zones()

[Damien Le Moal]

Given that the max_open_zones and max_active_zones attributes of a zoned
block device have been moved to the device request queue limit, rename
the functions disk_set_max_open_zones() and disk_set_max_active_zones()
to blk_queue_max_open_zones() and blk_queue_max_active_zones() to be
consistent with other request queue limit setting functions.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/nvme/host/zns.c |  4 ++--
 drivers/scsi/sd_zbc.c   |  6 +++---
 include/linux/blkdev.h  | 26 ++++++++++++++------------
 3 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 96d9206efc5e..7f86b169c373 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -111,8 +111,8 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
 
 	blk_queue_zoned(q);
 	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
-	disk_set_max_open_zones(ns->disk, le32_to_cpu(id->mor) + 1);
-	disk_set_max_active_zones(ns->disk, le32_to_cpu(id->mar) + 1);
+	blk_queue_max_open_zones(q, le32_to_cpu(id->mor) + 1);
+	blk_queue_max_active_zones(q, le32_to_cpu(id->mar) + 1);
 free_data:
 	kfree(id);
 	return status;
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 26af5ab7d7c1..2971c3269296 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -943,10 +943,10 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE])
 	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
 	blk_queue_required_elevator_features(q, ELEVATOR_F_ZBD_SEQ_WRITE);
 	if (sdkp->zones_max_open == U32_MAX)
-		disk_set_max_open_zones(disk, 0);
+		blk_queue_max_open_zones(q, 0);
 	else
-		disk_set_max_open_zones(disk, sdkp->zones_max_open);
-	disk_set_max_active_zones(disk, 0);
+		blk_queue_max_open_zones(q, sdkp->zones_max_open);
+	blk_queue_max_active_zones(q, 0);
 	nr_zones = round_up(sdkp->capacity, zone_blocks) >> ilog2(zone_blocks);
 
 	sdkp->early_zone_info.nr_zones = nr_zones;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1081c2d9e6bd..55a68983fae8 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -615,6 +615,20 @@ static inline bool blk_queue_is_zoned(struct request_queue *q)
 	return IS_ENABLED(CONFIG_BLK_DEV_ZONED) && q->limits.zoned;
 }
 
+static inline void blk_queue_max_open_zones(struct request_queue *q,
+					    unsigned int max_open_zones)
+{
+	WARN_ON_ONCE(!IS_ENABLED(CONFIG_BLK_DEV_ZONED));
+	q->limits.max_open_zones = max_open_zones;
+}
+
+static inline void blk_queue_max_active_zones(struct request_queue *q,
+					      unsigned int max_active_zones)
+{
+	WARN_ON_ONCE(!IS_ENABLED(CONFIG_BLK_DEV_ZONED));
+	q->limits.max_active_zones = max_active_zones;
+}
+
 #ifdef CONFIG_BLK_DEV_ZONED
 unsigned int bdev_nr_zones(struct block_device *bdev);
 
@@ -639,18 +653,6 @@ static inline bool disk_zone_is_seq(struct gendisk *disk, sector_t sector)
 	return !test_bit(disk_zone_no(disk, sector), disk->conv_zones_bitmap);
 }
 
-static inline void disk_set_max_open_zones(struct gendisk *disk,
-		unsigned int max_open_zones)
-{
-	disk->queue->limits.max_open_zones = max_open_zones;
-}
-
-static inline void disk_set_max_active_zones(struct gendisk *disk,
-		unsigned int max_active_zones)
-{
-	disk->queue->limits.max_active_zones = max_active_zones;
-}
-
 static inline unsigned int bdev_max_open_zones(struct block_device *bdev)
 {
 	return bdev->bd_disk->queue->limits.max_open_zones;
-- 
2.44.0

Re: [PATCH 1/3] virtio_blk: Do not use disk_set_max_open/active_zones()

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

no idea why this got dropped from my patch because I did have it
when testing virtio_blk.

Re: [PATCH 2/3] block: Rename disk_set_zoned()

[Christoph Hellwig]

On Sat, Mar 02, 2024 at 04:26:38AM +0900, Damien Le Moal wrote:
> The disk_set_zoned() function operates on the zoned request queue limit
> of a block device and does not change anything to the gendisk of the
> device. To reflect this behavior and to be consistent with other request
> queue limit setting functions, rename disk_set_zoned() to
> blk_queue_zoned().

Can we just hold this off a bit?  I have the big nvme queue limits series
that removes the zns usage, and early next merge window scsi will be
converted as well and this function will go away entirely.

Re: [PATCH 3/3] block: Rename disk_set_max_open/active_zones()

[Christoph Hellwig]

On Sat, Mar 02, 2024 at 04:26:39AM +0900, Damien Le Moal wrote:
> Given that the max_open_zones and max_active_zones attributes of a zoned
> block device have been moved to the device request queue limit, rename
> the functions disk_set_max_open_zones() and disk_set_max_active_zones()
> to blk_queue_max_open_zones() and blk_queue_max_active_zones() to be
> consistent with other request queue limit setting functions.

Same comment as for the last one - this one will go away early next
merge window too.

Re: [PATCH 1/3] virtio_blk: Do not use disk_set_max_open/active_zones()

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 2/3] block: Rename disk_set_zoned()

[Damien Le Moal]

On 3/2/24 23:07, Christoph Hellwig wrote:
> On Sat, Mar 02, 2024 at 04:26:38AM +0900, Damien Le Moal wrote:
>> The disk_set_zoned() function operates on the zoned request queue limit
>> of a block device and does not change anything to the gendisk of the
>> device. To reflect this behavior and to be consistent with other request
>> queue limit setting functions, rename disk_set_zoned() to
>> blk_queue_zoned().
> 
> Can we just hold this off a bit?  I have the big nvme queue limits series
> that removes the zns usage, and early next merge window scsi will be
> converted as well and this function will go away entirely.

I suspected as much, but sent the patches anyway.. Fine with me !

-- 
Damien Le Moal
Western Digital Research

Re: (subset) [PATCH 0/3] queue limits cleanup

[Jens Axboe]

On Sat, 02 Mar 2024 04:26:36 +0900, Damien Le Moal wrote:
> One fix and 2 cleanups to go on top of Christoph's queue limits work.
> 
> Damien Le Moal (3):
>   virtio_blk: Do not use disk_set_max_open/active_zones()
>   block: Rename disk_set_zoned()
>   block: Rename disk_set_max_open/active_zones()
> 
> [...]

Applied, thanks!

[1/3] virtio_blk: Do not use disk_set_max_open/active_zones()
      commit: 0e46064ebebb90b02c53283106f26600aa38c986

Best regards,
-- 
Jens Axboe