Prev: Linux IO service time - a perfect source of randomness ?
Next: NOHZ: local_softirq_pending 08
From: Benjamin Herrenschmidt on 10 Oct 2009 18:10

> Further REQUEST SENSE commands therefore requested 96 bytes of data
> instead of the standard 18 bytes. With LUN 0 this worked okay. But
> with LUN 1 it didn't; the device reported a failure of the REQUEST
> SENSE. This is what caused usb-storage to issue the device reset.
>
> After the reset usb-storage continued to ask for 96 bytes of sense
> data, and LUN 1 continued to fail the commands. Hence the repeated
> resets.

Maybe a better approach would be to go back to 18 bytes when it fails,
what do you think ?

Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Benjamin Herrenschmidt on 10 Oct 2009 18:30
On Sun, 2009-10-11 at 08:20 +1100, Benjamin Herrenschmidt wrote:

> Yes, that works, you can see the ttyUSBx ports showing up. It looks like
> it may just be the resets coming from usb-storage that are breaking
> things.

Ok so I did a quick hack to usb-storage, basically put the block that
tests for the response size and sets USB_FL_SANE_SENSE into an #if 0 :

(This is 2.6.31.2 since that's what I have a built source at hand
right now)

#if 0
/* If the sense data returned is larger than 18-bytes then we
* assume this device supports requesting more in the future.
* The response code must be 70h through 73h inclusive.
*/
if (srb->sense_buffer[7] > (US_SENSE_SIZE - 8) &&
!(us->fflags & US_FL_SANE_SENSE) &&
(srb->sense_buffer[0] & 0x7C) == 0x70) {
US_DEBUGP("-- SANE_SENSE support enabled\n");
us->fflags |= US_FL_SANE_SENSE;

/* Indicate to the user that we truncated their sense
* because we didn't know it supported larger sense.
*/
US_DEBUGP("-- Sense data truncated to %i from %i\n",
US_SENSE_SIZE,
srb->sense_buffer[7] + 8);
srb->sense_buffer[7] = (US_SENSE_SIZE - 8);
}
#endif

Now, the result in the dmesg log is :

Oct 11 09:04:05 pasglop kernel: [ 52.916100] usb 4-1: new full speed USB device using uhci_hcd and address 2
Oct 11 09:04:05 pasglop kernel: [ 53.076402] usb 4-1: configuration #1 chosen from 1 choice
Oct 11 09:04:06 pasglop kernel: [ 53.104090] Initializing USB Mass Storage driver...
Oct 11 09:04:06 pasglop kernel: [ 53.105405] scsi2 : SCSI emulation for USB Mass Storage devices
Oct 11 09:04:06 pasglop kernel: [ 53.105585] usbcore: registered new interface driver usb-storage
Oct 11 09:04:06 pasglop kernel: [ 53.105589] USB Mass Storage support registered.
Oct 11 09:04:06 pasglop kernel: [ 53.107164] usb-storage: device found at 2
Oct 11 09:04:06 pasglop kernel: [ 53.107167] usb-storage: waiting for device to settle before scanning
Oct 11 09:04:06 pasglop kernel: [ 53.224104] usb 4-1: USB disconnect, address 2
Oct 11 09:04:06 pasglop kernel: [ 53.960100] usb 4-1: new full speed USB device using uhci_hcd and address 3
Oct 11 09:04:11 pasglop kernel: [ 54.121941] usb 4-1: configuration #1 chosen from 1 choice
Oct 11 09:04:11 pasglop kernel: [ 54.140861] scsi6 : SCSI emulation for USB Mass Storage devices
Oct 11 09:04:11 pasglop kernel: [ 54.143007] usb-storage: device found at 3
Oct 11 09:04:11 pasglop kernel: [ 54.143010] usb-storage: waiting for device to settle before scanning
Oct 11 09:04:11 pasglop kernel: [ 59.141422] usb-storage: device scan complete
Oct 11 09:04:11 pasglop kernel: [ 59.144370] scsi 6:0:0:0: CD-ROM HUAWEI Mass Storage 2.31 PQ: 0 ANSI: 2
Oct 11 09:04:11 pasglop kernel: [ 59.147379] scsi 6:0:0:1: Direct-Access HUAWEI SD Storage 2.31 PQ: 0 ANSI: 2
Oct 11 09:04:11 pasglop kernel: [ 59.169383] sr1: scsi-1 drive

At which point nothing happens for a while (and no serial stuff shows up).

If I yank the device, I then see:

