diff mbox series

[v9,04/33] scsi: Have scsi-ml retry scsi_probe_lun errors

Message ID 20230711214620.87232-5-michael.christie@oracle.com (mailing list archive)
State Superseded
Headers show
Series scsi: Allow scsi_execute users to control retries | expand

Commit Message

Mike Christie July 11, 2023, 9:45 p.m. UTC 
This has scsi_probe_lun ask scsi-ml to retry UAs instead of driving them
itself.

There is one behavior change with this patch. We used to get a total of
3 retries for both UAs we were checking for. We now get 3 retries for
each.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_scan.c | 42 +++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 18 deletions(-)

Comments

John Garry July 12, 2023, 11:17 a.m. UTC | #1 
On 11/07/2023 22:45, Mike Christie wrote:
> This has scsi_probe_lun ask scsi-ml to retry UAs instead of driving them
> itself.
> 
> There is one behavior change with this patch. We used to get a total of
> 3 retries for both UAs we were checking for. We now get 3 retries for
> each.
> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: John Garry <john.g.garry@oracle.com>

> ---
>   drivers/scsi/scsi_scan.c | 42 +++++++++++++++++++++++-----------------
>   1 file changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index aa13feb17c62..519755952254 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -647,10 +647,29 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
>   	int first_inquiry_len, try_inquiry_len, next_inquiry_len;
>   	int response_len = 0;
>   	int pass, count, result, resid;
> -	struct scsi_sense_hdr sshdr;
> +	/*
> +	 * not-ready to ready transition [asc/ascq=0x28/0x0] or power-on,
> +	 * reset [asc/ascq=0x29/0x0], continue. INQUIRY should not yield
> +	 * UNIT_ATTENTION but many buggy devices do so anyway.
> +	 */
> +	struct scsi_failure failures[] = {
> +		{
> +			.sense = UNIT_ATTENTION,
> +			.asc = 0x28,
> +			.allowed = 3,
> +			.result = SAM_STAT_CHECK_CONDITION,
> +		},
> +		{
> +			.sense = UNIT_ATTENTION,
> +			.asc = 0x29,
> +			.allowed = 3,
> +			.result = SAM_STAT_CHECK_CONDITION,
> +		},
> +		{},

nit: no need for ',' on a sentinel

> +	};
>   	const struct scsi_exec_args exec_args = {
> -		.sshdr = &sshdr,
>   		.resid = &resid,
> +		.failures = failures,
>   	};
>   
>   	*bflags = 0;
> @@ -668,6 +687,8 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
>   				pass, try_inquiry_len));
>   
>   	/* Each pass gets up to three chances to ignore Unit Attention */
> +	scsi_reset_failures(failures);
> +
>   	for (count = 0; count < 3; ++count) {
>   		memset(scsi_cmd, 0, 6);
>   		scsi_cmd[0] = INQUIRY;
> @@ -684,22 +705,7 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
>   				"scsi scan: INQUIRY %s with code 0x%x\n",
>   				result ? "failed" : "successful", result));
>   
> -		if (result > 0) {
> -			/*
> -			 * not-ready to ready transition [asc/ascq=0x28/0x0]
> -			 * or power-on, reset [asc/ascq=0x29/0x0], continue.
> -			 * INQUIRY should not yield UNIT_ATTENTION
> -			 * but many buggy devices do so anyway.
> -			 */
> -			if (scsi_status_is_check_condition(result) &&
> -			    scsi_sense_valid(&sshdr)) {
> -				if ((sshdr.sense_key == UNIT_ATTENTION) &&
> -				    ((sshdr.asc == 0x28) ||
> -				     (sshdr.asc == 0x29)) &&
> -				    (sshdr.ascq == 0))
> -					continue;
> -			}
> -		} else if (result == 0) {
> +		if (result == 0) {
>   			/*
>   			 * if nothing was transferred, we try
>   			 * again. It's a workaround for some USB
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index aa13feb17c62..519755952254 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -647,10 +647,29 @@  static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	int first_inquiry_len, try_inquiry_len, next_inquiry_len;
 	int response_len = 0;
 	int pass, count, result, resid;
-	struct scsi_sense_hdr sshdr;
+	/*
+	 * not-ready to ready transition [asc/ascq=0x28/0x0] or power-on,
+	 * reset [asc/ascq=0x29/0x0], continue. INQUIRY should not yield
+	 * UNIT_ATTENTION but many buggy devices do so anyway.
+	 */
+	struct scsi_failure failures[] = {
+		{
+			.sense = UNIT_ATTENTION,
+			.asc = 0x28,
+			.allowed = 3,
+			.result = SAM_STAT_CHECK_CONDITION,
+		},
+		{
+			.sense = UNIT_ATTENTION,
+			.asc = 0x29,
+			.allowed = 3,
+			.result = SAM_STAT_CHECK_CONDITION,
+		},
+		{},
+	};
 	const struct scsi_exec_args exec_args = {
-		.sshdr = &sshdr,
 		.resid = &resid,
+		.failures = failures,
 	};
 
 	*bflags = 0;
@@ -668,6 +687,8 @@  static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
 				pass, try_inquiry_len));
 
 	/* Each pass gets up to three chances to ignore Unit Attention */
+	scsi_reset_failures(failures);
+
 	for (count = 0; count < 3; ++count) {
 		memset(scsi_cmd, 0, 6);
 		scsi_cmd[0] = INQUIRY;
@@ -684,22 +705,7 @@  static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
 				"scsi scan: INQUIRY %s with code 0x%x\n",
 				result ? "failed" : "successful", result));
 
-		if (result > 0) {
-			/*
-			 * not-ready to ready transition [asc/ascq=0x28/0x0]
-			 * or power-on, reset [asc/ascq=0x29/0x0], continue.
-			 * INQUIRY should not yield UNIT_ATTENTION
-			 * but many buggy devices do so anyway. 
-			 */
-			if (scsi_status_is_check_condition(result) &&
-			    scsi_sense_valid(&sshdr)) {
-				if ((sshdr.sense_key == UNIT_ATTENTION) &&
-				    ((sshdr.asc == 0x28) ||
-				     (sshdr.asc == 0x29)) &&
-				    (sshdr.ascq == 0))
-					continue;
-			}
-		} else if (result == 0) {
+		if (result == 0) {
 			/*
 			 * if nothing was transferred, we try
 			 * again. It's a workaround for some USB