On Thu, 18 May 2017, Ewan D. Milne wrote:
> On Thu, 2017-05-18 at 13:37 -0400, Alan Stern wrote:
> >
> > I had completely forgotten about this code. :-(
> >
> > Looks like you put your finger on the source of the problem. So if the
> > device sends back essentially empty sense data (SK = No Sense, ASC =
> > ASCQ = 0), but the USB transport indicates command failure, how should
> > we inform the SCSI core in a way that won't cause infinite retries or
> > obnoxious log messages?
> >
> > Should we be doing a better job of detecting empty sense data -- that
> > is, do we need to check for non-empty ATA status?
> >
> > Or has the SCSI core improved so that it no longer does infinite
> > retries (see commit f1a0743bc0e7 "USB: storage: When a device returns
> > no sense data, call it a Hardware Error" and Bugzilla entry #14118),
> > meaning that this code can be removed entirely?
> >
> > Alan Stern
>
> We added:
>
> commit ee60b2c52ec8ecdcbcd2f85cc117b525f649441f
> Author: Eiichi Tsukata <[email protected]>
> Date: Tue Feb 11 14:29:52 2014 +0900
>
> [SCSI] Add timeout to avoid infinite command retry
>
> but this may not give you the behavior you want, because it bounds
> the execution time to (# of retries + 1) * timeout. So if you get
> an immediate error return it could still take a while for this code
> to give up retrying, i.e. it does not have the same properties as
> your commit f1a0743bc0e7.
>
> I suppose you could decode the ATA Status Return sense data descriptor
> but I don't know how good the compliance is among all the ATA devices.
> Table 177 in section 1.2.2.8 of SAT-4 r06 seems to say that most of
> the fields in the sense data are unspecified for ATA PASS-THROUGH
> commands, so this probably explains why you see nothing else useful.
> Perhaps the logging should be delegated to the USB or ATA code for
> these commands, since they are not really part of SCSI?
>
> I have seen a case of a Fibre Channel array returning all zeroes
> in the sense data, but this was because it was malfunctioning.
All right, suppose we don't return DID_ERROR and don't call it a
hardware error. I don't know if this will help at all, and I don't
know if it will cause any regressions.
GeekGirl1, can you try applying the patch below to see if it makes any
difference? If you don't know how, I will attach it to the Bugzilla
report so somebody else can try it.
Alan Stern
Index: usb-4.x/drivers/usb/storage/transport.c
===================================================================
--- usb-4.x.orig/drivers/usb/storage/transport.c
+++ usb-4.x/drivers/usb/storage/transport.c
@@ -835,6 +835,7 @@ Retry_Sense:
srb->result = SAM_STAT_GOOD;
srb->sense_buffer[0] = 0x0;
+#if 0
/*
* If there was a problem, report an unspecified
* hardware error to prevent the higher layers from
@@ -846,6 +847,7 @@ Retry_Sense:
srb->sense_buffer[1] = HARDWARE_ERROR;
else
srb->sense_buffer[2] = HARDWARE_ERROR;
+#endif
}
}
}