Oct 11 09:05:28 pasglop kernel: [ 59.169494] sr 6:0:0:0: Attached scsi CD-ROM sr1
Oct 11 09:05:28 pasglop kernel: [ 59.169563] sr 6:0:0:0: Attached scsi generic sg2 type 5
Oct 11 09:05:28 pasglop kernel: [ 59.169668] sd 6:0:0:1: Attached scsi generic sg3 type 0
Oct 11 09:05:28 pasglop kernel: [ 59.227809] sd 6:0:0:1: [sdb] Attached SCSI removable disk
Oct 11 09:05:28 pasglop kernel: [ 71.048322] ISO 9660 Extensions: Microsoft Joliet Level 1
Oct 11 09:05:28 pasglop kernel: [ 71.057316] ISOFS: changing to secondary root
Oct 11 09:05:28 pasglop kernel: [ 128.369159] usb 4-1: USB disconnect, address 3
Oct 11 09:05:28 pasglop kernel: [ 128.429675] scsi 6:0:0:0: rejecting I/O to dead device

However, if I put it back in later on ... it works. The storage shows up
and the modem too.

Weird. I'll have to reboot to try to reproduce with usbmon logging.

Cheers,
Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on 10 Oct 2009 19:00
On Sun, 11 Oct 2009, Benjamin Herrenschmidt wrote:

>
> > Further REQUEST SENSE commands therefore requested 96 bytes of data
> > instead of the standard 18 bytes. With LUN 0 this worked okay. But
> > with LUN 1 it didn't; the device reported a failure of the REQUEST
> > SENSE. This is what caused usb-storage to issue the device reset.
> >
> > After the reset usb-storage continued to ask for 96 bytes of sense
> > data, and LUN 1 continued to fail the commands. Hence the repeated
> > resets.
>
> Maybe a better approach would be to go back to 18 bytes when it fails,
> what do you think ?

We certainly could do that. But should we turn off the SANE_SENSE flag
at the same time?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Benjamin Herrenschmidt on 10 Oct 2009 19:40
On Sat, 2009-10-10 at 18:52 -0400, Alan Stern wrote:
> > Maybe a better approach would be to go back to 18 bytes when it fails,
> > what do you think ?
>
> We certainly could do that. But should we turn off the SANE_SENSE flag
> at the same time?

I would think so, and not set it back the next time the sense buffer
claims more data in it (something like setting a NOT_REALLY_SANE_SENSE
flag ? :-)

Or not care and always do double sense on those devices. Whatever
works. It's not that sense was a fast path.

Cheers,
Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Benjamin Herrenschmidt on 10 Oct 2009 20:00
On Sun, 2009-10-11 at 10:26 +1100, Benjamin Herrenschmidt wrote:
> On Sat, 2009-10-10 at 18:52 -0400, Alan Stern wrote:
> > > Maybe a better approach would be to go back to 18 bytes when it fails,
> > > what do you think ?
> >
> > We certainly could do that. But should we turn off the SANE_SENSE flag
> > at the same time?
>
> I would think so, and not set it back the next time the sense buffer
> claims more data in it (something like setting a NOT_REALLY_SANE_SENSE
> flag ? :-)
>
> Or not care and always do double sense on those devices. Whatever
> works. It's not that sense was a fast path.

Ok, so here's an absolutely horrible hack that makes it work here,
but of course, I think that should be done more cleanly.

With some printk added I verified that it does ping pong
US_FL_SANE_SENSE (ie, sets it back as soon as the command
succeeds) but I don't see that as a big deal though.

I doubt you want to apply that as-is though but in case you do:

Signed-off-by: Benjamin Herrenschmidt <benh(a)kernel.crashing.org>

Cheers,
Ben.

--- a/drivers/usb/storage/transport.c 2009-10-09 14:34:14.638445171 +1100
+++ b/drivers/usb/storage/transport.c 2009-10-11 10:40:55.890868361 +1100
@@ -687,6 +687,7 @@
US_DEBUGP("-- unexpectedly short transfer\n");
}

+ Retry_Sense:
/* Now, if we need to do the auto-sense, let's do it */
if (need_auto_sense) {
int temp_result;
@@ -722,6 +723,11 @@
}
if (temp_result != USB_STOR_TRANSPORT_GOOD) {
US_DEBUGP("-- auto-sense failure\n");
+ if (us->fflags & US_FL_SANE_SENSE) {
+ us->fflags &= ~US_FL_SANE_SENSE;
+ 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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  |  Next  |  Last
Pages: 1 2 3 4
Prev: Linux IO service time - a perfect source of randomness ?
Next: NOHZ: local_softirq_pending 08