diff mbox

[v5,4/7] scsi_io_completion_action helper added

Message ID 20180329054702.28644-5-dgilbert@interlog.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Douglas Gilbert March 29, 2018, 5:46 a.m. UTC
Place scsi_io_completion()'s complex error processing associated with a
local enumeration into a static helper function. That enumeration's
values start with "ACTION_" so use the suffix "_action" in the helper
function's name.

Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

 drivers/scsi/scsi_lib.c | 321 ++++++++++++++++++++++++++----------------------
 1 file changed, 171 insertions(+), 150 deletions(-)

Comments

Bart Van Assche March 29, 2018, 11:45 p.m. UTC | #1
On Thu, 2018-03-29 at 01:46 -0400, Douglas Gilbert wrote:
> +		/*

> +		 * Unprep the request and put it back at the head of the

> +		 * queue. A new command will be prepared and issued.

> +		 * This block is the same as case ACTION_REPREP in

> +		 * scsi_io_completion_action() above.

>  		 */

>  		if (q->mq_ops) {

>  			scsi_mq_requeue_cmd(cmd);

> @@ -1061,16 +1090,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)

>  			scsi_release_buffers(cmd);

>  			scsi_requeue_command(q, cmd);

>  		}


Although I'm not happy about this code duplication:

Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
diff mbox

Patch

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 08c4db34af7b..831d1fad38b3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -751,6 +751,168 @@  static blk_status_t __scsi_error_from_host_byte(struct scsi_cmnd *cmd,
 	}
 }
 
