Re: [PATCH 2/2] scsi: sd: enforce SYNCHRONIZE CACHE (16) on host-managed devices

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

On 11/9/22 11:59, Shin'ichiro Kawasaki wrote:
> ZBC Zoned Block Commands specification mandates SYNCHRONIZE CACHE (16)
> for host-managed zoned block devices, but does not mandate SYNCHRONIZE
> CACHE (10). Call SYNCHRONIZE CACHE (16) in place of SYNCHRONIZE CACHE
> (10) to ensure that the command is always supported. For this purpose,
> add use_16_for_sync flag to struct scsi_device in same manner as
> use_16_for_rw flag.
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  drivers/scsi/sd.c          | 16 ++++++++++++----
>  drivers/scsi/sd_zbc.c      |  3 ++-
>  include/scsi/scsi_device.h |  1 +
>  3 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index eb76ba055021..faa2b55d1a21 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1026,8 +1026,13 @@ static blk_status_t sd_setup_flush_cmnd(struct scsi_cmnd *cmd)
>  	/* flush requests don't perform I/O, zero the S/G table */
>  	memset(&cmd->sdb, 0, sizeof(cmd->sdb));
>  
> -	cmd->cmnd[0] = SYNCHRONIZE_CACHE;
> -	cmd->cmd_len = 10;
> +	if (cmd->device->use_16_for_sync) {
> +		cmd->cmnd[0] = SYNCHRONIZE_CACHE_16;
> +		cmd->cmd_len = 16;
> +	} else {
> +		cmd->cmnd[0] = SYNCHRONIZE_CACHE;
> +		cmd->cmd_len = 10;
> +	}
>  	cmd->transfersize = 0;
>  	cmd->allowed = sdkp->max_retries;
>  
> @@ -1587,9 +1592,12 @@ static int sd_sync_cache(struct scsi_disk *sdkp, struct scsi_sense_hdr *sshdr)
>  		sshdr = &my_sshdr;
>  
>  	for (retries = 3; retries > 0; --retries) {
> -		unsigned char cmd[10] = { 0 };
> +		unsigned char cmd[16] = { 0 };
>  
> -		cmd[0] = SYNCHRONIZE_CACHE;
> +		if (sdp->use_16_for_sync)
> +			cmd[0] = SYNCHRONIZE_CACHE_16;
> +		else
> +			cmd[0] = SYNCHRONIZE_CACHE;
>  		/*
>  		 * Leave the rest of the command zero to indicate
>  		 * flush everything.
> diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
> index 4717a55dbf35..a998f9c091dd 100644
> --- a/drivers/scsi/sd_zbc.c
> +++ b/drivers/scsi/sd_zbc.c
> @@ -921,10 +921,11 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE])
>  		return 0;
>  	}
>  
> -	/* READ16/WRITE16 is mandatory for host-managed devices */
> +	/* READ16/WRITE16/SYNC16 is mandatory for host-managed devices */
>  	if (sdkp->device->type == TYPE_ZBC) {
>  		sdkp->device->use_16_for_rw = 1;
>  		sdkp->device->use_10_for_rw = 0;
> +		sdkp->device->use_16_for_sync = 1;
>  	}
>  
>  	if (!blk_queue_is_zoned(q)) {
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index c36656d8ac6c..afd2986007a4 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -184,6 +184,7 @@ struct scsi_device {
>  	unsigned no_report_opcodes:1;	/* no REPORT SUPPORTED OPERATION CODES */
>  	unsigned no_write_same:1;	/* no WRITE SAME command */
>  	unsigned use_16_for_rw:1; /* Use read/write(16) over read/write(10) */
> +	unsigned use_16_for_sync:1;	/* Use sync (16) over sync (10) */
>  	unsigned skip_ms_page_8:1;	/* do not use MODE SENSE page 0x08 */
>  	unsigned skip_ms_page_3f:1;	/* do not use MODE SENSE page 0x3f */
>  	unsigned skip_vpd_pages:1;	/* do not read VPD pages */

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

-- 
Damien Le Moal
Western Digital Research