* Re: usb-storage and Sony Handycam [not found] <1068207145.3fab8c2988d43@webmail.netregistry.net> @ 2003-11-07 16:21 ` Alan Stern 2003-11-07 17:29 ` Patrick Mansfield 0 siblings, 1 reply; 54+ messages in thread From: Alan Stern @ 2003-11-07 16:21 UTC (permalink / raw) To: Dmitri Katchalov, Patrick Mansfield Cc: USB development list, SCSI development list On Fri, 7 Nov 2003, Dmitri Katchalov wrote: > Greetings, > > I finally managet to get Sony DCR-TRV17E memory stick > working with usb-storage. > The solution turned out to be very simple: > > echo "Sony:Sony DSC:0" > /proc/scsi/device_info > > But I'm puzzled as to why didn't it work in the first > place despite several reports to the contrary. My guess > is it used to work in earlier kernels and then some > seemingly unrelated change broke it. > > I investigated it a little and that's what I found: > This device uses CB transport (SENSE status not reported). > This device does not support MODE_SENSE at all. > MODE_SENSE(6) fails with STALL condition. Subsequent > REQUEST-SENSE reports ILLEGAL REQUEST. SCSI layer is > able to handle this situation, the only drawback is > that write-protect status is not reported. > > Unfortunately usb-storage sets use_10_for_ms flag > by default which causes SCSI to try MODE_SENSE(10) > first. > The device responds to MODE_SENSE(10) with "babble" > (-EOVERFLOW). scsiglue sees transport error and > tries to do a soft reset (which also fails). > Note that scsiglue does not automatically send > REQUEST-SENSE in this case. > > scsiglue then gives up after several attempts and > reports the error to the upper layer. Upper layer > (SCSI) correctly realises that MODE_SENSE(10) does > not work and attempts to fallback to MODE_SENSE(6) > but before that it issues TEST_UNIT_READY. > > This time the device responds with ILLEGAL REQUEST. > Now SCSI gets totally confused and decides to give up. > > As I understand it this ILLEGAL REQUEST actually refers > to the previous MODE_SENSE(10) command because this > error condition has never been cleared properly. You're probably right. But that's not how TEST-UNIT-READY is _supposed_ to behave. It shouldn't be reporting the status from a previous command; it should indicate the device's current status. > If I change interpret_usb_result() in transport.c > to ignore "babble" then the next REQUEST-SENSE > reports ILLEGAL REQUEST, SCSI handles that and > everything sort of works. > > Regards, > Dmitri Patrick, what do you think about implementing that change to have the response checker retry when it gets ILLEGAL REQUEST from a TEST-UNIT-READY? Alan Stern ------------------------------------------------------- This SF.net email is sponsored by: SF.net Giveback Program. Does SourceForge.net help you be more productive? Does it help you create better code? SHARE THE LOVE, and help us help YOU! Click Here: http://sourceforge.net/donate/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-07 16:21 ` usb-storage and Sony Handycam Alan Stern @ 2003-11-07 17:29 ` Patrick Mansfield 2003-11-07 19:49 ` Alan Stern 2003-11-07 22:09 ` usb-storage and Sony Handycam Alan Stern 0 siblings, 2 replies; 54+ messages in thread From: Patrick Mansfield @ 2003-11-07 17:29 UTC (permalink / raw) To: Alan Stern, Dmitri Katchalov; +Cc: USB development list, SCSI development list On Fri, Nov 07, 2003 at 11:21:48AM -0500, Alan Stern wrote: > On Fri, 7 Nov 2003, Dmitri Katchalov wrote: > > I investigated it a little and that's what I found: > > This device uses CB transport (SENSE status not reported). > > This device does not support MODE_SENSE at all. > > MODE_SENSE(6) fails with STALL condition. Subsequent > > REQUEST-SENSE reports ILLEGAL REQUEST. SCSI layer is > > able to handle this situation, the only drawback is > > that write-protect status is not reported. > > > > Unfortunately usb-storage sets use_10_for_ms flag > > by default which causes SCSI to try MODE_SENSE(10) > > first. > > The device responds to MODE_SENSE(10) with "babble" > > (-EOVERFLOW). scsiglue sees transport error and > > tries to do a soft reset (which also fails). > > Note that scsiglue does not automatically send > > REQUEST-SENSE in this case. > > > > scsiglue then gives up after several attempts and > > reports the error to the upper layer. Upper layer > > (SCSI) correctly realises that MODE_SENSE(10) does > > not work and attempts to fallback to MODE_SENSE(6) > > but before that it issues TEST_UNIT_READY. > > This time the device responds with ILLEGAL REQUEST. > > Now SCSI gets totally confused and decides to give up. The above sequence is not quite correct (assuming the same behaviour as logs for another failing Sony/Sony DSC), the MODE SENSE goes out, but we do not notice in scsi core that it failed (host byte DID_ERROR). The MODE SENSE is treated as if it succeeded, and yes the TEST UNIT READY gets an ILLEGAL REQUEST. I don't recall what happens to further MODE SENSE commands. > > As I understand it this ILLEGAL REQUEST actually refers > > to the previous MODE_SENSE(10) command because this > > error condition has never been cleared properly. > > You're probably right. But that's not how TEST-UNIT-READY is _supposed_ > to behave. It shouldn't be reporting the status from a previous command; > it should indicate the device's current status. > > > If I change interpret_usb_result() in transport.c > > to ignore "babble" then the next REQUEST-SENSE > > reports ILLEGAL REQUEST, SCSI handles that and > > everything sort of works. I don't see how that worked, I would expect nearly the same command sequence, since scsi core is not checking the error. USB storage debug logs are needed to verify what happened (when ignoring babble/overflow). > > Regards, > > Dmitri > > Patrick, what do you think about implementing that change to have the > response checker retry when it gets ILLEGAL REQUEST from a > TEST-UNIT-READY? No, since we should fix scsi_status_is_good, and then we see a different failure mode for the Sony/Sony DSC. Dmitri - Reference to the patch and discussion about scsi_status_is_good change, you will likely get the same bad result if you apply the patch: http://marc.theaimsgroup.com/?t=106744619500004&r=1&w=2 We should figure out what commands work for the device using SG_IO, try using Pat L's plscsi or modify one of the sg_util programs to send the exact failing command, reference: http://marc.theaimsgroup.com/?l=linux-scsi&m=106755844319144&w=2 A USB trace under Windows would be helpful. -- Patrick Mansfield ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-07 17:29 ` Patrick Mansfield @ 2003-11-07 19:49 ` Alan Stern 2003-11-08 2:54 ` Dmitri Katchalov 2003-11-07 22:09 ` usb-storage and Sony Handycam Alan Stern 1 sibling, 1 reply; 54+ messages in thread From: Alan Stern @ 2003-11-07 19:49 UTC (permalink / raw) To: Patrick Mansfield Cc: Dmitri Katchalov, USB development list, SCSI development list On Fri, 7 Nov 2003, Patrick Mansfield wrote: > > On Fri, 7 Nov 2003, Dmitri Katchalov wrote: > > > If I change interpret_usb_result() in transport.c > > > to ignore "babble" then the next REQUEST-SENSE > > > reports ILLEGAL REQUEST, SCSI handles that and > > > everything sort of works. > > I don't see how that worked, I would expect nearly the same command > sequence, since scsi core is not checking the error. > > USB storage debug logs are needed to verify what happened (when ignoring > babble/overflow). In any case, usb-storage should _never_ ignore a "babble" error. It indicates a serious communications problem. Alan Stern ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-07 19:49 ` Alan Stern @ 2003-11-08 2:54 ` Dmitri Katchalov 2003-11-08 6:34 ` Patrick Mansfield 0 siblings, 1 reply; 54+ messages in thread From: Dmitri Katchalov @ 2003-11-08 2:54 UTC (permalink / raw) To: Alan Stern; +Cc: linux-usb-devel, linux-scsi Quoting Alan Stern <stern@rowland.harvard.edu>: > On Fri, 7 Nov 2003, Patrick Mansfield wrote: > > > > On Fri, 7 Nov 2003, Dmitri Katchalov wrote: > > > > If I change interpret_usb_result() in transport.c > > > > to ignore "babble" then the next REQUEST-SENSE > > > > reports ILLEGAL REQUEST, SCSI handles that and > > > > everything sort of works. > > > > I don't see how that worked, I would expect nearly the same command > > sequence, since scsi core is not checking the error. > > > > USB storage debug logs are needed to verify what happened (when ignoring > > babble/overflow). > > In any case, usb-storage should _never_ ignore a "babble" error. It > indicates a serious communications problem. Well, when I said "ignore babble" I actually did this: case -EOVERFLOW: - return USB_STOR_XFER_LONG; + return USB_STOR_XFER_STALL; I agree, it was not a good idea. I only tried it to see what happens. As I said, my camera works just fine. All I need is a module parameter in modprobe.conf (or a new entry in scsi_devinfo.c), no other code changes required. I just thought you may be interested in this rather peculiar behaviour. Regards, Dmitri ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-08 2:54 ` Dmitri Katchalov @ 2003-11-08 6:34 ` Patrick Mansfield 2003-11-08 13:29 ` Dmitri Katchalov 2003-11-08 16:28 ` Alan Stern 0 siblings, 2 replies; 54+ messages in thread From: Patrick Mansfield @ 2003-11-08 6:34 UTC (permalink / raw) To: Dmitri Katchalov; +Cc: Alan Stern, linux-usb-devel, linux-scsi On Sat, Nov 08, 2003 at 01:54:11PM +1100, Dmitri Katchalov wrote: > Well, when I said "ignore babble" I actually did this: > case -EOVERFLOW: > - return USB_STOR_XFER_LONG; > + return USB_STOR_XFER_STALL; > I agree, it was not a good idea. I only tried it to see what happens. > > As I said, my camera works just fine. All I need is a module parameter > in modprobe.conf (or a new entry in scsi_devinfo.c), no other code changes > required. I just thought you may be interested in this rather peculiar > behaviour. We need to figure out what is different here, and why (based on other reports for this device) this worked on 2.4 but not on 2.6. Looking at usb/storage/transport.c, we are not generating the usb "fake_sense" with your change. Maybe the failed MODE SENSE had a check condition, but the fake_sense prevented us from retrieving it? So the next command sent got a check condition, illegal request. That still doesn't tell us why the MODE SENSE failed, and why the failure does not show up with 2.4. Can you run the variants on MODE SENE via user space SG_IO, and figure out which ones work? AFAICT at least three different problems are contributing to the failure: 1) The MODE SENSE seems to work fine in 2.4, but is failing in 2.6. In 2.4, we always send a MODE SENSE page 3f with length 255; in 2.6 we start by asking for 4 bytes. 2) scsi core ignores host byte errors (DID_ERROR). This means we behave as if the MODE SENSE worked OK, even though USB tells scsi core that it failed. Fixing this problem (for another user) caused even worse problems. 3) The next command - TEST UNIT READY - gets a check condition, illegal request. Maybe the fake sense handling is screwing things up - the device tells us it sent back more data than we asked for, but it is also trying to check us. -- Patrick Mansfield ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-08 6:34 ` Patrick Mansfield @ 2003-11-08 13:29 ` Dmitri Katchalov 2003-11-08 16:28 ` Alan Stern 1 sibling, 0 replies; 54+ messages in thread From: Dmitri Katchalov @ 2003-11-08 13:29 UTC (permalink / raw) To: Patrick Mansfield; +Cc: Alan Stern, linux-usb-devel, linux-scsi Quoting Patrick Mansfield <patmans@us.ibm.com>: > We need to figure out what is different here, and why (based on other > reports for this device) this worked on 2.4 but not on 2.6. USB log from "the other" OS is attached for your perusal. I've compressed and parsed it a little. From what I see the device is anything but compliant, but then it does not claim to be. Regards, Dmitri GET_DESCRIPTOR_FROM_DEVICE Descriptor Type: 0x0001 DEVICE 0000: 12 01 00 01 00 00 00 08 4c 05 2e 00 00 03 01 02 0010: 00 01 bLength : 0x12 (18) bDescriptorType : 0x01 (1) bcdUSB : 0x0100 (256) bDeviceClass : 0x00 (0) bDeviceSubClass : 0x00 (0) bDeviceProtocol : 0x00 (0) bMaxPacketSize0 : 0x08 (8) idVendor : 0x054c (1356) idProduct : 0x002e (46) bcdDevice : 0x0300 (768) iManufacturer : 0x01 (1) iProduct : 0x02 (2) iSerialNumber : 0x00 (0) bNumConfigurations : 0x01 (1) GET_DESCRIPTOR_FROM_DEVICE Descriptor Type: 0x0002 CONFIGURATION 0000: 09 02 27 00 01 01 00 40 01 09 04 00 00 03 08 ff 0010: 01 00 07 05 01 02 40 00 00 07 05 82 02 40 00 00 0020: 07 05 83 03 08 00 ff bLength : 0x09 (9) bDescriptorType : 0x02 (2) wTotalLength : 0x0027 (39) bNumInterfaces : 0x01 (1) bConfigurationValue: 0x01 (1) iConfiguration : 0x00 (0) bmAttributes : 0x40 (64) MaxPower : 0x01 (1) SELECT_CONFIGURATION Configuration Descriptor: bLength: 9 (0x09) bDescriptorType: 2 (0x02) wTotalLength: 39 (0x0027) bNumInterfaces: 1 (0x01) bConfigurationValue: 1 (0x01) iConfiguration: 0 (0x00) bmAttributes: 64 (0x40) 0x40: Self Powered MaxPower: 1 (0x01) (in 2 mA units, therefore 2 mA power consumption) Number of interfaces: 1 Interface[0]: Length: 0x004c InterfaceNumber: 0x00 AlternateSetting: 0x00 Class = 0x34 SubClass = 0x6d Protocol = 0x37 InterfaceHandle = 0x00000000 NumberOfPipes = 0x00000003 Pipe[0]: MaximumPacketSize = 0xaff0 EndpointAddress = 0x4b Interval = 0xc1 PipeType = 0x00 UsbdPipeTypeControl PipeHandle = 0x0000ce54 MaxTransferSize = 0x00010000 PipeFlags = 0x00 Pipe[1]: MaximumPacketSize = 0x2f68 EndpointAddress = 0x5b Interval = 0xbc PipeType = 0x206a2b1b !!! INVALID !!! PipeHandle = 0x4e4f5300 MaxTransferSize = 0x00010000 PipeFlags = 0x00 Pipe[2]: MaximumPacketSize = 0x000d EndpointAddress = 0x00 Interval = 0x00 PipeType = 0xc15667d0 !!! INVALID !!! PipeHandle = 0xc1566810 MaxTransferSize = 0x00010000 PipeFlags = 0x00 SELECT_CONFIGURATION Configuration Descriptor: bLength: 9 (0x09) bDescriptorType: 2 (0x02) wTotalLength: 39 (0x0027) bNumInterfaces: 1 (0x01) bConfigurationValue: 1 (0x01) iConfiguration: 0 (0x00) bmAttributes: 64 (0x40) 0x40: Self Powered MaxPower: 1 (0x01) (in 2 mA units, therefore 2 mA power consumption) Number of interfaces: 1 Interface[0]: Length: 0x004c InterfaceNumber: 0x00 AlternateSetting: 0x00 Class = 0x08 SubClass = 0xff Protocol = 0x01 InterfaceHandle = 0xc1567700 NumberOfPipes = 0x00000003 Pipe[0]: MaximumPacketSize = 0x0040 EndpointAddress = 0x01 Interval = 0x00 PipeType = 0x02 UsbdPipeTypeBulk PipeHandle = 0xc1567718 MaxTransferSize = 0x00010000 PipeFlags = 0x00 Pipe[1]: MaximumPacketSize = 0x0040 EndpointAddress = 0x82 Interval = 0x00 PipeType = 0x02 UsbdPipeTypeBulk PipeHandle = 0xc156772c MaxTransferSize = 0x00010000 PipeFlags = 0x00 Pipe[2]: MaximumPacketSize = 0x0008 EndpointAddress = 0x83 Interval = 0xff PipeType = 0x03 UsbdPipeTypeInterrupt PipeHandle = 0xc1567740 MaxTransferSize = 0x00010000 PipeFlags = 0x00 --------------------------------------------- out 0c (12) length 0000: 12 00 00 00 24 00 00 00 00 00 00 00 INQUIRY in 0x00000024 (36) length 0000: 00 80 00 01 1f 00 00 00 53 6f 6e 79 20 20 20 20 0010: 53 6f 6e 79 20 44 53 43 20 20 20 20 20 20 20 20 0020: 33 2e 30 30 out 0x0000000c (12) length 0000: 25 00 00 00 00 00 00 00 00 00 00 00 READ CAPACITY in 0x00000008 (8) length 0000: 00 00 1e df 00 00 02 00 7903 blocks x 512 = 4,046,366 bytes out 0x0000000c (12) length 0000: 00 00 00 00 00 00 00 00 00 00 00 00 TEST UNIT READY out 0x0000000c (12) length 0000: 25 00 00 00 00 00 00 00 00 00 00 00 READ CAPACITY in 0x00000008 (8) length 0000: 00 00 1e df 00 00 02 00 7903 blocks x 512 = 4,046,366 bytes out 0x0000000c (12) length 0000: 28 00 00 00 00 00 00 00 80 00 00 00 READ 128 blocks, starting from 0 <end of log> ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-08 6:34 ` Patrick Mansfield 2003-11-08 13:29 ` Dmitri Katchalov @ 2003-11-08 16:28 ` Alan Stern 2003-11-08 20:37 ` Patrick Mansfield 1 sibling, 1 reply; 54+ messages in thread From: Alan Stern @ 2003-11-08 16:28 UTC (permalink / raw) To: Patrick Mansfield Cc: Dmitri Katchalov, Idan Sofer, USB development list, SCSI development list On Fri, 7 Nov 2003, Patrick Mansfield wrote: > We need to figure out what is different here, and why (based on other > reports for this device) this worked on 2.4 but not on 2.6. > > Looking at usb/storage/transport.c, we are not generating the usb > "fake_sense" with your change. > > Maybe the failed MODE SENSE had a check condition, but the fake_sense > prevented us from retrieving it? No, the "babble" prevented that. How to deal with babble errors is a serious question that's been under discussion in the usb-storage mailing list, but we haven't found a good answer. It doesn't come up often except for certain devices that go crazy when given some of the less-common commands. Babble is a nasty violation of the USB protocol (and in particular the USB Mass Storage class protocols); it indicates that something has gone very wrong but doesn't provide any clue as to how to fix it. I vaguely recall getting a report from someone that even unplugging the USB cable and reinserting it wasn't enough to unwedge his device. The "fake sense" is a compromise. It lets us give a clear indication to the higher layers that something is wrong and asks them not to send the same command again. But it doesn't do much about getting the device back into a working state (if that's even possible). > So the next command sent got a check condition, illegal request. > > That still doesn't tell us why the MODE SENSE failed, and why the failure > does not show up with 2.4. The best approach for doing this is to have a side-by-side comparison of the usb-storage debugging logs for 2.4 and 2.6. I think we have to assume that the MODE-SENSE failed because the device simply doesn't support it correctly. > Can you run the variants on MODE SENE via user space SG_IO, and figure out > which ones work? > > AFAICT at least three different problems are contributing to the failure: > > 1) The MODE SENSE seems to work fine in 2.4, but is failing in 2.6. In > 2.4, we always send a MODE SENSE page 3f with length 255; in 2.6 we start by > asking for 4 bytes. > > 2) scsi core ignores host byte errors (DID_ERROR). This means we behave as > if the MODE SENSE worked OK, even though USB tells scsi core that it > failed. Fixing this problem (for another user) caused even worse problems. I must have missed that. It seems odd that fixing an oversight in the kernel could make interoperability _worse_. Maybe that indicates something else needs to be fixed. > 3) The next command - TEST UNIT READY - gets a check condition, illegal > request. Maybe the fake sense handling is screwing things up - the device > tells us it sent back more data than we asked for, but it is also trying > to check us. I don't understand your speculation here. It seems clear to me that this is a situation where the device and the host are out of sync -- the host sends a command and the device sends back status from the _previous_ command. Repeating the TEST-UNIT-READY may bring us back in sync. Or maybe some sort of reset will -- no way to tell without trying it. The ultimate problem is that this device doesn't like some of the MODE-SENSE commands used to determine read-write and cache status. We need to have either per-device information saying which variants of the commands the device will accept or else per-device information saying not to send the MODE-SENSE commands at all. I don't see any other way of coping. Even trying to send a command and then trying to recover if it fails is not a viable approach if recovery requires the user to cycle the power on the device (or to unplug its cable). Alan Stern ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-08 16:28 ` Alan Stern @ 2003-11-08 20:37 ` Patrick Mansfield 2003-11-09 3:47 ` [linux-usb-devel] " Alan Stern 0 siblings, 1 reply; 54+ messages in thread From: Patrick Mansfield @ 2003-11-08 20:37 UTC (permalink / raw) To: Alan Stern Cc: Dmitri Katchalov, Idan Sofer, ronald, USB development list, SCSI development list On Sat, Nov 08, 2003 at 11:28:37AM -0500, Alan Stern wrote: > On Fri, 7 Nov 2003, Patrick Mansfield wrote: > > Looking at usb/storage/transport.c, we are not generating the usb > > "fake_sense" with your change. > > > > Maybe the failed MODE SENSE had a check condition, but the fake_sense > > prevented us from retrieving it? > > No, the "babble" prevented that. > The "fake sense" is a compromise. It lets us give a clear indication to > the higher layers that something is wrong and asks them not to send the > same command again. But it doesn't do much about getting the device back > into a working state (if that's even possible). Looking closer at the USB code, is the fake_sense code even hit? The logs do not show any of the US_DEBUGP in usb_stor_Bulk_transport(), like "Bulk Command S ...". A DID_ERROR causes scsi core to retry the command - it will not even check the sense data. A DID_ABORT might be better, scsi core will not retry. (We still have issue 1, where the host byte is not checked in scsi_status_is_good.) > > So the next command sent got a check condition, illegal request. > > > > That still doesn't tell us why the MODE SENSE failed, and why the failure > > does not show up with 2.4. > > The best approach for doing this is to have a side-by-side comparison of > the usb-storage debugging logs for 2.4 and 2.6. Yes, we do not have a 2.4 trace of the same device. > I think we have to assume > that the MODE-SENSE failed because the device simply doesn't support it > correctly. > > > Can you run the variants on MODE SENE via user space SG_IO, and figure out > > which ones work? Still no data on the above (from Dmitri or Ronald). > > 3) The next command - TEST UNIT READY - gets a check condition, illegal > > request. Maybe the fake sense handling is screwing things up - the device > > tells us it sent back more data than we asked for, but it is also trying > > to check us. > > I don't understand your speculation here. It seems clear to me that this > is a situation where the device and the host are out of sync -- the host > sends a command and the device sends back status from the _previous_ > command. Repeating the TEST-UNIT-READY may bring us back in sync. Or > maybe some sort of reset will -- no way to tell without trying it. The sense data is not cleared until it is retrieved, so if there is sense waiting, and we do not send a REQUEST SENSE, the next REQUEST SENSE will retrieve the pre-existing sense data (see SAM-3, I am looking at sam3r07.pdf, section 5.9.6 sense data). If the actual result of the command was checked in the babble cases, and a REQUEST SENSE sent, it might have gotten the ILLEGAL REQUEST, and the devices sense data might be cleared. It might be bad to do anything for babble cases, but the REQUEST SENSE should be sent to at least clear out any real sense data if we are going to generate our own fake sense. Dmitri's change (return USB_STOR_XFER_STALL instead of USB_STOR_XFER_LONG) causes us to return USB_STOR_TRANSPORT_GOOD vs USB_STOR_TRANSPORT_ERROR in usb_stor_CB_transport. Then in usb_stor_invoke_transport we issue a REQUEST SENSE. If for a USB_STOR_TRANSPORT_ERROR we also did a REQUEST SENSE, the following command (TEST UNIT READY) would not get the ILLEGAL REQUEST. I don't know why the TEST UNIT READY caused an auto REQUEST SENSE to be sent. Changing the sense stuff does not address the MODE SENSE failing, we still have 3 separate issues that are all hit by this device. -- Patrick Mansfield ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] usb-storage and Sony Handycam 2003-11-08 20:37 ` Patrick Mansfield @ 2003-11-09 3:47 ` Alan Stern 2003-11-09 8:45 ` Dmitri Katchalov 2003-11-10 17:59 ` Patrick Mansfield 0 siblings, 2 replies; 54+ messages in thread From: Alan Stern @ 2003-11-09 3:47 UTC (permalink / raw) To: Patrick Mansfield Cc: Dmitri Katchalov, Idan Sofer, ronald, USB development list, SCSI development list On Sat, 8 Nov 2003, Patrick Mansfield wrote: > On Sat, Nov 08, 2003 at 11:28:37AM -0500, Alan Stern wrote: > > > The "fake sense" is a compromise. It lets us give a clear indication to > > the higher layers that something is wrong and asks them not to send the > > same command again. But it doesn't do much about getting the device back > > into a working state (if that's even possible). > > Looking closer at the USB code, is the fake_sense code even hit? The logs > do not show any of the US_DEBUGP in usb_stor_Bulk_transport(), like > "Bulk Command S ...". It probably wasn't hit. The fake sense data is only sent for devices using the Bulk-only transport, and Dmitri's and Idan's devices use the Control-Bulk transport IIRC. > A DID_ERROR causes scsi core to retry the command - it will not even check > the sense data. A DID_ABORT might be better, scsi core will not retry. > (We still have issue 1, where the host byte is not checked in > scsi_status_is_good.) For CB and CBI devices, babble causes a DID_ERROR return. For Bulk-only it causes a Check-Condition status to be returned along with Invalid Command sense data. Like I said earlier, we haven't yet figured out the best way to handle these errors. > > > That still doesn't tell us why the MODE SENSE failed, and why the failure > > > does not show up with 2.4. > > > > The best approach for doing this is to have a side-by-side comparison of > > the usb-storage debugging logs for 2.4 and 2.6. > > Yes, we do not have a 2.4 trace of the same device. > > > I think we have to assume > > that the MODE-SENSE failed because the device simply doesn't support it > > correctly. > > > > > Can you run the variants on MODE SENE via user space SG_IO, and figure out > > > which ones work? > > Still no data on the above (from Dmitri or Ronald). Doing that will probably be a waste of time. I firmly believe that there is _no_ least common denominator among USB mass storage devices: Some work with any MODE-SENSE command, some work with only a few variants, others work with others. Maybe some devices don't work with MODE-SENSE at all. In any case, there isn't any single combination of pages/transfer-lengths that will be accepted by every device. And when a device fails to accept such a command it often does so catastrophically, requiring a power-cycle to start working again. The only safe course is to avoid sending these commands at all to the devices that can't handle them. > > > 3) The next command - TEST UNIT READY - gets a check condition, illegal > > > request. Maybe the fake sense handling is screwing things up - the device > > > tells us it sent back more data than we asked for, but it is also trying > > > to check us. > > > > I don't understand your speculation here. It seems clear to me that this > > is a situation where the device and the host are out of sync -- the host > > sends a command and the device sends back status from the _previous_ > > command. Repeating the TEST-UNIT-READY may bring us back in sync. Or > > maybe some sort of reset will -- no way to tell without trying it. > > The sense data is not cleared until it is retrieved, so if there is sense > waiting, and we do not send a REQUEST SENSE, the next REQUEST SENSE will > retrieve the pre-existing sense data (see SAM-3, I am looking at > sam3r07.pdf, section 5.9.6 sense data). Hmmm... I'm more used to an earlier version of the SCSI protocol, according to which old sense data is lost whenever a new command is received unless that command is REQUEST-SENSE. > If the actual result of the command was checked in the babble cases, and a > REQUEST SENSE sent, it might have gotten the ILLEGAL REQUEST, and the > devices sense data might be cleared. It might be bad to do anything for > babble cases, but the REQUEST SENSE should be sent to at least clear out > any real sense data if we are going to generate our own fake sense. There's isn't any reliable way to get the command result or the sense data after a babble occurs -- in fact, there isn't a reliable way to re-establish communication with the device at all. My best thought on this has been to try reading (and throwing away) data from the device until a 5-second (for example) timeout expires without receiving anything, then issue a device reset. That's more likely to recover from this sort of error than anything else I can think of, but it's never been implemented. I have seen logs from users where a device reset was sent following a babble error. It didn't help; the device remained wedged. The USB Mass Storage specification documents are silent on the issue of babble. And even if they weren't it probably wouldn't help; the only reason these devices start babbling is because they aren't compliant with the standards in the first place. > Dmitri's change (return USB_STOR_XFER_STALL instead of USB_STOR_XFER_LONG) > causes us to return USB_STOR_TRANSPORT_GOOD vs USB_STOR_TRANSPORT_ERROR in > usb_stor_CB_transport. > > Then in usb_stor_invoke_transport we issue a REQUEST SENSE. > > If for a USB_STOR_TRANSPORT_ERROR we also did a REQUEST SENSE, the > following command (TEST UNIT READY) would not get the ILLEGAL REQUEST. That doesn't make sense. USB_STOR_TRANSPORT_ERROR means we aren't able to communicate with the device. So how can we send it a REQUEST-SENSE? > I don't know why the TEST UNIT READY caused an auto REQUEST SENSE to > be sent. The CB transport doesn't include any means for the device to return status. (Yes, it's stupid.) The only way we can find out is to issue auto REQUEST-SENSE for virtually every command. > Changing the sense stuff does not address the MODE SENSE failing, we still > have 3 separate issues that are all hit by this device. If we simply avoided sending the MODE-SENSE in the first place, none of the other issues would come up. IMHO that's the best solution. Alan Stern ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] usb-storage and Sony Handycam 2003-11-09 3:47 ` [linux-usb-devel] " Alan Stern @ 2003-11-09 8:45 ` Dmitri Katchalov 2003-11-10 20:45 ` Patrick Mansfield 2003-11-10 17:59 ` Patrick Mansfield 1 sibling, 1 reply; 54+ messages in thread From: Dmitri Katchalov @ 2003-11-09 8:45 UTC (permalink / raw) To: Alan Stern Cc: Patrick Mansfield, Idan Sofer, ronald, USB development list, SCSI development list [My previous msg didn't make it to the list, resending] Attached here for your perusal is USB log for this device on win98. Regards, Dmitri 1 in down n/a 0.062 GET_DESCRIPTOR_FROM_DEVICE URB Header (length: 80) SequenceNumber: 1 Function: 000b (GET_DESCRIPTOR_FROM_DEVICE) 1 in up n/a 0.066 CONTROL_TRANSFER 12 01 00 01 00 00 00 08 0x00000000 URB Header (length: 80) SequenceNumber: 1 Function: 0008 (CONTROL_TRANSFER) PipeHandle: c1565d8c SetupPacket: 0000: 80 06 00 01 00 00 12 00 bmRequestType: 80 DIR: Device-To-Host TYPE: Standard RECIPIENT: Device bRequest: 06 GET_DESCRIPTOR Descriptor Type: 0x0001 DEVICE TransferBuffer: 0x00000012 (18) length 0000: 12 01 00 01 00 00 00 08 4c 05 2e 00 00 03 01 02 0010: 00 01 bLength : 0x12 (18) bDescriptorType : 0x01 (1) bcdUSB : 0x0100 (256) bDeviceClass : 0x00 (0) bDeviceSubClass : 0x00 (0) bDeviceProtocol : 0x00 (0) bMaxPacketSize0 : 0x08 (8) idVendor : 0x054c (1356) idProduct : 0x002e (46) bcdDevice : 0x0300 (768) iManufacturer : 0x01 (1) iProduct : 0x02 (2) iSerialNumber : 0x00 (0) bNumConfigurations : 0x01 (1) 2 in down n/a 0.066 GET_DESCRIPTOR_FROM_DEVICE URB Header (length: 80) SequenceNumber: 2 Function: 000b (GET_DESCRIPTOR_FROM_DEVICE) 2 in up n/a 0.076 CONTROL_TRANSFER 09 02 27 00 01 01 00 40 0x00000000 URB Header (length: 80) SequenceNumber: 2 Function: 0008 (CONTROL_TRANSFER) PipeHandle: c1565d8c SetupPacket: 0000: 80 06 00 02 00 00 09 02 bmRequestType: 80 DIR: Device-To-Host TYPE: Standard RECIPIENT: Device bRequest: 06 GET_DESCRIPTOR Descriptor Type: 0x0002 CONFIGURATION TransferBuffer: 0x00000027 (39) length 0000: 09 02 27 00 01 01 00 40 01 09 04 00 00 03 08 ff 0010: 01 00 07 05 01 02 40 00 00 07 05 82 02 40 00 00 0020: 07 05 83 03 08 00 ff bLength : 0x09 (9) bDescriptorType : 0x02 (2) wTotalLength : 0x0027 (39) bNumInterfaces : 0x01 (1) bConfigurationValue: 0x01 (1) iConfiguration : 0x00 (0) bmAttributes : 0x40 (64) MaxPower : 0x01 (1) 3 ??? down n/a 0.076 SELECT_CONFIGURATION URB Header (length: 100) SequenceNumber: 3 Function: 0000 (SELECT_CONFIGURATION) Configuration Descriptor: bLength: 9 (0x09) bDescriptorType: 2 (0x02) wTotalLength: 39 (0x0027) bNumInterfaces: 1 (0x01) bConfigurationValue: 1 (0x01) iConfiguration: 0 (0x00) bmAttributes: 64 (0x40) 0x40: Self Powered MaxPower: 1 (0x01) (in 2 mA units, therefore 2 mA power consumption) Number of interfaces: 1 Interface[0]: Length: 0x004c InterfaceNumber: 0x00 AlternateSetting: 0x00 Class = 0x34 SubClass = 0x6d Protocol = 0x37 InterfaceHandle = 0x00000000 NumberOfPipes = 0x00000003 Pipe[0]: MaximumPacketSize = 0xaff0 EndpointAddress = 0x4b Interval = 0xc1 PipeType = 0x00 UsbdPipeTypeControl PipeHandle = 0x0000ce54 MaxTransferSize = 0x00010000 PipeFlags = 0x00 Pipe[1]: MaximumPacketSize = 0x2f68 EndpointAddress = 0x5b Interval = 0xbc PipeType = 0x206a2b1b !!! INVALID !!! PipeHandle = 0x4e4f5300 MaxTransferSize = 0x00010000 PipeFlags = 0x00 Pipe[2]: MaximumPacketSize = 0x000d EndpointAddress = 0x00 Interval = 0x00 PipeType = 0xc15667d0 !!! INVALID !!! PipeHandle = 0xc1566810 MaxTransferSize = 0x00010000 PipeFlags = 0x00 3 ??? up n/a 0.076 SELECT_CONFIGURATION 0x00000000 URB Header (length: 100) SequenceNumber: 3 Function: 0000 (SELECT_CONFIGURATION) Configuration Descriptor: bLength: 9 (0x09) bDescriptorType: 2 (0x02) wTotalLength: 39 (0x0027) bNumInterfaces: 1 (0x01) bConfigurationValue: 1 (0x01) iConfiguration: 0 (0x00) bmAttributes: 64 (0x40) 0x40: Self Powered MaxPower: 1 (0x01) (in 2 mA units, therefore 2 mA power consumption) Number of interfaces: 1 Interface[0]: Length: 0x004c InterfaceNumber: 0x00 AlternateSetting: 0x00 Class = 0x08 SubClass = 0xff Protocol = 0x01 InterfaceHandle = 0xc1567700 NumberOfPipes = 0x00000003 Pipe[0]: MaximumPacketSize = 0x0040 EndpointAddress = 0x01 Interval = 0x00 PipeType = 0x02 UsbdPipeTypeBulk PipeHandle = 0xc1567718 MaxTransferSize = 0x00010000 PipeFlags = 0x00 Pipe[1]: MaximumPacketSize = 0x0040 EndpointAddress = 0x82 Interval = 0x00 PipeType = 0x02 UsbdPipeTypeBulk PipeHandle = 0xc156772c MaxTransferSize = 0x00010000 PipeFlags = 0x00 Pipe[2]: MaximumPacketSize = 0x0008 EndpointAddress = 0x83 Interval = 0xff PipeType = 0x03 UsbdPipeTypeInterrupt PipeHandle = 0xc1567740 MaxTransferSize = 0x00010000 PipeFlags = 0x00 4 out down n/a 3.142 CLASS_INTERFACE 12 00 00 00 24 00 00 00 URB Header (length: 80) SequenceNumber: 4 Function: 001b (CLASS_INTERFACE) PipeHandle: c146f640 SetupPacket: 0000: 21 00 00 00 00 00 00 00 bmRequestType: 21 DIR: Host-To-Device TYPE: Class RECIPIENT: Interface bRequest: 00 TransferBuffer: 0x0000000c (12) length 0000: 12 00 00 00 24 00 00 00 00 00 00 00 4 out up n/a 3.146 CONTROL_TRANSFER - 0x00000000 URB Header (length: 80) SequenceNumber: 4 Function: 0008 (CONTROL_TRANSFER) PipeHandle: c1565d8c SetupPacket: 0000: 21 00 00 00 00 00 0c 00 bmRequestType: 21 DIR: Host-To-Device TYPE: Class RECIPIENT: Interface bRequest: 00 No TransferBuffer 5 in down 0x82 3.146 BULK_OR_INTERRUPT_TRANSFER - URB Header (length: 72) SequenceNumber: 5 Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) TransferFlags: 0x00000003 No TransferBuffer 5 in up 0x82 3.146 BULK_OR_INTERRUPT_TRANSFER 00 80 00 01 1f 00 00 00 0x00000000 URB Header (length: 72) SequenceNumber: 5 Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) TransferFlags: 0x00000003 TransferBuffer: 0x00000024 (36) length 0000: 00 80 00 01 1f 00 00 00 53 6f 6e 79 20 20 20 20 0010: 53 6f 6e 79 20 44 53 43 20 20 20 20 20 20 20 20 0020: 33 2e 30 30 6 in down 0x82 3.146 CLASS_INTERFACE 25 00 00 00 00 00 00 00 URB Header (length: 80) SequenceNumber: 6 Function: 001b (CLASS_INTERFACE) PipeHandle: c156772c SetupPacket: 0000: 21 00 00 00 00 00 0c 00 bmRequestType: 21 DIR: Host-To-Device TYPE: Class RECIPIENT: Interface bRequest: 00 TransferBuffer: 0x0000000c (12) length 0000: 25 00 00 00 00 00 00 00 00 00 00 00 6 out up n/a 3.152 CONTROL_TRANSFER - 0x00000000 URB Header (length: 80) SequenceNumber: 6 Function: 0008 (CONTROL_TRANSFER) PipeHandle: c1565d8c SetupPacket: 0000: 21 00 00 00 00 00 0c 00 bmRequestType: 21 DIR: Host-To-Device TYPE: Class RECIPIENT: Interface bRequest: 00 No TransferBuffer 7 in down 0x82 3.152 BULK_OR_INTERRUPT_TRANSFER - URB Header (length: 72) SequenceNumber: 7 Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) TransferFlags: 0x00000003 No TransferBuffer 7 in up 0x82 3.156 BULK_OR_INTERRUPT_TRANSFER 00 00 1e df 00 00 02 00 0x00000000 URB Header (length: 72) SequenceNumber: 7 Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) TransferFlags: 0x00000003 TransferBuffer: 0x00000008 (8) length 0000: 00 00 1e df 00 00 02 00 8 in down 0x82 3.166 CLASS_INTERFACE 00 00 00 00 00 00 00 00 URB Header (length: 80) SequenceNumber: 8 Function: 001b (CLASS_INTERFACE) PipeHandle: c156772c SetupPacket: 0000: 21 00 00 00 00 00 0c 00 bmRequestType: 21 DIR: Host-To-Device TYPE: Class RECIPIENT: Interface bRequest: 00 TransferBuffer: 0x0000000c (12) length 0000: 00 00 00 00 00 00 00 00 00 00 00 00 8 out up n/a 3.171 CONTROL_TRANSFER - 0x00000000 URB Header (length: 80) SequenceNumber: 8 Function: 0008 (CONTROL_TRANSFER) PipeHandle: c1565d8c SetupPacket: 0000: 21 00 00 00 00 00 0c 00 bmRequestType: 21 DIR: Host-To-Device TYPE: Class RECIPIENT: Interface bRequest: 00 No TransferBuffer 9 out down n/a 3.172 CLASS_INTERFACE 25 00 00 00 00 00 00 00 URB Header (length: 80) SequenceNumber: 9 Function: 001b (CLASS_INTERFACE) PipeHandle: c1565d8c SetupPacket: 0000: 21 00 00 00 00 00 0c 00 bmRequestType: 21 DIR: Host-To-Device TYPE: Class RECIPIENT: Interface bRequest: 00 TransferBuffer: 0x0000000c (12) length 0000: 25 00 00 00 00 00 00 00 00 00 00 00 9 out up n/a 3.176 CONTROL_TRANSFER - 0x00000000 URB Header (length: 80) SequenceNumber: 9 Function: 0008 (CONTROL_TRANSFER) PipeHandle: c1565d8c SetupPacket: 0000: 21 00 00 00 00 00 0c 00 bmRequestType: 21 DIR: Host-To-Device TYPE: Class RECIPIENT: Interface bRequest: 00 No TransferBuffer 10 in down 0x82 3.181 BULK_OR_INTERRUPT_TRANSFER - URB Header (length: 72) SequenceNumber: 10 Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) TransferFlags: 0x00000003 No TransferBuffer 10 in up 0x82 3.182 BULK_OR_INTERRUPT_TRANSFER 00 00 1e df 00 00 02 00 0x00000000 URB Header (length: 72) SequenceNumber: 10 Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) TransferFlags: 0x00000003 TransferBuffer: 0x00000008 (8) length 0000: 00 00 1e df 00 00 02 00 11 in down 0x82 3.191 CLASS_INTERFACE 28 00 00 00 00 00 00 00 URB Header (length: 80) SequenceNumber: 11 Function: 001b (CLASS_INTERFACE) PipeHandle: c156772c SetupPacket: 0000: 21 00 00 00 00 00 0c 00 bmRequestType: 21 DIR: Host-To-Device TYPE: Class RECIPIENT: Interface bRequest: 00 TransferBuffer: 0x0000000c (12) length 0000: 28 00 00 00 00 00 00 00 80 00 00 00 11 out up n/a 3.196 CONTROL_TRANSFER - 0x00000000 URB Header (length: 80) SequenceNumber: 11 Function: 0008 (CONTROL_TRANSFER) PipeHandle: c1565d8c SetupPacket: 0000: 21 00 00 00 00 00 0c 00 bmRequestType: 21 DIR: Host-To-Device TYPE: Class RECIPIENT: Interface bRequest: 00 No TransferBuffer 12 in down 0x82 3.201 BULK_OR_INTERRUPT_TRANSFER - URB Header (length: 72) SequenceNumber: 12 Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) TransferFlags: 0x00000003 No TransferBuffer ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-09 8:45 ` Dmitri Katchalov @ 2003-11-10 20:45 ` Patrick Mansfield 0 siblings, 0 replies; 54+ messages in thread From: Patrick Mansfield @ 2003-11-10 20:45 UTC (permalink / raw) To: Dmitri Katchalov Cc: Alan Stern, Idan Sofer, ronald, USB development list, SCSI development list On Sun, Nov 09, 2003 at 07:45:46PM +1100, Dmitri Katchalov wrote: > [My previous msg didn't make it to the list, resending] > > Attached here for your perusal is USB log for this device on win98. Thanks. Can someone verify my decoding? (This matches what Dmitri sent earlier that did not make it to the list, execept it looks like it starts with an INQUIRY VPD page 0 - get a list of supported VPD pages). I can't figure out the apparent nonsense returned for INQUIRY VPD page 0. And the "DIR" output seems odd. In short: INQUIRY VPD page 0 INQUIRY length 0x24 (36) bytes READ CAPACITY TEST UNIT READY READ CAPACITY READ 10 Sure enough no MODE SENSE. And no WRITE to see if it is write protected. Any logs for write protected USB media, or for the device with linux 2.4? A few comments inlined below. > 1 in down n/a 0.062 GET_DESCRIPTOR_FROM_DEVICE > URB Header (length: 80) > SequenceNumber: 1 > Function: 000b (GET_DESCRIPTOR_FROM_DEVICE) > 1 in up n/a 0.066 CONTROL_TRANSFER 12 01 00 01 00 00 00 08 > 0x00000000 > URB Header (length: 80) > SequenceNumber: 1 > Function: 0008 (CONTROL_TRANSFER) > PipeHandle: c1565d8c > > SetupPacket: > 0000: 80 06 00 01 00 00 12 00 > bmRequestType: 80 > DIR: Device-To-Host > TYPE: Standard > RECIPIENT: Device > bRequest: 06 > GET_DESCRIPTOR > Descriptor Type: 0x0001 > DEVICE > > > TransferBuffer: 0x00000012 (18) length > 0000: 12 01 00 01 00 00 00 08 4c 05 2e 00 00 03 01 02 > 0010: 00 01 INQUIRY (0x12) VPD page 0 request. > bLength : 0x12 (18) > bDescriptorType : 0x01 (1) > bcdUSB : 0x0100 (256) > bDeviceClass : 0x00 (0) > bDeviceSubClass : 0x00 (0) > bDeviceProtocol : 0x00 (0) > bMaxPacketSize0 : 0x08 (8) > idVendor : 0x054c (1356) > idProduct : 0x002e (46) > bcdDevice : 0x0300 (768) > iManufacturer : 0x01 (1) > iProduct : 0x02 (2) > iSerialNumber : 0x00 (0) > bNumConfigurations : 0x01 (1) > 2 in down n/a 0.066 GET_DESCRIPTOR_FROM_DEVICE > URB Header (length: 80) > SequenceNumber: 2 > Function: 000b (GET_DESCRIPTOR_FROM_DEVICE) > 2 in up n/a 0.076 CONTROL_TRANSFER 09 02 27 00 01 01 00 40 > 0x00000000 > URB Header (length: 80) > SequenceNumber: 2 > Function: 0008 (CONTROL_TRANSFER) > PipeHandle: c1565d8c > > SetupPacket: > 0000: 80 06 00 02 00 00 09 02 > bmRequestType: 80 > DIR: Device-To-Host > TYPE: Standard > RECIPIENT: Device > bRequest: 06 > GET_DESCRIPTOR > Descriptor Type: 0x0002 > CONFIGURATION > > > TransferBuffer: 0x00000027 (39) length > 0000: 09 02 27 00 01 01 00 40 01 09 04 00 00 03 08 ff > 0010: 01 00 07 05 01 02 40 00 00 07 05 82 02 40 00 00 > 0020: 07 05 83 03 08 00 ff WTF? > bLength : 0x09 (9) > bDescriptorType : 0x02 (2) > wTotalLength : 0x0027 (39) > bNumInterfaces : 0x01 (1) > bConfigurationValue: 0x01 (1) > iConfiguration : 0x00 (0) > bmAttributes : 0x40 (64) > MaxPower : 0x01 (1) > 3 ??? down n/a 0.076 SELECT_CONFIGURATION > URB Header (length: 100) > SequenceNumber: 3 > Function: 0000 (SELECT_CONFIGURATION) > Configuration Descriptor: > bLength: 9 (0x09) > bDescriptorType: 2 (0x02) > wTotalLength: 39 (0x0027) > bNumInterfaces: 1 (0x01) > bConfigurationValue: 1 (0x01) > iConfiguration: 0 (0x00) > bmAttributes: 64 (0x40) > 0x40: Self Powered > MaxPower: 1 (0x01) > (in 2 mA units, therefore 2 mA power consumption) > > Number of interfaces: 1 > Interface[0]: > Length: 0x004c > InterfaceNumber: 0x00 > AlternateSetting: 0x00 > Class = 0x34 > SubClass = 0x6d > Protocol = 0x37 > InterfaceHandle = 0x00000000 > NumberOfPipes = 0x00000003 > Pipe[0]: > MaximumPacketSize = 0xaff0 > EndpointAddress = 0x4b > Interval = 0xc1 > PipeType = 0x00 > UsbdPipeTypeControl > PipeHandle = 0x0000ce54 > MaxTransferSize = 0x00010000 > PipeFlags = 0x00 > Pipe[1]: > MaximumPacketSize = 0x2f68 > EndpointAddress = 0x5b > Interval = 0xbc > PipeType = 0x206a2b1b > !!! INVALID !!! > PipeHandle = 0x4e4f5300 > MaxTransferSize = 0x00010000 > PipeFlags = 0x00 > Pipe[2]: > MaximumPacketSize = 0x000d > EndpointAddress = 0x00 > Interval = 0x00 > PipeType = 0xc15667d0 > !!! INVALID !!! > PipeHandle = 0xc1566810 > MaxTransferSize = 0x00010000 > PipeFlags = 0x00 > 3 ??? up n/a 0.076 SELECT_CONFIGURATION 0x00000000 > URB Header (length: 100) > SequenceNumber: 3 > Function: 0000 (SELECT_CONFIGURATION) > Configuration Descriptor: > bLength: 9 (0x09) > bDescriptorType: 2 (0x02) > wTotalLength: 39 (0x0027) > bNumInterfaces: 1 (0x01) > bConfigurationValue: 1 (0x01) > iConfiguration: 0 (0x00) > bmAttributes: 64 (0x40) > 0x40: Self Powered > MaxPower: 1 (0x01) > (in 2 mA units, therefore 2 mA power consumption) > > Number of interfaces: 1 > Interface[0]: > Length: 0x004c > InterfaceNumber: 0x00 > AlternateSetting: 0x00 > Class = 0x08 > SubClass = 0xff > Protocol = 0x01 > InterfaceHandle = 0xc1567700 > NumberOfPipes = 0x00000003 > Pipe[0]: > MaximumPacketSize = 0x0040 > EndpointAddress = 0x01 > Interval = 0x00 > PipeType = 0x02 > UsbdPipeTypeBulk > PipeHandle = 0xc1567718 > MaxTransferSize = 0x00010000 > PipeFlags = 0x00 > Pipe[1]: > MaximumPacketSize = 0x0040 > EndpointAddress = 0x82 > Interval = 0x00 > PipeType = 0x02 > UsbdPipeTypeBulk > PipeHandle = 0xc156772c > MaxTransferSize = 0x00010000 > PipeFlags = 0x00 > Pipe[2]: > MaximumPacketSize = 0x0008 > EndpointAddress = 0x83 > Interval = 0xff > PipeType = 0x03 > UsbdPipeTypeInterrupt > PipeHandle = 0xc1567740 > MaxTransferSize = 0x00010000 > PipeFlags = 0x00 > 4 out down n/a 3.142 CLASS_INTERFACE 12 00 00 00 24 00 00 00 > URB Header (length: 80) > SequenceNumber: 4 > Function: 001b (CLASS_INTERFACE) > PipeHandle: c146f640 > > SetupPacket: > 0000: 21 00 00 00 00 00 00 00 > bmRequestType: 21 > DIR: Host-To-Device > TYPE: Class > RECIPIENT: Interface > bRequest: 00 > > > TransferBuffer: 0x0000000c (12) length > 0000: 12 00 00 00 24 00 00 00 00 00 00 00 Standard INQUIRY, 0x24 (36) bytes. > 4 out up n/a 3.146 CONTROL_TRANSFER - 0x00000000 > URB Header (length: 80) > SequenceNumber: 4 > Function: 0008 (CONTROL_TRANSFER) > PipeHandle: c1565d8c > > SetupPacket: > 0000: 21 00 00 00 00 00 0c 00 > bmRequestType: 21 > DIR: Host-To-Device > TYPE: Class > RECIPIENT: Interface > bRequest: 00 > > > No TransferBuffer > > 5 in down 0x82 3.146 BULK_OR_INTERRUPT_TRANSFER - > URB Header (length: 72) > SequenceNumber: 5 > Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) > TransferFlags: 0x00000003 > > No TransferBuffer > > 5 in up 0x82 3.146 BULK_OR_INTERRUPT_TRANSFER 00 80 00 01 1f > 00 00 00 0x00000000 > URB Header (length: 72) > SequenceNumber: 5 > Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) > TransferFlags: 0x00000003 > > TransferBuffer: 0x00000024 (36) length > 0000: 00 80 00 01 1f 00 00 00 53 6f 6e 79 20 20 20 20 > 0010: 53 6f 6e 79 20 44 53 43 20 20 20 20 20 20 20 20 > 0020: 33 2e 30 30 INQUIRY reponse > 6 in down 0x82 3.146 CLASS_INTERFACE 25 00 00 00 00 00 00 00 > URB Header (length: 80) > SequenceNumber: 6 > Function: 001b (CLASS_INTERFACE) > PipeHandle: c156772c > > SetupPacket: > 0000: 21 00 00 00 00 00 0c 00 > bmRequestType: 21 > DIR: Host-To-Device > TYPE: Class > RECIPIENT: Interface > bRequest: 00 > > > TransferBuffer: 0x0000000c (12) length > 0000: 25 00 00 00 00 00 00 00 00 00 00 00 READ CAPACITY > 6 out up n/a 3.152 CONTROL_TRANSFER - 0x00000000 > URB Header (length: 80) > SequenceNumber: 6 > Function: 0008 (CONTROL_TRANSFER) > PipeHandle: c1565d8c > > SetupPacket: > 0000: 21 00 00 00 00 00 0c 00 > bmRequestType: 21 > DIR: Host-To-Device > TYPE: Class > RECIPIENT: Interface > bRequest: 00 > > > No TransferBuffer > > 7 in down 0x82 3.152 BULK_OR_INTERRUPT_TRANSFER - > URB Header (length: 72) > SequenceNumber: 7 > Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) > TransferFlags: 0x00000003 > > No TransferBuffer > > 7 in up 0x82 3.156 BULK_OR_INTERRUPT_TRANSFER 00 00 1e df 00 > 00 02 00 0x00000000 > URB Header (length: 72) > SequenceNumber: 7 > Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) > TransferFlags: 0x00000003 > > TransferBuffer: 0x00000008 (8) length > 0000: 00 00 1e df 00 00 02 00 0x00001edf, 7903 blocks decimal 0x00000200, 512 byte blocks > 8 in down 0x82 3.166 CLASS_INTERFACE 00 00 00 00 00 00 00 00 > URB Header (length: 80) > SequenceNumber: 8 > Function: 001b (CLASS_INTERFACE) > PipeHandle: c156772c > > SetupPacket: > 0000: 21 00 00 00 00 00 0c 00 > bmRequestType: 21 > DIR: Host-To-Device > TYPE: Class > RECIPIENT: Interface > bRequest: 00 > > > TransferBuffer: 0x0000000c (12) length > 0000: 00 00 00 00 00 00 00 00 00 00 00 00 TEST UNIT READY > 8 out up n/a 3.171 CONTROL_TRANSFER - 0x00000000 > URB Header (length: 80) > SequenceNumber: 8 > Function: 0008 (CONTROL_TRANSFER) > PipeHandle: c1565d8c > > SetupPacket: > 0000: 21 00 00 00 00 00 0c 00 > bmRequestType: 21 > DIR: Host-To-Device > TYPE: Class > RECIPIENT: Interface > bRequest: 00 > > > No TransferBuffer > > 9 out down n/a 3.172 CLASS_INTERFACE 25 00 00 00 00 00 00 00 > URB Header (length: 80) > SequenceNumber: 9 > Function: 001b (CLASS_INTERFACE) > PipeHandle: c1565d8c > > SetupPacket: > 0000: 21 00 00 00 00 00 0c 00 > bmRequestType: 21 > DIR: Host-To-Device > TYPE: Class > RECIPIENT: Interface > bRequest: 00 Nothing back for the TEST UNIT READY? > TransferBuffer: 0x0000000c (12) length > 0000: 25 00 00 00 00 00 00 00 00 00 00 00 READ CAPACITY again > 9 out up n/a 3.176 CONTROL_TRANSFER - 0x00000000 > URB Header (length: 80) > SequenceNumber: 9 > Function: 0008 (CONTROL_TRANSFER) > PipeHandle: c1565d8c > > SetupPacket: > 0000: 21 00 00 00 00 00 0c 00 > bmRequestType: 21 > DIR: Host-To-Device > TYPE: Class > RECIPIENT: Interface > bRequest: 00 > > > No TransferBuffer > > 10 in down 0x82 3.181 BULK_OR_INTERRUPT_TRANSFER - > URB Header (length: 72) > SequenceNumber: 10 > Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) > TransferFlags: 0x00000003 > > No TransferBuffer > > 10 in up 0x82 3.182 BULK_OR_INTERRUPT_TRANSFER 00 00 1e df 00 > 00 02 00 0x00000000 > URB Header (length: 72) > SequenceNumber: 10 > Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) > TransferFlags: 0x00000003 > > TransferBuffer: 0x00000008 (8) length > 0000: 00 00 1e df 00 00 02 00 READ CAPACITY result again > 11 in down 0x82 3.191 CLASS_INTERFACE 28 00 00 00 00 00 00 00 > URB Header (length: 80) > SequenceNumber: 11 > Function: 001b (CLASS_INTERFACE) > PipeHandle: c156772c > > SetupPacket: > 0000: 21 00 00 00 00 00 0c 00 > bmRequestType: 21 > DIR: Host-To-Device > TYPE: Class > RECIPIENT: Interface > bRequest: 00 > > > TransferBuffer: 0x0000000c (12) length > 0000: 28 00 00 00 00 00 00 00 80 00 00 00 READ 10 > 11 out up n/a 3.196 CONTROL_TRANSFER - 0x00000000 > URB Header (length: 80) > SequenceNumber: 11 > Function: 0008 (CONTROL_TRANSFER) > PipeHandle: c1565d8c > > SetupPacket: > 0000: 21 00 00 00 00 00 0c 00 > bmRequestType: 21 > DIR: Host-To-Device > TYPE: Class > RECIPIENT: Interface > bRequest: 00 > > > No TransferBuffer > > 12 in down 0x82 3.201 BULK_OR_INTERRUPT_TRANSFER - > URB Header (length: 72) > SequenceNumber: 12 > Function: 0009 (BULK_OR_INTERRUPT_TRANSFER) > TransferFlags: 0x00000003 > > No TransferBuffer -- Patrick Mansfield ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-09 3:47 ` [linux-usb-devel] " Alan Stern 2003-11-09 8:45 ` Dmitri Katchalov @ 2003-11-10 17:59 ` Patrick Mansfield 2003-11-10 18:46 ` Alan Stern 2003-11-18 15:20 ` Unaligned scatter-gather buffers and usb-storage Alan Stern 1 sibling, 2 replies; 54+ messages in thread From: Patrick Mansfield @ 2003-11-10 17:59 UTC (permalink / raw) To: Alan Stern Cc: Dmitri Katchalov, Idan Sofer, ronald, USB development list, SCSI development list On Sat, Nov 08, 2003 at 10:47:26PM -0500, Alan Stern wrote: > On Sat, 8 Nov 2003, Patrick Mansfield wrote: > > > On Sat, Nov 08, 2003 at 11:28:37AM -0500, Alan Stern wrote: > > A DID_ERROR causes scsi core to retry the command - it will not even check > > the sense data. A DID_ABORT might be better, scsi core will not retry. > > (We still have issue 1, where the host byte is not checked in > > scsi_status_is_good.) > > For CB and CBI devices, babble causes a DID_ERROR return. For Bulk-only > it causes a Check-Condition status to be returned along with Invalid > Command sense data. Like I said earlier, we haven't yet figured out the > best way to handle these errors. The DID_ERROR should be changed to DID_ABORT for the babble case so scsi core won't retry the command generating the babble. I don't know why scsi core retries on DID_ERROR, as code comments say DID_ERROR is an internal error. DID_SOFT_ERROR is the same as DID_ERROR (as far as scsi core cares), execept for a check for reservation conflict. > > Dmitri's change (return USB_STOR_XFER_STALL instead of USB_STOR_XFER_LONG) > > causes us to return USB_STOR_TRANSPORT_GOOD vs USB_STOR_TRANSPORT_ERROR in > > usb_stor_CB_transport. > > > > Then in usb_stor_invoke_transport we issue a REQUEST SENSE. > > > > If for a USB_STOR_TRANSPORT_ERROR we also did a REQUEST SENSE, the > > following command (TEST UNIT READY) would not get the ILLEGAL REQUEST. > > That doesn't make sense. USB_STOR_TRANSPORT_ERROR means we aren't able to > communicate with the device. So how can we send it a REQUEST-SENSE? No that should not be done, I was trying to figure out and explain why he did not get the ILLEGAL REQUEST for TEST UNIT READY. Has anyone looked at why the reset failed? I assume that would clear the sense. > > I don't know why the TEST UNIT READY caused an auto REQUEST SENSE to > > be sent. > > The CB transport doesn't include any means for the device to return > status. (Yes, it's stupid.) The only way we can find out is to issue > auto REQUEST-SENSE for virtually every command. > > > Changing the sense stuff does not address the MODE SENSE failing, we still > > have 3 separate issues that are all hit by this device. > > If we simply avoided sending the MODE-SENSE in the first place, none of > the other issues would come up. IMHO that's the best solution. Yes. We still have the missed check in scsi_status_is_good. And, the sense coming back for a previous command returning babble is still an issue. -- Patrick Mansfield ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-10 17:59 ` Patrick Mansfield @ 2003-11-10 18:46 ` Alan Stern 2003-11-10 19:04 ` [linux-usb-devel] " Patrick Mansfield 2003-11-10 22:46 ` Sancho Dauskardt 2003-11-18 15:20 ` Unaligned scatter-gather buffers and usb-storage Alan Stern 1 sibling, 2 replies; 54+ messages in thread From: Alan Stern @ 2003-11-10 18:46 UTC (permalink / raw) To: Patrick Mansfield Cc: Dmitri Katchalov, Idan Sofer, ronald, USB development list, SCSI development list On Mon, 10 Nov 2003, Patrick Mansfield wrote: > On Sat, Nov 08, 2003 at 10:47:26PM -0500, Alan Stern wrote: > > > > For CB and CBI devices, babble causes a DID_ERROR return. For Bulk-only > > it causes a Check-Condition status to be returned along with Invalid > > Command sense data. Like I said earlier, we haven't yet figured out the > > best way to handle these errors. > > The DID_ERROR should be changed to DID_ABORT for the babble case so scsi > core won't retry the command generating the babble. > > I don't know why scsi core retries on DID_ERROR, as code comments say > DID_ERROR is an internal error. DID_SOFT_ERROR is the same as DID_ERROR > (as far as scsi core cares), execept for a check for reservation conflict. I'll pass that on to Matt Dharm. It's not always clear to me which DID_XXX code is best to use in a given situation for an emulated SCSI bus like USB. > Has anyone looked at why the reset failed? I assume that would clear the > sense. How could anyone attempt to find out why the reset failed? The only externally visible state is the device's response to incoming USB requests. IIRC, it simply didn't respond to the reset request. (I'm not sure about that; I may be confusing this with other devices that failed in similar ways.) > We still have the missed check in scsi_status_is_good. > > And, the sense coming back for a previous command returning babble is still > an issue. Agreed. I tend to ascribe things like that to buggy SCSI implementations in the device firmware. These things are produced quickly and often without thorough testing. On the other hand, even if it's true it doesn't suggest any good avenues of approach. Alan Stern ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] usb-storage and Sony Handycam 2003-11-10 18:46 ` Alan Stern @ 2003-11-10 19:04 ` Patrick Mansfield 2003-11-10 19:57 ` Alan Stern 2003-11-10 22:46 ` Sancho Dauskardt 1 sibling, 1 reply; 54+ messages in thread From: Patrick Mansfield @ 2003-11-10 19:04 UTC (permalink / raw) To: Alan Stern Cc: Dmitri Katchalov, Idan Sofer, ronald, USB development list, SCSI development list On Mon, Nov 10, 2003 at 01:46:44PM -0500, Alan Stern wrote: > On Mon, 10 Nov 2003, Patrick Mansfield wrote: > How could anyone attempt to find out why the reset failed? The only > externally visible state is the device's response to incoming USB > requests. IIRC, it simply didn't respond to the reset request. (I'm not > sure about that; I may be confusing this with other devices that failed in > similar ways.) I don't know, the bug report (and other logs) show: Nov 7 00:12:19 [kernel] usb-storage: Command MODE_SENSE_10 (10 bytes) Nov 7 00:12:19 [kernel] usb-storage: usb_stor_ctrl_transfer: rq=00 rqtype=21 value=0000 index=00 len=10 Nov 7 00:12:19 [kernel] usb-storage: Status code 0; transferred 10/10 Nov 7 00:12:19 [kernel] usb-storage: -- transfer complete Nov 7 00:12:19 [kernel] usb-storage: Call to usb_stor_ctrl_transfer() returned 0 Nov 7 00:12:19 [kernel] usb-storage: usb_stor_bulk_transfer_buf: xfer 8 bytes Nov 7 00:12:19 [kernel] usb-storage: Status code -75; transferred 8/8 Nov 7 00:12:19 [kernel] usb-storage: -- babble Nov 7 00:12:19 [kernel] usb-storage: CB data stage result is 0x3 Nov 7 00:12:19 [kernel] usb-storage: -- transport indicates error, resetting Nov 7 00:12:19 [kernel] usb-storage: usb_stor_CB_reset called Nov 7 00:12:19 [kernel] usb-storage: usb_stor_control_msg: rq=00 rqtype=21 value=0000 index=00 len=12 Nov 7 00:12:19 [kernel] usb-storage: Soft reset failed: -32 Nov 7 00:12:19 [kernel] usb-storage: scsi cmd done, result=0x70000 -- Patrick Mansfield ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-10 19:04 ` [linux-usb-devel] " Patrick Mansfield @ 2003-11-10 19:57 ` Alan Stern 0 siblings, 0 replies; 54+ messages in thread From: Alan Stern @ 2003-11-10 19:57 UTC (permalink / raw) To: Patrick Mansfield Cc: Dmitri Katchalov, Idan Sofer, ronald, USB development list, SCSI development list On Mon, 10 Nov 2003, Patrick Mansfield wrote: > On Mon, Nov 10, 2003 at 01:46:44PM -0500, Alan Stern wrote: > > On Mon, 10 Nov 2003, Patrick Mansfield wrote: > > > How could anyone attempt to find out why the reset failed? The only > > externally visible state is the device's response to incoming USB > > requests. IIRC, it simply didn't respond to the reset request. (I'm not > > sure about that; I may be confusing this with other devices that failed in > > similar ways.) > > I don't know, the bug report (and other logs) show: > > Nov 7 00:12:19 [kernel] usb-storage: Command MODE_SENSE_10 (10 bytes) > Nov 7 00:12:19 [kernel] usb-storage: usb_stor_ctrl_transfer: rq=00 rqtype=21 value=0000 index=00 len=10 > Nov 7 00:12:19 [kernel] usb-storage: Status code 0; transferred 10/10 > Nov 7 00:12:19 [kernel] usb-storage: -- transfer complete > Nov 7 00:12:19 [kernel] usb-storage: Call to usb_stor_ctrl_transfer() returned 0 > Nov 7 00:12:19 [kernel] usb-storage: usb_stor_bulk_transfer_buf: xfer 8 bytes > Nov 7 00:12:19 [kernel] usb-storage: Status code -75; transferred 8/8 > Nov 7 00:12:19 [kernel] usb-storage: -- babble > Nov 7 00:12:19 [kernel] usb-storage: CB data stage result is 0x3 > Nov 7 00:12:19 [kernel] usb-storage: -- transport indicates error, resetting > Nov 7 00:12:19 [kernel] usb-storage: usb_stor_CB_reset called > Nov 7 00:12:19 [kernel] usb-storage: usb_stor_control_msg: rq=00 rqtype=21 value=0000 index=00 len=12 > Nov 7 00:12:19 [kernel] usb-storage: Soft reset failed: -32 > Nov 7 00:12:19 [kernel] usb-storage: scsi cmd done, result=0x70000 The -32 (-EPIPE) error code indicates a "protocol stall" -- the device is saying that for some reason it can't accept or can't carry out the reset command. Maybe because it's busy still trying to finish up its broken response to the MODE-SENSE command. You would think that a reset would take precedence over a regular command-in-progress, but apparently not here. Alan Stern ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-10 18:46 ` Alan Stern 2003-11-10 19:04 ` [linux-usb-devel] " Patrick Mansfield @ 2003-11-10 22:46 ` Sancho Dauskardt 1 sibling, 0 replies; 54+ messages in thread From: Sancho Dauskardt @ 2003-11-10 22:46 UTC (permalink / raw) To: Alan Stern, Patrick Mansfield Cc: Dmitri Katchalov, Idan Sofer, ronald, USB development list, SCSI development list > > Has anyone looked at why the reset failed? I assume that would clear the > > sense. > >How could anyone attempt to find out why the reset failed? Just some ideas: a) not supported - AFAIK many storage devices don't seem to support the bulk-reset, as the 'Other Popular OS' doesn't seem to issue them. b) possibly VIA UHCI chipset which got killed by babble ? - sda ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Unaligned scatter-gather buffers and usb-storage 2003-11-10 17:59 ` Patrick Mansfield 2003-11-10 18:46 ` Alan Stern @ 2003-11-18 15:20 ` Alan Stern 2003-11-18 22:37 ` Patrick Mansfield 1 sibling, 1 reply; 54+ messages in thread From: Alan Stern @ 2003-11-18 15:20 UTC (permalink / raw) To: Patrick Mansfield; +Cc: SCSI development list, USB development list Although most scatter-gather buffers are page aligned, occasionally one of them isn't. This can happen when the sg and st drivers use direct I/O and the user's buffer doesn't start on a page boundary; the first s-g buffer in the list will coincide with the start of the user buffer even though all the later ones will start at page boundaries. The usb-storage driver has trouble with unaligned buffers. More accurately, it has trouble with s-g entries having bad lengths. A buffer that occupies an entire page or is the _last_ entry in the s-g list is okay, but if the _first_ entry isn't aligned properly it is likely to have a bad length. [For linux-scsi readers: This has to do with the way USB works. Data transfers are divided up into packets, and packets have a maximum size (typically 64 or 512 bytes). A packet can't span two s-g entries; hence if an s-g entry's length isn't divisible by the maxpacket size then one of the packets for that entry must be short, i.e., smaller than the maximum size. But USB specifies that short packets always indicate the end of a transfer. Thus if any s-g entry but the last has a bad length, the data transfer will terminate prematurely.] Alignment to a 512-byte boundary would suffice and page alignment would be more than enough. The cleanest way to fix the problem would be to have st and sg refuse to do direct I/O when the user's buffer isn't page aligned. That's probably too draconian (but it would allow us to give the user process an appropriate error code right away). So instead I'm going to change the usb-storage driver to make it reject any s-g transfer that doesn't meet the entry length requirements. My question is this: What's the best way to inform the SCSI midlayer about what went wrong? Return a DID_ABORT code? Or would something else be better? Alan Stern ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Unaligned scatter-gather buffers and usb-storage 2003-11-18 15:20 ` Unaligned scatter-gather buffers and usb-storage Alan Stern @ 2003-11-18 22:37 ` Patrick Mansfield 2003-11-19 8:47 ` Jens Axboe 0 siblings, 1 reply; 54+ messages in thread From: Patrick Mansfield @ 2003-11-18 22:37 UTC (permalink / raw) To: Alan Stern, Jens Axboe, Douglas Gilbert Cc: SCSI development list, USB development list On Tue, Nov 18, 2003 at 10:20:17AM -0500, Alan Stern wrote: > Alignment to a 512-byte boundary would suffice and page alignment would be > more than enough. The cleanest way to fix the problem would be to have st > and sg refuse to do direct I/O when the user's buffer isn't page aligned. > That's probably too draconian (but it would allow us to give the user > process an appropriate error code right away). The above restriction sounds reasonable to me, but I do not have experience in this area. I thought that the raw and direct block interfaces already had such restrictions. Jens or Doug G can you comment on this? You could add a scsi_host fields or bits specifying alignment restrictions, and enforce them as needed in upper level drivers. > So instead I'm going to change the usb-storage driver to make it reject > any s-g transfer that doesn't meet the entry length requirements. My > question is this: What's the best way to inform the SCSI midlayer about > what went wrong? Return a DID_ABORT code? Or would something else be > better? > > Alan Stern Assuming a printk is OK, DID_ABORT is OK. If there is an existing appropriate sense code, you could generate that and pass it back. I searched the SPC3 spec and did not find any that have "align" but there are some with "invalid" that might be OK, though you might not be able to tell for certain if the sense was from the device or not, it is less vague than a DID_ABORT. -- Patrick Mansfield ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Unaligned scatter-gather buffers and usb-storage 2003-11-18 22:37 ` Patrick Mansfield @ 2003-11-19 8:47 ` Jens Axboe 2003-11-19 13:01 ` [linux-usb-devel] " Oliver Neukum 2003-11-19 15:44 ` Alan Stern 0 siblings, 2 replies; 54+ messages in thread From: Jens Axboe @ 2003-11-19 8:47 UTC (permalink / raw) To: Patrick Mansfield Cc: Alan Stern, Douglas Gilbert, SCSI development list, USB development list On Tue, Nov 18 2003, Patrick Mansfield wrote: > On Tue, Nov 18, 2003 at 10:20:17AM -0500, Alan Stern wrote: > > > Alignment to a 512-byte boundary would suffice and page alignment would be > > more than enough. The cleanest way to fix the problem would be to have st > > and sg refuse to do direct I/O when the user's buffer isn't page aligned. > > That's probably too draconian (but it would allow us to give the user > > process an appropriate error code right away). > > The above restriction sounds reasonable to me, but I do not have > experience in this area. I thought that the raw and direct block > interfaces already had such restrictions. > > Jens or Doug G can you comment on this? > > You could add a scsi_host fields or bits specifying alignment > restrictions, and enforce them as needed in upper level drivers. The queue already has such a restriction embedded, see bio_map_user() and queue_dma_alignment(). -- Jens Axboe ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 8:47 ` Jens Axboe @ 2003-11-19 13:01 ` Oliver Neukum 2003-11-19 13:04 ` Jens Axboe 2003-11-19 15:44 ` Alan Stern 1 sibling, 1 reply; 54+ messages in thread From: Oliver Neukum @ 2003-11-19 13:01 UTC (permalink / raw) To: Jens Axboe, Patrick Mansfield Cc: Alan Stern, Douglas Gilbert, SCSI development list, USB development list > > You could add a scsi_host fields or bits specifying alignment > > restrictions, and enforce them as needed in upper level drivers. > > The queue already has such a restriction embedded, see bio_map_user() > and queue_dma_alignment(). Very well, as far as sd and sr are concerned, but it doesn't help for sg and st, does it? Regards Oliver ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 13:01 ` [linux-usb-devel] " Oliver Neukum @ 2003-11-19 13:04 ` Jens Axboe 2003-11-19 14:37 ` James Bottomley 0 siblings, 1 reply; 54+ messages in thread From: Jens Axboe @ 2003-11-19 13:04 UTC (permalink / raw) To: Oliver Neukum Cc: Patrick Mansfield, Alan Stern, Douglas Gilbert, SCSI development list, USB development list On Wed, Nov 19 2003, Oliver Neukum wrote: > > > > You could add a scsi_host fields or bits specifying alignment > > > restrictions, and enforce them as needed in upper level drivers. > > > > The queue already has such a restriction embedded, see bio_map_user() > > and queue_dma_alignment(). > > Very well, as far as sd and sr are concerned, but it doesn't help for > sg and st, does it? They both have a queue through the scsi device structure. But you could put the value someplace else if you wanted. -- Jens Axboe ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 13:04 ` Jens Axboe @ 2003-11-19 14:37 ` James Bottomley 2003-11-19 14:39 ` Jens Axboe 0 siblings, 1 reply; 54+ messages in thread From: James Bottomley @ 2003-11-19 14:37 UTC (permalink / raw) To: Jens Axboe Cc: Oliver Neukum, Patrick Mansfield, Alan Stern, Douglas Gilbert, SCSI development list, USB development list On Wed, 2003-11-19 at 07:04, Jens Axboe wrote: > On Wed, Nov 19 2003, Oliver Neukum wrote: > > > > > > You could add a scsi_host fields or bits specifying alignment > > > > restrictions, and enforce them as needed in upper level drivers. > > > > > > The queue already has such a restriction embedded, see bio_map_user() > > > and queue_dma_alignment(). > > > > Very well, as far as sd and sr are concerned, but it doesn't help for > > sg and st, does it? > > They both have a queue through the scsi device structure. But you could > put the value someplace else if you wanted. Actually, the sg case should be fine, because it shares the queue with the other attachment (unless it's a processor or other unrecognised device). but, regardless, the queue is already set up (for all devices, including st) in either slave_alloc, or slave_configure, so you could call the alignment setting routine there. Alternatively, we could add this as yet another parameter to the host and template structures. Any preferences? James ------------------------------------------------------- This SF.net email is sponsored by: SF.net Giveback Program. Does SourceForge.net help you be more productive? Does it help you create better code? SHARE THE LOVE, and help us help YOU! Click Here: http://sourceforge.net/donate/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 14:37 ` James Bottomley @ 2003-11-19 14:39 ` Jens Axboe 2003-11-19 14:58 ` James Bottomley 0 siblings, 1 reply; 54+ messages in thread From: Jens Axboe @ 2003-11-19 14:39 UTC (permalink / raw) To: James Bottomley Cc: Oliver Neukum, Patrick Mansfield, Alan Stern, Douglas Gilbert, SCSI development list, USB development list On Wed, Nov 19 2003, James Bottomley wrote: > On Wed, 2003-11-19 at 07:04, Jens Axboe wrote: > > On Wed, Nov 19 2003, Oliver Neukum wrote: > > > > > > > > You could add a scsi_host fields or bits specifying alignment > > > > > restrictions, and enforce them as needed in upper level drivers. > > > > > > > > The queue already has such a restriction embedded, see bio_map_user() > > > > and queue_dma_alignment(). > > > > > > Very well, as far as sd and sr are concerned, but it doesn't help for > > > sg and st, does it? > > > > They both have a queue through the scsi device structure. But you could > > put the value someplace else if you wanted. > > Actually, the sg case should be fine, because it shares the queue with > the other attachment (unless it's a processor or other unrecognised > device). > > but, regardless, the queue is already set up (for all devices, including > st) in either slave_alloc, or slave_configure, so you could call the > alignment setting routine there. That was my point, yes. > Alternatively, we could add this as yet another parameter to the host > and template structures. Any preferences? I suppose you have to, if you want LLD to be able to pass it down. -- Jens Axboe ------------------------------------------------------- This SF.net email is sponsored by: SF.net Giveback Program. Does SourceForge.net help you be more productive? Does it help you create better code? SHARE THE LOVE, and help us help YOU! Click Here: http://sourceforge.net/donate/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 14:39 ` Jens Axboe @ 2003-11-19 14:58 ` James Bottomley 2003-11-19 15:00 ` [linux-usb-devel] " Jens Axboe 0 siblings, 1 reply; 54+ messages in thread From: James Bottomley @ 2003-11-19 14:58 UTC (permalink / raw) To: Jens Axboe Cc: Oliver Neukum, Patrick Mansfield, Alan Stern, Douglas Gilbert, SCSI development list, USB development list On Wed, 2003-11-19 at 08:39, Jens Axboe wrote: > > but, regardless, the queue is already set up (for all devices, including > > st) in either slave_alloc, or slave_configure, so you could call the > > alignment setting routine there. > > That was my point, yes. I know, just driving it home. I'll hand back the reins now... > > > Alternatively, we could add this as yet another parameter to the host > > and template structures. Any preferences? > > I suppose you have to, if you want LLD to be able to pass it down. Well, what I was fishing for: Is this property useful enough to want every device driver writer to think about it (i.e. document and place in the template), or is it rare enough that it should only be accessible to those who know what they're doing (via the direct block queue interfaces in slave_alloc/configure)? James ------------------------------------------------------- This SF.net email is sponsored by: SF.net Giveback Program. Does SourceForge.net help you be more productive? Does it help you create better code? SHARE THE LOVE, and help us help YOU! Click Here: http://sourceforge.net/donate/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 14:58 ` James Bottomley @ 2003-11-19 15:00 ` Jens Axboe 2003-11-19 16:56 ` Kai Makisara 0 siblings, 1 reply; 54+ messages in thread From: Jens Axboe @ 2003-11-19 15:00 UTC (permalink / raw) To: James Bottomley Cc: Oliver Neukum, Patrick Mansfield, Alan Stern, Douglas Gilbert, SCSI development list, USB development list On Wed, Nov 19 2003, James Bottomley wrote: > > > Alternatively, we could add this as yet another parameter to the host > > > and template structures. Any preferences? > > > > I suppose you have to, if you want LLD to be able to pass it down. > > Well, what I was fishing for: Is this property useful enough to want > every device driver writer to think about it (i.e. document and place in > the template), or is it rare enough that it should only be accessible > to those who know what they're doing (via the direct block queue > interfaces in slave_alloc/configure)? As far as I'm concerned, requiring 512-byte alignment is 'just easier'... Only a select few host adapters will set it differently anyways, so you gain basically nothing. -- Jens Axboe ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 15:00 ` [linux-usb-devel] " Jens Axboe @ 2003-11-19 16:56 ` Kai Makisara 2003-11-19 20:19 ` Jens Axboe 0 siblings, 1 reply; 54+ messages in thread From: Kai Makisara @ 2003-11-19 16:56 UTC (permalink / raw) To: Jens Axboe Cc: James Bottomley, Oliver Neukum, Patrick Mansfield, Alan Stern, Douglas Gilbert, SCSI development list, USB development list On Wed, 19 Nov 2003, Jens Axboe wrote: > On Wed, Nov 19 2003, James Bottomley wrote: > > > > Alternatively, we could add this as yet another parameter to the host > > > > and template structures. Any preferences? > > > > > > I suppose you have to, if you want LLD to be able to pass it down. > > > > Well, what I was fishing for: Is this property useful enough to want > > every device driver writer to think about it (i.e. document and place in > > the template), or is it rare enough that it should only be accessible > > to those who know what they're doing (via the direct block queue > > interfaces in slave_alloc/configure)? > > As far as I'm concerned, requiring 512-byte alignment is 'just > easier'... Only a select few host adapters will set it differently > anyways, so you gain basically nothing. > As usual, I am against this 512-byte alignment (thinking as st maintainer). Don't default to more strict aligment requirements than is necessary. It creates hidden performance problems for character device drivers. -- Kai ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 16:56 ` Kai Makisara @ 2003-11-19 20:19 ` Jens Axboe 2003-11-19 22:06 ` Kai Makisara 0 siblings, 1 reply; 54+ messages in thread From: Jens Axboe @ 2003-11-19 20:19 UTC (permalink / raw) To: Kai Makisara Cc: James Bottomley, Oliver Neukum, Patrick Mansfield, Alan Stern, Douglas Gilbert, SCSI development list, USB development list On Wed, Nov 19 2003, Kai Makisara wrote: > On Wed, 19 Nov 2003, Jens Axboe wrote: > > > On Wed, Nov 19 2003, James Bottomley wrote: > > > > > Alternatively, we could add this as yet another parameter to the host > > > > > and template structures. Any preferences? > > > > > > > > I suppose you have to, if you want LLD to be able to pass it down. > > > > > > Well, what I was fishing for: Is this property useful enough to want > > > every device driver writer to think about it (i.e. document and place in > > > the template), or is it rare enough that it should only be accessible > > > to those who know what they're doing (via the direct block queue > > > interfaces in slave_alloc/configure)? > > > > As far as I'm concerned, requiring 512-byte alignment is 'just > > easier'... Only a select few host adapters will set it differently > > anyways, so you gain basically nothing. > > > As usual, I am against this 512-byte alignment (thinking as st > maintainer). Don't default to more strict aligment requirements than is > necessary. It creates hidden performance problems for character device > drivers. But you have to honor hardware restrictions, though. Where is the performance impact? -- Jens Axboe ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 20:19 ` Jens Axboe @ 2003-11-19 22:06 ` Kai Makisara 2003-11-20 6:53 ` Jens Axboe 0 siblings, 1 reply; 54+ messages in thread From: Kai Makisara @ 2003-11-19 22:06 UTC (permalink / raw) To: Jens Axboe Cc: James Bottomley, Oliver Neukum, Patrick Mansfield, Alan Stern, Douglas Gilbert, SCSI development list, USB development list On Wed, 19 Nov 2003, Jens Axboe wrote: > On Wed, Nov 19 2003, Kai Makisara wrote: ... > > As usual, I am against this 512-byte alignment (thinking as st > > maintainer). Don't default to more strict aligment requirements than is > > necessary. It creates hidden performance problems for character device > > drivers. > > But you have to honor hardware restrictions, though. Where is the > performance impact? > Yes, I agree that we must not try to do things the hardware does not allow. On i386, most SCSI adapter accept any alignment. It is a waste to require 512-byte alignment with these adapters just because USB requires that. I think there should be a way for the high-level drivers to know about the absolutely necessary alignment requirements for the actual adapter being used with a device. If the user buffer is not aligned according to the required rules, the driver must copy the data into a temporary buffer for i/o. This consumes bandwidth and with the current faster tape drives this leads to significant waste of cpu cycles (according to measurements). The software using tapes tends to malloc() the buffers and then the alignment is not guaranteed to 512 byte boundaries. Other Unices don't have alignment requirements for tape buffers and I think we should avoid requiring this in Linux if possible. Someone may think that this is theory. I checked one common low-end backup program tar (from SuSE 9.0). In the first test (tar cb 64 xxx) the user buffer address was 0806e508 except in one write it was 08051000. A test with dd gave the address 0806e508. cpio used the addresses 0806e508 and 08057cd8. -- Kai ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 22:06 ` Kai Makisara @ 2003-11-20 6:53 ` Jens Axboe 2003-11-20 15:20 ` Alan Stern 0 siblings, 1 reply; 54+ messages in thread From: Jens Axboe @ 2003-11-20 6:53 UTC (permalink / raw) To: Kai Makisara Cc: James Bottomley, Oliver Neukum, Patrick Mansfield, Alan Stern, Douglas Gilbert, SCSI development list, USB development list On Thu, Nov 20 2003, Kai Makisara wrote: > On Wed, 19 Nov 2003, Jens Axboe wrote: > > > On Wed, Nov 19 2003, Kai Makisara wrote: > .. > > > As usual, I am against this 512-byte alignment (thinking as st > > > maintainer). Don't default to more strict aligment requirements than is > > > necessary. It creates hidden performance problems for character device > > > drivers. > > > > But you have to honor hardware restrictions, though. Where is the > > performance impact? > > > Yes, I agree that we must not try to do things the hardware does not > allow. On i386, most SCSI adapter accept any alignment. It is a waste to > require 512-byte alignment with these adapters just because USB requires > that. I think there should be a way for the high-level drivers to know > about the absolutely necessary alignment requirements for the actual > adapter being used with a device. > > If the user buffer is not aligned according to the required rules, the > driver must copy the data into a temporary buffer for i/o. This consumes > bandwidth and with the current faster tape drives this leads to > significant waste of cpu cycles (according to measurements). > > The software using tapes tends to malloc() the buffers and then the > alignment is not guaranteed to 512 byte boundaries. Other Unices don't > have alignment requirements for tape buffers and I think we should avoid > requiring this in Linux if possible. Then you just cannot do zero-copy dma, if the buffer isn't aligned properly. It's as simple as that. And surely that wont kill you performance wise. Doing bounce buffering to just maintain the use of mapping user pages into the kernel ls out right silly. So no, I still don't think we have to add anything. Just don't do_dio if the alignment doesn't allow it. Ditto for sg. -- Jens Axboe ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-20 6:53 ` Jens Axboe @ 2003-11-20 15:20 ` Alan Stern 2003-11-20 15:30 ` Jens Axboe 2003-11-20 17:18 ` Kai Makisara 0 siblings, 2 replies; 54+ messages in thread From: Alan Stern @ 2003-11-20 15:20 UTC (permalink / raw) To: Jens Axboe Cc: Kai Makisara, James Bottomley, Oliver Neukum, Patrick Mansfield, Douglas Gilbert, SCSI development list, USB development list On Thu, 20 Nov 2003, Jens Axboe wrote: > Then you just cannot do zero-copy dma, if the buffer isn't aligned > properly. It's as simple as that. And surely that wont kill you > performance wise. > > Doing bounce buffering to just maintain the use of mapping user pages > into the kernel ls out right silly. > > So no, I still don't think we have to add anything. Just don't do_dio if > the alignment doesn't allow it. Ditto for sg. The answer seems very simple. There should be a host template entry for dma buffer alignment (there's already a dma_boundary member). It would be copied into the device's request queue stucture if it is nonzero, overriding the default value of 512. sg and st should check the user buffer against the request queue's dma_alignment mask and avoid doing direct I/O if the alignment is wrong -- just fall back to normal I/O. Any objections to this scheme? Alan Stern ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-20 15:20 ` Alan Stern @ 2003-11-20 15:30 ` Jens Axboe 2003-11-20 16:09 ` Alan Stern 2003-11-20 16:28 ` [linux-usb-devel] " Oliver Neukum 2003-11-20 17:18 ` Kai Makisara 1 sibling, 2 replies; 54+ messages in thread From: Jens Axboe @ 2003-11-20 15:30 UTC (permalink / raw) To: Alan Stern Cc: Kai Makisara, James Bottomley, Oliver Neukum, Patrick Mansfield, Douglas Gilbert, SCSI development list, USB development list On Thu, Nov 20 2003, Alan Stern wrote: > On Thu, 20 Nov 2003, Jens Axboe wrote: > > > Then you just cannot do zero-copy dma, if the buffer isn't aligned > > properly. It's as simple as that. And surely that wont kill you > > performance wise. > > > > Doing bounce buffering to just maintain the use of mapping user pages > > into the kernel ls out right silly. > > > > So no, I still don't think we have to add anything. Just don't do_dio if > > the alignment doesn't allow it. Ditto for sg. > > The answer seems very simple. There should be a host template entry for > dma buffer alignment (there's already a dma_boundary member). It would be > copied into the device's request queue stucture if it is nonzero, > overriding the default value of 512. sg and st should check the user > buffer against the request queue's dma_alignment mask and avoid doing > direct I/O if the alignment is wrong -- just fall back to normal I/O. > > Any objections to this scheme? Well yes, that's what my objection is against - adding that member. Did you not read any of my mails? And it's quite simple why - basically noone will add it, so it'll end up being 512 anyways. I just don't see the point. It's a miniscule optimization. If you need that last bit of performance, then align your buffers and noone loses. See? -- Jens Axboe ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-20 15:30 ` Jens Axboe @ 2003-11-20 16:09 ` Alan Stern 2003-11-20 16:24 ` Jens Axboe 2003-11-20 16:28 ` [linux-usb-devel] " Oliver Neukum 1 sibling, 1 reply; 54+ messages in thread From: Alan Stern @ 2003-11-20 16:09 UTC (permalink / raw) To: Jens Axboe Cc: Kai Makisara, James Bottomley, Oliver Neukum, Patrick Mansfield, Douglas Gilbert, SCSI development list, USB development list On Thu, 20 Nov 2003, Jens Axboe wrote: > Well yes, that's what my objection is against - adding that member. Did > you not read any of my mails? And it's quite simple why - basically > noone will add it, so it'll end up being 512 anyways. All right, fine. Host adapter drivers have the option of setting the dma_alignment mask in their slave_configure() routines. Doing it there won't add anything to the host template, will leave the value at its default 512 for host drivers that don't care, and will let drivers that do care set it to the optimal value. The important thing is that sg and st should check the alignment of the actual buffer against the dma_alignment mask, which they currently don't do. > I just don't see the point. It's a miniscule optimization. If you need > that last bit of performance, then align your buffers and noone loses. > See? I'm not sure -- was that "you" directed at me personally or at userspace program writers in general? Telling me to align the user buffers won't help anything; it's the program writers who need to be informed of the restrictions. And it's the host-driver writers who need to loosen those restrictions where applicable. Alan Stern ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Re: Unaligned scatter-gather buffers and usb-storage 2003-11-20 16:09 ` Alan Stern @ 2003-11-20 16:24 ` Jens Axboe 0 siblings, 0 replies; 54+ messages in thread From: Jens Axboe @ 2003-11-20 16:24 UTC (permalink / raw) To: Alan Stern Cc: Kai Makisara, James Bottomley, Oliver Neukum, Patrick Mansfield, Douglas Gilbert, SCSI development list, USB development list On Thu, Nov 20 2003, Alan Stern wrote: > On Thu, 20 Nov 2003, Jens Axboe wrote: > > > Well yes, that's what my objection is against - adding that member. Did > > you not read any of my mails? And it's quite simple why - basically > > noone will add it, so it'll end up being 512 anyways. > > All right, fine. Host adapter drivers have the option of setting the > dma_alignment mask in their slave_configure() routines. Doing it there > won't add anything to the host template, will leave the value at its > default 512 for host drivers that don't care, and will let drivers that do > care set it to the optimal value. > > The important thing is that sg and st should check the alignment of the > actual buffer against the dma_alignment mask, which they currently don't > do. Sure fine. I still don't think it's worth anybodies time messing with. > > I just don't see the point. It's a miniscule optimization. If you need > > that last bit of performance, then align your buffers and noone loses. > > See? > > I'm not sure -- was that "you" directed at me personally or at userspace > program writers in general? Telling me to align the user buffers won't > help anything; it's the program writers who need to be informed of the > restrictions. And it's the host-driver writers who need to loosen those > restrictions where applicable. You as in program writers, clearly you cannot do anything about it. Of course sg/st/whatever cannot pass down badly aligned sg addresses, that's a given. And I think they are currently broken that they don't attempt to align to 512 bytes. I just think we've waisted way more time discussion the issue than what will ever be saved in cpu time. -- Jens Axboe ------------------------------------------------------- This SF.net email is sponsored by: SF.net Giveback Program. Does SourceForge.net help you be more productive? Does it help you create better code? SHARE THE LOVE, and help us help YOU! Click Here: http://sourceforge.net/donate/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-20 15:30 ` Jens Axboe 2003-11-20 16:09 ` Alan Stern @ 2003-11-20 16:28 ` Oliver Neukum 2003-11-20 19:23 ` Kai Makisara 1 sibling, 1 reply; 54+ messages in thread From: Oliver Neukum @ 2003-11-20 16:28 UTC (permalink / raw) To: Jens Axboe, Alan Stern Cc: Kai Makisara, James Bottomley, Patrick Mansfield, Douglas Gilbert, SCSI development list, USB development list > > The answer seems very simple. There should be a host template entry for > > dma buffer alignment (there's already a dma_boundary member). It would be > > copied into the device's request queue stucture if it is nonzero, > > overriding the default value of 512. sg and st should check the user > > buffer against the request queue's dma_alignment mask and avoid doing > > direct I/O if the alignment is wrong -- just fall back to normal I/O. > > > > Any objections to this scheme? > > Well yes, that's what my objection is against - adding that member. Did > you not read any of my mails? And it's quite simple why - basically > noone will add it, so it'll end up being 512 anyways. But neither sg nor st currently honor the 512 byte alignment requirement. Whether this really hurts st in terms of performance is another question. Regards Oliver ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-20 16:28 ` [linux-usb-devel] " Oliver Neukum @ 2003-11-20 19:23 ` Kai Makisara 0 siblings, 0 replies; 54+ messages in thread From: Kai Makisara @ 2003-11-20 19:23 UTC (permalink / raw) To: Oliver Neukum; +Cc: SCSI development list, USB development list On Thu, 20 Nov 2003, Oliver Neukum wrote: ... > But neither sg nor st currently honor the 512 byte alignment requirement. > Whether this really hurts st in terms of performance is another question. > It depends on hardware :-) Before implementing direct i/o, I made some measurements with the hardware available. The tape drive was HP DDS-4 and computer dual PIII 700 MHz. The results with a test program using well-compressible data (zeroes): Buffer length 32768 bytes, 32768 buffers written (1024.000 MB, 1073741824 bytes) Variable block mode (writing 32768 byte blocks). dio: write: wall 60.436 user 0.015 ( 0.0 %) system 0.485 ( 0.8 %) speed 16.944 MB/s read: wall 64.580 user 0.014 ( 0.0 %) system 0.500 ( 0.8 %) speed 15.856 MB/s no dio: write: wall 61.373 user 0.024 ( 0.0 %) system 2.897 ( 4.7 %) speed 16.685 MB/s read: wall 66.435 user 0.055 ( 0.1 %) system 6.347 ( 9.6 %) speed 15.413 MB/s The common high-end drives at that time streamed at 30-40 MB/s assuming 2:1 compression. It meant that about 20 % of cpu was needed for extra copies when reading. Other people reported even higher percentages with other hardware. I just repeated the tests using the same drive but a 2.6 GHz PIV on a Intel D875 motherboard and dual channel memory. The results were: dio: write: wall 58.668 user 0.013 ( 0.0 %) system 0.217 ( 0.4 %) speed 17.045 MB/s read: wall 63.425 user 0.011 ( 0.0 %) system 0.245 ( 0.4 %) speed 15.767 MB/s no dio: write: wall 58.713 user 0.017 ( 0.0 %) system 0.534 ( 0.9 %) speed 17.032 MB/s read: wall 63.265 user 0.020 ( 0.0 %) system 0.805 ( 1.3 %) speed 15.807 MB/s Current ordinary drives stream up to 70 MB/s (assuming 2:1 compression). This would lead to over 5 % overhead per drive when not doing dio. -- Kai ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Re: Unaligned scatter-gather buffers and usb-storage 2003-11-20 15:20 ` Alan Stern 2003-11-20 15:30 ` Jens Axboe @ 2003-11-20 17:18 ` Kai Makisara 2003-11-20 19:18 ` [linux-usb-devel] " Kai Mäkisara 1 sibling, 1 reply; 54+ messages in thread From: Kai Makisara @ 2003-11-20 17:18 UTC (permalink / raw) To: Alan Stern Cc: Jens Axboe, James Bottomley, Oliver Neukum, Patrick Mansfield, Douglas Gilbert, SCSI development list, USB development list On Thu, 20 Nov 2003, Alan Stern wrote: > On Thu, 20 Nov 2003, Jens Axboe wrote: > > > Then you just cannot do zero-copy dma, if the buffer isn't aligned > > properly. It's as simple as that. And surely that wont kill you > > performance wise. > > > > Doing bounce buffering to just maintain the use of mapping user pages > > into the kernel ls out right silly. > > > > So no, I still don't think we have to add anything. Just don't do_dio if > > the alignment doesn't allow it. Ditto for sg. > > The answer seems very simple. There should be a host template entry for > dma buffer alignment (there's already a dma_boundary member). It would be > copied into the device's request queue stucture if it is nonzero, > overriding the default value of 512. sg and st should check the user > buffer against the request queue's dma_alignment mask and avoid doing > direct I/O if the alignment is wrong -- just fall back to normal I/O. > > Any objections to this scheme? > My only objection is the default. It should be zero. If I read the usb-storage sources correctly, the scsi_host_template is defined in one file (scsiglue.c). If the mask is there set to 511, all USB storage devices get 512-byte alignment and other get no constraints. All get optimal result with minimal work. (Adding the check to st and sg is trivial.) -- Kai ------------------------------------------------------- This SF.net email is sponsored by: SF.net Giveback Program. Does SourceForge.net help you be more productive? Does it help you create better code? SHARE THE LOVE, and help us help YOU! Click Here: http://sourceforge.net/donate/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [linux-usb-devel] Re: Unaligned scatter-gather buffers and usb-storage 2003-11-20 17:18 ` Kai Makisara @ 2003-11-20 19:18 ` Kai Mäkisara 2003-11-21 18:03 ` PATCH: (as141) " Alan Stern 0 siblings, 1 reply; 54+ messages in thread From: Kai Mäkisara @ 2003-11-20 19:18 UTC (permalink / raw) To: Alan Stern Cc: Jens Axboe, James Bottomley, Oliver Neukum, Patrick Mansfield, Douglas Gilbert, SCSI development list, USB development list Thus spake Kai Makisara (Kai.Makisara@kolumbus.fi): > On Thu, 20 Nov 2003, Alan Stern wrote: > ... > > The answer seems very simple. There should be a host template entry for > > dma buffer alignment (there's already a dma_boundary member). It would be > > copied into the device's request queue stucture if it is nonzero, > > overriding the default value of 512. sg and st should check the user > > buffer against the request queue's dma_alignment mask and avoid doing > > direct I/O if the alignment is wrong -- just fall back to normal I/O. > > > > Any objections to this scheme? > > > My only objection is the default. It should be zero. If I read the > usb-storage sources correctly, the scsi_host_template is defined in one > file (scsiglue.c). If the mask is there set to 511, all USB storage > devices get 512-byte alignment and other get no constraints. All get > optimal result with minimal work. (Adding the check to st and sg is > trivial.) > To make this a little more than talk, here is a patch for 2.6.0-test9 plus csets up to today (compiled and tested but does not include the check for sg): ---8<--- diff -ur linux-2.6-cset/drivers/scsi/st.c linux-2.6-cset-k/drivers/scsi/st.c --- linux-2.6-cset/drivers/scsi/st.c 2003-09-09 18:19:45.000000000 +0300 +++ linux-2.6-cset-k/drivers/scsi/st.c 2003-11-20 20:49:25.000000000 +0200 @@ -1267,7 +1267,8 @@ i = STp->try_dio && try_rdio; else i = STp->try_dio && try_wdio; - if (i) { + + if (i && ((unsigned int)buf & STp->device->host->hostt->dma_align_mask) == 0) { i = st_map_user_pages(&(STbp->sg[0]), STbp->use_sg, (unsigned long)buf, count, (is_read ? READ : WRITE), STp->max_pfn); diff -ur linux-2.6-cset/drivers/usb/storage/scsiglue.c linux-2.6-cset-k/drivers/usb/storage/scsiglue.c --- linux-2.6-cset/drivers/usb/storage/scsiglue.c 2003-10-25 22:35:58.000000000 +0300 +++ linux-2.6-cset-k/drivers/usb/storage/scsiglue.c 2003-11-20 19:34:56.000000000 +0200 @@ -312,6 +312,9 @@ /* lots of sg segments can be handled */ .sg_tablesize = SG_ALL, + /* alignment to 512-byte required */ + .dma_align_mask = 0x1ff, + /* merge commands... this seems to help performance, but * periodically someone should test to see which setting is more * optimal. Only in linux-2.6-cset/include/asm: page.h~ Only in linux-2.6-cset/include/asm-i386: page.h~ diff -ur linux-2.6-cset/include/scsi/scsi_host.h linux-2.6-cset-k/include/scsi/scsi_host.h --- linux-2.6-cset/include/scsi/scsi_host.h 2003-10-25 22:35:58.000000000 +0300 +++ linux-2.6-cset-k/include/scsi/scsi_host.h 2003-11-20 19:33:32.000000000 +0200 @@ -268,6 +268,12 @@ unsigned long dma_boundary; /* + * Alignment mask for scatter/gather segments. The offset and'ed + * with the mask must be zero for segments. + */ + unsigned int dma_align_mask; + + /* * This specifies "machine infinity" for host templates which don't * limit the transfer size. Note this limit represents an absolute * maximum, and may be over the transfer limits allowed for ---8<--- -- Kai ^ permalink raw reply [flat|nested] 54+ messages in thread
* PATCH: (as141) Unaligned scatter-gather buffers and usb-storage 2003-11-20 19:18 ` [linux-usb-devel] " Kai Mäkisara @ 2003-11-21 18:03 ` Alan Stern 2003-11-21 20:07 ` Kai Makisara ` (2 more replies) 0 siblings, 3 replies; 54+ messages in thread From: Alan Stern @ 2003-11-21 18:03 UTC (permalink / raw) To: Kai Mäkisara, Douglas Gilbert Cc: Matthew Dharm, James Bottomley, Patrick Mansfield, Jens Axboe, Oliver Neukum, SCSI development list, USB development list I'm not sure whether Jens is objecting to adding the extra member to the host template or to making the default alignment something other than 512. Here's my attempt at a compromise patch, which boils the area of disagreement between Kai and Jens down to a single line of code (plus a comment). It doesn't change the host template and it shouldn't slow down any tape I/O programs when using a non-USB device. If there aren't any more objections to this, I urge James to apply the SCSI parts to a post-2.6.0-final tree, and likewise for Matt and the USB part. Alan Stern ===== scsiglue.c 1.60 vs edited ===== --- 1.60/drivers/usb/storage/scsiglue.c Fri Oct 24 14:53:38 2003 +++ edited/drivers/usb/storage/scsiglue.c Fri Nov 21 12:45:21 2003 @@ -65,6 +65,16 @@ static int slave_configure (struct scsi_device *sdev) { + /* Scatter-gather buffers (all but the last) must have a length + * divisible by the bulk maxpacket size. Otherwise a data packet + * would end up being short, causing a premature end to the data + * transfer. Since high-speed bulk pipes have a maxpacket size + * of 512, we'll use that as the scsi device queue's DMA alignment + * mask. Guaranteeing proper alignment of the first buffer will + * have the desired effect because, except at the beginning and + * the end, scatter-gather buffers follow page boundaries. */ + blk_queue_dma_alignment(sdev->request_queue, (512 - 1)); + /* this is to satisify the compiler, tho I don't think the * return code is ever checked anywhere. */ return 0; ===== sg.c 1.48 vs edited ===== --- 1.48/drivers/scsi/sg.c Fri Oct 24 14:53:37 2003 +++ edited/drivers/scsi/sg.c Fri Nov 21 12:28:52 2003 @@ -1741,7 +1741,11 @@ int sg_tablesize = sfp->parentdp->sg_tablesize; struct scatterlist *sgl; int mx_sc_elems, res; + struct scsi_device *sdev = sfp->parentdp->device; + if (((unsigned long)hp->dxferp & + queue_dma_alignment(sdev->request_queue)) != 0) + return 1; mx_sc_elems = sg_build_sgat(schp, sfp, sg_tablesize); if (mx_sc_elems <= 0) { return 1; ===== st.c 1.45 vs edited ===== --- 1.45/drivers/scsi/st.c Fri Sep 5 12:16:40 2003 +++ edited/drivers/scsi/st.c Fri Nov 21 12:30:34 2003 @@ -1267,7 +1267,8 @@ i = STp->try_dio && try_rdio; else i = STp->try_dio && try_wdio; - if (i) { + if (i && ((unsigned int)buf & queue_dma_alignment( + STp->device->request_queue)) == 0) { i = st_map_user_pages(&(STbp->sg[0]), STbp->use_sg, (unsigned long)buf, count, (is_read ? READ : WRITE), STp->max_pfn); ===== scsi_scan.c 1.52 vs edited ===== --- 1.52/drivers/scsi/scsi_scan.c Fri Oct 24 14:53:37 2003 +++ edited/drivers/scsi/scsi_scan.c Fri Nov 21 12:34:00 2003 @@ -231,6 +231,15 @@ sdev->request_queue->queuedata = sdev; scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun); + /* + * Set the queue's mask to require a mere 8-byte alignment for + * DMA buffers, rather than the default 512. This shouldn't + * inconvenience any user programs and should be okay for most + * host adapters. A host driver can alter this mask in its + * slave_alloc() or slave_configure() callback if necessary. + */ + blk_queue_dma_alignment(sdev->request_queue, (8 - 1)); + if (shost->hostt->slave_alloc) { if (shost->hostt->slave_alloc(sdev)) goto out_free_queue; ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: PATCH: (as141) Unaligned scatter-gather buffers and usb-storage 2003-11-21 18:03 ` PATCH: (as141) " Alan Stern @ 2003-11-21 20:07 ` Kai Makisara 2003-12-01 1:30 ` Matthew Dharm 2004-01-05 0:41 ` Matthew Dharm 2 siblings, 0 replies; 54+ messages in thread From: Kai Makisara @ 2003-11-21 20:07 UTC (permalink / raw) To: Alan Stern Cc: Douglas Gilbert, Matthew Dharm, James Bottomley, Patrick Mansfield, Jens Axboe, Oliver Neukum, SCSI development list, USB development list On Fri, 21 Nov 2003, Alan Stern wrote: > I'm not sure whether Jens is objecting to adding the extra member to the > host template or to making the default alignment something other than 512. > Here's my attempt at a compromise patch, which boils the area of > disagreement between Kai and Jens down to a single line of code (plus a > comment). It doesn't change the host template and it shouldn't slow down > any tape I/O programs when using a non-USB device. > Looks very good. Using existing mechanisms is better than adding a new member to a struct. I tested the patch with a tape and did not find any problems. > If there aren't any more objections to this, I urge James to apply the > SCSI parts to a post-2.6.0-final tree, and likewise for Matt and the USB > part. > As far as I am concerned, please do. -- Kai ------------------------------------------------------- This SF.net email is sponsored by: SF.net Giveback Program. Does SourceForge.net help you be more productive? Does it help you create better code? SHARE THE LOVE, and help us help YOU! Click Here: http://sourceforge.net/donate/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: PATCH: (as141) Unaligned scatter-gather buffers and usb-storage 2003-11-21 18:03 ` PATCH: (as141) " Alan Stern 2003-11-21 20:07 ` Kai Makisara @ 2003-12-01 1:30 ` Matthew Dharm 2004-01-05 0:41 ` Matthew Dharm 2 siblings, 0 replies; 54+ messages in thread From: Matthew Dharm @ 2003-12-01 1:30 UTC (permalink / raw) To: Alan Stern Cc: Kai Mäkisara, Douglas Gilbert, James Bottomley, Patrick Mansfield, Jens Axboe, Oliver Neukum, SCSI development list, USB development list [-- Attachment #1: Type: text/plain, Size: 3979 bytes --] I'm going to hold on to this patch until I see the SCSI parts go in. Alan, try to remind me if you don't see this get merged after a while. Matt On Fri, Nov 21, 2003 at 01:03:05PM -0500, Alan Stern wrote: > I'm not sure whether Jens is objecting to adding the extra member to the > host template or to making the default alignment something other than 512. > Here's my attempt at a compromise patch, which boils the area of > disagreement between Kai and Jens down to a single line of code (plus a > comment). It doesn't change the host template and it shouldn't slow down > any tape I/O programs when using a non-USB device. > > If there aren't any more objections to this, I urge James to apply the > SCSI parts to a post-2.6.0-final tree, and likewise for Matt and the USB > part. > > Alan Stern > > > ===== scsiglue.c 1.60 vs edited ===== > --- 1.60/drivers/usb/storage/scsiglue.c Fri Oct 24 14:53:38 2003 > +++ edited/drivers/usb/storage/scsiglue.c Fri Nov 21 12:45:21 2003 > @@ -65,6 +65,16 @@ > > static int slave_configure (struct scsi_device *sdev) > { > + /* Scatter-gather buffers (all but the last) must have a length > + * divisible by the bulk maxpacket size. Otherwise a data packet > + * would end up being short, causing a premature end to the data > + * transfer. Since high-speed bulk pipes have a maxpacket size > + * of 512, we'll use that as the scsi device queue's DMA alignment > + * mask. Guaranteeing proper alignment of the first buffer will > + * have the desired effect because, except at the beginning and > + * the end, scatter-gather buffers follow page boundaries. */ > + blk_queue_dma_alignment(sdev->request_queue, (512 - 1)); > + > /* this is to satisify the compiler, tho I don't think the > * return code is ever checked anywhere. */ > return 0; > ===== sg.c 1.48 vs edited ===== > --- 1.48/drivers/scsi/sg.c Fri Oct 24 14:53:37 2003 > +++ edited/drivers/scsi/sg.c Fri Nov 21 12:28:52 2003 > @@ -1741,7 +1741,11 @@ > int sg_tablesize = sfp->parentdp->sg_tablesize; > struct scatterlist *sgl; > int mx_sc_elems, res; > + struct scsi_device *sdev = sfp->parentdp->device; > > + if (((unsigned long)hp->dxferp & > + queue_dma_alignment(sdev->request_queue)) != 0) > + return 1; > mx_sc_elems = sg_build_sgat(schp, sfp, sg_tablesize); > if (mx_sc_elems <= 0) { > return 1; > ===== st.c 1.45 vs edited ===== > --- 1.45/drivers/scsi/st.c Fri Sep 5 12:16:40 2003 > +++ edited/drivers/scsi/st.c Fri Nov 21 12:30:34 2003 > @@ -1267,7 +1267,8 @@ > i = STp->try_dio && try_rdio; > else > i = STp->try_dio && try_wdio; > - if (i) { > + if (i && ((unsigned int)buf & queue_dma_alignment( > + STp->device->request_queue)) == 0) { > i = st_map_user_pages(&(STbp->sg[0]), STbp->use_sg, > (unsigned long)buf, count, (is_read ? READ : WRITE), > STp->max_pfn); > ===== scsi_scan.c 1.52 vs edited ===== > --- 1.52/drivers/scsi/scsi_scan.c Fri Oct 24 14:53:37 2003 > +++ edited/drivers/scsi/scsi_scan.c Fri Nov 21 12:34:00 2003 > @@ -231,6 +231,15 @@ > sdev->request_queue->queuedata = sdev; > scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun); > > + /* > + * Set the queue's mask to require a mere 8-byte alignment for > + * DMA buffers, rather than the default 512. This shouldn't > + * inconvenience any user programs and should be okay for most > + * host adapters. A host driver can alter this mask in its > + * slave_alloc() or slave_configure() callback if necessary. > + */ > + blk_queue_dma_alignment(sdev->request_queue, (8 - 1)); > + > if (shost->hostt->slave_alloc) { > if (shost->hostt->slave_alloc(sdev)) > goto out_free_queue; -- Matthew Dharm Home: mdharm-usb@one-eyed-alien.net Maintainer, Linux USB Mass Storage Driver I want my GPFs!!! -- Stef User Friendly, 11/9/1998 [-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --] ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: PATCH: (as141) Unaligned scatter-gather buffers and usb-storage 2003-11-21 18:03 ` PATCH: (as141) " Alan Stern 2003-11-21 20:07 ` Kai Makisara 2003-12-01 1:30 ` Matthew Dharm @ 2004-01-05 0:41 ` Matthew Dharm 2004-01-05 10:08 ` Jens Axboe 2 siblings, 1 reply; 54+ messages in thread From: Matthew Dharm @ 2004-01-05 0:41 UTC (permalink / raw) To: Alan Stern Cc: Kai Mäkisara, Douglas Gilbert, James Bottomley, Patrick Mansfield, Jens Axboe, Oliver Neukum, SCSI development list, USB development list [-- Attachment #1: Type: text/plain, Size: 3920 bytes --] Is there any indication that the SCSI parts of this patch will get accepted? Matt On Fri, Nov 21, 2003 at 01:03:05PM -0500, Alan Stern wrote: > I'm not sure whether Jens is objecting to adding the extra member to the > host template or to making the default alignment something other than 512. > Here's my attempt at a compromise patch, which boils the area of > disagreement between Kai and Jens down to a single line of code (plus a > comment). It doesn't change the host template and it shouldn't slow down > any tape I/O programs when using a non-USB device. > > If there aren't any more objections to this, I urge James to apply the > SCSI parts to a post-2.6.0-final tree, and likewise for Matt and the USB > part. > > Alan Stern > > > ===== scsiglue.c 1.60 vs edited ===== > --- 1.60/drivers/usb/storage/scsiglue.c Fri Oct 24 14:53:38 2003 > +++ edited/drivers/usb/storage/scsiglue.c Fri Nov 21 12:45:21 2003 > @@ -65,6 +65,16 @@ > > static int slave_configure (struct scsi_device *sdev) > { > + /* Scatter-gather buffers (all but the last) must have a length > + * divisible by the bulk maxpacket size. Otherwise a data packet > + * would end up being short, causing a premature end to the data > + * transfer. Since high-speed bulk pipes have a maxpacket size > + * of 512, we'll use that as the scsi device queue's DMA alignment > + * mask. Guaranteeing proper alignment of the first buffer will > + * have the desired effect because, except at the beginning and > + * the end, scatter-gather buffers follow page boundaries. */ > + blk_queue_dma_alignment(sdev->request_queue, (512 - 1)); > + > /* this is to satisify the compiler, tho I don't think the > * return code is ever checked anywhere. */ > return 0; > ===== sg.c 1.48 vs edited ===== > --- 1.48/drivers/scsi/sg.c Fri Oct 24 14:53:37 2003 > +++ edited/drivers/scsi/sg.c Fri Nov 21 12:28:52 2003 > @@ -1741,7 +1741,11 @@ > int sg_tablesize = sfp->parentdp->sg_tablesize; > struct scatterlist *sgl; > int mx_sc_elems, res; > + struct scsi_device *sdev = sfp->parentdp->device; > > + if (((unsigned long)hp->dxferp & > + queue_dma_alignment(sdev->request_queue)) != 0) > + return 1; > mx_sc_elems = sg_build_sgat(schp, sfp, sg_tablesize); > if (mx_sc_elems <= 0) { > return 1; > ===== st.c 1.45 vs edited ===== > --- 1.45/drivers/scsi/st.c Fri Sep 5 12:16:40 2003 > +++ edited/drivers/scsi/st.c Fri Nov 21 12:30:34 2003 > @@ -1267,7 +1267,8 @@ > i = STp->try_dio && try_rdio; > else > i = STp->try_dio && try_wdio; > - if (i) { > + if (i && ((unsigned int)buf & queue_dma_alignment( > + STp->device->request_queue)) == 0) { > i = st_map_user_pages(&(STbp->sg[0]), STbp->use_sg, > (unsigned long)buf, count, (is_read ? READ : WRITE), > STp->max_pfn); > ===== scsi_scan.c 1.52 vs edited ===== > --- 1.52/drivers/scsi/scsi_scan.c Fri Oct 24 14:53:37 2003 > +++ edited/drivers/scsi/scsi_scan.c Fri Nov 21 12:34:00 2003 > @@ -231,6 +231,15 @@ > sdev->request_queue->queuedata = sdev; > scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun); > > + /* > + * Set the queue's mask to require a mere 8-byte alignment for > + * DMA buffers, rather than the default 512. This shouldn't > + * inconvenience any user programs and should be okay for most > + * host adapters. A host driver can alter this mask in its > + * slave_alloc() or slave_configure() callback if necessary. > + */ > + blk_queue_dma_alignment(sdev->request_queue, (8 - 1)); > + > if (shost->hostt->slave_alloc) { > if (shost->hostt->slave_alloc(sdev)) > goto out_free_queue; -- Matthew Dharm Home: mdharm-usb@one-eyed-alien.net Maintainer, Linux USB Mass Storage Driver I need a computer? -- Customer User Friendly, 2/19/1998 [-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: PATCH: (as141) Unaligned scatter-gather buffers and usb-storage 2004-01-05 0:41 ` Matthew Dharm @ 2004-01-05 10:08 ` Jens Axboe 2004-01-05 21:58 ` PATCH: (as141b) " Alan Stern 0 siblings, 1 reply; 54+ messages in thread From: Jens Axboe @ 2004-01-05 10:08 UTC (permalink / raw) To: Alan Stern, Kai Mäkisara, Douglas Gilbert, James Bottomley, Patrick Mansfield, Oliver Neukum, SCSI development list, USB development list On Sun, Jan 04 2004, Matthew Dharm wrote: > Is there any indication that the SCSI parts of this patch will get > accepted? I'm fine with the patch, FWIW, just move it to scsi_alloc_queue() instead? > > ===== scsi_scan.c 1.52 vs edited ===== > > --- 1.52/drivers/scsi/scsi_scan.c Fri Oct 24 14:53:37 2003 > > +++ edited/drivers/scsi/scsi_scan.c Fri Nov 21 12:34:00 2003 > > @@ -231,6 +231,15 @@ > > sdev->request_queue->queuedata = sdev; > > scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun); > > > > + /* > > + * Set the queue's mask to require a mere 8-byte alignment for > > + * DMA buffers, rather than the default 512. This shouldn't > > + * inconvenience any user programs and should be okay for most > > + * host adapters. A host driver can alter this mask in its > > + * slave_alloc() or slave_configure() callback if necessary. > > + */ > > + blk_queue_dma_alignment(sdev->request_queue, (8 - 1)); > > + > > if (shost->hostt->slave_alloc) { > > if (shost->hostt->slave_alloc(sdev)) > > goto out_free_queue; -- Jens Axboe ------------------------------------------------------- This SF.net email is sponsored by: IBM Linux Tutorials. Become an expert in LINUX or just sharpen your skills. Sign up for IBM's Free Linux Tutorials. Learn everything from the bash shell to sys admin. Click now! http://ads.osdn.com/?ad_id=1278&alloc_id=3371&op=click _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* PATCH: (as141b) Unaligned scatter-gather buffers and usb-storage 2004-01-05 10:08 ` Jens Axboe @ 2004-01-05 21:58 ` Alan Stern 2004-01-06 11:28 ` Oliver Neukum 2004-02-02 15:51 ` James Bottomley 0 siblings, 2 replies; 54+ messages in thread From: Alan Stern @ 2004-01-05 21:58 UTC (permalink / raw) To: Jens Axboe Cc: Kai Mäkisara, Douglas Gilbert, James Bottomley, Patrick Mansfield, Oliver Neukum, SCSI development list, USB development list On Mon, 5 Jan 2004, Jens Axboe wrote: > On Sun, Jan 04 2004, Matthew Dharm wrote: > > Is there any indication that the SCSI parts of this patch will get > > accepted? > > I'm fine with the patch, FWIW, just move it to scsi_alloc_queue() > instead? Here's the revised patch. Alan Stern ===== scsiglue.c 1.60 vs edited ===== --- 1.60/drivers/usb/storage/scsiglue.c Fri Oct 24 14:53:38 2003 +++ edited/drivers/usb/storage/scsiglue.c Fri Nov 21 12:45:21 2003 @@ -65,6 +65,16 @@ static int slave_configure (struct scsi_device *sdev) { + /* Scatter-gather buffers (all but the last) must have a length + * divisible by the bulk maxpacket size. Otherwise a data packet + * would end up being short, causing a premature end to the data + * transfer. Since high-speed bulk pipes have a maxpacket size + * of 512, we'll use that as the scsi device queue's DMA alignment + * mask. Guaranteeing proper alignment of the first buffer will + * have the desired effect because, except at the beginning and + * the end, scatter-gather buffers follow page boundaries. */ + blk_queue_dma_alignment(sdev->request_queue, (512 - 1)); + /* this is to satisify the compiler, tho I don't think the * return code is ever checked anywhere. */ return 0; ===== sg.c 1.48 vs edited ===== --- 1.48/drivers/scsi/sg.c Fri Oct 24 14:53:37 2003 +++ edited/drivers/scsi/sg.c Fri Nov 21 12:28:52 2003 @@ -1741,7 +1741,11 @@ int sg_tablesize = sfp->parentdp->sg_tablesize; struct scatterlist *sgl; int mx_sc_elems, res; + struct scsi_device *sdev = sfp->parentdp->device; + if (((unsigned long)hp->dxferp & + queue_dma_alignment(sdev->request_queue)) != 0) + return 1; mx_sc_elems = sg_build_sgat(schp, sfp, sg_tablesize); if (mx_sc_elems <= 0) { return 1; ===== st.c 1.45 vs edited ===== --- 1.45/drivers/scsi/st.c Fri Sep 5 12:16:40 2003 +++ edited/drivers/scsi/st.c Fri Nov 21 12:30:34 2003 @@ -1267,7 +1267,8 @@ i = STp->try_dio && try_rdio; else i = STp->try_dio && try_wdio; - if (i) { + if (i && ((unsigned int)buf & queue_dma_alignment( + STp->device->request_queue)) == 0) { i = st_map_user_pages(&(STbp->sg[0]), STbp->use_sg, (unsigned long)buf, count, (is_read ? READ : WRITE), STp->max_pfn); ===== scsi_lib.c 1.49 vs edited ===== --- 1.49/drivers/scsi/scsi_lib.c Sat Nov 22 11:20:45 2003 +++ edited/drivers/scsi/scsi_lib.c Mon Jan 5 16:55:32 2004 @@ -1287,6 +1287,15 @@ blk_queue_max_sectors(q, shost->max_sectors); blk_queue_bounce_limit(q, scsi_calculate_bounce_limit(shost)); blk_queue_segment_boundary(q, shost->dma_boundary); + + /* + * Set the queue's mask to require a mere 8-byte alignment for + * DMA buffers, rather than the default 512. This shouldn't + * inconvenience any user programs and should be okay for most + * host adapters. A host driver can alter this mask in its + * slave_alloc() or slave_configure() callback if necessary. + */ + blk_queue_dma_alignment(sdev->request_queue, (8 - 1)); if (!shost->use_clustering) clear_bit(QUEUE_FLAG_CLUSTER, &q->queue_flags); ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: PATCH: (as141b) Unaligned scatter-gather buffers and usb-storage 2004-01-05 21:58 ` PATCH: (as141b) " Alan Stern @ 2004-01-06 11:28 ` Oliver Neukum 2004-01-06 16:10 ` Alan Stern 2004-02-02 15:51 ` James Bottomley 1 sibling, 1 reply; 54+ messages in thread From: Oliver Neukum @ 2004-01-06 11:28 UTC (permalink / raw) To: Alan Stern, Jens Axboe Cc: Kai Mäkisara, Douglas Gilbert, James Bottomley, Patrick Mansfield, SCSI development list, USB development list > static int slave_configure (struct scsi_device *sdev) > { > + /* Scatter-gather buffers (all but the last) must have a length > + * divisible by the bulk maxpacket size. Otherwise a data packet > + * would end up being short, causing a premature end to the data > + * transfer. Since high-speed bulk pipes have a maxpacket size > + * of 512, we'll use that as the scsi device queue's DMA alignment > + * mask. Guaranteeing proper alignment of the first buffer will > + * have the desired effect because, except at the beginning and > + * the end, scatter-gather buffers follow page boundaries. */ > + blk_queue_dma_alignment(sdev->request_queue, (512 - 1)); What about the cacheline of the last buffer? It seems to me that we have to worry about DMA coherency. Regards Oliver ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: PATCH: (as141b) Unaligned scatter-gather buffers and usb-storage 2004-01-06 11:28 ` Oliver Neukum @ 2004-01-06 16:10 ` Alan Stern 0 siblings, 0 replies; 54+ messages in thread From: Alan Stern @ 2004-01-06 16:10 UTC (permalink / raw) To: Oliver Neukum Cc: Jens Axboe, Kai Mäkisara, Douglas Gilbert, James Bottomley, Patrick Mansfield, SCSI development list, USB development list On Tue, 6 Jan 2004, Oliver Neukum wrote: > > + /* Scatter-gather buffers (all but the last) must have a length > > + * divisible by the bulk maxpacket size. Otherwise a data packet > > + * would end up being short, causing a premature end to the data > > + * transfer. Since high-speed bulk pipes have a maxpacket size > > + * of 512, we'll use that as the scsi device queue's DMA alignment > > + * mask. Guaranteeing proper alignment of the first buffer will > > + * have the desired effect because, except at the beginning and > > + * the end, scatter-gather buffers follow page boundaries. */ > > + blk_queue_dma_alignment(sdev->request_queue, (512 - 1)); > > What about the cacheline of the last buffer? It seems to me that we have > to worry about DMA coherency. We don't have to worry about that, at least not here in the low-level driver. These buffers are passed from user processes. Suppose a process wants to write an amount that's smaller than a cacheline. But that small amount might be stored in part of a buffer that does end on a cacheline boundary -- there's no way for the driver to know. Ultimately the user process must be responsible for insuring that DMA coherency is maintained. Alan Stern ------------------------------------------------------- This SF.net email is sponsored by: IBM Linux Tutorials. Become an expert in LINUX or just sharpen your skills. Sign up for IBM's Free Linux Tutorials. Learn everything from the bash shell to sys admin. Click now! http://ads.osdn.com/?ad_id=1278&alloc_id=3371&op=click _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: PATCH: (as141b) Unaligned scatter-gather buffers and usb-storage 2004-01-05 21:58 ` PATCH: (as141b) " Alan Stern 2004-01-06 11:28 ` Oliver Neukum @ 2004-02-02 15:51 ` James Bottomley 2004-02-03 15:47 ` Alan Stern 1 sibling, 1 reply; 54+ messages in thread From: James Bottomley @ 2004-02-02 15:51 UTC (permalink / raw) To: Alan Stern Cc: Jens Axboe, Kai Mäkisara, Douglas Gilbert, Patrick Mansfield, Oliver Neukum, SCSI development list, USB development list On Mon, 2004-01-05 at 16:58, Alan Stern wrote: > ===== st.c 1.45 vs edited ===== > --- 1.45/drivers/scsi/st.c Fri Sep 5 12:16:40 2003 > +++ edited/drivers/scsi/st.c Fri Nov 21 12:30:34 2003 > @@ -1267,7 +1267,8 @@ > i = STp->try_dio && try_rdio; > else > i = STp->try_dio && try_wdio; > - if (i) { > + if (i && ((unsigned int)buf & queue_dma_alignment( This needs to be unsigned long, otherwise it will print a warning on a 64 bit compile. > ===== scsi_lib.c 1.49 vs edited ===== > --- 1.49/drivers/scsi/scsi_lib.c Sat Nov 22 11:20:45 2003 > +++ edited/drivers/scsi/scsi_lib.c Mon Jan 5 16:55:32 2004 > @@ -1287,6 +1287,15 @@ > blk_queue_max_sectors(q, shost->max_sectors); > blk_queue_bounce_limit(q, scsi_calculate_bounce_limit(shost)); > blk_queue_segment_boundary(q, shost->dma_boundary); > + > + /* > + * Set the queue's mask to require a mere 8-byte alignment for > + * DMA buffers, rather than the default 512. This shouldn't > + * inconvenience any user programs and should be okay for most > + * host adapters. A host driver can alter this mask in its > + * slave_alloc() or slave_configure() callback if necessary. > + */ > + blk_queue_dma_alignment(sdev->request_queue, (8 - 1)); sdev->request_queue is NULL here (it's filled in by the return from this function. The argument should be q. I'll make the changes and apply the patch. James ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: PATCH: (as141b) Unaligned scatter-gather buffers and usb-storage 2004-02-02 15:51 ` James Bottomley @ 2004-02-03 15:47 ` Alan Stern 2004-02-03 15:55 ` James Bottomley 2004-02-03 16:02 ` Matthew Wilcox 0 siblings, 2 replies; 54+ messages in thread From: Alan Stern @ 2004-02-03 15:47 UTC (permalink / raw) To: James Bottomley; +Cc: SCSI development list On 2 Feb 2004, James Bottomley wrote: > On Mon, 2004-01-05 at 16:58, Alan Stern wrote: > > ===== st.c 1.45 vs edited ===== > > --- 1.45/drivers/scsi/st.c Fri Sep 5 12:16:40 2003 > > +++ edited/drivers/scsi/st.c Fri Nov 21 12:30:34 2003 > > @@ -1267,7 +1267,8 @@ > > i = STp->try_dio && try_rdio; > > else > > i = STp->try_dio && try_wdio; > > - if (i) { > > + if (i && ((unsigned int)buf & queue_dma_alignment( > > This needs to be unsigned long, otherwise it will print a warning on a > 64 bit compile. Really? I'm surprised. Although I know nothing about the details of using gcc on a 64-bit platform, it seems odd that an explicit cast would generate a warning. Was that done deliberately, to try and catch inadvertent bugs in address arithmetic? > > ===== scsi_lib.c 1.49 vs edited ===== > > --- 1.49/drivers/scsi/scsi_lib.c Sat Nov 22 11:20:45 2003 > > +++ edited/drivers/scsi/scsi_lib.c Mon Jan 5 16:55:32 2004 > > @@ -1287,6 +1287,15 @@ > > blk_queue_max_sectors(q, shost->max_sectors); > > blk_queue_bounce_limit(q, scsi_calculate_bounce_limit(shost)); > > blk_queue_segment_boundary(q, shost->dma_boundary); > > + > > + /* > > + * Set the queue's mask to require a mere 8-byte alignment for > > + * DMA buffers, rather than the default 512. This shouldn't > > + * inconvenience any user programs and should be okay for most > > + * host adapters. A host driver can alter this mask in its > > + * slave_alloc() or slave_configure() callback if necessary. > > + */ > > + blk_queue_dma_alignment(sdev->request_queue, (8 - 1)); > > sdev->request_queue is NULL here (it's filled in by the return from this > function. The argument should be q. Whoops. I cut and pasted the code from somewhere else and didn't notice that the variable needed to be changed. > I'll make the changes and apply the patch. Thank you. Alan Stern ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: PATCH: (as141b) Unaligned scatter-gather buffers and usb-storage 2004-02-03 15:47 ` Alan Stern @ 2004-02-03 15:55 ` James Bottomley 2004-02-03 16:02 ` Matthew Wilcox 1 sibling, 0 replies; 54+ messages in thread From: James Bottomley @ 2004-02-03 15:55 UTC (permalink / raw) To: Alan Stern; +Cc: SCSI development list On Tue, 2004-02-03 at 10:47, Alan Stern wrote: > Really? I'm surprised. Although I know nothing about the details of > using gcc on a 64-bit platform, it seems odd that an explicit cast would > generate a warning. Was that done deliberately, to try and catch > inadvertent bugs in address arithmetic? Yes, it's specifically to detect incorrect casting. The warning is 'cast from pointer to integer of different size'. If you specifically mean the truncation, you have to first cast to the correct integer size and then truncate: (unsigned int)((unsigned long)buf) To tell the compiler you really, really mean it. James ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: PATCH: (as141b) Unaligned scatter-gather buffers and usb-storage 2004-02-03 15:47 ` Alan Stern 2004-02-03 15:55 ` James Bottomley @ 2004-02-03 16:02 ` Matthew Wilcox 1 sibling, 0 replies; 54+ messages in thread From: Matthew Wilcox @ 2004-02-03 16:02 UTC (permalink / raw) To: Alan Stern; +Cc: James Bottomley, SCSI development list On Tue, Feb 03, 2004 at 10:47:38AM -0500, Alan Stern wrote: > On 2 Feb 2004, James Bottomley wrote: > > This needs to be unsigned long, otherwise it will print a warning on a > > 64 bit compile. > > Really? I'm surprised. Although I know nothing about the details of > using gcc on a 64-bit platform, it seems odd that an explicit cast would > generate a warning. Was that done deliberately, to try and catch > inadvertent bugs in address arithmetic? Yes, it's something like "Cast from pointer to integer of different size". If you really want to cast a pointer to an int, you have to do (int)(long)ptr; or you can use an intermediate variable like "int foo = (long)ptr;" -- "Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception." -- Mark Twain ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 8:47 ` Jens Axboe 2003-11-19 13:01 ` [linux-usb-devel] " Oliver Neukum @ 2003-11-19 15:44 ` Alan Stern 2003-11-19 15:49 ` Jens Axboe 1 sibling, 1 reply; 54+ messages in thread From: Alan Stern @ 2003-11-19 15:44 UTC (permalink / raw) To: Jens Axboe Cc: Patrick Mansfield, Douglas Gilbert, James Bottomley, Oliver Neukum, SCSI development list, USB development list On Wed, 19 Nov 2003, Jens Axboe wrote: > The queue already has such a restriction embedded, see bio_map_user() > and queue_dma_alignment(). Searching through the 2.6 kernel source gave some interesting results. queue_dma_alignment() does nothing but set the dma_alignment field of the request queue structure. The _only_ place this field is used is in fs/bio.c by __bio_map_user(), called from bio_map_user(). In turn, the _only_ place bio_map_user() is called is from drivers/block/scsi_ioctl.c, and that's only for a certain specific type of SCSI ioctl. It's not on any main code path, and it wouldn't get invoked by anything using the st or sg drivers. In fact, it looks like scsi_ioctl.c:scsi_cmd_ioctl() is only called from within the sd driver. So something needs to be fixed up. Is there some spot in the block layer that's supposed to be checking dma alignments but isn't doing so? Alan Stern ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 15:44 ` Alan Stern @ 2003-11-19 15:49 ` Jens Axboe 2003-11-19 16:58 ` Alan Stern 0 siblings, 1 reply; 54+ messages in thread From: Jens Axboe @ 2003-11-19 15:49 UTC (permalink / raw) To: Alan Stern Cc: Patrick Mansfield, Douglas Gilbert, James Bottomley, Oliver Neukum, SCSI development list, USB development list On Wed, Nov 19 2003, Alan Stern wrote: > On Wed, 19 Nov 2003, Jens Axboe wrote: > > > The queue already has such a restriction embedded, see bio_map_user() > > and queue_dma_alignment(). > > Searching through the 2.6 kernel source gave some interesting results. > > queue_dma_alignment() does nothing but set the dma_alignment field of the > request queue structure. The _only_ place this field is used is in > fs/bio.c by __bio_map_user(), called from bio_map_user(). In turn, the > _only_ place bio_map_user() is called is from drivers/block/scsi_ioctl.c, > and that's only for a certain specific type of SCSI ioctl. It's not on It's for SG_IO. > any main code path, and it wouldn't get invoked by anything using the st > or sg drivers. In fact, it looks like scsi_ioctl.c:scsi_cmd_ioctl() is > only called from within the sd driver. It's invoked from user space. Don't mix block and scsi scsi_ioctl() up. bio_map_user() would be used by a block layer sg driver too, for instance. > So something needs to be fixed up. Is there some spot in the block layer > that's supposed to be checking dma alignments but isn't doing so? The queue dma alignment was added to better cater to cdrecord, for instance. We need better than 512b granularity there, or you cannot use DMA on certain types of burns (lots of sector types are non-2kb aligned there). If you can 512-byte align the data and transfer from user space, _that is enough_. The program just needs to be aware of this, it's not an odd restriction. There are just scenarious where you cannot do this, and that is why I added the queue dma alignment attribute. To me, it doesn't sound like your case applies. -- Jens Axboe ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 15:49 ` Jens Axboe @ 2003-11-19 16:58 ` Alan Stern 2003-11-19 17:03 ` Jens Axboe 0 siblings, 1 reply; 54+ messages in thread From: Alan Stern @ 2003-11-19 16:58 UTC (permalink / raw) To: Jens Axboe Cc: Patrick Mansfield, Douglas Gilbert, James Bottomley, Oliver Neukum, SCSI development list, USB development list On Wed, 19 Nov 2003, Jens Axboe wrote: > > any main code path, and it wouldn't get invoked by anything using the st > > or sg drivers. In fact, it looks like scsi_ioctl.c:scsi_cmd_ioctl() is > > only called from within the sd driver. > > It's invoked from user space. Don't mix block and scsi scsi_ioctl() up. > bio_map_user() would be used by a block layer sg driver too, for > instance. You've lost me. scsi_cmd_ioctl() and bio_map_user() are _not_ used by the sg driver -- did you mean that a hypothetical block layer sg driver would call them? That doesn't seem relevant to the point of this thread. Although come to think of it, you probably never saw the start of the thread. Take a look at http://marc.theaimsgroup.com/?l=linux-usb-devel&m=106916943618090&w=2 to understand my problem. > > So something needs to be fixed up. Is there some spot in the block layer > > that's supposed to be checking dma alignments but isn't doing so? > > The queue dma alignment was added to better cater to cdrecord, for > instance. We need better than 512b granularity there, or you cannot use > DMA on certain types of burns (lots of sector types are non-2kb aligned > there). > > If you can 512-byte align the data and transfer from user space, _that > is enough_. The program just needs to be aware of this, it's not an odd > restriction. There are just scenarious where you cannot do this, and > that is why I added the queue dma alignment attribute. To me, it doesn't > sound like your case applies. This isn't a userspace problem as such; it's a problem of enforcing dma alignment restrictions within the block layer or lower. I'm not working on a user program; I'm working on the usb-storage driver. It too needs 512-byte alignment, but under the right circumstances the st and sg drivers can submit scatter-gather buffers that aren't aligned. You said earlier that misaligned buffers would be caught in the block layer queueing code, but I don't see any place where that happens. Alan Stern ------------------------------------------------------- This SF.net email is sponsored by: SF.net Giveback Program. Does SourceForge.net help you be more productive? Does it help you create better code? SHARE THE LOVE, and help us help YOU! Click Here: http://sourceforge.net/donate/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Unaligned scatter-gather buffers and usb-storage 2003-11-19 16:58 ` Alan Stern @ 2003-11-19 17:03 ` Jens Axboe 0 siblings, 0 replies; 54+ messages in thread From: Jens Axboe @ 2003-11-19 17:03 UTC (permalink / raw) To: Alan Stern Cc: Patrick Mansfield, Douglas Gilbert, James Bottomley, Oliver Neukum, SCSI development list, USB development list On Wed, Nov 19 2003, Alan Stern wrote: > On Wed, 19 Nov 2003, Jens Axboe wrote: > > > > any main code path, and it wouldn't get invoked by anything using the st > > > or sg drivers. In fact, it looks like scsi_ioctl.c:scsi_cmd_ioctl() is > > > only called from within the sd driver. > > > > It's invoked from user space. Don't mix block and scsi scsi_ioctl() up. > > bio_map_user() would be used by a block layer sg driver too, for > > instance. > > You've lost me. scsi_cmd_ioctl() and bio_map_user() are _not_ used by the > sg driver -- did you mean that a hypothetical block layer sg driver would > call them? That doesn't seem relevant to the point of this thread. Well you brought up bio_map_user(), so I'm telling where it's used and why. The block layer provides SG_IO functionality for non-scsi devices, that is its current use. > Although come to think of it, you probably never saw the start of the > thread. Take a look at > > http://marc.theaimsgroup.com/?l=linux-usb-devel&m=106916943618090&w=2 > > to understand my problem. Ok I see. Then make sg/st return -EINVAL if the address isn't 512-byte aligned would be the rigth fix, imo. > > > So something needs to be fixed up. Is there some spot in the block layer > > > that's supposed to be checking dma alignments but isn't doing so? > > > > The queue dma alignment was added to better cater to cdrecord, for > > instance. We need better than 512b granularity there, or you cannot use > > DMA on certain types of burns (lots of sector types are non-2kb aligned > > there). > > > > If you can 512-byte align the data and transfer from user space, _that > > is enough_. The program just needs to be aware of this, it's not an odd > > restriction. There are just scenarious where you cannot do this, and > > that is why I added the queue dma alignment attribute. To me, it doesn't > > sound like your case applies. > > This isn't a userspace problem as such; it's a problem of enforcing dma > alignment restrictions within the block layer or lower. I'm not working > on a user program; I'm working on the usb-storage driver. It too needs > 512-byte alignment, but under the right circumstances the st and sg > drivers can submit scatter-gather buffers that aren't aligned. You said > earlier that misaligned buffers would be caught in the block layer > queueing code, but I don't see any place where that happens. sg/st submissions would never pass through the block layer, even if they use the queue as a transport between the scsi mid layer. They need to impose this restrictions themselves. I still don't think this warrants adding a host template attribute for this. 512-byte alignment is perfectly fine. -- Jens Axboe ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: usb-storage and Sony Handycam 2003-11-07 17:29 ` Patrick Mansfield 2003-11-07 19:49 ` Alan Stern @ 2003-11-07 22:09 ` Alan Stern 1 sibling, 0 replies; 54+ messages in thread From: Alan Stern @ 2003-11-07 22:09 UTC (permalink / raw) To: Patrick Mansfield Cc: Dmitri Katchalov, Idan Sofer, USB development list, SCSI development list Patrick: For a usb-storage debugging trace of what appears to be exactly the same problem, look at http://bugme.osdl.org/show_bug.cgi?id=1501 Alan Stern ------------------------------------------------------- This SF.Net email sponsored by: ApacheCon 2003, 16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel ^ permalink raw reply [flat|nested] 54+ messages in thread
end of thread, other threads:[~2004-02-03 16:02 UTC | newest] Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1068207145.3fab8c2988d43@webmail.netregistry.net> 2003-11-07 16:21 ` usb-storage and Sony Handycam Alan Stern 2003-11-07 17:29 ` Patrick Mansfield 2003-11-07 19:49 ` Alan Stern 2003-11-08 2:54 ` Dmitri Katchalov 2003-11-08 6:34 ` Patrick Mansfield 2003-11-08 13:29 ` Dmitri Katchalov 2003-11-08 16:28 ` Alan Stern 2003-11-08 20:37 ` Patrick Mansfield 2003-11-09 3:47 ` [linux-usb-devel] " Alan Stern 2003-11-09 8:45 ` Dmitri Katchalov 2003-11-10 20:45 ` Patrick Mansfield 2003-11-10 17:59 ` Patrick Mansfield 2003-11-10 18:46 ` Alan Stern 2003-11-10 19:04 ` [linux-usb-devel] " Patrick Mansfield 2003-11-10 19:57 ` Alan Stern 2003-11-10 22:46 ` Sancho Dauskardt 2003-11-18 15:20 ` Unaligned scatter-gather buffers and usb-storage Alan Stern 2003-11-18 22:37 ` Patrick Mansfield 2003-11-19 8:47 ` Jens Axboe 2003-11-19 13:01 ` [linux-usb-devel] " Oliver Neukum 2003-11-19 13:04 ` Jens Axboe 2003-11-19 14:37 ` James Bottomley 2003-11-19 14:39 ` Jens Axboe 2003-11-19 14:58 ` James Bottomley 2003-11-19 15:00 ` [linux-usb-devel] " Jens Axboe 2003-11-19 16:56 ` Kai Makisara 2003-11-19 20:19 ` Jens Axboe 2003-11-19 22:06 ` Kai Makisara 2003-11-20 6:53 ` Jens Axboe 2003-11-20 15:20 ` Alan Stern 2003-11-20 15:30 ` Jens Axboe 2003-11-20 16:09 ` Alan Stern 2003-11-20 16:24 ` Jens Axboe 2003-11-20 16:28 ` [linux-usb-devel] " Oliver Neukum 2003-11-20 19:23 ` Kai Makisara 2003-11-20 17:18 ` Kai Makisara 2003-11-20 19:18 ` [linux-usb-devel] " Kai Mäkisara 2003-11-21 18:03 ` PATCH: (as141) " Alan Stern 2003-11-21 20:07 ` Kai Makisara 2003-12-01 1:30 ` Matthew Dharm 2004-01-05 0:41 ` Matthew Dharm 2004-01-05 10:08 ` Jens Axboe 2004-01-05 21:58 ` PATCH: (as141b) " Alan Stern 2004-01-06 11:28 ` Oliver Neukum 2004-01-06 16:10 ` Alan Stern 2004-02-02 15:51 ` James Bottomley 2004-02-03 15:47 ` Alan Stern 2004-02-03 15:55 ` James Bottomley 2004-02-03 16:02 ` Matthew Wilcox 2003-11-19 15:44 ` Alan Stern 2003-11-19 15:49 ` Jens Axboe 2003-11-19 16:58 ` Alan Stern 2003-11-19 17:03 ` Jens Axboe 2003-11-07 22:09 ` usb-storage and Sony Handycam Alan Stern
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.