Re: [PATCH v11 15/34] scsi: rdac: Have scsi-ml retry send_mode_select errors

[Martin Wilck <mwilck@suse.com>]

On Tue, 2023-09-05 at 18:15 -0500, Mike Christie wrote:
> This has rdac have scsi-ml retry errors instead of driving them
> itself.
> 
> There is one behavior change with this patch. We used to get a total
> of
> 5 retries for errors mode_select_handle_sense returned SCSI_DH_RETRY.
> We
> now get 5 retries for each failure.

... making me think of the total retry count again.

> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/device_handler/scsi_dh_rdac.c | 87 ++++++++++++--------
> --
>  1 file changed, 49 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c
> b/drivers/scsi/device_handler/scsi_dh_rdac.c
> index 1ac2ae17e8be..771108a332cb 100644
> --- a/drivers/scsi/device_handler/scsi_dh_rdac.c
> +++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
> @@ -485,43 +485,17 @@ static int set_mode_select(struct scsi_device
> *sdev, struct rdac_dh_data *h)
>  static int mode_select_handle_sense(struct scsi_device *sdev,
>                                     struct scsi_sense_hdr *sense_hdr)
>  {
> -       int err = SCSI_DH_IO;
>         struct rdac_dh_data *h = sdev->handler_data;
>  
>         if (!scsi_sense_valid(sense_hdr))
> -               goto done;
> -
> -       switch (sense_hdr->sense_key) {
> -       case NO_SENSE:
> -       case ABORTED_COMMAND:
> -       case UNIT_ATTENTION:
> -               err = SCSI_DH_RETRY;
> -               break;
> -       case NOT_READY:
> -               if (sense_hdr->asc == 0x04 && sense_hdr->ascq ==
> 0x01)
> -                       /* LUN Not Ready and is in the Process of
> Becoming
> -                        * Ready
> -                        */
> -                       err = SCSI_DH_RETRY;
> -               break;
> -       case ILLEGAL_REQUEST:
> -               if (sense_hdr->asc == 0x91 && sense_hdr->ascq ==
> 0x36)
> -                       /*
> -                        * Command Lock contention
> -                        */
> -                       err = SCSI_DH_IMM_RETRY;
> -               break;
> -       default:
> -               break;
> -       }
> +               return SCSI_DH_IO;
>  
>         RDAC_LOG(RDAC_LOG_FAILOVER, sdev, "array %s, ctlr %d, "
>                 "MODE_SELECT returned with sense %02x/%02x/%02x",
>                 (char *) h->ctlr->array_name, h->ctlr->index,
>                 sense_hdr->sense_key, sense_hdr->asc, sense_hdr-
> >ascq);
>  
> -done:
> -       return err;
> +       return SCSI_DH_IO;
>  }
>  
>  static void send_mode_select(struct work_struct *work)
> @@ -530,7 +504,7 @@ static void send_mode_select(struct work_struct
> *work)
>                 container_of(work, struct rdac_controller, ms_work);
>         struct scsi_device *sdev = ctlr->ms_sdev;
>         struct rdac_dh_data *h = sdev->handler_data;
> -       int rc, err, retry_cnt = RDAC_RETRY_COUNT;
> +       int rc, err;
>         struct rdac_queue_data *tmp, *qdata;
>         LIST_HEAD(list);
>         unsigned char cdb[MAX_COMMAND_SIZE];
> @@ -538,8 +512,52 @@ static void send_mode_select(struct work_struct
> *work)
>         unsigned int data_size;
>         blk_opf_t opf = REQ_OP_DRV_OUT | REQ_FAILFAST_DEV |
>                                 REQ_FAILFAST_TRANSPORT |
> REQ_FAILFAST_DRIVER;
> +       struct scsi_failure failures[] = {
> +               {
> +                       .sense = NO_SENSE,
> +                       .asc = SCMD_FAILURE_ASC_ANY,
> +                       .ascq = SCMD_FAILURE_ASCQ_ANY,
> +                       .allowed = RDAC_RETRY_COUNT,
> +                       .result = SAM_STAT_CHECK_CONDITION,
> +               },
> +               {
> +                       .sense = ABORTED_COMMAND,
> +                       .asc = SCMD_FAILURE_ASC_ANY,
> +                       .ascq = SCMD_FAILURE_ASCQ_ANY,
> +                       .allowed = RDAC_RETRY_COUNT,
> +                       .result = SAM_STAT_CHECK_CONDITION,
> +               },
> +               {
> +                       .sense = UNIT_ATTENTION,
> +                       .asc = SCMD_FAILURE_ASC_ANY,
> +                       .ascq = SCMD_FAILURE_ASCQ_ANY,
> +                       .allowed = RDAC_RETRY_COUNT,
> +                       .result = SAM_STAT_CHECK_CONDITION,
> +               },
> +               {
> +                       /*
> +                        * LUN Not Ready and is in the Process of
> Becoming
> +                        * Ready
> +                        */
> +                       .sense = NOT_READY,
> +                       .asc = 0x04,
> +                       .ascq = 0x01,
> +                       .allowed = RDAC_RETRY_COUNT,
> +                       .result = SAM_STAT_CHECK_CONDITION,
> +               },
> +               {
> +                       /* Command Lock contention */
> +                       .sense = ILLEGAL_REQUEST,
> +                       .asc = 0x91,
> +                       .ascq = 0x36,
> +                       .allowed = SCMD_FAILURE_NO_LIMIT,
> +                       .result = SAM_STAT_CHECK_CONDITION,
> +               },
> +               {}
> +       };
>         const struct scsi_exec_args exec_args = {
>                 .sshdr = &sshdr,
> +               .failures = failures,
>         };
>  
>         spin_lock(&ctlr->ms_lock);
> @@ -548,15 +566,12 @@ static void send_mode_select(struct work_struct
> *work)
>         ctlr->ms_sdev = NULL;
>         spin_unlock(&ctlr->ms_lock);
>  
> - retry:
>         memset(cdb, 0, sizeof(cdb));
>  
>         data_size = rdac_failover_get(ctlr, &list, cdb);
>  
> -       RDAC_LOG(RDAC_LOG_FAILOVER, sdev, "array %s, ctlr %d, "
> -               "%s MODE_SELECT command",
> -               (char *) h->ctlr->array_name, h->ctlr->index,
> -               (retry_cnt == RDAC_RETRY_COUNT) ? "queueing" :
> "retrying");
> +       RDAC_LOG(RDAC_LOG_FAILOVER, sdev, "array %s, ctlr %d,
> queueingMODE_SELECT command",

missing space?


> +               (char *) h->ctlr->array_name, h->ctlr->index);
>  
>         rc = scsi_execute_cmd(sdev, cdb, opf, &h->ctlr->mode_select,
> data_size,
>                               RDAC_TIMEOUT * HZ, RDAC_RETRIES,
> &exec_args);
> @@ -570,10 +585,6 @@ static void send_mode_select(struct work_struct
> *work)
>                 err = SCSI_DH_IO;
>         } else {
>                 err = mode_select_handle_sense(sdev, &sshdr);
> -               if (err == SCSI_DH_RETRY && retry_cnt--)
> -                       goto retry;
> -               if (err == SCSI_DH_IMM_RETRY)
> -                       goto retry;
>         }
>  
>         list_for_each_entry_safe(qdata, tmp, &list, entry) {