RE: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
From: Sarah Sharp Greg, Dave, Freddy, question about cross-subsystem reverts below: On Fri, Feb 28, 2014 at 04:15:12PM -0500, Alan Stern wrote: On Fri, 28 Feb 2014, Sarah Sharp wrote: When testing 3.14-rc1 with a USB 3.0 Lexar flash drive, the drive fails to be mounted. I added a bit of debugging to the USB core: That revealed the SCSI request fails because the USB core is rejecting a scatter-gather list with an entry that isn't aligned to the max packet size: Feb 28 09:45:30 xanatos kernel: [ 376.449316] usb 2-2: URB sg entry 0 of 17, size 1536 not a multiple of ep1in max packet 1024 Notice the request length: 1536. That's three 512-byte sectors. A little unusual, since most I/O is done in units of pages, which are 4096 bytes. ... Some of this behavior is to be expected. 1536 isn't a multiple of 1024 (the maxpacket size when running at SuperSpeed), but it _is_ a multiple of 512 (the maxpacket size when running at high speed). Therefore the failure won't occur when the drive is attached to an EHCI controller or is behind a USB-2 hub. ... If we can't figure out how to get max-packet sized scatter-gather list entries from the mass storage driver, Mathias is going to need to: The SG entries don't come from usb-storage; they come from the block layer. As far as I know, there is no way to tell the block layer that each element in an SG list (except the last) must be a multiple of some specific size. revert commit 3804fad45411 USBNET: ax88179_178a: enable tso if usb host supports sg dma revert commit 247bf557273d xhci 1.0: Limit arbitrarily-aligned scatter gather. And we'll need to focus on getting the TD fragments supported in 3.16. So far we've gotten away with this at high speed or below, because no USB mass-storage devices have a block size smaller than 512 (at least, none that I've ever heard of). But when the maxpacket size is 1024, a request for an odd number of blocks can cause trouble. Ok, we can't have SuperSpeed mass storage devices broken, so it looks like we'll have to revert the last patch to add scatter-gather to the ASIX driver to avoid that breakage. That means Mathias is going to need to revert those two commits then, since he's taking over pushing xHCI driver bug fixes this kernel. Greg, Dave, Freddy, how do you want to handle reverting commit 3804fad45411? Should that come through Dave's networking tree or Greg's USB tree? I'm not sure what those two commits have to do with this problem. In order to support a request with a non-terminal buffer that isn't a multiple of 1k you need something to stop a LINK TRB being in the middle of the transfer. Otherwise the 1536 byte transfer (expected as a 1k block followed by 512byte one) can probably get sent as a 512byte block (terminating the bulk data request) followed by a spurious 1k block. These should generate horrid errors from the target disk. The only code I've seen anyone suggest that will correctly send these requests starts from the patches I've sent. I suspect you need to apply ALL of the patches I've sent, and then fixup the remaining problems. David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
On Tue, 4 Mar 2014, David Laight wrote: Notice the request length: 1536. That's three 512-byte sectors. A little unusual, since most I/O is done in units of pages, which are 4096 bytes. Ok, we can't have SuperSpeed mass storage devices broken, so it looks like we'll have to revert the last patch to add scatter-gather to the ASIX driver to avoid that breakage. That means Mathias is going to need to revert those two commits then, since he's taking over pushing xHCI driver bug fixes this kernel. Greg, Dave, Freddy, how do you want to handle reverting commit 3804fad45411? Should that come through Dave's networking tree or Greg's USB tree? I'm not sure what those two commits have to do with this problem. In order to support a request with a non-terminal buffer that isn't a multiple of 1k you need something to stop a LINK TRB being in the middle of the transfer. Otherwise the 1536 byte transfer (expected as a 1k block followed by 512byte one) can probably get sent as a 512byte block (terminating the bulk data request) followed by a spurious 1k block. These should generate horrid errors from the target disk. David is right; this problem can't be fixed simply by reverting patches. The real problem is that the block layer has handed the USB stack an SG list that xhci-hcd cannot handle at all, in its current form. There are only two reasonable ways to fix this: Add appropriate TRB fragment handling into xhci-hcd, or use bounce buffers for non-aligned requests. In theory the block layer could be taught about the need for these bounce buffers, but that would be only a partial solution. It would help for mass-storage transfers, but not for networking (which doesn't use the block layer). An alternative is to work around this particular problem by identifying the code that submits the 3-sector SG element, and changing it to use an even number of sectors. But obviously that doesn't solve the underlying issue. I think in the end there is no real choice but to bite the bullet and implement the TRB fragmentation rules. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
On Tue, Mar 4, 2014 at 10:56 PM, David Laight david.lai...@aculab.com wrote: From: Bjørn Mork If xHCI won't plan to support arbitrary-length scatter-gather any more, that is fine to revert the commit forever. Otherwise, it should be better to just clear no_sg_constraint in xhcd, shouldn't it? No, that's what's currently causing bugs with the storage driver. IIUC, the bug was added by commit 10e232c597ac (USB: check sg buffer size in usb_submit_urb) which introduced an additional restriction on SG URBs not present before: + for_each_sg(urb-sg, sg, urb-num_sgs - 1, i) + if (sg-length % max) + return -EINVAL; where max is usb_endpoint_maxp(ep-desc). As has been shown by numerous bug reports lately, the storage driver will submit SG lists with 512 byte elements on superspeed endpoints with max == 1024. That has never been 'fine'. So when this SS storage device(maxp isn't 512) comes and block layer may send scatter-gather list with an non-tail entry that isn't aligned to the max packet size, looks xHCD has to enable no_sg_constraint and has to deal with it correctly, and I am wondering if there is other solutions for the case. Basically this case is very similar with current usbnet(axis 88179/ 178a). Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
From: Alan Stern There are only two reasonable ways to fix this: Add appropriate TRB fragment handling into xhci-hcd, or use bounce buffers for non-aligned requests. In theory the block layer could be taught about the need for these bounce buffers, but that would be only a partial solution. It would help for mass-storage transfers, but not for networking (which doesn't use the block layer). An alternative is to work around this particular problem by identifying the code that submits the 3-sector SG element, and changing it to use an even number of sectors. But obviously that doesn't solve the underlying issue. Actually most of the block layer code could be taught to split requests into multiple URBs. Or even resubmit bounced requests split in two. Although splitting requests won't help the network code. The dma setup code could allocate bounce buffers - but you really don't want to be doing that. I think in the end there is no real choice but to bite the bullet and implement the TRB fragmentation rules. Indeed :-) David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
On Tue, Mar 4, 2014 at 11:21 PM, David Laight david.lai...@aculab.com wrote: Actually most of the block layer code could be taught to split requests into multiple URBs. Or even resubmit bounced requests split in two. Although splitting requests won't help the network code. It might not help storage too since the single URB in the middle of transfer produces short packet. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
On Tue, 4 Mar 2014, David Laight wrote: An alternative is to work around this particular problem by identifying the code that submits the 3-sector SG element, and changing it to use an even number of sectors. But obviously that doesn't solve the underlying issue. Actually most of the block layer code could be taught to split requests into multiple URBs. Or even resubmit bounced requests split in two. Although splitting requests won't help the network code. It won't help the mass-storage code either. If an SG element is shorter than the maxpacket length, splitting it up isn't going to make it longer. What _would_ help would be for the block layer to split up both this element and the next one, and then allocate a maxpacket-sized bounce buffer to hold the last part of this element plus the first part of the next. But _that_ won't help the network code. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
On Tue, 4 Mar 2014, Sarah Sharp wrote:
Scatter-gather under xHCI seems to work fine on mass storage, but it
fails with that particular ASIX device, because of how the xHCI driver
sets up the buffers on the endpoint rings. We need to implement the TD
fragment rules to avoid breaking this particular USB ethernet device.
In the meantime, mass storage works fine with scatter-gather, so we
don't want to lose hard drive performance.
Otherwise, it should be better to just
clear no_sg_constraint in xhcd, shouldn't it?
We tried that. That's what commit 247bf557273d does. It clears
no_sg_constraint for 1.0 xHCI hosts that need TD fragments (and thus
would cause the ASIX chipsets to drop packets). However, we've found
that commit breaks USB 3.0 mass storage devices.
The block layer may submit scatter-gather lists with entries that
are multiples of 512-byte blocks. That's fine for USB 2.0 devices,
where the bulk endpoint max packet size is 512 bytes. But USB 3.0
devices have bulk endpoints with a 1024 byte max packet size.
That means when the block layer submits a scatter-gather list with one
entry that includes, say, three 512-byte blocks, this code will reject
the URB if it's submitted to a USB 3.0 bulk endpoint:
int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
{
...
max = usb_endpoint_maxp(ep-desc);
...
} else if (urb-num_sgs !urb-dev-bus-no_sg_constraint
dev-speed != USB_SPEED_WIRELESS) {
struct scatterlist *sg;
int i;
for_each_sg(urb-sg, sg, urb-num_sgs - 1, i)
if (sg-length % max)
return -EINVAL;
}
This results in failures with USB 3.0 drives. For me, a failure to
auto-mount the device. For others, a read or write SCSI command
failure. We can't introduce a regression in USB 3.0 mass storage in
order to get better performance on one USB ethernet adapter.
It isn't a regression. Even without this test, the transfer will fail.
(Actually, depending on the details of your device, maybe it won't
fail. But it _should_!)
Until we get TD fragments implemented, we need to revert the commit
3804fad45411 and commit 247bf557273d. That will mean USB 3.0 mass
storage works again, and (unfortunately) the ASIX driver performance
won't be as good until we implement TD fragments.
USB-3 mass storage won't work, even after you revert those two commits.
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
From: Sarah Sharp
...
The block layer may submit scatter-gather lists with entries that
are multiples of 512-byte blocks. That's fine for USB 2.0 devices,
where the bulk endpoint max packet size is 512 bytes. But USB 3.0
devices have bulk endpoints with a 1024 byte max packet size.
That means when the block layer submits a scatter-gather list with one
entry that includes, say, three 512-byte blocks, this code will reject
the URB if it's submitted to a USB 3.0 bulk endpoint:
int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
{
...
max = usb_endpoint_maxp(ep-desc);
...
} else if (urb-num_sgs !urb-dev-bus-no_sg_constraint
dev-speed != USB_SPEED_WIRELESS) {
struct scatterlist *sg;
int i;
for_each_sg(urb-sg, sg, urb-num_sgs - 1, i)
if (sg-length % max)
return -EINVAL;
}
This results in failures with USB 3.0 drives. For me, a failure to
auto-mount the device. For others, a read or write SCSI command
failure. We can't introduce a regression in USB 3.0 mass storage in
order to get better performance on one USB ethernet adapter.
Until we get TD fragments implemented, we need to revert the commit
3804fad45411 and commit 247bf557273d. That will mean USB 3.0 mass
storage works again, and (unfortunately) the ASIX driver performance
won't be as good until we implement TD fragments.
You need to find out what happens when a request that fails the above
test gets split by a LINK TRB on the Intel 1.0 controller.
If, for example, there is a 3 sector read transfer the target will send
a 1k USB data block followed by a 512 byte one.
I suspect that the xhci controller will read the first 512 bytes into the
first buffer, hit the LINK TRB, and then terminate the data TRB with some
kind of error. It might read the last 512 bytes into the second buffer,
or might skip to the TRB list and treat the last 512 bytes as a separate
bulk transfer.
A similar write transfer is likely to generate two bulk transfers, the
first of 512 bytes, the second of 1k. What the target makes of that is
anybodies guess.
Just disabling the test won't make it work.
All it does it make it fail less often.
David
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
On Tue, Mar 04, 2014 at 10:00:16AM -0500, Alan Stern wrote: On Tue, 4 Mar 2014, David Laight wrote: Notice the request length: 1536. That's three 512-byte sectors. A little unusual, since most I/O is done in units of pages, which are 4096 bytes. Ok, we can't have SuperSpeed mass storage devices broken, so it looks like we'll have to revert the last patch to add scatter-gather to the ASIX driver to avoid that breakage. That means Mathias is going to need to revert those two commits then, since he's taking over pushing xHCI driver bug fixes this kernel. Greg, Dave, Freddy, how do you want to handle reverting commit 3804fad45411? Should that come through Dave's networking tree or Greg's USB tree? I'm not sure what those two commits have to do with this problem. In order to support a request with a non-terminal buffer that isn't a multiple of 1k you need something to stop a LINK TRB being in the middle of the transfer. Otherwise the 1536 byte transfer (expected as a 1k block followed by 512byte one) can probably get sent as a 512byte block (terminating the bulk data request) followed by a spurious 1k block. These should generate horrid errors from the target disk. David is right; this problem can't be fixed simply by reverting patches. The real problem is that the block layer has handed the USB stack an SG list that xhci-hcd cannot handle at all, in its current form. We do not know if the driver not implementing TD fragment rules impacts USB storage devices. I can certainly look into that today, with the xHCI 1.0 hosts I have on hand (Ivy Bridge and Haswell-ULT). I can experiment with shorting the ring segment so that almost every SCSI transfer has a link TRB in the middle, and use a USB 3.0 analyzer to see whether there are any short packets or abnormal traffic on the bus. There are only two reasonable ways to fix this: Add appropriate TRB fragment handling into xhci-hcd, or use bounce buffers for non-aligned requests. Or disable scatter-gather for xHCI 1.0 hosts all together. Alan, what do you suggest we do for the stable kernels in the meantime before we have TD fragment rules in place? 3.12 and 3.13 already have those two patches, and I keep getting failure reports. From a user perspective, USB 3.0 mass storage devices used to work before 3.14-rc1. Theoretically, the TD fragment rules could have caused an occasional disk glitch. Now the devices *will* fail, instead of theoretically failing. From a user perspective, this looks like a regression; the USB device obviously fails on 3.14-rc1, and may sometimes silently fail on prior kernels. So what would you have me do to fix stable kernels? In theory the block layer could be taught about the need for these bounce buffers, but that would be only a partial solution. It would help for mass-storage transfers, but not for networking (which doesn't use the block layer). An alternative is to work around this particular problem by identifying the code that submits the 3-sector SG element, and changing it to use an even number of sectors. But obviously that doesn't solve the underlying issue. I think in the end there is no real choice but to bite the bullet and implement the TRB fragmentation rules. I agree we should implement TD fragment rules. It's too late in the -rc cycle to get those finished before 3.14 final. So what do we do with stable in the meantime? Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
On Tue, 4 Mar 2014, Sarah Sharp wrote: David is right; this problem can't be fixed simply by reverting patches. The real problem is that the block layer has handed the USB stack an SG list that xhci-hcd cannot handle at all, in its current form. We do not know if the driver not implementing TD fragment rules impacts USB storage devices. I can certainly look into that today, with the xHCI 1.0 hosts I have on hand (Ivy Bridge and Haswell-ULT). I can experiment with shorting the ring segment so that almost every SCSI transfer has a link TRB in the middle, and use a USB 3.0 analyzer to see whether there are any short packets or abnormal traffic on the bus. Okay, that's certainly worth doing. It might be even easier to use testusb with gadget zero. Tests 5 and 6 exercise bulk SG, and you can set the length parameter to 512. There are only two reasonable ways to fix this: Add appropriate TRB fragment handling into xhci-hcd, or use bounce buffers for non-aligned requests. Or disable scatter-gather for xHCI 1.0 hosts all together. That won't help. In fact, it would cause all of these questionable transfers to fail, not just those that cross a ring segment boundary. Without SG support in the HCD, the scatter-gather library routine would end up creating a single URB for 1536 bytes. For READ, it would fail with an overflow error when the device sent two 1024-byte packets. For WRITE, goodness knows what the device would do with an unexpected short packet. Alan, what do you suggest we do for the stable kernels in the meantime before we have TD fragment rules in place? 3.12 and 3.13 already have those two patches, and I keep getting failure reports. At the moment, I guess the best you can do is keep the SG support in xhci-hcd, set the no_sg_constraint flag, and live with an occasional failure when a link TRB occurs at an odd sector boundary. Also disable SG in the ax88179 network driver. In other words, revert those two commits. From a user perspective, USB 3.0 mass storage devices used to work before 3.14-rc1. Theoretically, the TD fragment rules could have caused an occasional disk glitch. Now the devices *will* fail, instead of theoretically failing. From a user perspective, this looks like a regression; the USB device obviously fails on 3.14-rc1, and may sometimes silently fail on prior kernels. So what would you have me do to fix stable kernels? Reverting them seems to be the only choice for now. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
From: Alan Stern When testing 3.14-rc1 with a USB 3.0 Lexar flash drive, the drive fails to be mounted. ... That revealed the SCSI request fails because the USB core is rejecting a scatter-gather list with an entry that isn't aligned to the max packet size: Feb 28 09:45:30 xanatos kernel: [ 376.449316] usb 2-2: URB sg entry 0 of 17, size 1536 not a multiple of ep1in max packet 1024 Notice the request length: 1536. That's three 512-byte sectors. A little unusual, since most I/O is done in units of pages, which are 4096 bytes. ... Some of this behavior is to be expected. 1536 isn't a multiple of 1024 (the maxpacket size when running at SuperSpeed), but it _is_ a multiple of 512 (the maxpacket size when running at high speed). Therefore the failure won't occur when the drive is attached to an EHCI controller or is behind a USB-2 hub. It should be possible to modify the driver so that it is only necessary to have a 1024 byte boundary within a ring segment. The last fragment that crosses a 1k boundary would need to be truncated and then followed by a link TRB. The only real difficulty is that you then don't know how many TRBs will be needed until you actually start writing them. Which might mean moving the ring expansion down into the code that actually writes the TRBs. I also think it might be worth queueing some requests as URBs rather than expanding the rings so that they fit. However to do that you probably need to apply some of the 'simplifiaction' patches I wrote first (or equivalent patches). David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
Greg, Dave, Freddy, question about cross-subsystem reverts below: On Fri, Feb 28, 2014 at 04:15:12PM -0500, Alan Stern wrote: On Fri, 28 Feb 2014, Sarah Sharp wrote: When testing 3.14-rc1 with a USB 3.0 Lexar flash drive, the drive fails to be mounted. I added a bit of debugging to the USB core: That revealed the SCSI request fails because the USB core is rejecting a scatter-gather list with an entry that isn't aligned to the max packet size: Feb 28 09:45:30 xanatos kernel: [ 376.449316] usb 2-2: URB sg entry 0 of 17, size 1536 not a multiple of ep1in max packet 1024 Notice the request length: 1536. That's three 512-byte sectors. A little unusual, since most I/O is done in units of pages, which are 4096 bytes. It's failing because of commit 247bf557273d xhci 1.0: Limit arbitrarily-aligned scatter gather. That commit clears the hcd-self.no_sg_constraint flag if the host is a 1.0 host (which my Panther Point host is). It was put in to avoid TD fragment issues on 1.0 hosts with ethernet devices. (Note, this also means that David Laight's potential work-around patch [1] wouldn't help if arbitrary-length scatter gather bigger than a ring segment was submitted.) The behavior for reproducing this is odd. I can only reproduce this on my Ubuntu 13.10 laptop with Intel Panther Point xHCI, when the device is running at SuperSpeed. If I plug the device into an EHCI port, or behind a USB 2.0 hub plugged into an xHCI port, I never see these arbitrary-length scatter-gather list entries. Dan can't reproduce this on his Intel Haswell machine running Fedora at all. Some of this behavior is to be expected. 1536 isn't a multiple of 1024 (the maxpacket size when running at SuperSpeed), but it _is_ a multiple of 512 (the maxpacket size when running at high speed). Therefore the failure won't occur when the drive is attached to an EHCI controller or is behind a USB-2 hub. Ah, that makes sense now. Does the same thing happen if you prevent the system from automatically trying to mount the volume? If the system doesn't automatically mount the volume, the error doesn't occur. If we can't figure out how to get max-packet sized scatter-gather list entries from the mass storage driver, Mathias is going to need to: The SG entries don't come from usb-storage; they come from the block layer. As far as I know, there is no way to tell the block layer that each element in an SG list (except the last) must be a multiple of some specific size. revert commit 3804fad45411 USBNET: ax88179_178a: enable tso if usb host supports sg dma revert commit 247bf557273d xhci 1.0: Limit arbitrarily-aligned scatter gather. And we'll need to focus on getting the TD fragments supported in 3.16. So far we've gotten away with this at high speed or below, because no USB mass-storage devices have a block size smaller than 512 (at least, none that I've ever heard of). But when the maxpacket size is 1024, a request for an odd number of blocks can cause trouble. Ok, we can't have SuperSpeed mass storage devices broken, so it looks like we'll have to revert the last patch to add scatter-gather to the ASIX driver to avoid that breakage. That means Mathias is going to need to revert those two commits then, since he's taking over pushing xHCI driver bug fixes this kernel. Greg, Dave, Freddy, how do you want to handle reverting commit 3804fad45411? Should that come through Dave's networking tree or Greg's USB tree? Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
On Tue, Mar 4, 2014 at 5:47 AM, Sarah Sharp sarah.a.sh...@linux.intel.com wrote: Greg, Dave, Freddy, question about cross-subsystem reverts below: On Fri, Feb 28, 2014 at 04:15:12PM -0500, Alan Stern wrote: On Fri, 28 Feb 2014, Sarah Sharp wrote: When testing 3.14-rc1 with a USB 3.0 Lexar flash drive, the drive fails to be mounted. I added a bit of debugging to the USB core: That revealed the SCSI request fails because the USB core is rejecting a scatter-gather list with an entry that isn't aligned to the max packet size: Feb 28 09:45:30 xanatos kernel: [ 376.449316] usb 2-2: URB sg entry 0 of 17, size 1536 not a multiple of ep1in max packet 1024 Notice the request length: 1536. That's three 512-byte sectors. A little unusual, since most I/O is done in units of pages, which are 4096 bytes. It's failing because of commit 247bf557273d xhci 1.0: Limit arbitrarily-aligned scatter gather. That commit clears the hcd-self.no_sg_constraint flag if the host is a 1.0 host (which my Panther Point host is). It was put in to avoid TD fragment issues on 1.0 hosts with ethernet devices. (Note, this also means that David Laight's potential work-around patch [1] wouldn't help if arbitrary-length scatter gather bigger than a ring segment was submitted.) The behavior for reproducing this is odd. I can only reproduce this on my Ubuntu 13.10 laptop with Intel Panther Point xHCI, when the device is running at SuperSpeed. If I plug the device into an EHCI port, or behind a USB 2.0 hub plugged into an xHCI port, I never see these arbitrary-length scatter-gather list entries. Dan can't reproduce this on his Intel Haswell machine running Fedora at all. Some of this behavior is to be expected. 1536 isn't a multiple of 1024 (the maxpacket size when running at SuperSpeed), but it _is_ a multiple of 512 (the maxpacket size when running at high speed). Therefore the failure won't occur when the drive is attached to an EHCI controller or is behind a USB-2 hub. Ah, that makes sense now. Does the same thing happen if you prevent the system from automatically trying to mount the volume? If the system doesn't automatically mount the volume, the error doesn't occur. If we can't figure out how to get max-packet sized scatter-gather list entries from the mass storage driver, Mathias is going to need to: The SG entries don't come from usb-storage; they come from the block layer. As far as I know, there is no way to tell the block layer that each element in an SG list (except the last) must be a multiple of some specific size. revert commit 3804fad45411 USBNET: ax88179_178a: enable tso if usb host supports sg dma revert commit 247bf557273d xhci 1.0: Limit arbitrarily-aligned scatter gather. And we'll need to focus on getting the TD fragments supported in 3.16. So far we've gotten away with this at high speed or below, because no USB mass-storage devices have a block size smaller than 512 (at least, none that I've ever heard of). But when the maxpacket size is 1024, a request for an odd number of blocks can cause trouble. Ok, we can't have SuperSpeed mass storage devices broken, so it looks like we'll have to revert the last patch to add scatter-gather to the ASIX driver to avoid that breakage. That means Mathias is going to need to revert those two commits then, since he's taking over pushing xHCI driver bug fixes this kernel. Greg, Dave, Freddy, how do you want to handle reverting commit 3804fad45411? Should that come through Dave's networking tree or Greg's USB tree? Sorry, could you explain why you want to revert commit 3804fad45411 (USBNET: ax88179_178a)? If xHCI won't plan to support arbitrary-length scatter-gather any more, that is fine to revert the commit forever. Otherwise, it should be better to just clear no_sg_constraint in xhcd, shouldn't it? Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
When testing 3.14-rc1 with a USB 3.0 Lexar flash drive, the drive fails
to be mounted. I added a bit of debugging to the USB core:
diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 9ff665f1322f..eff59ac37865 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -430,9 +430,16 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
struct scatterlist *sg;
int i;
- for_each_sg(urb-sg, sg, urb-num_sgs - 1, i)
- if (sg-length % max)
+ for_each_sg(urb-sg, sg, urb-num_sgs - 1, i) {
+ if (sg-length % max) {
+ dev_dbg(dev-dev,
+ URB sg entry %d of %d, size %d
not a multiple of ep%d%s max packet %d\n,
+ i, urb-num_sgs, sg-length,
+ usb_endpoint_num(ep-desc),
+ is_out ? out : in, max);
return -EINVAL;
+ }
+ }
}
That revealed the SCSI request fails because the USB core is rejecting a
scatter-gather list with an entry that isn't aligned to the max packet size:
Feb 28 09:45:28 xanatos kernel: [ 374.351963] scsi 7:0:0:0: Direct-Access
LexarJumpDrive1.00 PQ: 0 ANSI: 6
Feb 28 09:45:28 xanatos kernel: [ 374.352751] sd 7:0:0:0: Attached scsi
generic sg1 type 0
Feb 28 09:45:28 xanatos kernel: [ 374.352847] sd 7:0:0:0: [sdb] 15667200
512-byte logical blocks: (8.02 GB/7.47 GiB)
Feb 28 09:45:28 xanatos kernel: [ 374.353014] sd 7:0:0:0: [sdb] Write Protect
is off
Feb 28 09:45:28 xanatos kernel: [ 374.353021] sd 7:0:0:0: [sdb] Mode Sense: 23
00 00 00
Feb 28 09:45:28 xanatos kernel: [ 374.353187] sd 7:0:0:0: [sdb] Write cache:
disabled, read cache: disabled, doesn't support DPO or FUA
Feb 28 09:45:28 xanatos kernel: [ 374.358458] sdb: sdb1
Feb 28 09:45:28 xanatos kernel: [ 374.359358] sd 7:0:0:0: [sdb] Attached SCSI
removable disk
Feb 28 09:45:30 xanatos kernel: [ 376.36] FAT-fs (sdb1): Volume was not
properly unmounted. Some data may be corrupt. Please run fsck.
Feb 28 09:45:30 xanatos kernel: [ 376.449316] usb 2-2: URB sg entry 0 of 17,
size 1536 not a multiple of ep1in max packet 1024
Feb 28 09:45:30 xanatos kernel: [ 376.561395] usb 2-2: reset SuperSpeed USB
device number 3 using xhci_hcd
Feb 28 09:45:30 xanatos kernel: [ 376.577350] xhci_hcd :00:14.0: xHCI
xhci_drop_endpoint called with disabled ep 880115f9f9c0
Feb 28 09:45:30 xanatos kernel: [ 376.577355] xhci_hcd :00:14.0: xHCI
xhci_drop_endpoint called with disabled ep 880115f9f980
Feb 28 09:45:30 xanatos kernel: [ 376.577804] sd 7:0:0:0: [sdb] Unhandled
error code
Feb 28 09:45:30 xanatos kernel: [ 376.577807] sd 7:0:0:0: [sdb]
Feb 28 09:45:30 xanatos kernel: [ 376.577808] Result: hostbyte=DID_ERROR
driverbyte=DRIVER_OK
Feb 28 09:45:30 xanatos kernel: [ 376.577809] sd 7:0:0:0: [sdb] CDB:
Feb 28 09:45:30 xanatos kernel: [ 376.577810] Read(10): 28 00 00 00 08 bd 00
00 80 00
Feb 28 09:45:30 xanatos kernel: [ 376.577816] end_request: I/O error, dev sdb,
sector 2237
Feb 28 09:45:48 xanatos kernel: [ 394.569635] usb 2-2: USB disconnect, device
number 3
Feb 28 09:46:01 xanatos kernel: [ 407.165252] sd 7:0:0:0: [sdb] Unhandled
error code
Feb 28 09:46:01 xanatos kernel: [ 407.165260] sd 7:0:0:0: [sdb]
Feb 28 09:46:01 xanatos kernel: [ 407.165264] Result: hostbyte=DID_NO_CONNECT
driverbyte=DRIVER_OK
Feb 28 09:46:01 xanatos kernel: [ 407.165267] sd 7:0:0:0: [sdb] CDB:
Feb 28 09:46:01 xanatos kernel: [ 407.165269] Read(10): 28 00 00 00 08 be 00
00 7f 00
Feb 28 09:46:01 xanatos kernel: [ 407.165287] end_request: I/O error, dev sdb,
sector 2238
Feb 28 09:46:01 xanatos kernel: [ 407.165404] FAT-fs (sdb1): FAT read failed
(blocknr 2109)
Feb 28 09:46:01 xanatos kernel: [ 407.165413] sd 7:0:0:0: [sdb] Unhandled
error code
Feb 28 09:46:01 xanatos kernel: [ 407.165419] sd 7:0:0:0: [sdb]
Feb 28 09:46:01 xanatos kernel: [ 407.165424] Result: hostbyte=DID_NO_CONNECT
driverbyte=DRIVER_OK
Feb 28 09:46:01 xanatos kernel: [ 407.165429] sd 7:0:0:0: [sdb] CDB:
Feb 28 09:46:01 xanatos kernel: [ 407.165434] Read(10): 28 00 00 00 09 3d 00
00 7d 00
Feb 28 09:46:01 xanatos kernel: [ 407.165469] end_request: I/O error, dev sdb,
sector 2365
Feb 28 09:46:01 xanatos kernel: [ 407.233418] FAT-fs (sdb1): Directory
bread(block 32640) failed
It's failing because of commit 247bf557273d xhci 1.0: Limit
arbitrarily-aligned scatter gather. That commit clears the
hcd-self.no_sg_constraint flag if the host is a 1.0 host (which my
Panther Point host is). It was put in to avoid TD fragment issues on
1.0 hosts with ethernet devices. (Note, this also means that David
Laight's potential work-around patch [1] wouldn't help if
arbitrary-length scatter gather bigger than a ring segment was
Re: 3.13-rc1 regression: Scatter-gather list issues at SuperSpeed only
On Fri, 28 Feb 2014, Sarah Sharp wrote:
When testing 3.14-rc1 with a USB 3.0 Lexar flash drive, the drive fails
to be mounted. I added a bit of debugging to the USB core:
diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 9ff665f1322f..eff59ac37865 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -430,9 +430,16 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
struct scatterlist *sg;
int i;
- for_each_sg(urb-sg, sg, urb-num_sgs - 1, i)
- if (sg-length % max)
+ for_each_sg(urb-sg, sg, urb-num_sgs - 1, i) {
+ if (sg-length % max) {
+ dev_dbg(dev-dev,
+ URB sg entry %d of %d, size
%d not a multiple of ep%d%s max packet %d\n,
+ i, urb-num_sgs, sg-length,
+ usb_endpoint_num(ep-desc),
+ is_out ? out : in, max);
return -EINVAL;
+ }
+ }
}
That revealed the SCSI request fails because the USB core is rejecting a
scatter-gather list with an entry that isn't aligned to the max packet size:
Feb 28 09:45:30 xanatos kernel: [ 376.449316] usb 2-2: URB sg entry 0 of 17,
size 1536 not a multiple of ep1in max packet 1024
Notice the request length: 1536. That's three 512-byte sectors. A
little unusual, since most I/O is done in units of pages, which are
4096 bytes.
It's failing because of commit 247bf557273d xhci 1.0: Limit
arbitrarily-aligned scatter gather. That commit clears the
hcd-self.no_sg_constraint flag if the host is a 1.0 host (which my
Panther Point host is). It was put in to avoid TD fragment issues on
1.0 hosts with ethernet devices. (Note, this also means that David
Laight's potential work-around patch [1] wouldn't help if
arbitrary-length scatter gather bigger than a ring segment was
submitted.)
The behavior for reproducing this is odd. I can only reproduce this on
my Ubuntu 13.10 laptop with Intel Panther Point xHCI, when the device is
running at SuperSpeed. If I plug the device into an EHCI port, or
behind a USB 2.0 hub plugged into an xHCI port, I never see these
arbitrary-length scatter-gather list entries. Dan can't reproduce this
on his Intel Haswell machine running Fedora at all.
Some of this behavior is to be expected. 1536 isn't a multiple of 1024
(the maxpacket size when running at SuperSpeed), but it _is_ a multiple
of 512 (the maxpacket size when running at high speed). Therefore the
failure won't occur when the drive is attached to an EHCI controller or
is behind a USB-2 hub.
The only speed-based decision I see usb-storage making is whether to
clear the US_FLOW_GO_SLOW flag. Changing that to set the flag for
SuperSpeed devices as well didn't help:
That has nothing to do with this. The GO_SLOW flag tells usb-storage
to add a delay between the command phase and the data phase; it doesn't
affect the sizes of the transfers.
I'm stumped as to why we get arbitrary-length scatter-gather for
SuperSpeed devices only,
You don't. The same transfers occur at high speed -- it's just that
they don't cause an error.
and only under Ubuntu. Perhaps this comes
directly from userspace?
Maybe. I doubt the kernel would issue such a request on its own. A
usbmon trace might shed a little light, although it won't tell what
program (if any) issued the request.
Does the same thing happen if you prevent the system from automatically
trying to mount the volume?
If we can't figure out how to get max-packet sized scatter-gather list
entries from the mass storage driver, Mathias is going to need to:
The SG entries don't come from usb-storage; they come from the block
layer. As far as I know, there is no way to tell the block layer that
each element in an SG list (except the last) must be a multiple of
some specific size.
revert commit 3804fad45411 USBNET: ax88179_178a: enable tso if usb host
supports sg dma
revert commit 247bf557273d xhci 1.0: Limit arbitrarily-aligned scatter gather.
And we'll need to focus on getting the TD fragments supported in 3.16.
So far we've gotten away with this at high speed or below, because no
USB mass-storage devices have a block size smaller than 512 (at least,
none that I've ever heard of). But when the maxpacket size is 1024, a
request for an odd number of blocks can cause trouble.
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
