Skip to contentSkip to conversation

Zone related cleanups

PATCH Superseded Damien Le Moal 3 participants 6 messages Nov 6, 2025

Subsystems (2)
Block layer Zoned block device (block layer)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Damien Le Moal

author maintainer
#0 lore raw

cover Zone related cleanups

A couple more patches to go on top of the cached report zone support. The first one removes blk_zone_wp_offset() to simplify the code. The second patch introduces bdev_zone_start() as a replacement to using ALIGN_DOWN() for getting the start sector of a zone.

Damien Le Moal (2):
block: remove blk_zone_wp_offset()
block: introduce bdev_zone_start()

 block/blk-zoned.c      | 68 +++++++++++++++++++++---------------------
 include/linux/blkdev.h |  6 ++++
 2 files changed, 40 insertions(+), 34 deletions(-)
Signature · 1 line
2.51.1
author maintainer
#1 lore raw

PATCH 1/2 block: remove blk_zone_wp_offset()

The helper function blk_zone_wp_offset() is called from disk_zone_wplug_sync_wp_offset(), and again called from blk_revalidate_seq_zone() right after the call to disk_zone_wplug_sync_wp_offset().

Change disk_zone_wplug_sync_wp_offset() to return the wp_offset it used for updating the target zone write plug to avoid this double call. With this change, blk_zone_wp_offset() can be open coded directly in disk_zone_wplug_sync_wp_offset(). This open-coding introduces 2 changes:
handle the BLK_COND_ZONE_ACTIVE case, and return UINT_MAX as the wp_offset for a full zone, since the write pointer of full zones is invalid.

For the case where a zone does not have a zone write plug, disk_zone_wplug_sync_wp_offset() does nothing and returns 0. This in turn leads to blk_revalidate_seq_zone() to immediately return, which is exactly what we want (because there is no need to attempt removing a zone write plug that does not exist).

Signed-off-by Damien Le Moal <[email protected]>
---
 block/blk-zoned.c | 64 +++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 32 deletions(-)
block/blk-zoned.c +32 -32
index c5226bcaaa94..2f4e45638601 100644
@@ -794,46 +794,48 @@ static void disk_zone_wplug_set_wp_offset(struct gendisk *disk,
 		disk_remove_zone_wplug(disk, zwplug);
 }
 
