diff mbox series

[v12,01/20] scsi: Allow passthrough to request scsi-ml retries

Message ID 20231114013750.76609-2-michael.christie@oracle.com (mailing list archive)
State Changes Requested
Headers show
Series scsi: Allow scsi_execute users to request retries | expand

Commit Message

Mike Christie Nov. 14, 2023, 1:37 a.m. UTC
For passthrough we don't retry any error we get a check condition for.
This results in a lot of callers driving their own retries for all UAs,
specific UAs, NOT_READY, specific sense values or any type of failure.

This adds the core code to allow passthrough users to specify what errors
they want scsi-ml to retry for them. We can then convert users to drop
a lot of their sense parsing and retry handling.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/scsi/scsi_lib.c    | 92 ++++++++++++++++++++++++++++++++++++++
 include/scsi/scsi_device.h | 48 ++++++++++++++++++++
 2 files changed, 140 insertions(+)

Comments

John Garry Nov. 16, 2023, 10:53 a.m. UTC | #1
On 14/11/2023 01:37, Mike Christie wrote:
> For passthrough we don't retry any error we get a check condition for.

nit: For passthrough we don't retry any error which we get a check 
condition for.

> This results in a lot of callers driving their own retries for all UAs,
> specific UAs, NOT_READY, specific sense values or any type of failure.
> 
> This adds the core code to allow passthrough users to specify what errors
> they want scsi-ml to retry for them. We can then convert users to drop
> a lot of their sense parsing and retry handling.
> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>

Looks ok to me, so feel free to pick up:
Reviewed-by: John Garry <john.g.garry@oracle.com>

> ---
>   drivers/scsi/scsi_lib.c    | 92 ++++++++++++++++++++++++++++++++++++++
>   include/scsi/scsi_device.h | 48 ++++++++++++++++++++
>   2 files changed, 140 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index cf3864f72093..dee43c6f7ad0 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -184,6 +184,92 @@ void scsi_queue_insert(struct scsi_cmnd *cmd, int reason)
>   	__scsi_queue_insert(cmd, reason, true);
>   }
>   
> +void scsi_reset_failures(struct scsi_failures *failures)

nit: maybe scsi_reset_failures_retries as the name, to be more specific

