diff mbox

[1/6] scsi: Introduce scsi_zbc_noretry_cmd()

Message ID 20180301194024.25532-2-damien.lemoal@wdc.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Damien Le Moal March 1, 2018, 7:40 p.m. UTC
For ZBC/ZAC devices, retrying a command with a condition known to lead
to a failure is useless. One example is an unaligned write past the
write pointer of a sequential zone. Retrying the same command will
result in an error again.

Currently, these iknown error condition cases are handled in sd_zbc.c
using the sd_zbc_complete() function which is called from sd_done() when
a command completes. However, these known error conditions are not
handled in libata, nor is scsi_noretry_cmd() considering them.

Fix this by introducing the function scsi_zbc_noretry_cmd() and use this
function in scsi_noretry_cmd(). This allows simplifying
sd_zbc_complete() which now only has to deal with report zones command
reply.

scsi_zbc_noretry_cmd() is also exported so that it can be used from
libata.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/scsi/scsi_error.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/sd.c         |  2 +-
 drivers/scsi/sd.h         |  8 +++---
 drivers/scsi/sd_zbc.c     | 47 ++++-----------------------------
 include/scsi/scsi_eh.h    |  1 +
 5 files changed, 77 insertions(+), 47 deletions(-)

Comments

Hannes Reinecke March 2, 2018, 7:31 a.m. UTC | #1
On 03/01/2018 08:40 PM, Damien Le Moal wrote:
> For ZBC/ZAC devices, retrying a command with a condition known to lead
> to a failure is useless. One example is an unaligned write past the
> write pointer of a sequential zone. Retrying the same command will
> result in an error again.
> 
> Currently, these iknown error condition cases are handled in sd_zbc.c
> using the sd_zbc_complete() function which is called from sd_done() when
> a command completes. However, these known error conditions are not
> handled in libata, nor is scsi_noretry_cmd() considering them.
> 
> Fix this by introducing the function scsi_zbc_noretry_cmd() and use this
> function in scsi_noretry_cmd(). This allows simplifying
> sd_zbc_complete() which now only has to deal with report zones command
> reply.
> 
> scsi_zbc_noretry_cmd() is also exported so that it can be used from
> libata.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  drivers/scsi/scsi_error.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/scsi/sd.c         |  2 +-
>  drivers/scsi/sd.h         |  8 +++---
>  drivers/scsi/sd_zbc.c     | 47 ++++-----------------------------
>  include/scsi/scsi_eh.h    |  1 +
>  5 files changed, 77 insertions(+), 47 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
diff mbox

Patch

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index ca53a5f785ee..abb33d250176 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1671,6 +1671,66 @@  static void scsi_eh_offline_sdevs(struct list_head *work_q,
 	return;
 }
 
+/**
+ * scsi_zbc_noretry_cmd - Determine if ZBC device command can be retried
+ * @scmd:       Failed cmd to check
+ *
+ * Test the error condition of a failed ZBC device command to determine cases
+ * that are known to be not worth retrying.
+ * If the specified command is not intended for a ZBC device, do nothing.
+ */
+bool scsi_zbc_noretry_cmd(struct scsi_cmnd *scmd)
+{
+	struct request *rq = scmd->request;
+	struct scsi_sense_hdr sshdr;
+
+	/*
+	 * The request queue zone model may not be set when this is called
+	 * during device probe/revalidation. In that case, just fall back to
+	 * default behavior and let the caller decide what to do with failures.
+	 */
+	if (!blk_queue_is_zoned(rq->q))
+		return false;
+
+	if (!scsi_command_normalize_sense(scmd, &sshdr))
+		/* no valid sense data, don't know, so maybe retry */
+		return false;
+
+	if (sshdr.sense_key != ILLEGAL_REQUEST)
+		return false;
+
+	switch (req_op(rq)) {
+	case REQ_OP_ZONE_RESET:
+
+		if (sshdr.asc == 0x24) {
+			/*
+			 * INVALID FIELD IN CDB error: reset of a conventional
+			 * zone was attempted. Nothing to worry about, so be
+			 * quiet about the error.
+			 */
+			if (!blk_rq_is_passthrough(rq))
+				rq->rq_flags |= RQF_QUIET;
+			return true;
+		}
+		return false;
+
+	case REQ_OP_WRITE:
+	case REQ_OP_WRITE_ZEROES:
+	case REQ_OP_WRITE_SAME:
+
+		/*
+		 * INVALID ADDRESS FOR WRITE error: It is unlikely that
+		 * retrying write requests failed with any kind of
+		 * alignement error will result in success. So don't.
+		 */
+		return sshdr.asc == 0x21;
+
+	default:
+		return false;
+	}
+}
+EXPORT_SYMBOL_GPL(scsi_zbc_noretry_cmd);
+
 /**
  * scsi_noretry_cmd - determine if command should be failed fast
  * @scmd:	SCSI cmd to examine.
@@ -1699,6 +1759,12 @@  int scsi_noretry_cmd(struct scsi_cmnd *scmd)
 		return 0;
 
 check_type:
+	/*
+	 * For ZBC, do not retry conditions that will only fail again.
+	 */
+	if (scmd->device->type == TYPE_ZBC &&
+	    scsi_zbc_noretry_cmd(scmd))
+		return 1;
 	/*
 	 * assume caller has checked sense and determined
 	 * the check condition was retryable.
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index bff21e636ddd..93c6baa7d677 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2041,7 +2041,7 @@  static int sd_done(struct scsi_cmnd *SCpnt)
 
  out:
 	if (sd_is_zoned(sdkp))
-		sd_zbc_complete(SCpnt, good_bytes, &sshdr);
+		sd_zbc_complete(SCpnt, good_bytes);
 
 	SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt,
 					   "sd_done: completed %d of %d bytes\n",
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 0d663b5e45bb..b777ffecf386 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -284,8 +284,7 @@  extern void sd_zbc_remove(struct scsi_disk *sdkp);
 extern void sd_zbc_print_zones(struct scsi_disk *sdkp);
 extern int sd_zbc_setup_report_cmnd(struct scsi_cmnd *cmd);
 extern int sd_zbc_setup_reset_cmnd(struct scsi_cmnd *cmd);
-extern void sd_zbc_complete(struct scsi_cmnd *cmd, unsigned int good_bytes,
-			    struct scsi_sense_hdr *sshdr);
+extern void sd_zbc_complete(struct scsi_cmnd *cmd, unsigned int good_bytes);
 
 #else /* CONFIG_BLK_DEV_ZONED */
 
