Break out several interwined paths when cmd->result is non zero and
place them in scsi_io_completion_nz_result helper function. The logic
is not changed.

Signed-off-by: Douglas Gilbert <[email protected]>
---
A reviewer requested the original helper function's two return values
be reduced to one: the blk_stat variable. This required a hack to
differentiate the default setting of blk_stat (BLK_STS_OK) from the case
when the helper assigns BLK_STS_OK as the return value. The hack is to
return the otherwise unused BLK_STS_NOTSUPP value as an indication that
the helper didn't change anything.

 drivers/scsi/scsi_lib.c | 134 +++++++++++++++++++++++++++++-------------------
 1 file changed, 82 insertions(+), 52 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a4da5773d42d..9731d0d3cc80 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -751,6 +751,77 @@ static blk_status_t __scsi_error_from_host_byte(struct 
scsi_cmnd *cmd,
        }
 }
 
+/*
+ * Helper for scsi_io_completion() when cmd->result is non-zero. Returns
+ * BLK_STS_NOTSUPP if this function does not change blk_status .
+ */
+static blk_status_t scsi_io_completion_nz_result(struct scsi_cmnd *cmd,
+                                                int result)
+{
+       bool sense_valid;
+       bool about_current;
+       /* __scsi_error_from_host_byte() does not return BLK_STS_NOTSUPP */
+       blk_status_t blk_stat = BLK_STS_NOTSUPP;
+       struct request *req = cmd->request;
+       struct scsi_sense_hdr sshdr;
+
+       sense_valid = scsi_command_normalize_sense(cmd, &sshdr);
+       about_current = sense_valid ? !scsi_sense_is_deferred(&sshdr) : true;
+
+       if (blk_rq_is_passthrough(req)) {
+               if (sense_valid) {
+                       /*
+                        * SG_IO wants current and deferred errors
+                        */
+                       scsi_req(req)->sense_len =
+                               min(8 + cmd->sense_buffer[7],
+                                   SCSI_SENSE_BUFFERSIZE);
+               }
+               if (about_current)
+                       blk_stat = __scsi_error_from_host_byte(cmd, result);
+       } else if (blk_rq_bytes(req) == 0 && about_current) {
+               /*
+                * Flush commands do not transfers any data, and thus cannot use
+                * good_bytes != blk_rq_bytes(req) as the signal for an error.
+                * This sets blk_stat explicitly for the problem case.
+                */
+               blk_stat = __scsi_error_from_host_byte(cmd, result);
+       }
+       /*
+        * Recovered errors need reporting, but they're always treated as
+        * success, so fiddle the result code here.  For passthrough requests
+        * we already took a copy of the original into sreq->result which
+        * is what gets returned to the user
+        */
+       if (sense_valid && (sshdr.sense_key == RECOVERED_ERROR)) {
+               bool do_print = true;
+               /*
+                * if ATA PASS-THROUGH INFORMATION AVAILABLE [0x0, 0x1d]
+                * skip print since caller wants ATA registers. Only occurs
+                * on SCSI ATA PASS_THROUGH commands when CK_COND=1
+                */
+               if ((sshdr.asc == 0x0) && (sshdr.ascq == 0x1d))
+                       do_print = false;
+               else if (req->rq_flags & RQF_QUIET)
+                       do_print = false;
+               if (do_print)
+                       scsi_print_sense(cmd);
+               /* for passthrough, blk_stat may be set */
+               blk_stat = BLK_STS_OK;
+       }
+       /*
+        * Another corner case: the SCSI status byte is non-zero but 'good'.
+        * Example: PRE-FETCH command returns SAM_STAT_CONDITION_MET when
+        * it is able to fit nominated LBs in its cache (and SAM_STAT_GOOD
+        * if it can't fit). Treat SAM_STAT_CONDITION_MET and the related
+        * intermediate statuses (both obsolete in SAM-4) as good.
+        */
+       if (status_byte(result) && scsi_status_is_good(result))
+               blk_stat = BLK_STS_OK;
+
+       return blk_stat;
+}
+
 /*
  * Function:    scsi_io_completion()
  *
@@ -794,26 +865,23 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned 
int good_bytes)
              ACTION_DELAYED_RETRY} action;
        unsigned long wait_for = (cmd->allowed + 1) * req->timeout;
 
-       if (result) {
+       if (result) {   /* does not necessarily mean there is an error */
                sense_valid = scsi_command_normalize_sense(cmd, &sshdr);
                if (sense_valid)
                        sense_current = !scsi_sense_is_deferred(&sshdr);