> +{
> +	struct scsi_failure *failure;
> +
> +	failures->total_retries = 0;
> +
> +	for (failure = failures->failure_definitions; failure->result;
> +	     failure++)
> +		failure->retries = 0;
> +}
> +EXPORT_SYMBOL_GPL(scsi_reset_failures);
> +
> +/**
> + * scsi_check_passthrough - Determine if passthrough scsi_cmnd needs a retry.
> + * @scmd: scsi_cmnd to check.
> + * @failures: scsi_failures struct that lists failures to check for.
> + *
> + * Returns -EAGAIN if the caller should retry else 0.
> + */
> +static int scsi_check_passthrough(struct scsi_cmnd *scmd,
> +				  struct scsi_failures *failures)
> +{
> +	struct scsi_failure *failure;
> +	struct scsi_sense_hdr sshdr;
> +	enum sam_status status;
> +
> +	if (!failures)
> +		return 0;
> +
> +	for (failure = failures->failure_definitions; failure->result;
> +	     failure++) {
> +		if (failure->result == SCMD_FAILURE_RESULT_ANY)
> +			goto maybe_retry;
> +
> +		if (host_byte(scmd->result) &&
> +		    host_byte(scmd->result) == host_byte(failure->result))
> +			goto maybe_retry;
> +
> +		status = status_byte(scmd->result);
> +		if (!status)
> +			continue;
> +
> +		if (failure->result == SCMD_FAILURE_STAT_ANY &&
> +		    !scsi_status_is_good(scmd->result))
> +			goto maybe_retry;
> +
> +		if (status != status_byte(failure->result))
> +			continue;
> +
> +		if (status_byte(failure->result) != SAM_STAT_CHECK_CONDITION ||
> +		    failure->sense == SCMD_FAILURE_SENSE_ANY)
> +			goto maybe_retry;
> +
> +		if (!scsi_command_normalize_sense(scmd, &sshdr))
> +			return 0;
> +
> +		if (failure->sense != sshdr.sense_key)
> +			continue;
> +
> +		if (failure->asc == SCMD_FAILURE_ASC_ANY)
> +			goto maybe_retry;
> +
> +		if (failure->asc != sshdr.asc)
> +			continue;
> +
> +		if (failure->ascq == SCMD_FAILURE_ASCQ_ANY ||
> +		    failure->ascq == sshdr.ascq)
> +			goto maybe_retry;
> +	}
> +
> +	return 0;
> +
> +maybe_retry:
> +	if (failure->allowed) {
> +		if (failure->allowed == SCMD_FAILURE_NO_LIMIT ||
> +		    ++failure->retries <= failure->allowed)
> +			return -EAGAIN;
> +	} else {
> +		if (failures->total_allowed == SCMD_FAILURE_NO_LIMIT ||
> +		    ++failures->total_retries <= failures->total_allowed)
> +			return -EAGAIN;
> +	}
> +
> +	return 0;
> +}
> +
>   /**
>    * scsi_execute_cmd - insert request and wait for the result
>    * @sdev:	scsi_device
> @@ -214,6 +300,7 @@ int scsi_execute_cmd(struct scsi_device *sdev, const unsigned char *cmd,
>   			      args->sense_len != SCSI_SENSE_BUFFERSIZE))
>   		return -EINVAL;
>   
> +retry:
>   	req = scsi_alloc_request(sdev->request_queue, opf, args->req_flags);
>   	if (IS_ERR(req))
>   		return PTR_ERR(req);
> @@ -237,6 +324,11 @@ int scsi_execute_cmd(struct scsi_device *sdev, const unsigned char *cmd,
>   	 */
>   	blk_execute_rq(req, true);
>   
> +	if (scsi_check_passthrough(scmd, args->failures) == -EAGAIN) {
> +		blk_mq_free_request(req);
> +		goto retry;
> +	}
> +
>   	/*
>   	 * Some devices (USB mass-storage in particular) may transfer
>   	 * garbage data together with a residue indicating that the data
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 10480eb582b2..c92d6d9e644e 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -483,6 +483,52 @@ extern int scsi_is_sdev_device(const struct device *);
>   extern int scsi_is_target_device(const struct device *);
>   extern void scsi_sanitize_inquiry_string(unsigned char *s, int len);
>   
> +/*
> + * scsi_execute_cmd users can set scsi_failure.result to have
> + * scsi_check_passthrough fail/retry a command. scsi_failure.result can be a
> + * specific host byte or message code, or SCMD_FAILURE_RESULT_ANY can be used
> + * to match any host or message code.
> + */
> +#define SCMD_FAILURE_RESULT_ANY	0x7fffffff
> +/*
> + * Set scsi_failure.result to SCMD_FAILURE_STAT_ANY to fail/retry any failure
> + * scsi_status_is_good returns false for.
> + */
> +#define SCMD_FAILURE_STAT_ANY	0xff
> +/*
> + * The following can be set to the scsi_failure sense, asc and ascq fields to
> + * match on any sense, ASC, or ASCQ value.
> + */
> +#define SCMD_FAILURE_SENSE_ANY	0xff
> +#define SCMD_FAILURE_ASC_ANY	0xff
> +#define SCMD_FAILURE_ASCQ_ANY	0xff
> +/* Always retry a matching failure. */
> +#define SCMD_FAILURE_NO_LIMIT	-1
> +
> +struct scsi_failure {
> +	int result;
> +	u8 sense;
> +	u8 asc;
> +	u8 ascq;
> +
> +	/*
> +	 * Number of attempts allowed for this failure. It does not count
> +	 * for the total_allowed.
> +	 */
> +	s8 allowed;
> +	s8 retries;
> +};
> +
> +struct scsi_failures {
> +	/*
> +	 * If the failure does not have a specific limit in the scsi_failure
> +	 * then this limit is followed.
> +	 */
> +	int total_allowed;
> +	int total_retries;
> +	struct scsi_failure *failure_definitions;
> +};
> +
>   /* Optional arguments to scsi_execute_cmd */
>   struct scsi_exec_args {
>   	unsigned char *sense;		/* sense buffer */
> @@ -491,12 +537,14 @@ struct scsi_exec_args {
>   	blk_mq_req_flags_t req_flags;	/* BLK_MQ_REQ flags */
>   	int scmd_flags;			/* SCMD flags */
>   	int *resid;			/* residual length */
> +	struct scsi_failures *failures;	/* failures to retry */
>   };
>   
>   int scsi_execute_cmd(struct scsi_device *sdev, const unsigned char *cmd,
>   		     blk_opf_t opf, void *buffer, unsigned int bufflen,
>   		     int timeout, int retries,
>   		     const struct scsi_exec_args *args);
> +void scsi_reset_failures(struct scsi_failures *failures);
>   
>   extern void sdev_disable_disk_events(struct scsi_device *sdev);
>   extern void sdev_enable_disk_events(struct scsi_device *sdev);
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index cf3864f72093..dee43c6f7ad0 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -184,6 +184,92 @@  void scsi_queue_insert(struct scsi_cmnd *cmd, int reason)
 	__scsi_queue_insert(cmd, reason, true);
 }
 
