Re: [PATCH 3/5] block: remove support for the host aware zone model

Damien Le Moal
2023-12-18T09:48:46Z

On 2023/12/18 1:53, Christoph Hellwig wrote:
> When zones were first added the SCSI and ATA specs, two different
> models were supported (in addition to the drive managed one that
> is invisible to the host):
>
> - host managed where non-conventional zones there is strict requirement
> to write at the write pointer, or else an error is returned
> - host aware where a write point is maintained if writes always happen
> at it, otherwise it is left in an under-defined state and the
> sequential write preferred zones behave like conventional zones
> (probably very badly performing ones, though)
>
> Not surprisingly this lukewarm model didn't prove to be very useful and
> was finally removed from the ZBC and SBC specs (NVMe never implemented
> it). Due to to the easily disappearing write pointer host software
> could never rely on the write pointer to actually be useful for say
> recovery.
>
> Fortunately only a few HDD prototypes shipped using this model which
> never made it to mass production. Drop the support before it is too
> late. Note that any such host aware prototype HDD can still be used
> with Linux as we'll now treat it as a conventional HDD.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

[...]

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 6d8218a4412264..d03d66f1149301 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -339,7 +339,7 @@ struct sdebug_dev_info {
> bool used;
>
> /* For ZBC devices */
> - enum blk_zoned_model zmodel;
> + bool zoned;
> unsigned int zcap;
> unsigned int zsize;
> unsigned int zsize_shift;
> @@ -844,8 +844,11 @@ static bool write_since_sync;
> static bool sdebug_statistics = DEF_STATISTICS;
> static bool sdebug_wp;
> static bool sdebug_allow_restart;
> -/* Following enum: 0: no zbc, def; 1: host aware; 2: host managed */
> -static enum blk_zoned_model sdeb_zbc_model = BLK_ZONED_NONE;
> +static enum {
> + BLK_ZONED_NONE = 0,
> + BLK_ZONED_HA = 1,
> + BLK_ZONED_HM = 2,
> +} sdeb_zbc_model = BLK_ZONED_NONE;
> static char *sdeb_zbc_model_s;
>
> enum sam_lun_addr_method {SAM_LUN_AM_PERIPHERAL = 0x0,
> @@ -1815,8 +1818,6 @@ static int inquiry_vpd_b1(struct sdebug_dev_info *devip, unsigned char *arr)
> arr[1] = 1; /* non rotating medium (e.g. solid state) */
> arr[2] = 0;
> arr[3] = 5; /* less than 1.8" */
> - if (devip->zmodel == BLK_ZONED_HA)
> - arr[4] = 1 << 4; /* zoned field = 01b */

I think we should keep everything related to HA in scsi debug as that is an easy
way to test the block layer and scsi. no ?

Other than this, very nice cleanup !

--
Damien Le Moal
Western Digital Research