diff mbox series

[02/13] block: Exclude conventional zones when faking max open limit

Message ID 20240430125131.668482-3-dlemoal@kernel.org (mailing list archive)
State Superseded, archived
Delegated to: Mike Snitzer
Headers show
Series Zone write plugging fixes and cleanup | expand

Commit Message

Damien Le Moal April 30, 2024, 12:51 p.m. UTC
For a device that has no limits for the maximum number of open and
active zones, we default to using the number of zones, limited to
BLK_ZONE_WPLUG_DEFAULT_POOL_SIZE (128), for the maximum number of open
zones indicated to the user. However, for a device that has conventional
zones and less zones than BLK_ZONE_WPLUG_DEFAULT_POOL_SIZE, we should
not account conventional zones and set the limit to the number of
sequential write required zones. Furthermore, for cases where the limit
is equal to the number of sequential write required zones, we can
advertize a limit of 0 to indicate "no limits".

Fix this by moving the zone write plug mempool resizing from
disk_revalidate_zone_resources() to disk_update_zone_resources() where
we can safely compute the number of conventional zones and update the
limits.

Fixes: 843283e96e5a ("block: Fake max open zones limit when there is no limit")
Reported-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 block/blk-zoned.c | 42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

Comments

Christoph Hellwig April 30, 2024, 3:24 p.m. UTC | #1
On Tue, Apr 30, 2024 at 09:51:20PM +0900, Damien Le Moal wrote:
> +	/* Resize the zone write plug memory pool if needed. */
> +	if (disk->zone_wplugs_pool->min_nr != pool_size)
> +		mempool_resize(disk->zone_wplugs_pool, pool_size);

No need for the if here, mempool_resize is a no-op if called for
the current value.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Jens Axboe April 30, 2024, 7:04 p.m. UTC | #2
On 4/30/24 9:24 AM, Christoph Hellwig wrote:
> On Tue, Apr 30, 2024 at 09:51:20PM +0900, Damien Le Moal wrote:
>> +	/* Resize the zone write plug memory pool if needed. */
>> +	if (disk->zone_wplugs_pool->min_nr != pool_size)
>> +		mempool_resize(disk->zone_wplugs_pool, pool_size);
> 
> No need for the if here, mempool_resize is a no-op if called for
> the current value.

Still cheaper than the function call though, so I think that's
the right way to do it.
Christoph Hellwig May 1, 2024, 4:57 a.m. UTC | #3
On Tue, Apr 30, 2024 at 01:04:51PM -0600, Jens Axboe wrote:
> On 4/30/24 9:24 AM, Christoph Hellwig wrote:
> > On Tue, Apr 30, 2024 at 09:51:20PM +0900, Damien Le Moal wrote:
> >> +	/* Resize the zone write plug memory pool if needed. */
> >> +	if (disk->zone_wplugs_pool->min_nr != pool_size)
> >> +		mempool_resize(disk->zone_wplugs_pool, pool_size);
> > 
> > No need for the if here, mempool_resize is a no-op if called for
> > the current value.
> 
> Still cheaper than the function call though, so I think that's
> the right way to do it.

It is only called during device probing and resize.  Try to avoid
the call and spinlock there is the poster definition of premature
micro-optimization..
diff mbox series

Patch

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index bad68277c0b2..6cf3e319513c 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -1513,10 +1513,6 @@  static int disk_revalidate_zone_resources(struct gendisk *disk,
 	if (!disk->zone_wplugs_hash)
 		return disk_alloc_zone_resources(disk, pool_size);
 
-	/* Resize the zone write plug memory pool if needed. */
-	if (disk->zone_wplugs_pool->min_nr != pool_size)
-		return mempool_resize(disk->zone_wplugs_pool, pool_size);
-
 	return 0;
 }
 
@@ -1536,27 +1532,51 @@  static int disk_update_zone_resources(struct gendisk *disk,
 				      struct blk_revalidate_zone_args *args)
 {
 	struct request_queue *q = disk->queue;
+	unsigned int nr_seq_zones, nr_conv_zones = 0;
+	unsigned int pool_size;
 	struct queue_limits lim;
 
 	disk->nr_zones = args->nr_zones;
 	disk->zone_capacity = args->zone_capacity;
 	swap(disk->conv_zones_bitmap, args->conv_zones_bitmap);
+	if (disk->conv_zones_bitmap)
+		nr_conv_zones = bitmap_weight(disk->conv_zones_bitmap,
+					      disk->nr_zones);
+	if (nr_conv_zones >= disk->nr_zones) {
+		pr_warn("%s: Invalid number of conventional zones %u / %u\n",
+			disk->disk_name, nr_conv_zones, disk->nr_zones);
+		return -ENODEV;
+	}
+
+	if (!disk->zone_wplugs_pool)
+		return 0;
 
 	/*
-	 * If the device has no limit on the maximum number of open and active
+	 * If the device has no limits on the maximum number of open and active
 	 * zones, set its max open zone limit to the mempool size to indicate
 	 * to the user that there is a potential performance impact due to
 	 * dynamic zone write plug allocation when simultaneously writing to
 	 * more zones than the size of the mempool.
 	 */
-	if (disk->zone_wplugs_pool) {
-		lim = queue_limits_start_update(q);
-		if (!lim.max_open_zones && !lim.max_active_zones)
-			lim.max_open_zones = disk->zone_wplugs_pool->min_nr;
-		return queue_limits_commit_update(q, &lim);
+	lim = queue_limits_start_update(q);
+
+	nr_seq_zones = disk->nr_zones - nr_conv_zones;
+	pool_size = max(lim.max_open_zones, lim.max_active_zones);
+	if (!pool_size)
+		pool_size = min(BLK_ZONE_WPLUG_DEFAULT_POOL_SIZE, nr_seq_zones);
+
+	/* Resize the zone write plug memory pool if needed. */
+	if (disk->zone_wplugs_pool->min_nr != pool_size)
+		mempool_resize(disk->zone_wplugs_pool, pool_size);
+
+	if (!lim.max_open_zones && !lim.max_active_zones) {
+		if (pool_size < nr_seq_zones)
+			lim.max_open_zones = pool_size;
+		else
+			lim.max_open_zones = 0;
 	}
 
-	return 0;
+	return queue_limits_commit_update(q, &lim);
 }
 
 /*