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.lem...@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(-)

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)
 {
-- 
2.14.3

Reply via email to