[PATCH] aic79xx: restore modes when exiting ahd_linux_queue_abort_cmd()

[PATCH] aic79xx: restore modes when exiting ahd_linux_queue_abort_cmd()

[Hannes Reinecke]

ahd_linux_queue_abort_cmd() calls ahd_save_modes() without calling
ahd_restore_modes() before exiting.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/aic7xxx/aic79xx_osm.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c b/drivers/scsi/aic7xxx/aic79xx_osm.c
index dc4fe334efd0..abb5447f1de4 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
@@ -2147,7 +2147,7 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd *cmd)
 	u_int  last_phase;
 	u_int  saved_scsiid;
 	u_int  cdb_byte;
-	int    retval;
+	int    retval = SUCCESS;
 	int    was_paused;
 	int    paused;
 	int    wait;
@@ -2185,8 +2185,7 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd *cmd)
 		 * so we must not still own the command.
 		 */
 		scmd_printk(KERN_INFO, cmd, "Is not an active device\n");
-		retval = SUCCESS;
-		goto no_cmd;
+		goto done;
 	}
 
 	/*
@@ -2199,7 +2198,7 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd *cmd)
 
 	if (pending_scb == NULL) {
 		scmd_printk(KERN_INFO, cmd, "Command not found\n");
-		goto no_cmd;
+		goto done;
 	}
 
 	if ((pending_scb->flags & SCB_RECOVERY_SCB) != 0) {
@@ -2207,7 +2206,7 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd *cmd)
 		 * We can't queue two recovery actions using the same SCB
 		 */
 		retval = FAILED;
-		goto  done;
+		goto done;
 	}
 
 	/*
@@ -2222,7 +2221,7 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd *cmd)
 
 	if ((pending_scb->flags & SCB_ACTIVE) == 0) {
 		scmd_printk(KERN_INFO, cmd, "Command already completed\n");
-		goto no_cmd;
+		goto done;
 	}
 
 	printk("%s: At time of recovery, card was %spaused\n",
@@ -2239,7 +2238,6 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd *cmd)
 		printk("%s:%d:%d:%d: Cmd aborted from QINFIFO\n",
 		       ahd_name(ahd), cmd->device->channel, 
 		       cmd->device->id, (u8)cmd->device->lun);
-		retval = SUCCESS;
 		goto done;
 	}
 
@@ -2336,17 +2334,10 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd *cmd)
 	} else {
 		scmd_printk(KERN_INFO, cmd, "Unable to deliver message\n");
 		retval = FAILED;
-		goto done;
 	}
 
-no_cmd:
-	/*
-	 * Our assumption is that if we don't have the command, no
-	 * recovery action was required, so we return success.  Again,
-	 * the semantics of the mid-layer recovery engine are not
-	 * well defined, so this may change in time.
-	 */
-	retval = SUCCESS;
+
+	ahd_restore_modes(ahd, saved_modes);
 done:
 	if (paused)
 		ahd_unpause(ahd);
-- 
2.16.4

Re: [PATCH] aic79xx: restore modes when exiting ahd_linux_queue_abort_cmd()

[James Bottomley]

On Tue, 2020-07-14 at 18:03 +0200, Hannes Reinecke wrote:
> ahd_linux_queue_abort_cmd() calls ahd_save_modes() without calling
> ahd_restore_modes() before exiting.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/aic7xxx/aic79xx_osm.c | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c
> b/drivers/scsi/aic7xxx/aic79xx_osm.c
> index dc4fe334efd0..abb5447f1de4 100644
> --- a/drivers/scsi/aic7xxx/aic79xx_osm.c
> +++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
> @@ -2147,7 +2147,7 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd
> *cmd)
>  	u_int  last_phase;
>  	u_int  saved_scsiid;
>  	u_int  cdb_byte;
> -	int    retval;
> +	int    retval = SUCCESS;
>  	int    was_paused;
>  	int    paused;
>  	int    wait;
> @@ -2185,8 +2185,7 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd
> *cmd)
>  		 * so we must not still own the command.
>  		 */
>  		scmd_printk(KERN_INFO, cmd, "Is not an active
> device\n");
> -		retval = SUCCESS;
> -		goto no_cmd;
> +		goto done;
>  	}
>  
>  	/*
> @@ -2199,7 +2198,7 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd
> *cmd)
>  
>  	if (pending_scb == NULL) {
>  		scmd_printk(KERN_INFO, cmd, "Command not found\n");
> -		goto no_cmd;
> +		goto done;
>  	}
>  
>  	if ((pending_scb->flags & SCB_RECOVERY_SCB) != 0) {
> @@ -2207,7 +2206,7 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd
> *cmd)
>  		 * We can't queue two recovery actions using the
> same SCB
>  		 */
>  		retval = FAILED;
> -		goto  done;
> +		goto done;
>  	}
>  
>  	/*
> @@ -2222,7 +2221,7 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd
> *cmd)
>  
>  	if ((pending_scb->flags & SCB_ACTIVE) == 0) {
>  		scmd_printk(KERN_INFO, cmd, "Command already
> completed\n");
> -		goto no_cmd;
> +		goto done;
>  	}
>  
>  	printk("%s: At time of recovery, card was %spaused\n",
> @@ -2239,7 +2238,6 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd
> *cmd)
>  		printk("%s:%d:%d:%d: Cmd aborted from QINFIFO\n",
>  		       ahd_name(ahd), cmd->device->channel, 
>  		       cmd->device->id, (u8)cmd->device->lun);
> -		retval = SUCCESS;
>  		goto done;
>  	}
>  
> @@ -2336,17 +2334,10 @@ ahd_linux_queue_abort_cmd(struct scsi_cmnd
> *cmd)
>  	} else {
>  		scmd_printk(KERN_INFO, cmd, "Unable to deliver
> message\n");
>  		retval = FAILED;
> -		goto done;
>  	}
>  
> -no_cmd:
> -	/*
> -	 * Our assumption is that if we don't have the command, no
> -	 * recovery action was required, so we return
> success.  Again,
> -	 * the semantics of the mid-layer recovery engine are not
> -	 * well defined, so this may change in time.
> -	 */
> -	retval = SUCCESS;
> +
> +	ahd_restore_modes(ahd, saved_modes);
>  done:
>  	if (paused)
>  		ahd_unpause(ahd);

Yes, that looks correct to me ... is there any way to test it?

James

Re: [PATCH] aic79xx: restore modes when exiting ahd_linux_queue_abort_cmd()

[Martin K. Petersen]

On Tue, 14 Jul 2020 18:03:01 +0200, Hannes Reinecke wrote:

> ahd_linux_queue_abort_cmd() calls ahd_save_modes() without calling
> ahd_restore_modes() before exiting.

Applied to 5.9/scsi-queue, thanks!

[1/1] scsi: aic79xx: Restore modes when exiting ahd_linux_queue_abort_cmd()
      https://git.kernel.org/mkp/scsi/c/6422e3c3b521

-- 
Martin K. Petersen	Oracle Linux Engineering