[PATCH v2 0/9] Support zoned devices with gap zones

[PATCH v2 0/9] Support zoned devices with gap zones

[Bart Van Assche]

Hi Martin,

In ZBC-2 support has been improved for zones with a size that is not a power
of two by allowing host-managed devices to report gap zones. This patch adds
support for zoned devices for which data zones and gap zones alternate if the
distance between zone start LBAs is a power of two.

Please consider this patch series for kernel v5.19.

Thanks,

Bart.

Changes compared to v1:
- Made this patch series compatible with the zone querying code in BTRFS.
- Addressed Damien's off-list review comments.
- Added patch "Return early in sd_zbc_check_zoned_characteristics()" to this
  series.

Bart Van Assche (9):
  scsi: sd_zbc: Improve source code documentation
  scsi: sd_zbc: Verify that the zone size is a power of two
  scsi: sd_zbc: Use logical blocks as unit when querying zones
  scsi: sd_zbc: Introduce struct zoned_disk_info
  scsi: sd_zbc: Return early in sd_zbc_check_zoned_characteristics()
  scsi: sd_zbc: Hide gap zones
  scsi_debug: Fix a typo
  scsi_debug: Rename zone type constants
  scsi_debug: Add gap zone support

 drivers/scsi/scsi_debug.c | 149 ++++++++++++++++++------
 drivers/scsi/sd.h         |  32 ++++--
 drivers/scsi/sd_zbc.c     | 236 +++++++++++++++++++++++++++++---------
 include/scsi/scsi_proto.h |   9 +-
 4 files changed, 331 insertions(+), 95 deletions(-)

[PATCH v2 1/9] scsi: sd_zbc: Improve source code documentation

[Bart Van Assche]

Add several kernel-doc headers. Declare input arrays const. Specify the
array size in function declarations.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/sd.h     |  5 ++--
 drivers/scsi/sd_zbc.c | 55 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 0a33a4b68ffb..4849cbe771a7 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -222,7 +222,7 @@ static inline int sd_is_zoned(struct scsi_disk *sdkp)
 #ifdef CONFIG_BLK_DEV_ZONED
 
 void sd_zbc_release_disk(struct scsi_disk *sdkp);
-int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned char *buffer);
+int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE]);
 int sd_zbc_revalidate_zones(struct scsi_disk *sdkp);
 blk_status_t sd_zbc_setup_zone_mgmt_cmnd(struct scsi_cmnd *cmd,
 					 unsigned char op, bool all);
@@ -238,8 +238,7 @@ blk_status_t sd_zbc_prepare_zone_append(struct scsi_cmnd *cmd, sector_t *lba,
 
 static inline void sd_zbc_release_disk(struct scsi_disk *sdkp) {}
 
-static inline int sd_zbc_read_zones(struct scsi_disk *sdkp,
-				    unsigned char *buf)
+static inline int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE])
 {
 	return 0;
 }
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 7f466280993b..2ae44bc52a5f 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -20,6 +20,12 @@
 
 #include "sd.h"
 
+/**
+ * sd_zbc_get_zone_wp_offset - Get zone write pointer offset.
+ * @zone: Zone for which to return the write pointer offset.
+ *
+ * Return: offset of the write pointer from the start of the zone.
+ */
 static unsigned int sd_zbc_get_zone_wp_offset(struct blk_zone *zone)
 {
 	if (zone->type == ZBC_ZONE_TYPE_CONV)
@@ -44,7 +50,21 @@ static unsigned int sd_zbc_get_zone_wp_offset(struct blk_zone *zone)
 	}
 }
 
-static int sd_zbc_parse_report(struct scsi_disk *sdkp, u8 *buf,
+/**
+ * sd_zbc_parse_report - Parse a SCSI zone descriptor
+ * @sdkp: SCSI disk pointer.
+ * @buf: SCSI zone descriptor.
+ * @idx: Index of the zone relative to the first zone reported by the current
+ *	sd_zbc_report_zones() call.
+ * @cb: Callback function pointer.
+ * @data: Second argument passed to @cb.
+ *
+ * Return: Value returned by @cb.
+ *
+ * Convert a SCSI zone descriptor into struct blk_zone format. Additionally,
+ * call @cb(blk_zone, @data).
+ */
+static int sd_zbc_parse_report(struct scsi_disk *sdkp, const u8 buf[64],
 			       unsigned int idx, report_zones_cb cb, void *data)
 {
 	struct scsi_device *sdp = sdkp->device;
@@ -189,6 +209,17 @@ static inline sector_t sd_zbc_zone_sectors(struct scsi_disk *sdkp)
 	return logical_to_sectors(sdkp->device, sdkp->zone_blocks);
 }
 
+/**
+ * sd_zbc_report_zones - SCSI .report_zones() callback.
+ * @disk: Disk to report zones for.
+ * @sector: Start sector.
+ * @nr_zones: Maximum number of zones to report.
+ * @cb: Callback function called to report zone information.
+ * @data: Second argument passed to @cb.
+ *
+ * Called by the block layer to iterate over zone information. See also the
+ * disk->fops->report_zones() calls in block/blk-zoned.c.
+ */
 int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
 			unsigned int nr_zones, report_zones_cb cb, void *data)
 {
@@ -276,6 +307,10 @@ static int sd_zbc_update_wp_offset_cb(struct blk_zone *zone, unsigned int idx,
 	return 0;
 }
 
+/*
+ * An attempt to append a zone triggered an invalid write pointer error.
+ * Reread the write pointer of the zone(s) in which the append failed.
+ */
 static void sd_zbc_update_wp_offset_workfn(struct work_struct *work)
 {
 	struct scsi_disk *sdkp;
@@ -585,7 +620,7 @@ static int sd_zbc_check_zoned_characteristics(struct scsi_disk *sdkp,
  * sd_zbc_check_capacity - Check the device capacity
  * @sdkp: Target disk
  * @buf: command buffer
- * @zblocks: zone size in number of blocks
+ * @zblocks: zone size in logical blocks
  *
  * Get the device zone size and check that the device capacity as reported
  * by READ CAPACITY matches the max_lba value (plus one) of the report zones
@@ -696,6 +731,11 @@ static void sd_zbc_revalidate_zones_cb(struct gendisk *disk)
 	swap(sdkp->zones_wp_offset, sdkp->rev_wp_offset);
 }
 
+/*
+ * Call blk_revalidate_disk_zones() if any of the zoned disk properties have
+ * changed that make it necessary to call that function. Called by
+ * sd_revalidate_disk() after the gendisk capacity has been set.
+ */
 int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)
 {
 	struct gendisk *disk = sdkp->disk;
@@ -774,7 +814,16 @@ int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)
 	return ret;
 }
 
