Re: [PATCH v2 24/24] scsi: aic7xxx: aic79xx_osm: Remove set but unused variabes 'saved_scsiid' and 'saved_modes'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/15/20 1:03 AM, James Bottomley wrote:
On Tue, 2020-07-14 at 22:39 +0100, Lee Jones wrote:
On Tue, 14 Jul 2020, James Bottomley wrote:

On Tue, 2020-07-14 at 09:46 +0200, Hannes Reinecke wrote:
On 7/13/20 10:00 AM, Lee Jones wrote:
Haven't been used since 2006.

Fixes the following W=1 kernel build warning(s):

  drivers/scsi/aic7xxx/aic79xx_osm.c: In function
‘ahd_linux_queue_abort_cmd’:
  drivers/scsi/aic7xxx/aic79xx_osm.c:2155:17: warning: variable
‘saved_modes’ set but not used [-Wunused-but-set-variable]
  drivers/scsi/aic7xxx/aic79xx_osm.c:2148:9: warning: variable
‘saved_scsiid’ set but not used [-Wunused-but-set-variable]

Cc: Hannes Reinecke <hare@xxxxxxxx>
Signed-off-by: Lee Jones <lee.jones@xxxxxxxxxx>
---
  drivers/scsi/aic7xxx/aic79xx_osm.c | 4 ----
  1 file changed, 4 deletions(-)

diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c
b/drivers/scsi/aic7xxx/aic79xx_osm.c
index 3782a20d58885..140c4e74ddd7e 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
@@ -2141,14 +2141,12 @@ ahd_linux_queue_abort_cmd(struct
scsi_cmnd
*cmd)
  	u_int  saved_scbptr;
  	u_int  active_scbptr;
  	u_int  last_phase;
-	u_int  saved_scsiid;
  	u_int  cdb_byte;
  	int    retval;
  	int    was_paused;
  	int    paused;
  	int    wait;
  	int    disconnected;
-	ahd_mode_state saved_modes;
  	unsigned long flags;
pending_scb = NULL;
@@ -2239,7 +2237,6 @@ ahd_linux_queue_abort_cmd(struct
scsi_cmnd
*cmd)
  		goto done;
  	}
- saved_modes = ahd_save_modes(ahd);
  	ahd_set_modes(ahd, AHD_MODE_SCSI, AHD_MODE_SCSI);
  	last_phase = ahd_inb(ahd, LASTPHASE);
  	saved_scbptr = ahd_get_scbptr(ahd);
@@ -2257,7 +2254,6 @@ ahd_linux_queue_abort_cmd(struct
scsi_cmnd
*cmd)
  	 * passed in command.  That command is currently
active on
the
  	 * bus or is in the disconnected state.
  	 */
-	saved_scsiid = ahd_inb(ahd, SAVED_SCSIID);
  	if (last_phase != P_BUSFREE
  	    && SCB_GET_TAG(pending_scb) == active_scbptr) {

Reviewed-by: Hannes Reinecke <hare@xxxxxxx>

Hey, you don't get to do that ... I asked you to figure out why
we're missing an ahd_restore_modes().  Removing the
ahd_save_modes() is cosmetic: it gets rid of a warning but doesn't
fix the problem.  I'd rather keep the warning until the problem is
fixed and the problem is we need a mode save/restore around the
ahd_set_modes() which is only partially implemented in this
function.

I had a look.  Traced it back to the dawn of time (time == Git), then
delved even further back by downloading and trawling through ~10-15
tarballs.  It looks as though drivers/scsi/aic7xxx/aic79xx_osm.c was
upstreamed in v2.5.60, nearly 20 years ago.  'saved_modes' has been
unused since at least then.  If no one has complained in 2 decades,
I'd say it probably isn't an issue worth perusing.

Well, we have what looks like a fix now.  The reason it matters is that
  if the bus is not in AHD_MODE_SCSI when the routine is called, it ends
up being in a wrong state when the routine exits, so you abort one
command and screw up another.  Ultimately I bet escalation to bus reset
fixes it, so everything appears to work, but it might have worked a lot
better if the original mode were restored.

This is error handling code, so most installations run just fine
without ever invoking it.

And since you mention it, it might also explain why every time this routine got invoked it failed to fixup anything and got escalated to bus reset. (And yes, I _did_ have some customer issues with aic79xx EH escalations over the years).

Testing will be tricky, though, as you'd have to inject timeouts onto the parallel bus or device.

Cheers,

Hannes
--
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@xxxxxxx                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux