diff mbox series

[03/13] block: Fix zone write plug initialization from blk_revalidate_zone_cb()

Message ID 20240430125131.668482-4-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
When revalidating the zones of a zoned block device,
blk_revalidate_zone_cb() must allocate a zone write plug for any
sequential write required zone that is not empty nor full. However, the
current code tests the latter case by comparing the zone write pointer
offset to the zone size instead of the zone capacity. Furthermore,
disk_get_and_lock_zone_wplug() is called with a sector argument equal to
the zone start instead of the current zone write pointer position.
This commit fixes both issues by calling disk_get_and_lock_zone_wplug()
for a zone that is not empty and with a write pointer offset lower than
the zone capacity and use the zone capacity sector as the sector
argument for disk_get_and_lock_zone_wplug().

Fixes: dd291d77cc90 ("block: Introduce zone write plugging")
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 block/blk-zoned.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Christoph Hellwig April 30, 2024, 3:26 p.m. UTC | #1
Looks good:

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

Although I suspect that at some point blk_revalidate_zone_cb should
grow separate helpers for the conventional and sequential required cases
as the amount of code in that switch statement is getting a bit out of
bounds..
Damien Le Moal April 30, 2024, 10:50 p.m. UTC | #2
On 5/1/24 00:26, Christoph Hellwig wrote:
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Although I suspect that at some point blk_revalidate_zone_cb should
> grow separate helpers for the conventional and sequential required cases
> as the amount of code in that switch statement is getting a bit out of
> bounds..

Yep, will clean that up later (next cycle).
Damien Le Moal April 30, 2024, 11:54 p.m. UTC | #3
On 5/1/24 07:50, Damien Le Moal wrote:
> On 5/1/24 00:26, Christoph Hellwig wrote:
>> Looks good:
>>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>>
>> Although I suspect that at some point blk_revalidate_zone_cb should
>> grow separate helpers for the conventional and sequential required cases
>> as the amount of code in that switch statement is getting a bit out of
>> bounds..
> 
> Yep, will clean that up later (next cycle).

I did it now. It is a nice cleanup. I added a patch to the series for that.
diff mbox series

Patch

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 6cf3e319513c..e92ae0729cf8 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -1666,10 +1666,11 @@  static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx,
 		 * empty nor full. So make sure we have a zone write plug for
 		 * such zone if the device has a zone write plug hash table.
 		 */
+		if (!disk->zone_wplugs_hash)
+			break;
 		wp_offset = blk_zone_wp_offset(zone);
-		if (disk->zone_wplugs_hash &&
-		    wp_offset && wp_offset < zone_sectors) {
-			zwplug = disk_get_and_lock_zone_wplug(disk, zone->start,
+		if (wp_offset && wp_offset < zone->capacity) {
+			zwplug = disk_get_and_lock_zone_wplug(disk, zone->wp,
 							      GFP_NOIO, &flags);
 			if (!zwplug)
 				return -ENOMEM;