-int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned char *buf)
+/**
+ * sd_zbc_read_zones - Read zone information and update the request queue
+ * @sdkp: SCSI disk pointer.
+ * @buf: 512 byte buffer used for storing SCSI command output.
+ *
+ * Read zone information and update the request queue zone characteristics and
+ * also the zoned device information in *sdkp. Called by sd_revalidate_disk()
+ * before the gendisk capacity has been set.
+ */
+int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE])
 {
 	struct gendisk *disk = sdkp->disk;
 	struct request_queue *q = disk->queue;

Re: [PATCH v2 1/9] scsi: sd_zbc: Improve source code documentation

[Himanshu Madhani]

> On Apr 21, 2022, at 11:30 AM, Bart Van Assche <bvanassche@acm.org> wrote:
> 
> Add several kernel-doc headers. Declare input arrays const. Specify the
> array size in function declarations.
> 
> Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/sd.h     |  5 ++--
> drivers/scsi/sd_zbc.c | 55 ++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 54 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 0a33a4b68ffb..4849cbe771a7 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -222,7 +222,7 @@ static inline int sd_is_zoned(struct scsi_disk *sdkp)
> #ifdef CONFIG_BLK_DEV_ZONED
> 
> void sd_zbc_release_disk(struct scsi_disk *sdkp);
> -int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned char *buffer);
> +int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE]);
> int sd_zbc_revalidate_zones(struct scsi_disk *sdkp);
> blk_status_t sd_zbc_setup_zone_mgmt_cmnd(struct scsi_cmnd *cmd,
> 					 unsigned char op, bool all);
> @@ -238,8 +238,7 @@ blk_status_t sd_zbc_prepare_zone_append(struct scsi_cmnd *cmd, sector_t *lba,
> 
> static inline void sd_zbc_release_disk(struct scsi_disk *sdkp) {}
> 
> -static inline int sd_zbc_read_zones(struct scsi_disk *sdkp,
> -				    unsigned char *buf)
> +static inline int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE])
> {
> 	return 0;
> }
> diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
> index 7f466280993b..2ae44bc52a5f 100644
> --- a/drivers/scsi/sd_zbc.c
> +++ b/drivers/scsi/sd_zbc.c
> @@ -20,6 +20,12 @@
> 
> #include "sd.h"
> 
> +/**
> + * sd_zbc_get_zone_wp_offset - Get zone write pointer offset.
> + * @zone: Zone for which to return the write pointer offset.
> + *
> + * Return: offset of the write pointer from the start of the zone.
> + */
> static unsigned int sd_zbc_get_zone_wp_offset(struct blk_zone *zone)
> {
> 	if (zone->type == ZBC_ZONE_TYPE_CONV)
> @@ -44,7 +50,21 @@ static unsigned int sd_zbc_get_zone_wp_offset(struct blk_zone *zone)
> 	}
> }
> 
> -static int sd_zbc_parse_report(struct scsi_disk *sdkp, u8 *buf,
> +/**
> + * sd_zbc_parse_report - Parse a SCSI zone descriptor
> + * @sdkp: SCSI disk pointer.
> + * @buf: SCSI zone descriptor.
> + * @idx: Index of the zone relative to the first zone reported by the current
> + *	sd_zbc_report_zones() call.
> + * @cb: Callback function pointer.
> + * @data: Second argument passed to @cb.
> + *
> + * Return: Value returned by @cb.
> + *
> + * Convert a SCSI zone descriptor into struct blk_zone format. Additionally,
> + * call @cb(blk_zone, @data).
> + */
> +static int sd_zbc_parse_report(struct scsi_disk *sdkp, const u8 buf[64],
> 			       unsigned int idx, report_zones_cb cb, void *data)
> {
> 	struct scsi_device *sdp = sdkp->device;
> @@ -189,6 +209,17 @@ static inline sector_t sd_zbc_zone_sectors(struct scsi_disk *sdkp)
> 	return logical_to_sectors(sdkp->device, sdkp->zone_blocks);
> }
> 
> +/**
> + * sd_zbc_report_zones - SCSI .report_zones() callback.
> + * @disk: Disk to report zones for.
> + * @sector: Start sector.
> + * @nr_zones: Maximum number of zones to report.
> + * @cb: Callback function called to report zone information.
> + * @data: Second argument passed to @cb.
> + *
> + * Called by the block layer to iterate over zone information. See also the
> + * disk->fops->report_zones() calls in block/blk-zoned.c.
> + */
> int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
> 			unsigned int nr_zones, report_zones_cb cb, void *data)
> {
> @@ -276,6 +307,10 @@ static int sd_zbc_update_wp_offset_cb(struct blk_zone *zone, unsigned int idx,
> 	return 0;
> }
> 
> +/*
> + * An attempt to append a zone triggered an invalid write pointer error.
> + * Reread the write pointer of the zone(s) in which the append failed.
> + */
> static void sd_zbc_update_wp_offset_workfn(struct work_struct *work)
> {
> 	struct scsi_disk *sdkp;
> @@ -585,7 +620,7 @@ static int sd_zbc_check_zoned_characteristics(struct scsi_disk *sdkp,
>  * sd_zbc_check_capacity - Check the device capacity
>  * @sdkp: Target disk
>  * @buf: command buffer
> - * @zblocks: zone size in number of blocks
> + * @zblocks: zone size in logical blocks
>  *
>  * Get the device zone size and check that the device capacity as reported
>  * by READ CAPACITY matches the max_lba value (plus one) of the report zones
> @@ -696,6 +731,11 @@ static void sd_zbc_revalidate_zones_cb(struct gendisk *disk)
> 	swap(sdkp->zones_wp_offset, sdkp->rev_wp_offset);
> }
> 
> +/*
> + * Call blk_revalidate_disk_zones() if any of the zoned disk properties have
> + * changed that make it necessary to call that function. Called by
> + * sd_revalidate_disk() after the gendisk capacity has been set.
> + */
> int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)
> {
> 	struct gendisk *disk = sdkp->disk;
> @@ -774,7 +814,16 @@ int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)
> 	return ret;
> }
> 
> -int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned char *buf)
> +/**
> + * sd_zbc_read_zones - Read zone information and update the request queue
> + * @sdkp: SCSI disk pointer.
> + * @buf: 512 byte buffer used for storing SCSI command output.
> + *
> + * Read zone information and update the request queue zone characteristics and
> + * also the zoned device information in *sdkp. Called by sd_revalidate_disk()
> + * before the gendisk capacity has been set.
> + */
> +int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE])
> {
> 	struct gendisk *disk = sdkp->disk;
> 	struct request_queue *q = disk->queue;


Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	Oracle Linux Engineering

[PATCH v2 2/9] scsi: sd_zbc: Verify that the zone size is a power of two

[Bart Van Assche]

The following check in sd_zbc_cmnd_checks() can only work correctly if
the zone size is a power of two:

	if (sector & (sd_zbc_zone_sectors(sdkp) - 1))
		/* Unaligned request */
		return BLK_STS_IOERR;

Hence this patch that verifies that the zone size is a power of two.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/sd_zbc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 2ae44bc52a5f..9ef5ad345185 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -664,6 +664,13 @@ static int sd_zbc_check_capacity(struct scsi_disk *sdkp, unsigned char *buf,
 		return -EFBIG;
 	}
 
+	if (!is_power_of_2(zone_blocks)) {
+		sd_printk(KERN_ERR, sdkp,
+			  "Zone size %llu is not a power of two.\n",
+			  zone_blocks);
+		return -EINVAL;
+	}
+
 	*zblocks = zone_blocks;
 
 	return 0;

Re: [PATCH v2 2/9] scsi: sd_zbc: Verify that the zone size is a power of two

[Himanshu Madhani]

> On Apr 21, 2022, at 11:30 AM, Bart Van Assche <bvanassche@acm.org> wrote:
> 
> The following check in sd_zbc_cmnd_checks() can only work correctly if
> the zone size is a power of two:
> 
> 	if (sector & (sd_zbc_zone_sectors(sdkp) - 1))
> 		/* Unaligned request */
> 		return BLK_STS_IOERR;
> 
> Hence this patch that verifies that the zone size is a power of two.
> 
> Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/sd_zbc.c | 7 +++++++
> 1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
> index 2ae44bc52a5f..9ef5ad345185 100644
> --- a/drivers/scsi/sd_zbc.c
> +++ b/drivers/scsi/sd_zbc.c
> @@ -664,6 +664,13 @@ static int sd_zbc_check_capacity(struct scsi_disk *sdkp, unsigned char *buf,
> 		return -EFBIG;
> 	}
> 
> +	if (!is_power_of_2(zone_blocks)) {
> +		sd_printk(KERN_ERR, sdkp,
> +			  "Zone size %llu is not a power of two.\n",
> +			  zone_blocks);
> +		return -EINVAL;
> +	}
> +
> 	*zblocks = zone_blocks;
> 
> 	return 0;

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	Oracle Linux Engineering

[PATCH v2 3/9] scsi: sd_zbc: Use logical blocks as unit when querying zones

[Bart Van Assche]

When querying zones, track the position in logical blocks instead of in
sectors. This change slightly simplifies sd_zbc_report_zones().

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
[ bvanassche: extracted this change from a larger patch ]
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/sd_zbc.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 9ef5ad345185..e76bcbfd0d1c 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -224,7 +224,7 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
 			unsigned int nr_zones, report_zones_cb cb, void *data)
 {
 	struct scsi_disk *sdkp = scsi_disk(disk);
-	sector_t capacity = logical_to_sectors(sdkp->device, sdkp->capacity);
+	sector_t lba = sectors_to_logical(sdkp->device, sector);
 	unsigned int nr, i;
 	unsigned char *buf;
 	size_t offset, buflen = 0;
@@ -235,7 +235,7 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
 		/* Not a zoned device */
 		return -EOPNOTSUPP;
 
-	if (!capacity)
+	if (!sdkp->capacity)
 		/* Device gone or invalid */
 		return -ENODEV;
 
@@ -243,9 +243,8 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
 	if (!buf)
 		return -ENOMEM;
 
-	while (zone_idx < nr_zones && sector < capacity) {
-		ret = sd_zbc_do_report_zones(sdkp, buf, buflen,
-				sectors_to_logical(sdkp->device, sector), true);
+	while (zone_idx < nr_zones && lba < sdkp->capacity) {
+		ret = sd_zbc_do_report_zones(sdkp, buf, buflen, lba, true);
 		if (ret)
 			goto out;
 
@@ -263,7 +262,7 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
 			zone_idx++;
 		}
 
-		sector += sd_zbc_zone_sectors(sdkp) * i;
+		lba += sdkp->zone_blocks * i;
 	}
 
 	ret = zone_idx;

Re: [PATCH v2 3/9] scsi: sd_zbc: Use logical blocks as unit when querying zones

[Himanshu Madhani]

> On Apr 21, 2022, at 11:30 AM, Bart Van Assche <bvanassche@acm.org> wrote:
> 
> When querying zones, track the position in logical blocks instead of in
> sectors. This change slightly simplifies sd_zbc_report_zones().
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> [ bvanassche: extracted this change from a larger patch ]
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/sd_zbc.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
> index 9ef5ad345185..e76bcbfd0d1c 100644
> --- a/drivers/scsi/sd_zbc.c
> +++ b/drivers/scsi/sd_zbc.c
> @@ -224,7 +224,7 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
> 			unsigned int nr_zones, report_zones_cb cb, void *data)
> {
> 	struct scsi_disk *sdkp = scsi_disk(disk);
> -	sector_t capacity = logical_to_sectors(sdkp->device, sdkp->capacity);
> +	sector_t lba = sectors_to_logical(sdkp->device, sector);
> 	unsigned int nr, i;
> 	unsigned char *buf;
> 	size_t offset, buflen = 0;
> @@ -235,7 +235,7 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
> 		/* Not a zoned device */
> 		return -EOPNOTSUPP;
> 
> -	if (!capacity)
> +	if (!sdkp->capacity)
> 		/* Device gone or invalid */
> 		return -ENODEV;
> 
> @@ -243,9 +243,8 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
> 	if (!buf)
> 		return -ENOMEM;
> 
> -	while (zone_idx < nr_zones && sector < capacity) {
> -		ret = sd_zbc_do_report_zones(sdkp, buf, buflen,
> -				sectors_to_logical(sdkp->device, sector), true);
> +	while (zone_idx < nr_zones && lba < sdkp->capacity) {
> +		ret = sd_zbc_do_report_zones(sdkp, buf, buflen, lba, true);
> 		if (ret)
> 			goto out;
> 
> @@ -263,7 +262,7 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
> 			zone_idx++;
> 		}
> 
> -		sector += sd_zbc_zone_sectors(sdkp) * i;
> +		lba += sdkp->zone_blocks * i;
> 	}
> 
> 	ret = zone_idx;

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	Oracle Linux Engineering

[PATCH v2 4/9] scsi: sd_zbc: Introduce struct zoned_disk_info

[Bart Van Assche]

Deriving the meaning of the nr_zones, rev_nr_zones, zone_blocks and
rev_zone_blocks member variables requires careful analysis of the source
code. Make the meaning of these member variables easier to understand by
introducing struct zoned_disk_info.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/sd.h     | 22 +++++++++++++++----
 drivers/scsi/sd_zbc.c | 49 ++++++++++++++++++++-----------------------
 2 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 4849cbe771a7..47434f905b0a 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -67,6 +67,20 @@ enum {
 	SD_ZERO_WS10_UNMAP,	/* Use WRITE SAME(10) with UNMAP */
 };
 
+/**
+ * struct zoned_disk_info - Specific properties of a ZBC SCSI device.
+ * @nr_zones: number of zones.
+ * @zone_blocks: number of logical blocks per zone.
+ *
+ * This data structure holds the ZBC SCSI device properties that are retrieved
+ * twice: a first time before the gendisk capacity is known and a second time
+ * after the gendisk capacity is known.
+ */
+struct zoned_disk_info {
+	u32		nr_zones;
+	u32		zone_blocks;
+};
+
 struct scsi_disk {
 	struct scsi_device *device;
 
@@ -78,10 +92,10 @@ struct scsi_disk {
 	struct gendisk	*disk;
 	struct opal_dev *opal_dev;
 #ifdef CONFIG_BLK_DEV_ZONED
-	u32		nr_zones;
-	u32		rev_nr_zones;
-	u32		zone_blocks;
-	u32		rev_zone_blocks;
+	/* Updated during revalidation before the gendisk capacity is known. */
+	struct zoned_disk_info	early_zone_info;
+	/* Updated during revalidation after the gendisk capacity is known. */
+	struct zoned_disk_info	zone_info;
 	u32		zones_optimal_open;
 	u32		zones_optimal_nonseq;
 	u32		zones_max_open;
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index e76bcbfd0d1c..ac557a5a65c8 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -181,7 +181,7 @@ static void *sd_zbc_alloc_report_buffer(struct scsi_disk *sdkp,
 	 * sure that the allocated buffer can always be mapped by limiting the
 	 * number of pages allocated to the HBA max segments limit.
 	 */
-	nr_zones = min(nr_zones, sdkp->nr_zones);
+	nr_zones = min(nr_zones, sdkp->zone_info.nr_zones);
 	bufsize = roundup((nr_zones + 1) * 64, SECTOR_SIZE);
 	bufsize = min_t(size_t, bufsize,
 			queue_max_hw_sectors(q) << SECTOR_SHIFT);
@@ -206,7 +206,7 @@ static void *sd_zbc_alloc_report_buffer(struct scsi_disk *sdkp,
  */
 static inline sector_t sd_zbc_zone_sectors(struct scsi_disk *sdkp)
 {
-	return logical_to_sectors(sdkp->device, sdkp->zone_blocks);
+	return logical_to_sectors(sdkp->device, sdkp->zone_info.zone_blocks);
 }
 
 /**
@@ -262,7 +262,7 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
 			zone_idx++;
 		}
 
-		lba += sdkp->zone_blocks * i;
+		lba += sdkp->zone_info.zone_blocks * i;
 	}
 
 	ret = zone_idx;
@@ -320,14 +320,14 @@ static void sd_zbc_update_wp_offset_workfn(struct work_struct *work)
 	sdkp = container_of(work, struct scsi_disk, zone_wp_offset_work);
 
 	spin_lock_irqsave(&sdkp->zones_wp_offset_lock, flags);
-	for (zno = 0; zno < sdkp->nr_zones; zno++) {
+	for (zno = 0; zno < sdkp->zone_info.nr_zones; zno++) {
 		if (sdkp->zones_wp_offset[zno] != SD_ZBC_UPDATING_WP_OFST)
 			continue;
 
 		spin_unlock_irqrestore(&sdkp->zones_wp_offset_lock, flags);
 		ret = sd_zbc_do_report_zones(sdkp, sdkp->zone_wp_update_buf,
 					     SD_BUF_SIZE,
-					     zno * sdkp->zone_blocks, true);
+					     zno * sdkp->zone_info.zone_blocks, true);
 		spin_lock_irqsave(&sdkp->zones_wp_offset_lock, flags);
 		if (!ret)
 			sd_zbc_parse_report(sdkp, sdkp->zone_wp_update_buf + 64,
@@ -394,7 +394,7 @@ blk_status_t sd_zbc_prepare_zone_append(struct scsi_cmnd *cmd, sector_t *lba,
 		break;
 	default:
 		wp_offset = sectors_to_logical(sdkp->device, wp_offset);
-		if (wp_offset + nr_blocks > sdkp->zone_blocks) {
+		if (wp_offset + nr_blocks > sdkp->zone_info.zone_blocks) {
 			ret = BLK_STS_IOERR;
 			break;
 		}
@@ -523,7 +523,7 @@ static unsigned int sd_zbc_zone_wp_update(struct scsi_cmnd *cmd,
 		break;
 	case REQ_OP_ZONE_RESET_ALL:
 		memset(sdkp->zones_wp_offset, 0,
-		       sdkp->nr_zones * sizeof(unsigned int));
+		       sdkp->zone_info.nr_zones * sizeof(unsigned int));
 		break;
 	default:
 		break;
@@ -680,16 +680,16 @@ static void sd_zbc_print_zones(struct scsi_disk *sdkp)
 	if (!sd_is_zoned(sdkp) || !sdkp->capacity)
 		return;
 
-	if (sdkp->capacity & (sdkp->zone_blocks - 1))
+	if (sdkp->capacity & (sdkp->zone_info.zone_blocks - 1))
 		sd_printk(KERN_NOTICE, sdkp,
 			  "%u zones of %u logical blocks + 1 runt zone\n",
-			  sdkp->nr_zones - 1,
-			  sdkp->zone_blocks);
+			  sdkp->zone_info.nr_zones - 1,
+			  sdkp->zone_info.zone_blocks);
 	else
 		sd_printk(KERN_NOTICE, sdkp,
 			  "%u zones of %u logical blocks\n",
-			  sdkp->nr_zones,
-			  sdkp->zone_blocks);
+			  sdkp->zone_info.nr_zones,
+			  sdkp->zone_info.zone_blocks);
 }
 
 static int sd_zbc_init_disk(struct scsi_disk *sdkp)
@@ -716,10 +716,8 @@ static void sd_zbc_clear_zone_info(struct scsi_disk *sdkp)
 	kfree(sdkp->zone_wp_update_buf);
 	sdkp->zone_wp_update_buf = NULL;
 
-	sdkp->nr_zones = 0;
-	sdkp->rev_nr_zones = 0;
-	sdkp->zone_blocks = 0;
-	sdkp->rev_zone_blocks = 0;
+	sdkp->early_zone_info = (struct zoned_disk_info){ };
+	sdkp->zone_info = (struct zoned_disk_info){ };
 
 	mutex_unlock(&sdkp->rev_mutex);
 }
@@ -746,8 +744,8 @@ int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)
 {
 	struct gendisk *disk = sdkp->disk;
 	struct request_queue *q = disk->queue;
-	u32 zone_blocks = sdkp->rev_zone_blocks;
-	unsigned int nr_zones = sdkp->rev_nr_zones;
+	u32 zone_blocks = sdkp->early_zone_info.zone_blocks;
+	unsigned int nr_zones = sdkp->early_zone_info.nr_zones;
 	u32 max_append;
 	int ret = 0;
 	unsigned int flags;
@@ -778,14 +776,14 @@ int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)
 	 */
 	mutex_lock(&sdkp->rev_mutex);
 
-	if (sdkp->zone_blocks == zone_blocks &&
-	    sdkp->nr_zones == nr_zones &&
+	if (sdkp->zone_info.zone_blocks == zone_blocks &&
+	    sdkp->zone_info.nr_zones == nr_zones &&
 	    disk->queue->nr_zones == nr_zones)
 		goto unlock;
 
 	flags = memalloc_noio_save();
-	sdkp->zone_blocks = zone_blocks;
-	sdkp->nr_zones = nr_zones;
+	sdkp->zone_info.zone_blocks = zone_blocks;
+	sdkp->zone_info.nr_zones = nr_zones;
 	sdkp->rev_wp_offset = kvcalloc(nr_zones, sizeof(u32), GFP_KERNEL);
 	if (!sdkp->rev_wp_offset) {
 		ret = -ENOMEM;
@@ -800,8 +798,7 @@ int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)
 	sdkp->rev_wp_offset = NULL;
 
 	if (ret) {
-		sdkp->zone_blocks = 0;
-		sdkp->nr_zones = 0;
+		sdkp->zone_info = (struct zoned_disk_info){ };
 		sdkp->capacity = 0;
 		goto unlock;
 	}
@@ -887,8 +884,8 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE])
 	if (blk_queue_zoned_model(q) == BLK_ZONED_HM)
 		blk_queue_zone_write_granularity(q, sdkp->physical_block_size);
 
-	sdkp->rev_nr_zones = nr_zones;
-	sdkp->rev_zone_blocks = zone_blocks;
+	sdkp->early_zone_info.nr_zones = nr_zones;
+	sdkp->early_zone_info.zone_blocks = zone_blocks;
 
 	return 0;
 

Re: [PATCH v2 4/9] scsi: sd_zbc: Introduce struct zoned_disk_info

[Himanshu Madhani]

> On Apr 21, 2022, at 11:30 AM, Bart Van Assche <bvanassche@acm.org> wrote:
> 
> Deriving the meaning of the nr_zones, rev_nr_zones, zone_blocks and
> rev_zone_blocks member variables requires careful analysis of the source
> code. Make the meaning of these member variables easier to understand by
> introducing struct zoned_disk_info.
> 
> Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/sd.h     | 22 +++++++++++++++----
> drivers/scsi/sd_zbc.c | 49 ++++++++++++++++++++-----------------------
> 2 files changed, 41 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 4849cbe771a7..47434f905b0a 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -67,6 +67,20 @@ enum {
> 	SD_ZERO_WS10_UNMAP,	/* Use WRITE SAME(10) with UNMAP */
> };
> 
> +/**
> + * struct zoned_disk_info - Specific properties of a ZBC SCSI device.
> + * @nr_zones: number of zones.
> + * @zone_blocks: number of logical blocks per zone.
> + *
> + * This data structure holds the ZBC SCSI device properties that are retrieved
> + * twice: a first time before the gendisk capacity is known and a second time
> + * after the gendisk capacity is known.
> + */
> +struct zoned_disk_info {
> +	u32		nr_zones;
> +	u32		zone_blocks;
> +};
> +
> struct scsi_disk {
> 	struct scsi_device *device;
> 
> @@ -78,10 +92,10 @@ struct scsi_disk {
> 	struct gendisk	*disk;
> 	struct opal_dev *opal_dev;
> #ifdef CONFIG_BLK_DEV_ZONED
> -	u32		nr_zones;
> -	u32		rev_nr_zones;
> -	u32		zone_blocks;
> -	u32		rev_zone_blocks;
> +	/* Updated during revalidation before the gendisk capacity is known. */
> +	struct zoned_disk_info	early_zone_info;
> +	/* Updated during revalidation after the gendisk capacity is known. */
> +	struct zoned_disk_info	zone_info;
> 	u32		zones_optimal_open;
> 	u32		zones_optimal_nonseq;
> 	u32		zones_max_open;
> diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
> index e76bcbfd0d1c..ac557a5a65c8 100644
> --- a/drivers/scsi/sd_zbc.c
> +++ b/drivers/scsi/sd_zbc.c
> @@ -181,7 +181,7 @@ static void *sd_zbc_alloc_report_buffer(struct scsi_disk *sdkp,
> 	 * sure that the allocated buffer can always be mapped by limiting the
> 	 * number of pages allocated to the HBA max segments limit.
> 	 */
> -	nr_zones = min(nr_zones, sdkp->nr_zones);
> +	nr_zones = min(nr_zones, sdkp->zone_info.nr_zones);
> 	bufsize = roundup((nr_zones + 1) * 64, SECTOR_SIZE);
> 	bufsize = min_t(size_t, bufsize,
> 			queue_max_hw_sectors(q) << SECTOR_SHIFT);
> @@ -206,7 +206,7 @@ static void *sd_zbc_alloc_report_buffer(struct scsi_disk *sdkp,
>  */
> static inline sector_t sd_zbc_zone_sectors(struct scsi_disk *sdkp)
> {
> -	return logical_to_sectors(sdkp->device, sdkp->zone_blocks);
> +	return logical_to_sectors(sdkp->device, sdkp->zone_info.zone_blocks);
> }
> 
> /**
> @@ -262,7 +262,7 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
> 			zone_idx++;
> 		}
> 
> -		lba += sdkp->zone_blocks * i;
> +		lba += sdkp->zone_info.zone_blocks * i;
> 	}
> 
> 	ret = zone_idx;
> @@ -320,14 +320,14 @@ static void sd_zbc_update_wp_offset_workfn(struct work_struct *work)
> 	sdkp = container_of(work, struct scsi_disk, zone_wp_offset_work);
> 
> 	spin_lock_irqsave(&sdkp->zones_wp_offset_lock, flags);
> -	for (zno = 0; zno < sdkp->nr_zones; zno++) {
> +	for (zno = 0; zno < sdkp->zone_info.nr_zones; zno++) {
> 		if (sdkp->zones_wp_offset[zno] != SD_ZBC_UPDATING_WP_OFST)
> 			continue;
> 
> 		spin_unlock_irqrestore(&sdkp->zones_wp_offset_lock, flags);
> 		ret = sd_zbc_do_report_zones(sdkp, sdkp->zone_wp_update_buf,
> 					     SD_BUF_SIZE,
> -					     zno * sdkp->zone_blocks, true);
> +					     zno * sdkp->zone_info.zone_blocks, true);
> 		spin_lock_irqsave(&sdkp->zones_wp_offset_lock, flags);
> 		if (!ret)
> 			sd_zbc_parse_report(sdkp, sdkp->zone_wp_update_buf + 64,
> @@ -394,7 +394,7 @@ blk_status_t sd_zbc_prepare_zone_append(struct scsi_cmnd *cmd, sector_t *lba,
> 		break;
> 	default:
> 		wp_offset = sectors_to_logical(sdkp->device, wp_offset);
> -		if (wp_offset + nr_blocks > sdkp->zone_blocks) {
> +		if (wp_offset + nr_blocks > sdkp->zone_info.zone_blocks) {
> 			ret = BLK_STS_IOERR;
> 			break;
> 		}
> @@ -523,7 +523,7 @@ static unsigned int sd_zbc_zone_wp_update(struct scsi_cmnd *cmd,
> 		break;
> 	case REQ_OP_ZONE_RESET_ALL:
> 		memset(sdkp->zones_wp_offset, 0,
> -		       sdkp->nr_zones * sizeof(unsigned int));
> +		       sdkp->zone_info.nr_zones * sizeof(unsigned int));
> 		break;
> 	default:
> 		break;
> @@ -680,16 +680,16 @@ static void sd_zbc_print_zones(struct scsi_disk *sdkp)
> 	if (!sd_is_zoned(sdkp) || !sdkp->capacity)
> 		return;
> 
> -	if (sdkp->capacity & (sdkp->zone_blocks - 1))
> +	if (sdkp->capacity & (sdkp->zone_info.zone_blocks - 1))
> 		sd_printk(KERN_NOTICE, sdkp,
> 			  "%u zones of %u logical blocks + 1 runt zone\n",
> -			  sdkp->nr_zones - 1,
> -			  sdkp->zone_blocks);
> +			  sdkp->zone_info.nr_zones - 1,
> +			  sdkp->zone_info.zone_blocks);
> 	else
> 		sd_printk(KERN_NOTICE, sdkp,
> 			  "%u zones of %u logical blocks\n",
> -			  sdkp->nr_zones,
> -			  sdkp->zone_blocks);
> +			  sdkp->zone_info.nr_zones,
> +			  sdkp->zone_info.zone_blocks);
> }
> 
> static int sd_zbc_init_disk(struct scsi_disk *sdkp)
> @@ -716,10 +716,8 @@ static void sd_zbc_clear_zone_info(struct scsi_disk *sdkp)
> 	kfree(sdkp->zone_wp_update_buf);
> 	sdkp->zone_wp_update_buf = NULL;
> 
> -	sdkp->nr_zones = 0;
> -	sdkp->rev_nr_zones = 0;
> -	sdkp->zone_blocks = 0;
> -	sdkp->rev_zone_blocks = 0;
> +	sdkp->early_zone_info = (struct zoned_disk_info){ };
> +	sdkp->zone_info = (struct zoned_disk_info){ };
> 
> 	mutex_unlock(&sdkp->rev_mutex);
> }
> @@ -746,8 +744,8 @@ int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)
> {
> 	struct gendisk *disk = sdkp->disk;
> 	struct request_queue *q = disk->queue;
> -	u32 zone_blocks = sdkp->rev_zone_blocks;
> -	unsigned int nr_zones = sdkp->rev_nr_zones;
> +	u32 zone_blocks = sdkp->early_zone_info.zone_blocks;
> +	unsigned int nr_zones = sdkp->early_zone_info.nr_zones;
> 	u32 max_append;
> 	int ret = 0;
> 	unsigned int flags;
> @@ -778,14 +776,14 @@ int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)
> 	 */
> 	mutex_lock(&sdkp->rev_mutex);
> 
> -	if (sdkp->zone_blocks == zone_blocks &&
> -	    sdkp->nr_zones == nr_zones &&
> +	if (sdkp->zone_info.zone_blocks == zone_blocks &&
> +	    sdkp->zone_info.nr_zones == nr_zones &&
> 	    disk->queue->nr_zones == nr_zones)
> 		goto unlock;
> 
> 	flags = memalloc_noio_save();
> -	sdkp->zone_blocks = zone_blocks;
> -	sdkp->nr_zones = nr_zones;
> +	sdkp->zone_info.zone_blocks = zone_blocks;
> +	sdkp->zone_info.nr_zones = nr_zones;
> 	sdkp->rev_wp_offset = kvcalloc(nr_zones, sizeof(u32), GFP_KERNEL);
> 	if (!sdkp->rev_wp_offset) {
> 		ret = -ENOMEM;
> @@ -800,8 +798,7 @@ int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)
> 	sdkp->rev_wp_offset = NULL;
> 
> 	if (ret) {
> -		sdkp->zone_blocks = 0;
> -		sdkp->nr_zones = 0;
> +		sdkp->zone_info = (struct zoned_disk_info){ };
> 		sdkp->capacity = 0;
> 		goto unlock;
> 	}
> @@ -887,8 +884,8 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE])
> 	if (blk_queue_zoned_model(q) == BLK_ZONED_HM)
> 		blk_queue_zone_write_granularity(q, sdkp->physical_block_size);
> 
> -	sdkp->rev_nr_zones = nr_zones;
> -	sdkp->rev_zone_blocks = zone_blocks;
> +	sdkp->early_zone_info.nr_zones = nr_zones;
> +	sdkp->early_zone_info.zone_blocks = zone_blocks;
> 
> 	return 0;
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	Oracle Linux Engineering

[PATCH v2 5/9] scsi: sd_zbc: Return early in sd_zbc_check_zoned_characteristics()

[Bart Van Assche]

Return early in sd_zbc_check_zoned_characteristics() for host-aware
disks. This patch does not change any functionality but makes a later
patch easier to read.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
[ bvanassche: extracted this change from a larger patch ]
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/sd_zbc.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index ac557a5a65c8..c53e166362b9 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -592,14 +592,15 @@ static int sd_zbc_check_zoned_characteristics(struct scsi_disk *sdkp,
 		sdkp->zones_optimal_open = get_unaligned_be32(&buf[8]);
 		sdkp->zones_optimal_nonseq = get_unaligned_be32(&buf[12]);
 		sdkp->zones_max_open = 0;
-	} else {
-		/* Host-managed */
-		sdkp->urswrz = buf[4] & 1;
-		sdkp->zones_optimal_open = 0;
-		sdkp->zones_optimal_nonseq = 0;
-		sdkp->zones_max_open = get_unaligned_be32(&buf[16]);
+		return 0;
 	}
 
+	/* Host-managed */
+	sdkp->urswrz = buf[4] & 1;
+	sdkp->zones_optimal_open = 0;
+	sdkp->zones_optimal_nonseq = 0;
+	sdkp->zones_max_open = get_unaligned_be32(&buf[16]);
+
 	/*
 	 * Check for unconstrained reads: host-managed devices with
 	 * constrained reads (drives failing read after write pointer)

Re: [PATCH v2 5/9] scsi: sd_zbc: Return early in sd_zbc_check_zoned_characteristics()

[Himanshu Madhani]

> On Apr 21, 2022, at 11:30 AM, Bart Van Assche <bvanassche@acm.org> wrote:
> 
> Return early in sd_zbc_check_zoned_characteristics() for host-aware
> disks. This patch does not change any functionality but makes a later
> patch easier to read.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> [ bvanassche: extracted this change from a larger patch ]
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/sd_zbc.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
> index ac557a5a65c8..c53e166362b9 100644
> --- a/drivers/scsi/sd_zbc.c
> +++ b/drivers/scsi/sd_zbc.c
> @@ -592,14 +592,15 @@ static int sd_zbc_check_zoned_characteristics(struct scsi_disk *sdkp,
> 		sdkp->zones_optimal_open = get_unaligned_be32(&buf[8]);
> 		sdkp->zones_optimal_nonseq = get_unaligned_be32(&buf[12]);
> 		sdkp->zones_max_open = 0;
> -	} else {
> -		/* Host-managed */
> -		sdkp->urswrz = buf[4] & 1;
> -		sdkp->zones_optimal_open = 0;
> -		sdkp->zones_optimal_nonseq = 0;
> -		sdkp->zones_max_open = get_unaligned_be32(&buf[16]);
> +		return 0;
> 	}
> 
> +	/* Host-managed */
> +	sdkp->urswrz = buf[4] & 1;
> +	sdkp->zones_optimal_open = 0;
> +	sdkp->zones_optimal_nonseq = 0;
> +	sdkp->zones_max_open = get_unaligned_be32(&buf[16]);
> +
> 	/*
> 	 * Check for unconstrained reads: host-managed devices with
> 	 * constrained reads (drives failing read after write pointer)


Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	Oracle Linux Engineering

[PATCH v2 6/9] scsi: sd_zbc: Hide gap zones

[Bart Van Assche]

ZBC-2 allows host-managed disks to report gap zones. This allow zoned disks
to report an offset between data zone starts that is a power of two even if
the number of logical blocks with data per zone is not a power of two.

Another new feature in ZBC-2 is support for constant zone starting LBA
offsets. For zoned disks that report a constant zone starting LBA offset,
hide the gap zones from the block layer. Report the offset between data
zone starts as zone size and report the number of logical blocks with data
per zone as the zone capacity.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
[ bvanassche: Reworked this patch ]
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/sd.h         |   5 ++
 drivers/scsi/sd_zbc.c     | 105 +++++++++++++++++++++++++++++++++-----
 include/scsi/scsi_proto.h |   9 +++-
 3 files changed, 105 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 47434f905b0a..d249933ba69e 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -99,6 +99,11 @@ struct scsi_disk {
 	u32		zones_optimal_open;
 	u32		zones_optimal_nonseq;
 	u32		zones_max_open;
+	/*
+	 * Either zero or a power of two. If not zero it means that the offset
+	 * between zone starting LBAs is constant.
+	 */
+	u32		zone_starting_lba_gran;
 	u32		*zones_wp_offset;
 	spinlock_t	zones_wp_offset_lock;
 	u32		*rev_wp_offset;
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index c53e166362b9..5b9fad70aa88 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -50,6 +50,12 @@ static unsigned int sd_zbc_get_zone_wp_offset(struct blk_zone *zone)
 	}
 }
 
+/* Whether or not a SCSI zone descriptor describes a gap zone. */
+static bool sd_zbc_is_gap_zone(const u8 buf[64])
+{
+	return (buf[0] & 0xf) == ZBC_ZONE_TYPE_GAP;
+}
+
 /**
  * sd_zbc_parse_report - Parse a SCSI zone descriptor
  * @sdkp: SCSI disk pointer.
@@ -69,8 +75,12 @@ static int sd_zbc_parse_report(struct scsi_disk *sdkp, const u8 buf[64],
 {
 	struct scsi_device *sdp = sdkp->device;
 	struct blk_zone zone = { 0 };
+	sector_t start_lba, gran;
 	int ret;
 
+	if (WARN_ON_ONCE(sd_zbc_is_gap_zone(buf)))
+		return -EINVAL;
+
 	zone.type = buf[0] & 0x0f;
 	zone.cond = (buf[1] >> 4) & 0xf;
 	if (buf[1] & 0x01)
@@ -78,9 +88,27 @@ static int sd_zbc_parse_report(struct scsi_disk *sdkp, const u8 buf[64],
 	if (buf[1] & 0x02)
 		zone.non_seq = 1;
 
-	zone.len = logical_to_sectors(sdp, get_unaligned_be64(&buf[8]));
-	zone.capacity = zone.len;
-	zone.start = logical_to_sectors(sdp, get_unaligned_be64(&buf[16]));
+	start_lba = get_unaligned_be64(&buf[16]);
+	zone.start = logical_to_sectors(sdp, start_lba);
+	zone.capacity = logical_to_sectors(sdp, get_unaligned_be64(&buf[8]));
+	zone.len = zone.capacity;
+	if (sdkp->zone_starting_lba_gran) {
+		gran = logical_to_sectors(sdp, sdkp->zone_starting_lba_gran);
+		if (zone.len > gran) {
+			sd_printk(KERN_ERR, sdkp,
+				  "Invalid zone at LBA %llu with capacity %llu and length %llu; granularity = %llu\n",
+				  start_lba,
+				  sectors_to_logical(sdp, zone.capacity),
+				  sectors_to_logical(sdp, zone.len),
+				  sectors_to_logical(sdp, gran));
+			return -EINVAL;
+		}
+		/*
+		 * Use the starting LBA granularity instead of the zone length
+		 * obtained from the REPORT ZONES command.
+		 */
+		zone.len = gran;
+	}
 	if (zone.cond == ZBC_ZONE_COND_FULL)
 		zone.wp = zone.start + zone.len;
 	else
@@ -227,6 +255,7 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
 	sector_t lba = sectors_to_logical(sdkp->device, sector);
 	unsigned int nr, i;
 	unsigned char *buf;
+	u64 zone_length, start_lba;
 	size_t offset, buflen = 0;
 	int zone_idx = 0;
 	int ret;
@@ -255,14 +284,36 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
 
 		for (i = 0; i < nr && zone_idx < nr_zones; i++) {
 			offset += 64;
+			start_lba = get_unaligned_be64(&buf[offset + 16]);
+			zone_length = get_unaligned_be64(&buf[offset + 8]);
+			if ((zone_idx == 0 &&
+			    (lba < start_lba ||
+			     lba >= start_lba + zone_length)) ||
+			    (zone_idx > 0 && start_lba != lba) ||
+			    start_lba + zone_length < start_lba) {
+				sd_printk(KERN_ERR, sdkp,
+					  "Zone %d at LBA %llu is invalid: %llu + %llu\n",
+					  zone_idx, lba, start_lba, zone_length);
+				ret = -EINVAL;
+				goto out;
+			}
+			lba = start_lba + zone_length;
+			if (sd_zbc_is_gap_zone(&buf[offset])) {
+				if (sdkp->zone_starting_lba_gran)
+					continue;
+				sd_printk(KERN_ERR, sdkp,
+					  "Gap zone without constant LBA offsets\n");
+				ret = -EINVAL;
+				goto out;
+			}
+
 			ret = sd_zbc_parse_report(sdkp, buf + offset, zone_idx,
 						  cb, data);
 			if (ret)
 				goto out;
+
 			zone_idx++;
 		}
-
-		lba += sdkp->zone_info.zone_blocks * i;
 	}
 
 	ret = zone_idx;
@@ -579,6 +630,7 @@ unsigned int sd_zbc_complete(struct scsi_cmnd *cmd, unsigned int good_bytes,
 static int sd_zbc_check_zoned_characteristics(struct scsi_disk *sdkp,
 					      unsigned char *buf)
 {
+	u64 zone_starting_lba_gran;
 
 	if (scsi_get_vpd_page(sdkp->device, 0xb6, buf, 64)) {
 		sd_printk(KERN_NOTICE, sdkp,
@@ -600,6 +652,29 @@ static int sd_zbc_check_zoned_characteristics(struct scsi_disk *sdkp,
 	sdkp->zones_optimal_open = 0;
 	sdkp->zones_optimal_nonseq = 0;
 	sdkp->zones_max_open = get_unaligned_be32(&buf[16]);
+	/* Check zone alignment method */
+	switch (buf[23] & 0xf) {
+	case 0:
+	case ZBC_CONSTANT_ZONE_LENGTH:
+		/* Use zone length */
+		break;
+	case ZBC_CONSTANT_ZONE_START_OFFSET:
+		zone_starting_lba_gran = get_unaligned_be64(&buf[24]);
+		if (zone_starting_lba_gran == 0 ||
+		    !is_power_of_2(zone_starting_lba_gran) ||
+		    logical_to_sectors(sdkp->device, zone_starting_lba_gran) >
+		    UINT_MAX) {
+			sd_printk(KERN_ERR, sdkp,
+				  "Invalid zone starting LBA granularity %llu\n",
+				  zone_starting_lba_gran);
+			return -ENODEV;
+		}
+		sdkp->zone_starting_lba_gran = zone_starting_lba_gran;
+		break;
+	default:
+		sd_printk(KERN_ERR, sdkp, "Invalid zone alignment method\n");
+		return -ENODEV;
+	}
 
 	/*
 	 * Check for unconstrained reads: host-managed devices with
@@ -654,14 +729,18 @@ static int sd_zbc_check_capacity(struct scsi_disk *sdkp, unsigned char *buf,
 		}
 	}
 
-	/* Get the size of the first reported zone */
-	rec = buf + 64;
-	zone_blocks = get_unaligned_be64(&rec[8]);
-	if (logical_to_sectors(sdkp->device, zone_blocks) > UINT_MAX) {
-		if (sdkp->first_scan)
-			sd_printk(KERN_NOTICE, sdkp,
-				  "Zone size too large\n");
-		return -EFBIG;
+	if (sdkp->zone_starting_lba_gran == 0) {
+		/* Get the size of the first reported zone */
+		rec = buf + 64;
+		zone_blocks = get_unaligned_be64(&rec[8]);
+		if (logical_to_sectors(sdkp->device, zone_blocks) > UINT_MAX) {
+			if (sdkp->first_scan)
+				sd_printk(KERN_NOTICE, sdkp,
+					  "Zone size too large\n");
+			return -EFBIG;
+		}
+	} else {
+		zone_blocks = sdkp->zone_starting_lba_gran;
 	}
 
 	if (!is_power_of_2(zone_blocks)) {
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index f017843a8124..c03e35fc382c 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -307,7 +307,9 @@ enum zbc_zone_type {
 	ZBC_ZONE_TYPE_CONV		= 0x1,
 	ZBC_ZONE_TYPE_SEQWRITE_REQ	= 0x2,
 	ZBC_ZONE_TYPE_SEQWRITE_PREF	= 0x3,
-	/* 0x4 to 0xf are reserved */
+	ZBC_ZONE_TYPE_SEQ_OR_BEFORE_REQ	= 0x4,
+	ZBC_ZONE_TYPE_GAP		= 0x5,
+	/* 0x6 to 0xf are reserved */
 };
 
 /* Zone conditions of REPORT ZONES zone descriptors */
@@ -323,6 +325,11 @@ enum zbc_zone_cond {
 	ZBC_ZONE_COND_OFFLINE		= 0xf,
 };
 
+enum zbc_zone_alignment_method {
+	ZBC_CONSTANT_ZONE_LENGTH	= 0x1,
+	ZBC_CONSTANT_ZONE_START_OFFSET	= 0x8,
+};
+
 /* Version descriptor values for INQUIRY */
 enum scsi_version_descriptor {
 	SCSI_VERSION_DESCRIPTOR_FCP4	= 0x0a40,

[PATCH v2 7/9] scsi_debug: Fix a typo

[Bart Van Assche]

Change a single occurrence of "nad" into "and".

Cc: Douglas Gilbert <dgilbert@interlog.com>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index c607755cce00..7cfae8206a4b 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4408,7 +4408,7 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 
 #define RZONES_DESC_HD 64
 
-/* Report zones depending on start LBA nad reporting options */
+/* Report zones depending on start LBA and reporting options */
 static int resp_report_zones(struct scsi_cmnd *scp,
 			     struct sdebug_dev_info *devip)
 {

Re: [PATCH v2 7/9] scsi_debug: Fix a typo

[Himanshu Madhani]

> On Apr 21, 2022, at 11:30 AM, Bart Van Assche <bvanassche@acm.org> wrote:
> 
> Change a single occurrence of "nad" into "and".
> 
> Cc: Douglas Gilbert <dgilbert@interlog.com>
> Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/scsi_debug.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index c607755cce00..7cfae8206a4b 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -4408,7 +4408,7 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
> 
> #define RZONES_DESC_HD 64
> 
> -/* Report zones depending on start LBA nad reporting options */
> +/* Report zones depending on start LBA and reporting options */
> static int resp_report_zones(struct scsi_cmnd *scp,
> 			     struct sdebug_dev_info *devip)
> {

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	Oracle Linux Engineering

[PATCH v2 8/9] scsi_debug: Rename zone type constants

[Bart Van Assche]

Rename the scsi_debug zone type constants to prevent a conflict with the
ZBC_ZONE_TYPE_GAP constant from include/scsi/scsi_proto.h.

Cc: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
[ bvanassche: Extracted these changes from a larger patch ]
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_debug.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 7cfae8206a4b..47cec83a4b7c 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -252,9 +252,9 @@ static const char *sdebug_version_date = "20210520";
 
 /* Zone types (zbcr05 table 25) */
 enum sdebug_z_type {
-	ZBC_ZONE_TYPE_CNV	= 0x1,
-	ZBC_ZONE_TYPE_SWR	= 0x2,
-	ZBC_ZONE_TYPE_SWP	= 0x3,
+	ZBC_ZTYPE_CNV	= 0x1,
+	ZBC_ZTYPE_SWR	= 0x2,
+	ZBC_ZTYPE_SWP	= 0x3,
 };
 
 /* enumeration names taken from table 26, zbcr05 */
@@ -2720,7 +2720,7 @@ static struct sdeb_zone_state *zbc_zone(struct sdebug_dev_info *devip,
 
 static inline bool zbc_zone_is_conv(struct sdeb_zone_state *zsp)
 {
-	return zsp->z_type == ZBC_ZONE_TYPE_CNV;
+	return zsp->z_type == ZBC_ZTYPE_CNV;
 }
 
 static void zbc_close_zone(struct sdebug_dev_info *devip,
@@ -2801,7 +2801,7 @@ static void zbc_inc_wp(struct sdebug_dev_info *devip,
 	if (zbc_zone_is_conv(zsp))
 		return;
 
-	if (zsp->z_type == ZBC_ZONE_TYPE_SWR) {
+	if (zsp->z_type == ZBC_ZTYPE_SWR) {
 		zsp->z_wp += num;
 		if (zsp->z_wp >= zend)
 			zsp->z_cond = ZC5_FULL;
@@ -2868,7 +2868,7 @@ static int check_zbc_access_params(struct scsi_cmnd *scp,
 		return 0;
 	}
 
-	if (zsp->z_type == ZBC_ZONE_TYPE_SWR) {
+	if (zsp->z_type == ZBC_ZTYPE_SWR) {
 		/* Writes cannot cross sequential zone boundaries */
 		if (zsp_end != zsp) {
 			mk_sense_buffer(scp, ILLEGAL_REQUEST,
@@ -5005,14 +5005,14 @@ static int sdebug_device_create_zones(struct sdebug_dev_info *devip)
 		zsp->z_start = zstart;
 
 		if (i < devip->nr_conv_zones) {
-			zsp->z_type = ZBC_ZONE_TYPE_CNV;
+			zsp->z_type = ZBC_ZTYPE_CNV;
 			zsp->z_cond = ZBC_NOT_WRITE_POINTER;
 			zsp->z_wp = (sector_t)-1;
 		} else {
 			if (devip->zmodel == BLK_ZONED_HM)
-				zsp->z_type = ZBC_ZONE_TYPE_SWR;
+				zsp->z_type = ZBC_ZTYPE_SWR;
 			else
-				zsp->z_type = ZBC_ZONE_TYPE_SWP;
+				zsp->z_type = ZBC_ZTYPE_SWP;
 			zsp->z_cond = ZC1_EMPTY;
 			zsp->z_wp = zsp->z_start;
 		}

[PATCH v2 9/9] scsi_debug: Add gap zone support

[Bart Van Assche]

Add the 'zone_cap_mb' kernel module parameter. This parameter defines
the zone capacity. The zone capacity must be less than or equal to the
zone size.

Report that sequential write zones and gap zones are paired in the Zoned
Block Device Characteristics VPD page (page B6h).

This patch has been tested as follows:

modprobe scsi_debug delay=0 sector_size=512 dev_size_mb=128 zbc=host-managed zone_nr_conv=16 zone_size_mb=4 zone_cap_mb=3
modprobe brd rd_nr=1 rd_size=$((1<<20))
mkfs.f2fs -m /dev/ram0 -c /dev/${scsi_debug_dev}
mount /dev/ram0 /mnt
 # Run a fio job that uses /mnt

Cc: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
[ bvanassche: Switched to reporting a constant zone starting LBA granularity ]
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_debug.c | 129 ++++++++++++++++++++++++++++++--------
 1 file changed, 104 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 47cec83a4b7c..d4ab21c5d26a 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -16,7 +16,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ":%s: " fmt, __func__
 
 #include <linux/module.h>
-
+#include <linux/align.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/jiffies.h>
@@ -99,6 +99,7 @@ static const char *sdebug_version_date = "20210520";
 #define WRITE_BOUNDARY_ASCQ 0x5
 #define READ_INVDATA_ASCQ 0x6
 #define READ_BOUNDARY_ASCQ 0x7
+#define ATTEMPT_ACCESS_GAP 0x9
 #define INSUFF_ZONE_ASCQ 0xe
 
 /* Additional Sense Code Qualifier (ASCQ) */
@@ -255,6 +256,8 @@ enum sdebug_z_type {
 	ZBC_ZTYPE_CNV	= 0x1,
 	ZBC_ZTYPE_SWR	= 0x2,
 	ZBC_ZTYPE_SWP	= 0x3,
+	/* ZBC_ZTYPE_SOBR = 0x4, */
+	ZBC_ZTYPE_GAP	= 0x5,
 };
 
 /* enumeration names taken from table 26, zbcr05 */
@@ -292,10 +295,12 @@ struct sdebug_dev_info {
 
 	/* For ZBC devices */
 	enum blk_zoned_model zmodel;
+	unsigned int zcap;
 	unsigned int zsize;
 	unsigned int zsize_shift;
 	unsigned int nr_zones;
 	unsigned int nr_conv_zones;
+	unsigned int nr_seq_zones;
 	unsigned int nr_imp_open;
 	unsigned int nr_exp_open;
 	unsigned int nr_closed;
@@ -833,6 +838,7 @@ static int dif_errors;
 
 /* ZBC global data */
 static bool sdeb_zbc_in_use;	/* true for host-aware and host-managed disks */
+static int sdeb_zbc_zone_cap_mb;
 static int sdeb_zbc_zone_size_mb;
 static int sdeb_zbc_max_open = DEF_ZBC_MAX_OPEN_ZONES;
 static int sdeb_zbc_nr_conv = DEF_ZBC_NR_CONV_ZONES;
@@ -1563,6 +1569,12 @@ static int inquiry_vpd_b6(struct sdebug_dev_info *devip, unsigned char *arr)
 		put_unaligned_be32(devip->max_open, &arr[12]);
 	else
 		put_unaligned_be32(0xffffffff, &arr[12]);
+	if (devip->zcap < devip->zsize) {
+		arr[19] = ZBC_CONSTANT_ZONE_START_OFFSET;
+		put_unaligned_be64(devip->zsize, &arr[20]);
+	} else {
+		arr[19] = 0;
+	}
 	return 0x3c;
 }
 
@@ -2715,7 +2727,23 @@ static inline bool sdebug_dev_is_zoned(struct sdebug_dev_info *devip)
 static struct sdeb_zone_state *zbc_zone(struct sdebug_dev_info *devip,
 					unsigned long long lba)
 {
-	return &devip->zstate[lba >> devip->zsize_shift];
+	u32 zno = lba >> devip->zsize_shift;
+	struct sdeb_zone_state *zsp;
+
+	if (devip->zcap == devip->zsize || zno < devip->nr_conv_zones)
+		return &devip->zstate[zno];
+
+	/*
+	 * If the zone capacity is less than the zone size, adjust for gap
+	 * zones.
+	 */
+	zno = 2 * zno - devip->nr_conv_zones;
+	WARN_ONCE(zno >= devip->nr_zones, "%u > %u\n", zno, devip->nr_zones);
+	zsp = &devip->zstate[zno];
+	if (lba >= zsp->z_start + zsp->z_size)
+		zsp++;
+	WARN_ON_ONCE(lba >= zsp->z_start + zsp->z_size);
+	return zsp;
 }
 
 static inline bool zbc_zone_is_conv(struct sdeb_zone_state *zsp)
@@ -2723,12 +2751,22 @@ static inline bool zbc_zone_is_conv(struct sdeb_zone_state *zsp)
 	return zsp->z_type == ZBC_ZTYPE_CNV;
 }
 
+static inline bool zbc_zone_is_gap(struct sdeb_zone_state *zsp)
+{
+	return zsp->z_type == ZBC_ZTYPE_GAP;
+}
+
+static inline bool zbc_zone_is_seq(struct sdeb_zone_state *zsp)
+{
+	return !zbc_zone_is_conv(zsp) && !zbc_zone_is_gap(zsp);
+}
+
 static void zbc_close_zone(struct sdebug_dev_info *devip,
 			   struct sdeb_zone_state *zsp)
 {
 	enum sdebug_z_cond zc;
 
-	if (zbc_zone_is_conv(zsp))
+	if (!zbc_zone_is_seq(zsp))
 		return;
 
 	zc = zsp->z_cond;
@@ -2766,7 +2804,7 @@ static void zbc_open_zone(struct sdebug_dev_info *devip,
 {
 	enum sdebug_z_cond zc;
 
-	if (zbc_zone_is_conv(zsp))
+	if (!zbc_zone_is_seq(zsp))
 		return;
 
 	zc = zsp->z_cond;
@@ -2798,7 +2836,7 @@ static void zbc_inc_wp(struct sdebug_dev_info *devip,
 	struct sdeb_zone_state *zsp = zbc_zone(devip, lba);
 	unsigned long long n, end, zend = zsp->z_start + zsp->z_size;
 
-	if (zbc_zone_is_conv(zsp))
+	if (!zbc_zone_is_seq(zsp))
 		return;
 
 	if (zsp->z_type == ZBC_ZTYPE_SWR) {
@@ -2846,9 +2884,7 @@ static int check_zbc_access_params(struct scsi_cmnd *scp,
 		if (devip->zmodel == BLK_ZONED_HA)
 			return 0;
 		/* For host-managed, reads cannot cross zone types boundaries */
-		if (zsp_end != zsp &&
-		    zbc_zone_is_conv(zsp) &&
-		    !zbc_zone_is_conv(zsp_end)) {
+		if (zsp->z_type != zsp_end->z_type) {
 			mk_sense_buffer(scp, ILLEGAL_REQUEST,
 					LBA_OUT_OF_RANGE,
 					READ_INVDATA_ASCQ);
@@ -2857,6 +2893,13 @@ static int check_zbc_access_params(struct scsi_cmnd *scp,
 		return 0;
 	}
 
+	/* Writing into a gap zone is not allowed */
+	if (zbc_zone_is_gap(zsp)) {
+		mk_sense_buffer(scp, ILLEGAL_REQUEST, LBA_OUT_OF_RANGE,
+				ATTEMPT_ACCESS_GAP);
+		return check_condition_result;
+	}
+
 	/* No restrictions for writes within conventional zones */
 	if (zbc_zone_is_conv(zsp)) {
 		if (!zbc_zone_is_conv(zsp_end)) {
@@ -4412,14 +4455,14 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 static int resp_report_zones(struct scsi_cmnd *scp,
 			     struct sdebug_dev_info *devip)
 {
-	unsigned int i, max_zones, rep_max_zones, nrz = 0;
+	unsigned int rep_max_zones, nrz = 0;
 	int ret = 0;
 	u32 alloc_len, rep_opts, rep_len;
 	bool partial;
 	u64 lba, zs_lba;
 	u8 *arr = NULL, *desc;
 	u8 *cmd = scp->cmnd;
-	struct sdeb_zone_state *zsp;
+	struct sdeb_zone_state *zsp = NULL;
 	struct sdeb_store_info *sip = devip2sip(devip, false);
 
 	if (!sdebug_dev_is_zoned(devip)) {
@@ -4438,9 +4481,7 @@ static int resp_report_zones(struct scsi_cmnd *scp,
 		return check_condition_result;
 	}
 
-	max_zones = devip->nr_zones - (zs_lba >> devip->zsize_shift);
-	rep_max_zones = min((alloc_len - 64) >> ilog2(RZONES_DESC_HD),
-			    max_zones);
+	rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD);
 
 	arr = kzalloc(alloc_len, GFP_ATOMIC);
 	if (!arr) {
@@ -4452,9 +4493,9 @@ static int resp_report_zones(struct scsi_cmnd *scp,
 	sdeb_read_lock(sip);
 
 	desc = arr + 64;
-	for (i = 0; i < max_zones; i++) {
-		lba = zs_lba + devip->zsize * i;
-		if (lba > sdebug_capacity)
+	for (lba = zs_lba; lba < sdebug_capacity;
+	     lba = zsp->z_start + zsp->z_size) {
+		if (WARN_ONCE(zbc_zone(devip, lba) == zsp, "lba = %llu\n", lba))
 			break;
 		zsp = zbc_zone(devip, lba);
 		switch (rep_opts) {
@@ -4499,9 +4540,14 @@ static int resp_report_zones(struct scsi_cmnd *scp,
 			if (!zsp->z_non_seq_resource)
 				continue;
 			break;
+		case 0x3e:
+			/* All zones except gap zones. */
+			if (zbc_zone_is_gap(zsp))
+				continue;
+			break;
 		case 0x3f:
 			/* Not write pointer (conventional) zones */
-			if (!zbc_zone_is_conv(zsp))
+			if (zbc_zone_is_seq(zsp))
 				continue;
 			break;
 		default:
@@ -4530,8 +4576,13 @@ static int resp_report_zones(struct scsi_cmnd *scp,
 	}
 
 	/* Report header */
+	/* Zone list length. */
 	put_unaligned_be32(nrz * RZONES_DESC_HD, arr + 0);
+	/* Maximum LBA */
 	put_unaligned_be64(sdebug_capacity - 1, arr + 8);
+	/* Zone starting LBA granularity. */
+	if (devip->zcap < devip->zsize)
+		put_unaligned_be64(devip->zsize, arr + 16);
 
 	rep_len = (unsigned long)desc - (unsigned long)arr;
 	ret = fill_from_dev_buffer(scp, arr, min_t(u32, alloc_len, rep_len));
@@ -4756,7 +4807,7 @@ static void zbc_rwp_zone(struct sdebug_dev_info *devip,
 	enum sdebug_z_cond zc;
 	struct sdeb_store_info *sip = devip2sip(devip, false);
 
-	if (zbc_zone_is_conv(zsp))
+	if (!zbc_zone_is_seq(zsp))
 		return;
 
 	zc = zsp->z_cond;
@@ -4946,6 +4997,7 @@ static int sdebug_device_create_zones(struct sdebug_dev_info *devip)
 {
 	struct sdeb_zone_state *zsp;
 	sector_t capacity = get_sdebug_capacity();
+	sector_t conv_capacity;
 	sector_t zstart = 0;
 	unsigned int i;
 
@@ -4980,11 +5032,30 @@ static int sdebug_device_create_zones(struct sdebug_dev_info *devip)
 	devip->zsize_shift = ilog2(devip->zsize);
 	devip->nr_zones = (capacity + devip->zsize - 1) >> devip->zsize_shift;
 
-	if (sdeb_zbc_nr_conv >= devip->nr_zones) {
+	if (sdeb_zbc_zone_cap_mb == 0) {
+		devip->zcap = devip->zsize;
+	} else {
+		devip->zcap = (sdeb_zbc_zone_cap_mb * SZ_1M) >>
+			      ilog2(sdebug_sector_size);
+		if (devip->zcap > devip->zsize) {
+			pr_err("Zone capacity too large\n");
+			return -EINVAL;
+		}
+	}
+
+	conv_capacity = (sector_t)sdeb_zbc_nr_conv << devip->zsize_shift;
+	if (conv_capacity >= capacity) {
 		pr_err("Number of conventional zones too large\n");
 		return -EINVAL;
 	}
 	devip->nr_conv_zones = sdeb_zbc_nr_conv;
+	devip->nr_seq_zones = ALIGN(capacity - conv_capacity, devip->zsize) >>
+			      devip->zsize_shift;
+	devip->nr_zones = devip->nr_conv_zones + devip->nr_seq_zones;
+
+	/* Add gap zones if zone capacity is smaller than the zone size */
+	if (devip->zcap < devip->zsize)
+		devip->nr_zones += devip->nr_seq_zones;
 
 	if (devip->zmodel == BLK_ZONED_HM) {
 		/* zbc_max_open_zones can be 0, meaning "not reported" */
@@ -5008,20 +5079,26 @@ static int sdebug_device_create_zones(struct sdebug_dev_info *devip)
 			zsp->z_type = ZBC_ZTYPE_CNV;
 			zsp->z_cond = ZBC_NOT_WRITE_POINTER;
 			zsp->z_wp = (sector_t)-1;
-		} else {
+			zsp->z_size =
+				min_t(u64, devip->zsize, capacity - zstart);
+		} else if ((zstart & (devip->zsize - 1)) == 0) {
 			if (devip->zmodel == BLK_ZONED_HM)
 				zsp->z_type = ZBC_ZTYPE_SWR;
 			else
 				zsp->z_type = ZBC_ZTYPE_SWP;
 			zsp->z_cond = ZC1_EMPTY;
 			zsp->z_wp = zsp->z_start;
+			zsp->z_size =
+				min_t(u64, devip->zcap, capacity - zstart);
+		} else {
+			zsp->z_type = ZBC_ZTYPE_GAP;
+			zsp->z_cond = ZBC_NOT_WRITE_POINTER;
+			zsp->z_wp = (sector_t)-1;
+			zsp->z_size = min_t(u64, devip->zsize - devip->zcap,
+					    capacity - zstart);
 		}
 
-		if (zsp->z_start + devip->zsize < capacity)
-			zsp->z_size = devip->zsize;
-		else
-			zsp->z_size = capacity - zsp->z_start;
-
+		WARN_ON_ONCE((int)zsp->z_size <= 0);
 		zstart += zsp->z_size;
 	}
 
@@ -5856,6 +5933,7 @@ module_param_named(wp, sdebug_wp, bool, S_IRUGO | S_IWUSR);
 module_param_named(write_same_length, sdebug_write_same_length, int,
 		   S_IRUGO | S_IWUSR);
 module_param_named(zbc, sdeb_zbc_model_s, charp, S_IRUGO);
+module_param_named(zone_cap_mb, sdeb_zbc_zone_cap_mb, int, S_IRUGO);
 module_param_named(zone_max_open, sdeb_zbc_max_open, int, S_IRUGO);
 module_param_named(zone_nr_conv, sdeb_zbc_nr_conv, int, S_IRUGO);
 module_param_named(zone_size_mb, sdeb_zbc_zone_size_mb, int, S_IRUGO);
@@ -5927,6 +6005,7 @@ MODULE_PARM_DESC(vpd_use_hostno, "0 -> dev ids ignore hostno (def=1 -> unique de
 MODULE_PARM_DESC(wp, "Write Protect (def=0)");
 MODULE_PARM_DESC(write_same_length, "Maximum blocks per WRITE SAME cmd (def=0xffff)");
 MODULE_PARM_DESC(zbc, "'none' [0]; 'aware' [1]; 'managed' [2] (def=0). Can have 'host-' prefix");
+MODULE_PARM_DESC(zone_cap_mb, "Zone capacity in MiB (def=zone size)");
 MODULE_PARM_DESC(zone_max_open, "Maximum number of open zones; [0] for no limit (def=auto)");
 MODULE_PARM_DESC(zone_nr_conv, "Number of conventional zones (def=1)");
 MODULE_PARM_DESC(zone_size_mb, "Zone size in MiB (def=auto)");

Re: [PATCH v2 0/9] Support zoned devices with gap zones

[Douglas Gilbert]

On 2022-04-21 14:30, Bart Van Assche wrote:
> Hi Martin,
> 
> In ZBC-2 support has been improved for zones with a size that is not a power
> of two by allowing host-managed devices to report gap zones. This patch adds
> support for zoned devices for which data zones and gap zones alternate if the
> distance between zone start LBAs is a power of two.
> 
> Please consider this patch series for kernel v5.19.

whole series:
Acked-by: Douglas Gilbert <dgilbert@interlog.com>

> Changes compared to v1:
> - Made this patch series compatible with the zone querying code in BTRFS.
> - Addressed Damien's off-list review comments.
> - Added patch "Return early in sd_zbc_check_zoned_characteristics()" to this
>    series.
> 
> Bart Van Assche (9):
>    scsi: sd_zbc: Improve source code documentation
>    scsi: sd_zbc: Verify that the zone size is a power of two
>    scsi: sd_zbc: Use logical blocks as unit when querying zones
>    scsi: sd_zbc: Introduce struct zoned_disk_info
>    scsi: sd_zbc: Return early in sd_zbc_check_zoned_characteristics()
>    scsi: sd_zbc: Hide gap zones
>    scsi_debug: Fix a typo
>    scsi_debug: Rename zone type constants
>    scsi_debug: Add gap zone support
> 
>   drivers/scsi/scsi_debug.c | 149 ++++++++++++++++++------
>   drivers/scsi/sd.h         |  32 ++++--
>   drivers/scsi/sd_zbc.c     | 236 +++++++++++++++++++++++++++++---------
>   include/scsi/scsi_proto.h |   9 +-
>   4 files changed, 331 insertions(+), 95 deletions(-)
> 

Re: [PATCH v2 0/9] Support zoned devices with gap zones

[Martin K. Petersen]

Bart,

> In ZBC-2 support has been improved for zones with a size that is not a
> power of two by allowing host-managed devices to report gap
> zones. This patch adds support for zoned devices for which data zones
> and gap zones alternate if the distance between zone start LBAs is a
> power of two.

Applied to 5.19/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 0/9] Support zoned devices with gap zones

[Martin K. Petersen]

On Thu, 21 Apr 2022 11:30:14 -0700, Bart Van Assche wrote:

> In ZBC-2 support has been improved for zones with a size that is not a power
> of two by allowing host-managed devices to report gap zones. This patch adds
> support for zoned devices for which data zones and gap zones alternate if the
> distance between zone start LBAs is a power of two.
> 
> Please consider this patch series for kernel v5.19.
> 
> [...]

Applied to 5.19/scsi-queue, thanks!

[1/9] scsi: sd_zbc: Improve source code documentation
      https://git.kernel.org/mkp/scsi/c/aa96bfb4caff
[2/9] scsi: sd_zbc: Verify that the zone size is a power of two
      https://git.kernel.org/mkp/scsi/c/9a93b9c9d38a
[3/9] scsi: sd_zbc: Use logical blocks as unit when querying zones
      https://git.kernel.org/mkp/scsi/c/43af5da09efb
[4/9] scsi: sd_zbc: Introduce struct zoned_disk_info
      https://git.kernel.org/mkp/scsi/c/628617be8968
[5/9] scsi: sd_zbc: Return early in sd_zbc_check_zoned_characteristics()
      https://git.kernel.org/mkp/scsi/c/60caf3758103
[6/9] scsi: sd_zbc: Hide gap zones
      https://git.kernel.org/mkp/scsi/c/c976e588b34e
[7/9] scsi_debug: Fix a typo
      https://git.kernel.org/mkp/scsi/c/897284e8a048
[8/9] scsi_debug: Rename zone type constants
      https://git.kernel.org/mkp/scsi/c/35dbe2b9a7b0
[9/9] scsi_debug: Add gap zone support
      https://git.kernel.org/mkp/scsi/c/4a5fc1c6d752

-- 
Martin K. Petersen	Oracle Linux Engineering