+/* Helper for scsi_io_completion() when special action required. */
+static void scsi_io_completion_action(struct scsi_cmnd *cmd, int result)
+{
+	struct request_queue *q = cmd->device->request_queue;
+	struct request *req = cmd->request;
+	int level = 0;
+	enum {ACTION_FAIL, ACTION_REPREP, ACTION_RETRY,
+	      ACTION_DELAYED_RETRY} action;
+	unsigned long wait_for = (cmd->allowed + 1) * req->timeout;
+	struct scsi_sense_hdr sshdr;
+	bool sense_valid;
+	bool sense_current = true;      /* false implies "deferred sense" */
+	blk_status_t blk_stat;
+
+	sense_valid = scsi_command_normalize_sense(cmd, &sshdr);
+	if (sense_valid)
+		sense_current = !scsi_sense_is_deferred(&sshdr);
+
+	blk_stat = __scsi_error_from_host_byte(cmd, result);
+
+	if (host_byte(result) == DID_RESET) {
+		/* Third party bus reset or reset for error recovery
+		 * reasons.  Just retry the command and see what
+		 * happens.
+		 */
+		action = ACTION_RETRY;
+	} else if (sense_valid && sense_current) {
+		switch (sshdr.sense_key) {
+		case UNIT_ATTENTION:
+			if (cmd->device->removable) {
+				/* Detected disc change.  Set a bit
+				 * and quietly refuse further access.
+				 */
+				cmd->device->changed = 1;
+				action = ACTION_FAIL;
+			} else {
+				/* Must have been a power glitch, or a
+				 * bus reset.  Could not have been a
+				 * media change, so we just retry the
+				 * command and see what happens.
+				 */
+				action = ACTION_RETRY;
+			}
+			break;
+		case ILLEGAL_REQUEST:
+			/* If we had an ILLEGAL REQUEST returned, then
+			 * we may have performed an unsupported
+			 * command.  The only thing this should be
+			 * would be a ten byte read where only a six
+			 * byte read was supported.  Also, on a system
+			 * where READ CAPACITY failed, we may have
+			 * read past the end of the disk.
+			 */
+			if ((cmd->device->use_10_for_rw &&
+			    sshdr.asc == 0x20 && sshdr.ascq == 0x00) &&
+			    (cmd->cmnd[0] == READ_10 ||
+			     cmd->cmnd[0] == WRITE_10)) {
+				/* This will issue a new 6-byte command. */
+				cmd->device->use_10_for_rw = 0;
+				action = ACTION_REPREP;
+			} else if (sshdr.asc == 0x10) /* DIX */ {
+				action = ACTION_FAIL;
+				blk_stat = BLK_STS_PROTECTION;
+			/* INVALID COMMAND OPCODE or INVALID FIELD IN CDB */
+			} else if (sshdr.asc == 0x20 || sshdr.asc == 0x24) {
+				action = ACTION_FAIL;
+				blk_stat = BLK_STS_TARGET;
+			} else
+				action = ACTION_FAIL;
+			break;
+		case ABORTED_COMMAND:
+			action = ACTION_FAIL;
+			if (sshdr.asc == 0x10) /* DIF */
+				blk_stat = BLK_STS_PROTECTION;
+			break;
+		case NOT_READY:
+			/* If the device is in the process of becoming
+			 * ready, or has a temporary blockage, retry.
+			 */
+			if (sshdr.asc == 0x04) {
+				switch (sshdr.ascq) {
+				case 0x01: /* becoming ready */
+				case 0x04: /* format in progress */
+				case 0x05: /* rebuild in progress */
+				case 0x06: /* recalculation in progress */
+				case 0x07: /* operation in progress */
+				case 0x08: /* Long write in progress */
+				case 0x09: /* self test in progress */
+				case 0x14: /* space allocation in progress */
+					action = ACTION_DELAYED_RETRY;
+					break;
+				default:
+					action = ACTION_FAIL;
+					break;
+				}
+			} else
+				action = ACTION_FAIL;
+			break;
+		case VOLUME_OVERFLOW:
+			/* See SSC3rXX or current. */
+			action = ACTION_FAIL;
+			break;
+		default:
+			action = ACTION_FAIL;
+			break;
+		}
+	} else
+		action = ACTION_FAIL;
+
+	if (action != ACTION_FAIL &&
+	    time_before(cmd->jiffies_at_alloc + wait_for, jiffies))
+		action = ACTION_FAIL;
+
+	switch (action) {
+	case ACTION_FAIL:
+		/* Give up and fail the remainder of the request */
+		if (!(req->rq_flags & RQF_QUIET)) {
+			static DEFINE_RATELIMIT_STATE(_rs,
+					DEFAULT_RATELIMIT_INTERVAL,
+					DEFAULT_RATELIMIT_BURST);
+
+			if (unlikely(scsi_logging_level))
+				level =
+				     SCSI_LOG_LEVEL(SCSI_LOG_MLCOMPLETE_SHIFT,
+						    SCSI_LOG_MLCOMPLETE_BITS);
+
+			/*
+			 * if logging is enabled the failure will be printed
+			 * in scsi_log_completion(), so avoid duplicate messages
+			 */
+			if (!level && __ratelimit(&_rs)) {
+				scsi_print_result(cmd, NULL, FAILED);
+				if (driver_byte(result) & DRIVER_SENSE)
+					scsi_print_sense(cmd);
+				scsi_print_command(cmd);
+			}
+		}
+		if (!scsi_end_request(req, blk_stat, blk_rq_err_bytes(req), 0))
+			return;
+		/*FALLTHRU*/
+	case ACTION_REPREP:
+		/* Unprep the request and put it back at the head of the queue.
+		 * A new command will be prepared and issued.
+		 */
+		if (q->mq_ops) {
+			scsi_mq_requeue_cmd(cmd);
+		} else {
+			scsi_release_buffers(cmd);
+			scsi_requeue_command(q, cmd);
+		}
+		break;
+	case ACTION_RETRY:
+		/* Retry the same command immediately */
+		__scsi_queue_insert(cmd, SCSI_MLQUEUE_EH_RETRY, false);
+		break;
+	case ACTION_DELAYED_RETRY:
+		/* Retry the same command after a delay */
+		__scsi_queue_insert(cmd, SCSI_MLQUEUE_DEVICE_BUSY, false);
+		break;
+	}
+}
+
 /*
  * Helper for scsi_io_completion() when cmd->result is non-zero. Returns a
  * new result that may suppress further error checking. Also modifies
@@ -859,20 +1021,9 @@  void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 	struct request_queue *q = cmd->device->request_queue;
 	struct request *req = cmd->request;
 	blk_status_t blk_stat = BLK_STS_OK;
-	struct scsi_sense_hdr sshdr;
-	bool sense_valid = false;
-	bool sense_current = true;	/* false implies "deferred sense" */
-	int level = 0;
-	enum {ACTION_FAIL, ACTION_REPREP, ACTION_RETRY,
-	      ACTION_DELAYED_RETRY} action;
-	unsigned long wait_for = (cmd->allowed + 1) * req->timeout;
 
-	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);
+	if (result)	/* does not necessarily mean there is an error */
 		result = scsi_io_completion_nz_result(cmd, result, &blk_stat);
