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

[Damien Le Moal <dlemoal@kernel.org>]

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