-static unsigned int blk_zone_wp_offset(struct blk_zone *zone)
-{
-	switch (zone->cond) {
-	case BLK_ZONE_COND_IMP_OPEN:
-	case BLK_ZONE_COND_EXP_OPEN:
-	case BLK_ZONE_COND_CLOSED:
-		return zone->wp - zone->start;
-	case BLK_ZONE_COND_FULL:
-		return zone->len;
-	case BLK_ZONE_COND_EMPTY:
-		return 0;
-	case BLK_ZONE_COND_NOT_WP:
-	case BLK_ZONE_COND_OFFLINE:
-	case BLK_ZONE_COND_READONLY:
-	default:
-		/*
-		 * Conventional, offline and read-only zones do not have a valid
-		 * write pointer.
-		 */
-		return UINT_MAX;
-	}
-}
-
-static void disk_zone_wplug_sync_wp_offset(struct gendisk *disk,
-					   struct blk_zone *zone)
+static unsigned int disk_zone_wplug_sync_wp_offset(struct gendisk *disk,
+						   struct blk_zone *zone)
 {
 	struct blk_zone_wplug *zwplug;
 	unsigned long flags;
+	unsigned int wp_offset;
 
 	zwplug = disk_get_zone_wplug(disk, zone->start);
 	if (!zwplug)
-		return;
+		return 0;
 
 	spin_lock_irqsave(&zwplug->lock, flags);
-	if (zwplug->flags & BLK_ZONE_WPLUG_NEED_WP_UPDATE)
-		disk_zone_wplug_set_wp_offset(disk, zwplug,
-					      blk_zone_wp_offset(zone));
+	if (zwplug->flags & BLK_ZONE_WPLUG_NEED_WP_UPDATE) {
+		switch (zone->cond) {
+		case BLK_ZONE_COND_IMP_OPEN:
+		case BLK_ZONE_COND_EXP_OPEN:
+		case BLK_ZONE_COND_CLOSED:
+		case BLK_ZONE_COND_ACTIVE:
+			wp_offset = zone->wp - zone->start;
+			break;
+		case BLK_ZONE_COND_EMPTY:
+			wp_offset = 0;
+			break;
+		case BLK_ZONE_COND_FULL:
+		case BLK_ZONE_COND_NOT_WP:
+		case BLK_ZONE_COND_OFFLINE:
+		case BLK_ZONE_COND_READONLY:
+		default:
+			/*
+			 * Conventional, full, offline and read-only zones do
+			 * not have a valid write pointer.
+			 */
+			wp_offset = UINT_MAX;
+			break;
+		}
+		disk_zone_wplug_set_wp_offset(disk, zwplug, wp_offset);
+	}
 	spin_unlock_irqrestore(&zwplug->lock, flags);
 
 	disk_put_zone_wplug(zwplug);
+
+	return wp_offset;
 }
 
 /**

@@ -2095,9 +2097,7 @@ static int blk_revalidate_seq_zone(struct blk_zone *zone, unsigned int idx,
 	if (!queue_emulates_zone_append(disk->queue) || !disk->zone_wplugs_hash)
 		return 0;
 
-	disk_zone_wplug_sync_wp_offset(disk, zone);
-
-	wp_offset = blk_zone_wp_offset(zone);
+	wp_offset = disk_zone_wplug_sync_wp_offset(disk, zone);
 	if (!wp_offset || wp_offset >= zone->capacity)
 		return 0;
Signature · 1 line
2.51.1
#2 lore raw

Re: [PATCH 1/2] block: remove blk_zone_wp_offset()

Quoted 13 lines · Damien Le Moal

The helper function blk_zone_wp_offset() is called from disk_zone_wplug_sync_wp_offset(), and again called from blk_revalidate_seq_zone() right after the call to disk_zone_wplug_sync_wp_offset().

Change disk_zone_wplug_sync_wp_offset() to return the wp_offset it used for updating the target zone write plug to avoid this double call. With this change, blk_zone_wp_offset() can be open coded directly in disk_zone_wplug_sync_wp_offset(). This open-coding introduces 2 changes:
handle the BLK_COND_ZONE_ACTIVE case, and return UINT_MAX as the wp_offset for a full zone, since the write pointer of full zones is invalid.

Quoted 5 lines

For the case where a zone does not have a zone write plug, disk_zone_wplug_sync_wp_offset() does nothing and returns 0. This in turn leads to blk_revalidate_seq_zone() to immediately return, which is exactly what we want (because there is no need to attempt removing a zone write plug that does not exist).

I like the use of disk_zone_wplug_sync_wp_offset in blk_revalidate_seq_zone, but having the switch on the zone conditions in a separate helper seems nicer than open coding it next to the write plug handling.

author maintainer
#3 lore raw

PATCH 2/2 block: introduce bdev_zone_start()

In reply to Damien Le Moal

Introduce the function bdev_zone_start() as a more explicit (and clear) replacement for ALIGN_DOWN() to get the start sector of a zone containing a particular sector of a zoned block device.

Use this new helper in blkdev_get_zone_info() and
blkdev_report_zones_cached().

Signed-off-by Damien Le Moal <[email protected]>
---
 block/blk-zoned.c      | 4 ++--
 include/linux/blkdev.h | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)
block/blk-zoned.c +2 -2
index 2f4e45638601..e7e5542c538e 100644
@@ -945,7 +945,7 @@ int blkdev_get_zone_info(struct block_device *bdev, sector_t sector,
 		return -EINVAL;
 
 	memset(zone, 0, sizeof(*zone));
-	sector = ALIGN_DOWN(sector, zone_sectors);
+	sector = bdev_zone_start(bdev, sector);
 
 	if (!blkdev_has_cached_report_zones(bdev))
 		return blkdev_report_zone_fallback(bdev, sector, zone);

@@ -1063,7 +1063,7 @@ int blkdev_report_zones_cached(struct block_device *bdev, sector_t sector,
 		return blkdev_do_report_zones(bdev, sector, nr_zones, &args);
 	}
 
-	for (sector = ALIGN_DOWN(sector, zone_sectors);
+	for (sector = bdev_zone_start(bdev, sector);
 	     sector < capacity && idx < nr_zones;
 	     sector += zone_sectors, idx++) {
 		ret = blkdev_get_zone_info(bdev, sector, &zone);
include/linux/blkdev.h +6 -0
index 6a498aa7f7e7..2fff8a80dbd2 100644
@@ -1522,6 +1522,12 @@ static inline sector_t bdev_zone_sectors(struct block_device *bdev)
 	return q->limits.chunk_sectors;
 }
 
+static inline sector_t bdev_zone_start(struct block_device *bdev,
+				       sector_t sector)
+{
+	return sector & ~(bdev_zone_sectors(bdev) - 1);
+}
+
 static inline sector_t bdev_offset_from_zone_start(struct block_device *bdev,
 						   sector_t sector)
 {
Signature · 1 line
2.51.1