diff mbox series

[2/2] scsi: core: simplify control flow in scmd_eh_abort_handler

Message ID 20211025143952.17128-3-emilne@redhat.com (mailing list archive)
State Superseded
Headers show
Series Fix SCSI async abort handling when eh_deadline is active | expand

Commit Message

Ewan Milne Oct. 25, 2021, 2:39 p.m. UTC 
Simplify the nested conditionals in the function by using
a label for the error path, and remove duplicate code.

Signed-off-by: Ewan D. Milne <emilne@redhat.com>
---
 drivers/scsi/scsi_error.c | 104 ++++++++++++++++++++--------------------------
 1 file changed, 46 insertions(+), 58 deletions(-)

Comments

Martin K. Petersen Oct. 28, 2021, 3:31 a.m. UTC | #1 
Ewan,

> Simplify the nested conditionals in the function by using
> a label for the error path, and remove duplicate code.

Oh, I see you shuffled things for the follow-on patch. That's
better. I'd still like the stable fix to be smaller, though.
Ewan Milne Oct. 28, 2021, 8:45 p.m. UTC | #2 
On Wed, 2021-10-27 at 23:31 -0400, Martin K. Petersen wrote:
> Ewan,
> 
> > Simplify the nested conditionals in the function by using
> > a label for the error path, and remove duplicate code.
> 
> Oh, I see you shuffled things for the follow-on patch. That's
> better. I'd still like the stable fix to be smaller, though.
> 

Sure, let me work on it a bit more to get the first patch as
small as possible.  I'll submit a v2.

-Ewan
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 9f4001e..e8248bb 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -159,72 +159,60 @@  scmd_eh_abort_handler(struct work_struct *work)
 		SCSI_LOG_ERROR_RECOVERY(3,
 			scmd_printk(KERN_INFO, scmd,
 				    "eh timeout, not aborting\n"));
-	} else {
-		SCSI_LOG_ERROR_RECOVERY(3,
+		goto out;
+	}
+
+	SCSI_LOG_ERROR_RECOVERY(3,
 			scmd_printk(KERN_INFO, scmd,
 				    "aborting command\n"));
-		rtn = scsi_try_to_abort_cmd(shost->hostt, scmd);
-		if (rtn == SUCCESS) {
-			set_host_byte(scmd, DID_TIME_OUT);
-			if (scsi_host_eh_past_deadline(shost)) {
-				SCSI_LOG_ERROR_RECOVERY(3,
-					scmd_printk(KERN_INFO, scmd,
-						    "eh timeout, not retrying "
-						    "aborted command\n"));
-			} else if (!scsi_noretry_cmd(scmd) &&
-				   scsi_cmd_retry_allowed(scmd) &&
-				scsi_eh_should_retry_cmd(scmd)) {
-				SCSI_LOG_ERROR_RECOVERY(3,
-					scmd_printk(KERN_WARNING, scmd,
-						    "retry aborted command\n"));
-
-				spin_lock_irqsave(shost->host_lock, flags);
-				list_del_init(&scmd->eh_entry);
-
-				/*
-				 * If the abort succeeds, and there is no further
-				 * EH action, clear the ->last_reset time.
-				 */
-				if (list_empty(&shost->eh_abort_list) &&
-				    list_empty(&shost->eh_cmd_q))
-					if (shost->eh_deadline != -1)
-						shost->last_reset = 0;
-
-				spin_unlock_irqrestore(shost->host_lock, flags);
-
-				scsi_queue_insert(scmd, SCSI_MLQUEUE_EH_RETRY);
-				return;
-			} else {
-				SCSI_LOG_ERROR_RECOVERY(3,
-					scmd_printk(KERN_WARNING, scmd,
-						    "finish aborted command\n"));
+	rtn = scsi_try_to_abort_cmd(shost->hostt, scmd);
+	if (rtn != SUCCESS) {
+		SCSI_LOG_ERROR_RECOVERY(3,
+			scmd_printk(KERN_INFO, scmd,
+				    "cmd abort %s\n",
+				    (rtn == FAST_IO_FAIL) ?
+				    "not send" : "failed"));
+		goto out;
+	}
+	set_host_byte(scmd, DID_TIME_OUT);
+	if (scsi_host_eh_past_deadline(shost)) {
+		SCSI_LOG_ERROR_RECOVERY(3,
+			scmd_printk(KERN_INFO, scmd,
+				    "eh timeout, not retrying "
+				    "aborted command\n"));
+		goto out;
+	}
 
-				spin_lock_irqsave(shost->host_lock, flags);
-				list_del_init(&scmd->eh_entry);
+	spin_lock_irqsave(shost->host_lock, flags);
+	list_del_init(&scmd->eh_entry);
 
-				/*
-				 * If the abort succeeds, and there is no further
-				 * EH action, clear the ->last_reset time.
-				 */
-				if (list_empty(&shost->eh_abort_list) &&
-				    list_empty(&shost->eh_cmd_q))
-					if (shost->eh_deadline != -1)
-						shost->last_reset = 0;
+	/*
+	 * If the abort succeeds, and there is no further
+	 * EH action, clear the ->last_reset time.
+	 */
+	if (list_empty(&shost->eh_abort_list) &&
+	    list_empty(&shost->eh_cmd_q))
+		if (shost->eh_deadline != -1)
+			shost->last_reset = 0;
 
-				spin_unlock_irqrestore(shost->host_lock, flags);
+	spin_unlock_irqrestore(shost->host_lock, flags);
 
-				scsi_finish_command(scmd);
-				return;
-			}
-		} else {
-			SCSI_LOG_ERROR_RECOVERY(3,
-				scmd_printk(KERN_INFO, scmd,
-					    "cmd abort %s\n",
-					    (rtn == FAST_IO_FAIL) ?
-					    "not send" : "failed"));
-		}
+	if (!scsi_noretry_cmd(scmd) &&
+	    scsi_cmd_retry_allowed(scmd) &&
+	    scsi_eh_should_retry_cmd(scmd)) {
+		SCSI_LOG_ERROR_RECOVERY(3,
+			scmd_printk(KERN_WARNING, scmd,
+				    "retry aborted command\n"));
+		scsi_queue_insert(scmd, SCSI_MLQUEUE_EH_RETRY);
+	} else {
+		SCSI_LOG_ERROR_RECOVERY(3,
+			scmd_printk(KERN_WARNING, scmd,
+				    "finish aborted command\n"));
+		scsi_finish_command(scmd);
 	}
+	return;
 
+out:
 	spin_lock_irqsave(shost->host_lock, flags);
 	list_del_init(&scmd->eh_entry);
 	spin_unlock_irqrestore(shost->host_lock, flags);