RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-21 Thread David Laight
My suspicion is that long SG lists are unusual - otherwise the ring expansion code would have been needed much earlier. Can anyone remember whether that was needed because of long SG lists or because of large numbers of outstanding requests? I've seen it for network cards - but only

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-20 Thread David Laight
From: Alan Stern [mailto:st...@rowland.harvard.edu] On Tue, 19 Nov 2013, Sarah Sharp wrote: The xHCI driver can limit the number of sg-list entries through hcd-self.sg_tablesize. It's currently set to ~0, which is however many entries you want. You could set that to the number of TRBs

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-20 Thread David Laight
From: Sarah Sharp ... (Also, usb-storage aligns the block sizes to 512K, which explains why we've never had an issue with TD fragments with that driver.) What is a 'block' in that context? 512K sounds more like the value that very long transfers get chopped up into. With 4k pages that might be

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-20 Thread Eric Dumazet
On Wed, 2013-11-20 at 09:36 +, David Laight wrote: Ben said the largest number of fragments from the current network stack will be 17, and that none of them will cross 32k boundaries. So the network stack won't send down long SG lists. Please note that skb-head itself _might_ cross a 32K

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-20 Thread David Laight
From: Eric Dumazet [mailto:eric.duma...@gmail.com] On Wed, 2013-11-20 at 09:36 +, David Laight wrote: Ben said the largest number of fragments from the current network stack will be 17, and that none of them will cross 32k boundaries. So the network stack won't send down long SG

Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-20 Thread Sarah Sharp
On Wed, Nov 20, 2013 at 09:36:11AM -, David Laight wrote: From: Alan Stern [mailto:st...@rowland.harvard.edu] On Tue, 19 Nov 2013, Sarah Sharp wrote: The xHCI driver can limit the number of sg-list entries through hcd-self.sg_tablesize. It's currently set to ~0, which is however

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-20 Thread Alan Stern
On Wed, 20 Nov 2013, David Laight wrote: From: Sarah Sharp ... (Also, usb-storage aligns the block sizes to 512K, which explains why we've never had an issue with TD fragments with that driver.) What is a 'block' in that context? I think Sarah means that usb-storage requires the block

Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-20 Thread Sarah Sharp
On Wed, Nov 20, 2013 at 09:46:08AM -, David Laight wrote: From: Sarah Sharp ... (Also, usb-storage aligns the block sizes to 512K, which explains why we've never had an issue with TD fragments with that driver.) What is a 'block' in that context? 512K sounds more like the value that

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-20 Thread David Laight
From: Sarah Sharp [mailto:sarah.a.sh...@linux.intel.com] On Wed, Nov 20, 2013 at 09:36:11AM -, David Laight wrote: From: Alan Stern [mailto:st...@rowland.harvard.edu] On Tue, 19 Nov 2013, Sarah Sharp wrote: The xHCI driver can limit the number of sg-list entries through

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-20 Thread David Laight
From: Alan Stern [mailto:st...@rowland.harvard.edu] On Wed, 20 Nov 2013, David Laight wrote: From: Sarah Sharp ... (Also, usb-storage aligns the block sizes to 512K, which explains why we've never had an issue with TD fragments with that driver.) What is a 'block' in that

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-20 Thread David Laight
Ok, so the networking layer should be fine. However, with the current patch, if the mass storage driver sends down a scatter-gather list that's bigger than a ring segment, or needs to be split up so it doesn't cross 64K boundaries, then the URB submission will fail. We don't want that to

Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-20 Thread Sarah Sharp
On Wed, Nov 20, 2013 at 05:16:12PM -, David Laight wrote: Ok, so the networking layer should be fine. However, with the current patch, if the mass storage driver sends down a scatter-gather list that's bigger than a ring segment, or needs to be split up so it doesn't cross 64K

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-20 Thread Paul Zimmerman
From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of David Laight Sent: Wednesday, November 20, 2013 9:16 AM Ok, so the networking layer should be fine. However, with the current patch, if the mass storage driver sends down a scatter-gather list

Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-19 Thread Sarah Sharp
: Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst. On Mon, 2013-11-18 at 09:48 +, David Laight wrote: But the minimum fragment size is (probably) 4k. For the network stack an OUT transfer might have a lot (and I mean lots) of fragments (there may

Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-19 Thread Alan Stern
On Tue, 19 Nov 2013, Sarah Sharp wrote: The xHCI driver can limit the number of sg-list entries through hcd-self.sg_tablesize. It's currently set to ~0, which is however many entries you want. You could set that to the number of TRBs in a segment (minus one for the link TRB). The

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-18 Thread David Laight
But the minimum fragment size is (probably) 4k. For the network stack an OUT transfer might have a lot (and I mean lots) of fragments (there may be constraints, and linearising the skb is a option). [...] The maximum number of fragments in the skb is going to be 17 (including the

Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-18 Thread Ben Hutchings
On Mon, 2013-11-18 at 09:48 +, David Laight wrote: But the minimum fragment size is (probably) 4k. For the network stack an OUT transfer might have a lot (and I mean lots) of fragments (there may be constraints, and linearising the skb is a option). [...] The maximum number

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-18 Thread David Laight
-Original Message- From: Ben Hutchings [mailto:bhutchi...@solarflare.com] Sent: 18 November 2013 15:03 To: David Laight Cc: Alan Stern; Sarah Sharp; net...@vger.kernel.org; linux-usb@vger.kernel.org Subject: Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst

Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-15 Thread Ben Hutchings
On Wed, 2013-11-13 at 16:58 +, David Laight wrote: [...] You can split a bulk TD on a 1k boundary and the target won't know the difference. The target won't know the difference, but the host will. Here's an example: Suppose the driver submits two URBs, each for a data-in

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-13 Thread David Laight
Since you don't really want to do all the work twice, the sensible way is to add each input fragment to the ring one a time. If you need to cross a link TRB and the last MBP boundary is within the previous data TRB then you can split the previous data TRB at the MBP boundary and

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-13 Thread David Laight
that doesn't matter; we don't get an interrupt when a ring segment is crossed. Instead we set the interrupt-on-completion flag on the last TRB of the TD, not on any earlier fragment or link TRB. That's because you don't worry about handling URBs which require too many TRBs (i.e., more

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-13 Thread Alan Stern
On Wed, 13 Nov 2013, David Laight wrote: Since you don't really want to do all the work twice, the sensible way is to add each input fragment to the ring one a time. If you need to cross a link TRB and the last MBP boundary is within the previous data TRB then you can split the

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-13 Thread Alan Stern
On Wed, 13 Nov 2013, David Laight wrote: that doesn't matter; we don't get an interrupt when a ring segment is crossed. Instead we set the interrupt-on-completion flag on the last TRB of the TD, not on any earlier fragment or link TRB. That's because you don't worry about handling

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-13 Thread David Laight
-Original Message- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: 13 November 2013 16:21 To: David Laight Cc: Sarah Sharp; net...@vger.kernel.org; linux-usb@vger.kernel.org Subject: RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst. On Wed, 13

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-12 Thread David Laight
You're right. I do wish the spec had been written more clearly. I've read a lot of hardware specs in my time ... Reading it all again makes me think that a LINK trb is only allowed on the burst boundary (which might be 16k bytes). The only real way to implement that is to ensure that TD

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-12 Thread Alan Stern
On Tue, 12 Nov 2013, David Laight wrote: You're right. I do wish the spec had been written more clearly. I've read a lot of hardware specs in my time ... Reading it all again makes me think that a LINK trb is only allowed on the burst boundary (which might be 16k bytes). The only

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-12 Thread David Laight
If all the fragments are larger than the MBP (assume 16k) then that would be relatively easy. However that is very dependant on the source of the data. It might be true for disk data, but is unlikely to be true for ethernet data. I don't quite understand your point. Are you saying that

Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-12 Thread Sarah Sharp
On Mon, Nov 11, 2013 at 03:34:53PM -0500, Alan Stern wrote: On Mon, 11 Nov 2013, David Laight wrote: Suppose, for example, the MBP is 1024. If you have a TD with length 1500, and if it had only one fragment, the last (and only) fragment's length would not less than the MBP and it

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-12 Thread David Laight
On Fri, Nov 08, 2013 at 11:07:41AM -, David Laight wrote: While this change improves things a lot (it runs for 20 minutes rather than a few seconds), there is still something else wrong. Almost certainly caused by an unrelated local change. Ok, good, I was concerned there was

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-12 Thread David Laight
That's one way to do it. Or you could allow a Link TRB at an intermediate MBP boundary. I like this idea instead. The xHCI driver should be modified to be able to handle link TRBs in the middle of the segments (the cancellation code would have to be touched as well). We would keep a

Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-12 Thread Alan Stern
On Tue, 12 Nov 2013, Sarah Sharp wrote: It comes down to a question of how often you want the controller to issue an interrupt. If a ring segment is 4 KB (one page), then it can hold 256 TRBs. With scatter-gather transfers, each SG element typically refers to something like a 2-page

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-11 Thread David Laight
From: Alan Stern [mailto:st...@rowland.harvard.edu] On Fri, 8 Nov 2013, David Laight wrote: ... GET_MAX_PACKET always returns MaxPacketSize, and for USB-3 bulk endpoints, MaxPacketSize is always 1024. MaxBurstSize can be anything from 1 to 16. I've just read something that explained about

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-11 Thread Alan Stern
On Mon, 11 Nov 2013, David Laight wrote: Suppose, for example, the MBP is 1024. If you have a TD with length 1500, and if it had only one fragment, the last (and only) fragment's length would not less than the MBP and it would not be an exact multiple of the MBP. That doesn't matter -

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-08 Thread David Laight
From: Sarah Sharp [mailto:sarah.a.sh...@linux.intel.com] On Thu, Nov 07, 2013 at 05:20:49PM -, David Laight wrote: Section 4.11.7.1 of rev 1.0 of the xhci specification states that a link TRB can only occur at a boundary between underlying USB frames (512 bytes for 480M). Which

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-08 Thread David Laight
While this change improves things a lot (it runs for 20 minutes rather than a few seconds), there is still something else wrong. Almost certainly caused by an unrelated local change. David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message

Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-08 Thread Alan Stern
On Thu, 7 Nov 2013, Sarah Sharp wrote: On Thu, Nov 07, 2013 at 05:20:49PM -, David Laight wrote: Section 4.11.7.1 of rev 1.0 of the xhci specification states that a link TRB can only occur at a boundary between underlying USB frames (512 bytes for 480M). Which version of the

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-08 Thread David Laight
-Original Message- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: 08 November 2013 16:46 To: Sarah Sharp Cc: David Laight; net...@vger.kernel.org; linux-usb@vger.kernel.org Subject: Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst. On Thu, 7

RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-08 Thread Alan Stern
On Fri, 8 Nov 2013, David Laight wrote: The whole idea of TD fragments is exceedingly cloudy. The definition doesn't make sense, and the spec doesn't say what the actual hardware restrictions are, i.e., what is the underlying reality that the TD fragment concept wants to express. I

[PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-07 Thread David Laight
Section 4.11.7.1 of rev 1.0 of the xhci specification states that a link TRB can only occur at a boundary between underlying USB frames (512 bytes for 480M). If this isn't done the USB frames aren't formatted correctly and, for example, the USB3 ethernet ax88179_178a card will stop sending

Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

2013-11-07 Thread Sarah Sharp
On Thu, Nov 07, 2013 at 05:20:49PM -, David Laight wrote: Section 4.11.7.1 of rev 1.0 of the xhci specification states that a link TRB can only occur at a boundary between underlying USB frames (512 bytes for 480M). Which version of the spec are you looking at? I'm looking at the