mbox series

[0/2] scsi: sd: Fix physical block size issues of host-managed zoned disks

Message ID 20230303014422.2466103-1-shinichiro.kawasaki@wdc.com
Headers show
Series scsi: sd: Fix physical block size issues of host-managed zoned disks | expand

Message

Shin'ichiro Kawasaki March 3, 2023, 1:44 a.m. UTC 
In general, writes to SCSI disks are required to align to the logical block
size. On the other hand, ZBC and ZAC specifications require that the writes to
sequential write required zones to align to the physical block size. When
ZBC/ZAC host-managed zoned disks have the physical block size different from the
logical block size, writes aligned to the logical block size fail. The sysfs
attribute zone_write_granularity was introduced so that userland programs can
tell what is the alignment size for sequential write required zones. As for
ZBC/ZAC host-managed zoned disks, zone_write_granularity shows the physical
block size.

However, there are two issues related to this requirement of the physical block
size alignment. The first issue is unclear failure report. On the write fail,
the disks return the unaligned write command error, which may happen for other
causes other than the physical block size alignment. The second issue is wrong
value of zone_write_granularity sysfs attribute. In most cases, it shows
correct values. But during revalidate of the disks, it shows wrong values. The
two patches in this series address the two issues.

Shin'ichiro Kawasaki (2):
  scsi: sd: Check physical sector alignment of sequential zone writes
  scsi: sd: Fix wrong zone_write_granularity value at revalidate

 drivers/scsi/sd.c     | 17 ++++++++++++++++-
 drivers/scsi/sd_zbc.c |  8 --------
 2 files changed, 16 insertions(+), 9 deletions(-)

Comments

Damien Le Moal March 4, 2023, 3:03 a.m. UTC | #1 
On 3/4/23 03:03, Bart Van Assche wrote:
> On 3/2/23 17:44, Shin'ichiro Kawasaki wrote:
>> +	if (sdkp->device->type == TYPE_ZBC && blk_rq_zone_is_seq(rq) &&
>> +	    (req_op(rq) == REQ_OP_WRITE || req_op(rq) == REQ_OP_ZONE_APPEND) &&
>> +	    (!IS_ALIGNED(blk_rq_pos(rq), pb_sectors) ||
>> +	     !IS_ALIGNED(blk_rq_sectors(rq), pb_sectors))) {
>> +		scmd_printk(KERN_ERR, cmd,
>> +			    "Sequential write request not aligned to the physical block size\n");
>> +		goto fail;
>> +	}
> 
> I vote -1 for this patch because my opinion is that we should not 
> duplicate checks that must be performed by the storage controller anyway 
> inside the sd driver.

Sure, the drive will fail this request, so the end result is the same. But what
is the point of issuing such unaligned request that we know will fail ? The
error message also make it easier to debug as it clarifies that this is not a
write pointer violation. So while this change is not critical, it does have
merits in my opinion.

> 
> Thanks,
> 
> Bart.