+void scsi_reset_failures(struct scsi_failures *failures)
+{
+	struct scsi_failure *failure;
+
+	failures->total_retries = 0;
+
+	for (failure = failures->failure_definitions; failure->result;
+	     failure++)
+		failure->retries = 0;
+}
+EXPORT_SYMBOL_GPL(scsi_reset_failures);
+
+/**
+ * scsi_check_passthrough - Determine if passthrough scsi_cmnd needs a retry.
+ * @scmd: scsi_cmnd to check.
+ * @failures: scsi_failures struct that lists failures to check for.
+ *
+ * Returns -EAGAIN if the caller should retry else 0.
+ */
+static int scsi_check_passthrough(struct scsi_cmnd *scmd,
+				  struct scsi_failures *failures)
+{
+	struct scsi_failure *failure;
+	struct scsi_sense_hdr sshdr;
+	enum sam_status status;
+
+	if (!failures)
+		return 0;
+
+	for (failure = failures->failure_definitions; failure->result;
+	     failure++) {
+		if (failure->result == SCMD_FAILURE_RESULT_ANY)
+			goto maybe_retry;
+
+		if (host_byte(scmd->result) &&
+		    host_byte(scmd->result) == host_byte(failure->result))
+			goto maybe_retry;
+
+		status = status_byte(scmd->result);
+		if (!status)
+			continue;
+
+		if (failure->result == SCMD_FAILURE_STAT_ANY &&
+		    !scsi_status_is_good(scmd->result))
+			goto maybe_retry;
+
+		if (status != status_byte(failure->result))
+			continue;
+
+		if (status_byte(failure->result) != SAM_STAT_CHECK_CONDITION ||
+		    failure->sense == SCMD_FAILURE_SENSE_ANY)
+			goto maybe_retry;
+
+		if (!scsi_command_normalize_sense(scmd, &sshdr))
+			return 0;
+
+		if (failure->sense != sshdr.sense_key)
+			continue;
+
+		if (failure->asc == SCMD_FAILURE_ASC_ANY)
+			goto maybe_retry;
+
+		if (failure->asc != sshdr.asc)
+			continue;
+
+		if (failure->ascq == SCMD_FAILURE_ASCQ_ANY ||
+		    failure->ascq == sshdr.ascq)
+			goto maybe_retry;
+	}
+
+	return 0;
+
+maybe_retry:
+	if (failure->allowed) {
+		if (failure->allowed == SCMD_FAILURE_NO_LIMIT ||
+		    ++failure->retries <= failure->allowed)
+			return -EAGAIN;
+	} else {
+		if (failures->total_allowed == SCMD_FAILURE_NO_LIMIT ||
+		    ++failures->total_retries <= failures->total_allowed)
+			return -EAGAIN;
+	}
+
+	return 0;
+}
+
 /**
  * scsi_execute_cmd - insert request and wait for the result
  * @sdev:	scsi_device
@@ -214,6 +300,7 @@  int scsi_execute_cmd(struct scsi_device *sdev, const unsigned char *cmd,
 			      args->sense_len != SCSI_SENSE_BUFFERSIZE))
 		return -EINVAL;
 
+retry:
 	req = scsi_alloc_request(sdev->request_queue, opf, args->req_flags);
 	if (IS_ERR(req))
 		return PTR_ERR(req);
@@ -237,6 +324,11 @@  int scsi_execute_cmd(struct scsi_device *sdev, const unsigned char *cmd,
 	 */
 	blk_execute_rq(req, true);
 
