On Sun, 2009-10-11 at 09:16 -0700, Ben Efros wrote:
> No I don't think its a good idea to turn off SANE_SENSE in this situation.
> Here is a patch similar to Ben Herrenschmidt's but will not turn off SANE_SENSE
> just because a transport failure.
>
> Retry with short sense when SANE_SENSE fails.
Works for me too.
Alan, what do you think ? I definitely don't like Ben E's most recent
patch with a quirk for all devices, it's simply a lot more code for
something that will come back and bite again when somebody does the
same mistake again. I'd rather have the request sense code be more
robust. But this patch is fine, as was my previous one.
So it boils down on clearing SANE_SENSE vs. not clearing it. If we
clear it, we probably want to keep it cleared (via an INSANE_SENSE
flag ?). But on the other hand, I don't think that always going
for a retry when a SANE_SENSE fails is going to hurt and sounds
like the robust thing to do, so I don't mind that simple patch
from Ben. So up to you :-)
Cheers,
Ben.
> Signed-off-by: Ben Efros <ben@xxxxxxxxxxxxx>
Tested-by: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
> --- linux-2.6.31.1/drivers/usb/storage/transport.c 2009-09-24 08:45:25.000000000 -0700
> +++ linux-2.6.31.1.new/drivers/usb/storage/transport.c 2009-10-11 08:06:26.000000000 -0700
> @@ -696,7 +696,7 @@ void usb_stor_invoke_transport(struct sc
> /* device supports and needs bigger sense buffer */
> if (us->fflags & US_FL_SANE_SENSE)
> sense_size = ~0;
> -
> +Retry_Sense:
> US_DEBUGP("Issuing auto-REQUEST_SENSE\n");
>
> scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
> @@ -723,6 +723,12 @@ void usb_stor_invoke_transport(struct sc
> if (temp_result != USB_STOR_TRANSPORT_GOOD) {
> US_DEBUGP("-- auto-sense failure\n");
>
> + if ((us->fflags & US_FL_SANE_SENSE) &&
> + sense_size != US_SENSE_SIZE) {
> + sense_size = US_SENSE_SIZE;
> + US_DEBUGP("-- retry without SANE_SENSE\n");
> + goto Retry_Sense;
> + }
> /* we skip the reset if this happens to be a
> * multi-target device, since failure of an
> * auto-sense is perfectly valid
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