@@ -310,8 +309,9 @@  static inline int sd_zbc_setup_reset_cmnd(struct scsi_cmnd *cmd)
 }
 
 static inline void sd_zbc_complete(struct scsi_cmnd *cmd,
-				   unsigned int good_bytes,
-				   struct scsi_sense_hdr *sshdr) {}
+				   unsigned int good_bytes)
+{
+}
 
 #endif /* CONFIG_BLK_DEV_ZONED */
 
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 6c348a211ebb..14174f26af98 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -271,53 +271,16 @@  int sd_zbc_setup_reset_cmnd(struct scsi_cmnd *cmd)
  * sd_zbc_complete - ZBC command post processing.
  * @cmd: Completed command
  * @good_bytes: Command reply bytes
- * @sshdr: command sense header
  *
- * Called from sd_done(). Process report zones reply and handle reset zone
- * and write commands errors.
+ * Called from sd_done(). Process report zones reply.
  */
-void sd_zbc_complete(struct scsi_cmnd *cmd, unsigned int good_bytes,
-		     struct scsi_sense_hdr *sshdr)
+void sd_zbc_complete(struct scsi_cmnd *cmd, unsigned int good_bytes)
 {
-	int result = cmd->result;
 	struct request *rq = cmd->request;
 
-	switch (req_op(rq)) {
-	case REQ_OP_ZONE_RESET:
-
-		if (result &&
-		    sshdr->sense_key == ILLEGAL_REQUEST &&
-		    sshdr->asc == 0x24)
-			/*
-			 * INVALID FIELD IN CDB error: reset of a conventional
-			 * zone was attempted. Nothing to worry about, so be
-			 * quiet about the error.
-			 */
-			rq->rq_flags |= RQF_QUIET;
-		break;
-
-	case REQ_OP_WRITE:
-	case REQ_OP_WRITE_ZEROES:
-	case REQ_OP_WRITE_SAME:
-
-		if (result &&
-		    sshdr->sense_key == ILLEGAL_REQUEST &&
-		    sshdr->asc == 0x21)
-			/*
-			 * INVALID ADDRESS FOR WRITE error: It is unlikely that
-			 * retrying write requests failed with any kind of
-			 * alignement error will result in success. So don't.
-			 */
-			cmd->allowed = 0;
-		break;
-
-	case REQ_OP_ZONE_REPORT:
-
-		if (!result)
-			sd_zbc_report_zones_complete(cmd, good_bytes);
-		break;
-
-	}
+	if (req_op(rq) == REQ_OP_ZONE_REPORT &&
+	    !cmd->result)
+		sd_zbc_report_zones_complete(cmd, good_bytes);
 }
 
 /**
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 2b7e227960e1..4c3626192255 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -18,6 +18,7 @@  extern int scsi_block_when_processing_errors(struct scsi_device *);
 extern bool scsi_command_normalize_sense(const struct scsi_cmnd *cmd,
 					 struct scsi_sense_hdr *sshdr);
 extern int scsi_check_sense(struct scsi_cmnd *);
+extern bool scsi_zbc_noretry_cmd(struct scsi_cmnd *);
 
 static inline bool scsi_sense_is_deferred(const struct scsi_sense_hdr *sshdr)
 {