-	}
 
 	if (blk_rq_is_passthrough(req)) {
 		/*
@@ -926,134 +1077,12 @@  void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 	 * If there had been no error, but we have leftover bytes in the
 	 * requeues just queue the command up again.
 	 */
-	if (result == 0)
-		goto requeue;
-
-	blk_stat = __scsi_error_from_host_byte(cmd, result);
-
-	if (host_byte(result) == DID_RESET) {
-		/* Third party bus reset or reset for error recovery
-		 * reasons.  Just retry the command and see what
-		 * happens.
-		 */
-		action = ACTION_RETRY;
-	} else if (sense_valid && sense_current) {
-		switch (sshdr.sense_key) {
-		case UNIT_ATTENTION:
-			if (cmd->device->removable) {
-				/* Detected disc change.  Set a bit
-				 * and quietly refuse further access.
-				 */
-				cmd->device->changed = 1;
-				action = ACTION_FAIL;
-			} else {
-				/* Must have been a power glitch, or a
-				 * bus reset.  Could not have been a
-				 * media change, so we just retry the
-				 * command and see what happens.
-				 */
-				action = ACTION_RETRY;
-			}
-			break;
-		case ILLEGAL_REQUEST:
-			/* If we had an ILLEGAL REQUEST returned, then
-			 * we may have performed an unsupported
-			 * command.  The only thing this should be
-			 * would be a ten byte read where only a six
-			 * byte read was supported.  Also, on a system
-			 * where READ CAPACITY failed, we may have
-			 * read past the end of the disk.
-			 */
-			if ((cmd->device->use_10_for_rw &&
-			    sshdr.asc == 0x20 && sshdr.ascq == 0x00) &&
-			    (cmd->cmnd[0] == READ_10 ||
-			     cmd->cmnd[0] == WRITE_10)) {
-				/* This will issue a new 6-byte command. */
-				cmd->device->use_10_for_rw = 0;
-				action = ACTION_REPREP;
-			} else if (sshdr.asc == 0x10) /* DIX */ {
-				action = ACTION_FAIL;
-				blk_stat = BLK_STS_PROTECTION;
-			/* INVALID COMMAND OPCODE or INVALID FIELD IN CDB */
-			} else if (sshdr.asc == 0x20 || sshdr.asc == 0x24) {
-				action = ACTION_FAIL;
-				blk_stat = BLK_STS_TARGET;
-			} else
-				action = ACTION_FAIL;
-			break;
-		case ABORTED_COMMAND:
-			action = ACTION_FAIL;
-			if (sshdr.asc == 0x10) /* DIF */
-				blk_stat = BLK_STS_PROTECTION;
-			break;
-		case NOT_READY:
-			/* If the device is in the process of becoming
-			 * ready, or has a temporary blockage, retry.
-			 */
-			if (sshdr.asc == 0x04) {
-				switch (sshdr.ascq) {
-				case 0x01: /* becoming ready */
-				case 0x04: /* format in progress */
-				case 0x05: /* rebuild in progress */
-				case 0x06: /* recalculation in progress */
-				case 0x07: /* operation in progress */
-				case 0x08: /* Long write in progress */
-				case 0x09: /* self test in progress */
-				case 0x14: /* space allocation in progress */
-					action = ACTION_DELAYED_RETRY;
-					break;
-				default:
-					action = ACTION_FAIL;
-					break;
-				}
-			} else
-				action = ACTION_FAIL;
-			break;
-		case VOLUME_OVERFLOW:
-			/* See SSC3rXX or current. */
-			action = ACTION_FAIL;
-			break;
-		default:
-			action = ACTION_FAIL;
-			break;
-		}
-	} else
-		action = ACTION_FAIL;
-
-	if (action != ACTION_FAIL &&
-	    time_before(cmd->jiffies_at_alloc + wait_for, jiffies))
-		action = ACTION_FAIL;
-
-	switch (action) {
-	case ACTION_FAIL:
-		/* Give up and fail the remainder of the request */
-		if (!(req->rq_flags & RQF_QUIET)) {
-			static DEFINE_RATELIMIT_STATE(_rs,
-					DEFAULT_RATELIMIT_INTERVAL,
-					DEFAULT_RATELIMIT_BURST);
-
-			if (unlikely(scsi_logging_level))
-				level = SCSI_LOG_LEVEL(SCSI_LOG_MLCOMPLETE_SHIFT,
-						       SCSI_LOG_MLCOMPLETE_BITS);
-
-			/*
-			 * if logging is enabled the failure will be printed
-			 * in scsi_log_completion(), so avoid duplicate messages
-			 */
-			if (!level && __ratelimit(&_rs)) {
-				scsi_print_result(cmd, NULL, FAILED);
-				if (driver_byte(result) & DRIVER_SENSE)
-					scsi_print_sense(cmd);
-				scsi_print_command(cmd);
-			}
-		}
-		if (!scsi_end_request(req, blk_stat, blk_rq_err_bytes(req), 0))
-			return;
-		/*FALLTHRU*/
-	case ACTION_REPREP:
-	requeue:
-		/* Unprep the request and put it back at the head of the queue.
-		 * A new command will be prepared and issued.
+	if (result == 0) {
+		/*
+		 * Unprep the request and put it back at the head of the
+		 * queue. A new command will be prepared and issued.
+		 * This block is the same as case ACTION_REPREP in
+		 * scsi_io_completion_action() above.
 		 */
 		if (q->mq_ops) {
 			scsi_mq_requeue_cmd(cmd);
@@ -1061,16 +1090,8 @@  void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 			scsi_release_buffers(cmd);
 			scsi_requeue_command(q, cmd);
 		}
-		break;
-	case ACTION_RETRY:
-		/* Retry the same command immediately */
-		__scsi_queue_insert(cmd, SCSI_MLQUEUE_EH_RETRY, false);
-		break;
-	case ACTION_DELAYED_RETRY:
-		/* Retry the same command after a delay */
-		__scsi_queue_insert(cmd, SCSI_MLQUEUE_DEVICE_BUSY, false);
-		break;
-	}
+	} else
+		scsi_io_completion_action(cmd, result);
 }
 
 static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb)