| Message ID | 20230109232738.169886-8-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable zoned write pipelining for UFS devices | expand |
On 1/10/23 08:27, Bart Van Assche wrote: > From ZBC-2: "The device server terminates with CHECK CONDITION status, with > the sense key set to ILLEGAL REQUEST, and the additional sense code set to > UNALIGNED WRITE COMMAND a write command, other than an entire medium write > same command, that specifies: a) the starting LBA in a sequential write > required zone set to a value that is not equal to the write pointer for that > sequential write required zone; or b) an ending LBA that is not equal to the > last logical block within a physical block (see SBC-5)." > > I am not aware of any other conditions that may trigger the UNALIGNED > WRITE COMMAND response. > > Retry unaligned writes in preparation of removing zone locking. > > Increase the number of retries for write commands sent to a sequential > zone to the maximum number of outstanding commands. > > Cc: Martin K. Petersen <martin.petersen@oracle.com> > Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/scsi/scsi_error.c | 7 +++++++ > drivers/scsi/sd.c | 3 +++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c > index a7960ad2d386..e17f36b0b18a 100644 > --- a/drivers/scsi/scsi_error.c > +++ b/drivers/scsi/scsi_error.c > @@ -676,6 +676,13 @@ enum scsi_disposition scsi_check_sense(struct scsi_cmnd *scmd) > fallthrough; > > case ILLEGAL_REQUEST: > + /* > + * Unaligned write command. Retry immediately if pipelining > + * zoned writes has been enabled. > + */ > + if (sshdr.asc == 0x21 && sshdr.ascq == 0x04 && > + blk_queue_pipeline_zoned_writes(sdev->request_queue)) > + return NEEDS_RETRY; > if (sshdr.asc == 0x20 || /* Invalid command operation code */ > sshdr.asc == 0x21 || /* Logical block address out of range */ > sshdr.asc == 0x22 || /* Invalid function */ > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > index 47dafe6b8a66..cd90b54a6597 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -1207,6 +1207,9 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) > cmd->transfersize = sdp->sector_size; > cmd->underflow = nr_blocks << 9; > cmd->allowed = sdkp->max_retries; > + if (blk_queue_pipeline_zoned_writes(rq->q) && > + blk_rq_is_seq_zone_write(rq)) > + cmd->allowed += rq->q->nr_requests; > cmd->sdb.length = nr_blocks * sdp->sector_size; > > SCSI_LOG_HLQUEUE(1, Aouch... The amount of IO errors logged in the the kernel log will not be pretty, no ? At least for scsi disks, the problem is that a write may partially fail (the command hit a bad sector). Such write failure will cause all queued writes after it to fail and no amount of retry will help. Not sure about UFS devices though. Can you get a partial failure ?
On 1/9/23 15:51, Damien Le Moal wrote: > On 1/10/23 08:27, Bart Van Assche wrote: >> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c >> index 47dafe6b8a66..cd90b54a6597 100644 >> --- a/drivers/scsi/sd.c >> +++ b/drivers/scsi/sd.c >> @@ -1207,6 +1207,9 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) >> cmd->transfersize = sdp->sector_size; >> cmd->underflow = nr_blocks << 9; >> cmd->allowed = sdkp->max_retries; >> + if (blk_queue_pipeline_zoned_writes(rq->q) && >> + blk_rq_is_seq_zone_write(rq)) >> + cmd->allowed += rq->q->nr_requests; >> cmd->sdb.length = nr_blocks * sdp->sector_size; >> >> SCSI_LOG_HLQUEUE(1, > > Aouch... The amount of IO errors logged in the the kernel log will not be > pretty, no ? > > At least for scsi disks, the problem is that a write may partially fail > (the command hit a bad sector). Such write failure will cause all queued > writes after it to fail and no amount of retry will help. Not sure about > UFS devices though. Can you get a partial failure ? Although the UFS specification supports residuals, one of the open bugs in the UFS driver is that the residual field from the UFS device response is ignored. I think this means that no UFS device vendor cares enough about partial failures to implement support for partial failures. Please let me know if you want me to look into suppressing error messages triggered by this new retry mechanism. Thanks, Bart.
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index a7960ad2d386..e17f36b0b18a 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -676,6 +676,13 @@ enum scsi_disposition scsi_check_sense(struct scsi_cmnd *scmd) fallthrough; case ILLEGAL_REQUEST: + /* + * Unaligned write command. Retry immediately if pipelining + * zoned writes has been enabled. + */ + if (sshdr.asc == 0x21 && sshdr.ascq == 0x04 && + blk_queue_pipeline_zoned_writes(sdev->request_queue)) + return NEEDS_RETRY; if (sshdr.asc == 0x20 || /* Invalid command operation code */ sshdr.asc == 0x21 || /* Logical block address out of range */ sshdr.asc == 0x22 || /* Invalid function */ diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 47dafe6b8a66..cd90b54a6597 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1207,6 +1207,9 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) cmd->transfersize = sdp->sector_size; cmd->underflow = nr_blocks << 9; cmd->allowed = sdkp->max_retries; + if (blk_queue_pipeline_zoned_writes(rq->q) && + blk_rq_is_seq_zone_write(rq)) + cmd->allowed += rq->q->nr_requests; cmd->sdb.length = nr_blocks * sdp->sector_size; SCSI_LOG_HLQUEUE(1,
From ZBC-2: "The device server terminates with CHECK CONDITION status, with the sense key set to ILLEGAL REQUEST, and the additional sense code set to UNALIGNED WRITE COMMAND a write command, other than an entire medium write same command, that specifies: a) the starting LBA in a sequential write required zone set to a value that is not equal to the write pointer for that sequential write required zone; or b) an ending LBA that is not equal to the last logical block within a physical block (see SBC-5)." I am not aware of any other conditions that may trigger the UNALIGNED WRITE COMMAND response. Retry unaligned writes in preparation of removing zone locking. Increase the number of retries for write commands sent to a sequential zone to the maximum number of outstanding commands. Cc: Martin K. Petersen <martin.petersen@oracle.com> Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/scsi/scsi_error.c | 7 +++++++ drivers/scsi/sd.c | 3 +++ 2 files changed, 10 insertions(+)