diff mbox series

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

Message ID 20230905231547.83945-16-michael.christie@oracle.com (mailing list archive)
State Changes Requested
Headers show
Series [v11,01/34] scsi: Add helper to prep sense during error handling | expand

Commit Message

Mike Christie Sept. 5, 2023, 11:15 p.m. UTC 
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.

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(-)

Comments

Martin Wilck Sept. 15, 2023, 8:58 p.m. UTC | #1 
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) {
Mike Christie Sept. 15, 2023, 9:52 p.m. UTC | #2 
On 9/15/23 3:58 PM, Martin Wilck wrote:
>> -       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?

Yeah, I see it. I had just kept the existing tab/spacing. Will fix.
diff mbox series

Patch

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",
+		(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) {