| Message ID | 20200714160301.4482-1-hare@suse.de (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 3eef38a14370c673134600130094472e2c3f4041 |
| Headers | show |
| Series | aic79xx: restore modes when exiting ahd_linux_queue_abort_cmd() | expand |
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
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
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);
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(-)