On 3/3/23 10:44, Shin'ichiro Kawasaki wrote:
> When host-managed SMR disks have different physical sector size and
> logical sector size, writes to conventional zones should be aligned to
> the logical sector size. On the other hand, ZBC/ZAC requires that writes
> to sequential write required zones shall be aligned to the physical
> sector size. Otherwise, the disks return the unaligned write command
> error. However, this error is common with other failure reasons. The
> error is also reported when the write start sector is not at the write
> pointer, or the write end sector is not in the same zone.
>
> To clarify the write failure cause is the physical sector alignment,
> confirm before issuing write commands that the writes to sequential
> write required zones are aligned to the physical sector size. If not,
> print a relevant error message. This makes failure analysis easier, and
> also avoids error handling in low level drivers.
>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
> ---
> drivers/scsi/sd.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 47dafe6b8a66..6d115b2fa99a 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1123,6 +1123,7 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd)
> sector_t lba = sectors_to_logical(sdp, blk_rq_pos(rq));
> sector_t threshold;
> unsigned int nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
> + unsigned int pb_sectors = sdkp->physical_block_size >> SECTOR_SHIFT;
> unsigned int mask = logical_to_sectors(sdp, 1) - 1;
> bool write = rq_data_dir(rq) == WRITE;
> unsigned char protect, fua;
> @@ -1145,6 +1146,15 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd)
> goto fail;
> }
>
> + 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;
> + }
A little helper for this complicated check would be better, and that will avoid
the built bot warning you got when CONFIG_BLK_DEV_ZONED is not set.
Something like this:
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a38c71511bc9..71e4e51898d8 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1146,6 +1146,9 @@ static blk_status_t sd_setup_read_write_cmnd(struct
scsi_cmnd *cmd)
goto fail;
}
+ if (sdkp->device->type == TYPE_ZBC && !sd_zbc_check_write(cmd))
+ goto fail;
+
if ((blk_rq_pos(rq) & mask) || (blk_rq_sectors(rq) & mask)) {
scmd_printk(KERN_ERR, cmd, "request not aligned to the logical block size\n");
goto fail;
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 5eea762f84d1..f19711b92f25 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -254,6 +254,8 @@ int sd_zbc_report_zones(struct gendisk *disk, sector_t sector,
blk_status_t sd_zbc_prepare_zone_append(struct scsi_cmnd *cmd, sector_t *lba,
unsigned int nr_blocks);
+bool sd_zbc_check_write(struct scsi_cmnd *cmd);
+
#else /* CONFIG_BLK_DEV_ZONED */
static inline void sd_zbc_free_zone_info(struct scsi_disk *sdkp) {}
@@ -290,6 +292,11 @@ static inline blk_status_t
sd_zbc_prepare_zone_append(struct scsi_cmnd *cmd,
#define sd_zbc_report_zones NULL
+static inline bool sd_zbc_check_write(struct scsi_cmnd *cmd)
+{
+ return true;
+}
+
#endif /* CONFIG_BLK_DEV_ZONED */
void sd_print_sense_hdr(struct scsi_disk *sdkp, struct scsi_sense_hdr *sshdr);
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 6b3a02d4406c..3025cb35f30c 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -983,3 +983,33 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, u8
buf[SD_BUF_SIZE])
return ret;
}
+
+/**
+ * sd_zbc_check_write - Check if a write to a sequential zone is aligned to
+ * the physical block size of the disk.
+ * @cmd: The command to check.
+ *
+ * Return false for write and zone append commands that are not aligned to
+ * the disk physical block size and true otherwise.
+ */
+bool sd_zbc_check_write(struct scsi_cmnd *cmd)
+{
+ struct request *rq = scsi_cmd_to_rq(cmd);
+ struct scsi_disk *sdkp = scsi_disk(rq->q->disk);
+ unsigned int pb_sectors = sdkp->physical_block_size >> SECTOR_SHIFT;
+
+ if (!blk_rq_zone_is_seq(rq))
+ return true;
+
+ if (req_op(rq) != REQ_OP_WRITE && req_op(rq) != REQ_OP_ZONE_APPEND)
+ return true;
+
+ if (!IS_ALIGNED(blk_rq_pos(rq), pb_sectors) ||
+ !IS_ALIGNED(blk_rq_sectors(rq), pb_sectors)) {
+ scmd_printk(KERN_ERR, cmd,
+ "Write request not aligned to the physical block size\n");
+ return false;
+ }
+
+ return true;
+}
--
2.39.2
> +
> if ((blk_rq_pos(rq) & mask) || (blk_rq_sectors(rq) & mask)) {
> scmd_printk(KERN_ERR, cmd, "request not aligned to the logical block size\n");
> goto fail;
--
Damien Le Moal
Western Digital Research