+	if (scsi_check_passthrough(scmd, args->failures) == -EAGAIN) {
+		blk_mq_free_request(req);
+		goto retry;
+	}
+
 	/*
 	 * Some devices (USB mass-storage in particular) may transfer
 	 * garbage data together with a residue indicating that the data
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 10480eb582b2..c92d6d9e644e 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -483,6 +483,52 @@  extern int scsi_is_sdev_device(const struct device *);
 extern int scsi_is_target_device(const struct device *);
 extern void scsi_sanitize_inquiry_string(unsigned char *s, int len);
 
+/*
+ * scsi_execute_cmd users can set scsi_failure.result to have
+ * scsi_check_passthrough fail/retry a command. scsi_failure.result can be a
+ * specific host byte or message code, or SCMD_FAILURE_RESULT_ANY can be used
+ * to match any host or message code.
+ */
+#define SCMD_FAILURE_RESULT_ANY	0x7fffffff
+/*
+ * Set scsi_failure.result to SCMD_FAILURE_STAT_ANY to fail/retry any failure
+ * scsi_status_is_good returns false for.
+ */
+#define SCMD_FAILURE_STAT_ANY	0xff
+/*
+ * The following can be set to the scsi_failure sense, asc and ascq fields to
+ * match on any sense, ASC, or ASCQ value.
+ */
+#define SCMD_FAILURE_SENSE_ANY	0xff
+#define SCMD_FAILURE_ASC_ANY	0xff
+#define SCMD_FAILURE_ASCQ_ANY	0xff
+/* Always retry a matching failure. */
+#define SCMD_FAILURE_NO_LIMIT	-1
+
+struct scsi_failure {
+	int result;
+	u8 sense;
+	u8 asc;
+	u8 ascq;
+
+	/*
+	 * Number of attempts allowed for this failure. It does not count
+	 * for the total_allowed.
+	 */
+	s8 allowed;
+	s8 retries;
+};
+
+struct scsi_failures {
+	/*
+	 * If the failure does not have a specific limit in the scsi_failure
+	 * then this limit is followed.
+	 */
+	int total_allowed;
+	int total_retries;
+	struct scsi_failure *failure_definitions;
+};
+
 /* Optional arguments to scsi_execute_cmd */
 struct scsi_exec_args {
 	unsigned char *sense;		/* sense buffer */
@@ -491,12 +537,14 @@  struct scsi_exec_args {
 	blk_mq_req_flags_t req_flags;	/* BLK_MQ_REQ flags */
 	int scmd_flags;			/* SCMD flags */
 	int *resid;			/* residual length */
+	struct scsi_failures *failures;	/* failures to retry */
 };
 
 int scsi_execute_cmd(struct scsi_device *sdev, const unsigned char *cmd,
 		     blk_opf_t opf, void *buffer, unsigned int bufflen,
 		     int timeout, int retries,
 		     const struct scsi_exec_args *args);
+void scsi_reset_failures(struct scsi_failures *failures);
 
 extern void sdev_disable_disk_events(struct scsi_device *sdev);
 extern void sdev_enable_disk_events(struct scsi_device *sdev);