On 11/10/22 12:50, 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. > > To be precise, ZBC does not mandate READ(16), WRITE (16) and SYNCHRONIZE > CACHE (16) commands for host-aware zoned block devices. However, modern > devices should support the 16 byte commands. Hence, enforce the 16 byte > commands on both types of ZBC devices, host-aware and host-managed. > > Of note is that this patch depends on the libata-scsi fix [1], and > should be merged to upstream after it. > > [1] https://lore.kernel.org/linux-ide/20221107040229.1548793-1-shinichiro.kawasaki@xxxxxxx/ > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx> Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx> > --- > Changes from v1: > * Dropped the first patch to relax check on host-aware devices > * Enforce SYNC 16 command on both host-aware and host-managed devices > > 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 bd15624c6322..b163bf936acc 100644 > --- a/drivers/scsi/sd_zbc.c > +++ b/drivers/scsi/sd_zbc.c > @@ -921,9 +921,10 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE]) > return 0; > } > > - /* READ16/WRITE16 is mandatory for ZBC disks */ > + /* READ16/WRITE16/SYNC16 is mandatory for ZBC devices */ > 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 */ -- Damien Le Moal Western Digital Research