+               blk_stat = scsi_io_completion_nz_result(cmd, result);
+               if (blk_stat == BLK_STS_OK)
+                       result = 0;     /* suppress error processing now */
+               /*
+                * if scsi_io_completion_nz_result() made no change,
+                * re-instate initial blk_stat value.
+                */
+               if (blk_stat == BLK_STS_NOTSUPP)
+                       blk_stat = BLK_STS_OK;
+
        }
 
        if (blk_rq_is_passthrough(req)) {
-               if (result) {
-                       if (sense_valid) {
-                               /*
-                                * SG_IO wants current and deferred errors
-                                */
-                               scsi_req(req)->sense_len =
-                                       min(8 + cmd->sense_buffer[7],
-                                           SCSI_SENSE_BUFFERSIZE);
-                       }
-                       if (sense_current)
-                               blk_stat = __scsi_error_from_host_byte(cmd,
-                                                                      result);
-               }
                /*
                 * __scsi_error_from_host_byte may have reset the host_byte
                 */
@@ -831,13 +899,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned 
int good_bytes)
                                BUG();
                        return;
                }
-       } else if (blk_rq_bytes(req) == 0 && result && sense_current) {
-               /*
-                * Flush commands do not transfers any data, and thus cannot use
-                * good_bytes != blk_rq_bytes(req) as the signal for an error.
-                * This sets blk_stat explicitly for the problem case.
-                */
-               blk_stat = __scsi_error_from_host_byte(cmd, result);
        }
 
        /* no bidi support for !blk_rq_is_passthrough yet */
@@ -851,37 +912,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned 
int good_bytes)
                "%u sectors total, %d bytes done.\n",
                blk_rq_sectors(req), good_bytes));
 
-       /*
-        * Recovered errors need reporting, but they're always treated as
-        * success, so fiddle the result code here.  For passthrough requests
-        * we already took a copy of the original into sreq->result which
-        * is what gets returned to the user
-        */
-       if (sense_valid && (sshdr.sense_key == RECOVERED_ERROR)) {
-               /* if ATA PASS-THROUGH INFORMATION AVAILABLE skip
-                * print since caller wants ATA registers. Only occurs on
-                * SCSI ATA PASS_THROUGH commands when CK_COND=1
-                */
-               if ((sshdr.asc == 0x0) && (sshdr.ascq == 0x1d))
-                       ;
-               else if (!(req->rq_flags & RQF_QUIET))
-                       scsi_print_sense(cmd);
-               result = 0;
-               /* for passthrough, blk_stat may be set */
-               blk_stat = BLK_STS_OK;
-       }
-       /*
-        * Another corner case: the SCSI status byte is non-zero but 'good'.
-        * Example: PRE-FETCH command returns SAM_STAT_CONDITION_MET when
-        * it is able to fit nominated LBs in its cache (and SAM_STAT_GOOD
-        * if it can't fit). Treat SAM_STAT_CONDITION_MET and the related
-        * intermediate statuses (both obsolete in SAM-4) as good.
-        */
-       if (status_byte(result) && scsi_status_is_good(result)) {
-               result = 0;
-               blk_stat = BLK_STS_OK;
-       }
-
        /*
         * Next deal with any sectors which we were able to correctly
         * handle. Failed, zero length commands always need to drop down
-- 
2.14.1

Reply via email to