On Jan 06, 2025 / 14:56, Damien Le Moal wrote:

On 12/25/24 7:09 PM, Shin'ichiro Kawasaki wrote:

quoted

The previous commit modified bad blocks handling to do the partial IOs.
When such partial IOs happen for zoned null_blk devices, it is expected
that the write pointers also move partially. To test and debug partial
write by userland tools for zoned block devices, move write pointers
partially.

Signed-off-by: Shin'ichiro Kawasaki <redacted>
---
 drivers/block/null_blk/main.c     |  5 ++++-
 drivers/block/null_blk/null_blk.h |  6 ++++++
 drivers/block/null_blk/zoned.c    | 10 ++++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index d155eb040077..1675dec0b0e6 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c

@@ -1330,6 +1330,7 @@ static inline blk_status_t null_handle_throttled(struct nullb_cmd *cmd)
 }
 
 static inline blk_status_t null_handle_badblocks(struct nullb_cmd *cmd,
+						 enum req_op op,
 						 sector_t sector,
 						 sector_t nr_sectors)
 {

@@ -1347,6 +1348,8 @@ static inline blk_status_t null_handle_badblocks(struct nullb_cmd *cmd,
 			transfer_bytes = (first_bad - sector) << SECTOR_SHIFT;
 			__null_handle_rq(cmd, transfer_bytes);
 		}
+		if (dev->zoned && op == REQ_OP_WRITE)

Forgot REQ_OP_ZONE_APPEND ?

Actually, null_process_zoned_cmd() handles both REQ_OP_WRITE and
REQ_OP_ZONE_APPEND in the same way, and both case results in op == REQ_OP_WRITE
here. Anyway, this if branch will not be required in the v3 series since this
patch is totally rewritten to follow your the other suggestion.

quoted

+			null_move_zone_wp(dev, sector, first_bad - sector);
 		return BLK_STS_IOERR;
 	}
 

@@ -1413,7 +1416,7 @@ blk_status_t null_process_cmd(struct nullb_cmd *cmd, enum req_op op,
 	blk_status_t ret;
 
 	if (dev->badblocks.shift != -1) {
-		ret = null_handle_badblocks(cmd, sector, nr_sectors);
+		ret = null_handle_badblocks(cmd, op, sector, nr_sectors);
 		if (ret != BLK_STS_OK)
 			return ret;
 	}

diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
index 6f9fe6171087..c6ceede691ba 100644
--- a/drivers/block/null_blk/null_blk.h
+++ b/drivers/block/null_blk/null_blk.h

@@ -144,6 +144,8 @@ size_t null_zone_valid_read_len(struct nullb *nullb,
 				sector_t sector, unsigned int len);
 ssize_t zone_cond_store(struct nullb_device *dev, const char *page,
 			size_t count, enum blk_zone_cond cond);
+void null_move_zone_wp(struct nullb_device *dev, sector_t zone_sector,
+		       sector_t nr_sectors);
 #else
 static inline int null_init_zoned_dev(struct nullb_device *dev,
 		struct queue_limits *lim)

@@ -173,6 +175,10 @@ static inline ssize_t zone_cond_store(struct nullb_device *dev,
 {
 	return -EOPNOTSUPP;
 }
+static inline void null_move_zone_wp(struct nullb_device *dev,
+				     sector_t zone_sector, sector_t nr_sectors)
+{
+}
 #define null_report_zones	NULL
 #endif /* CONFIG_BLK_DEV_ZONED */
 #endif /* __NULL_BLK_H */

diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
index 0d5f9bf95229..e2b8396aa318 100644
--- a/drivers/block/null_blk/zoned.c
+++ b/drivers/block/null_blk/zoned.c

@@ -347,6 +347,16 @@ static blk_status_t null_check_zone_resources(struct nullb_device *dev,
 	}
 }
 
+void null_move_zone_wp(struct nullb_device *dev, sector_t zone_sector,
+		       sector_t nr_sectors)
+{
+	unsigned int zno = null_zone_no(dev, zone_sector);
+	struct nullb_zone *zone = &dev->zones[zno];
+
+	if (zone->type != BLK_ZONE_TYPE_CONVENTIONAL)
+		zone->wp += nr_sectors;
+}

I do not think this is correct. We need to deal with the zone implicit open and
zone resources as well, exactly like for a regular write. So it is not that simple.

I think that overall, a simpler approach would be to reuse
null_handle_badblocks() inside null_process_zoned_cmd(), similar to
null_process_cmd(). If reusing null_handle_badblocks() inside
null_process_zoned_cmd() is not possible, then let's write a
null_handle_zone_badblocks() helper.

Right, I overlooked the zone resource management. I think we can reuse
null_handle_badblolcks() from null_zone_write(). Please review v3 that I will
post soon.