Re: [PATCH 2/5] scsi: Retry unaligned zoned writes

[Khazhy Kumykov <khazhy@google.com>]

[-- Attachment #1: Type: text/plain, Size: 3299 bytes --]

On Tue, Jun 14, 2022 at 10:49 AM Bart Van Assche <bvanassche@acm.org> 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.
Is /just/ retrying effective here? A series of writes to the same zone
would all need to be sent in order - in the worst case (requests
somehow ordered in reverse order) this becomes quadratic as only 1
request "succeeds" out of the N outstanding requests, with the rest
all needing to retry. (Imagine a user writes an entire "zone" - which
could be split into hundreds of requests).

Block layer / schedulers are free to do this reordering, which I
understand does happen whenever we need to requeue - and would result
in a retry of all writes after the first re-ordered request. (side
note: fwiw "requests somehow in reverse order" can happen - bfq
inherited cfq's odd behavior of sometimes issuing sequential IO in
reverse order due to back_seek, e.g.)

>
> 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>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/scsi_error.c | 6 ++++++
>  drivers/scsi/sd.c         | 2 ++
>  2 files changed, 8 insertions(+)
>
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 49ef864df581..8e22d4ba22a3 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -674,6 +674,12 @@ enum scsi_disposition scsi_check_sense(struct scsi_cmnd *scmd)
>                 fallthrough;
>
>         case ILLEGAL_REQUEST:
> +               /*
> +                * Unaligned write command. Retry immediately to handle
> +                * out-of-order zoned writes.
> +                */
> +               if (sshdr.asc == 0x21 && sshdr.ascq == 0x04)
> +                       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 a1a2ac09066f..8d68bd20723e 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1202,6 +1202,8 @@ 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_rq_is_seq_write(rq))
> +               cmd->allowed += rq->q->nr_hw_queues * rq->q->nr_requests;
>         cmd->sdb.length = nr_blocks * sdp->sector_size;
>
>         SCSI_LOG_HLQUEUE(1,

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3999 bytes --]