[GIT PULL] xhci: Regression fix for 3.13.

[GIT PULL] xhci: Regression fix for 3.13.

[Sarah Sharp]

The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:

  Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-linus (2013-11-27 09:49:03 -0800)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02

for you to fetch changes up to 35773dac5f862cb1c82ea151eba3e2f6de51ec3e:

  usb: xhci: Link TRB must not occur within a USB payload burst (2013-12-02 11:57:10 -0800)

----------------------------------------------------------------
xhci: Regression fix for 3.13.

Hi Greg,

Here's one bug fix for 3.13.

usb-net added support for bulk scatter-gather in 3.12, and it triggered a
bug in the xHCI driver.  This bug causes xHCI hosts to send an unexpected
short transfer, which will cause the USB ethernet device to stop sending
packets.

The patch is marked for the 3.12 stable kernel.  It's a long standing bug,
but the usb-net drivers are the first to trigger it.  The only other
driver that does bulk scatter-gather (usb-storage) will not trigger this
bug.

I'm not sure what the effect of the no-op TRBs will be on various xHCI
host controllers, so I would only like to be conservative and only queue
it for 3.13 and 3.12 stable.

Please queue this for 3.13.

Sarah Sharp

----------------------------------------------------------------
David Laight (1):
      usb: xhci: Link TRB must not occur within a USB payload burst

 drivers/usb/host/xhci-ring.c |   54 ++++++++++++++++++++++++++++++++++++++++-
 include/linux/usb.h          |    2 +
 2 files changed, 54 insertions(+), 2 deletions(-)

[PATCH 1/1] usb: xhci: Link TRB must not occur within a USB payload burst

[Sarah Sharp]

From: David Laight <David.Laight@ACULAB.COM>

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
high speed devices).

If this isn't done the USB frames aren't formatted correctly and, for example,
the USB3 ethernet ax88179_178a card will stop sending (while still receiving)
when running a netperf tcp transmit test with (say) and 8k buffer.

This should be a candidate for stable, the ax88179_178a driver defaults to
gso and tso enabled so it passes a lot of fragmented skb to the USB stack.

Notes from Sarah:

Discussion: http://marc.info/?l=linux-usb&m=138384509604981&w=2

This patch fixes a long-standing xHCI driver bug that was revealed by a
change in 3.12 in the usb-net driver.  Commit
638c5115a794981441246fa8fa5d95c1875af5ba "USBNET: support DMA SG" added
support to use bulk endpoint scatter-gather (urb->sg).  Only the USB
ethernet drivers trigger this bug, because the mass storage driver sends
sg list entries in page-sized chunks.

This patch only fixes the issue for bulk endpoint scatter-gather.  The
problem will still occur for periodic endpoints, because hosts will
interpret no-op transfers as a request to skip a service interval, which
is not what we want.

Luckily, the USB core isn't set up for scatter-gather on isochronous
endpoints, and no USB drivers use scatter-gather for interrupt
endpoints.  Document this known limitation so that developers won't try
to use urb->sg for interrupt endpoints until this issue is fixed.  The
more comprehensive fix would be to allow link TRBs in the middle of the
endpoint ring and revert this patch, but that fix would touch too much
code to be allowed in for stable.

This patch should be backported to kernels as old as 3.12, that contain
the commit 638c5115a794981441246fa8fa5d95c1875af5ba "USBNET: support DMA
SG".  Without this patch, the USB network device gets wedged, and stops
sending packets.  Mark Lord confirms this patch fixes the regression:

http://marc.info/?l=linux-netdev&m=138487107625966&w=2

Signed-off-by: David Laight <david.laight@aculab.com>
Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Tested-by: Mark Lord <mlord@pobox.com>
Cc: stable@vger.kernel.org
---
 drivers/usb/host/xhci-ring.c | 54 ++++++++++++++++++++++++++++++++++++++++++--
 include/linux/usb.h          |  2 ++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1e2f3f495843..53c2e296467f 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2973,8 +2973,58 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
 	}
 
 	while (1) {
-		if (room_on_ring(xhci, ep_ring, num_trbs))
-			break;
+		if (room_on_ring(xhci, ep_ring, num_trbs)) {
+			union xhci_trb *trb = ep_ring->enqueue;
+			unsigned int usable = ep_ring->enq_seg->trbs +
+					TRBS_PER_SEGMENT - 1 - trb;
+			u32 nop_cmd;
+
+			/*
+			 * Section 4.11.7.1 TD Fragments states that a link
+			 * TRB must only occur at the boundary between
+			 * data bursts (eg 512 bytes for 480M).
+			 * While it is possible to split a large fragment
+			 * we don't know the size yet.
+			 * Simplest solution is to fill the trb before the
+			 * LINK with nop commands.
+			 */
+			if (num_trbs == 1 || num_trbs <= usable || usable == 0)
+				break;
+
+			if (ep_ring->type != TYPE_BULK)
+				/*
+				 * While isoc transfers might have a buffer that
+				 * crosses a 64k boundary it is unlikely.
+				 * Since we can't add NOPs without generating
+				 * gaps in the traffic just hope it never
+				 * happens at the end of the ring.
+				 * This could be fixed by writing a LINK TRB
+				 * instead of the first NOP - however the
+				 * TRB_TYPE_LINK_LE32() calls would all need
+				 * changing to check the ring length.
+				 */
+				break;
+
+			if (num_trbs >= TRBS_PER_SEGMENT) {
+				xhci_err(xhci, "Too many fragments %d, max %d\n",
+						num_trbs, TRBS_PER_SEGMENT - 1);
+				return -ENOMEM;
+			}
+
+			nop_cmd = cpu_to_le32(TRB_TYPE(TRB_TR_NOOP) |
+					ep_ring->cycle_state);
+			ep_ring->num_trbs_free -= usable;
+			do {
+				trb->generic.field[0] = 0;
+				trb->generic.field[1] = 0;
+				trb->generic.field[2] = 0;
+				trb->generic.field[3] = nop_cmd;
+				trb++;
+			} while (--usable);
+			ep_ring->enqueue = trb;
+			if (room_on_ring(xhci, ep_ring, num_trbs))
+				break;
+		}
 
 		if (ep_ring == xhci->cmd_ring) {
 			xhci_err(xhci, "Do not support expand command ring\n");
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 7454865ad148..512ab162832c 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1264,6 +1264,8 @@ typedef void (*usb_complete_t)(struct urb *);
  * @sg: scatter gather buffer list, the buffer size of each element in
  * 	the list (except the last) must be divisible by the endpoint's
  * 	max packet size if no_sg_constraint isn't set in 'struct usb_bus'
+ * 	(FIXME: scatter-gather under xHCI is broken for periodic transfers.
+ * 	Do not use urb->sg for interrupt endpoints for now, only bulk.)
  * @num_mapped_sgs: (internal) number of mapped sg entries
  * @num_sgs: number of entries in the sg list
  * @transfer_buffer_length: How big is transfer_buffer.  The transfer may
-- 
1.8.3.3

Re: [GIT PULL] xhci: Regression fix for 3.13.

[Greg Kroah-Hartman]

On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
> 
>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-linus (2013-11-27 09:49:03 -0800)
> 
> are available in the git repository at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02

Pulled and pushed out, thanks.

greg k-h

Re: [GIT PULL] xhci: Regression fix for 3.13.

[Mark Lord]

[-- Attachment #1: Type: text/plain, Size: 2032 bytes --]

On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
> On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
>> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
>>
>>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-linus (2013-11-27 09:49:03 -0800)
>>
>> are available in the git repository at:
>>
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
> 
> Pulled and pushed out, thanks.

Did this commit make it into linux-3.12.3 ?

I ask, because the NIC still locks up with that kernel,
and even with the patch I had been using from David Laight.

Reverting the change that originally broke it still works though.
Could we please get this reverted until such time as a reworked
patch can be prepared for it?

This is what I am reverting locally to make it all work
as it did prior to linux-3.12 was released.  Unmangled copy also attached:

--- linux/drivers/net/usb/ax88179_178a.c.orig	2013-11-03 18:41:51.000000000 -0500
+++ linux/drivers/net/usb/ax88179_178a.c	2013-11-17 13:23:39.525734277 -0500
@@ -1177,18 +1177,31 @@
 	int frame_size = dev->maxpacket;
 	int mss = skb_shinfo(skb)->gso_size;
 	int headroom;
+	int tailroom;

 	tx_hdr1 = skb->len;
 	tx_hdr2 = mss;
 	if (((skb->len + 8) % frame_size) == 0)
 		tx_hdr2 |= 0x80008000;	/* Enable padding */

-	headroom = skb_headroom(skb) - 8;
+	headroom = skb_headroom(skb);
+	tailroom = skb_tailroom(skb);

-	if ((skb_header_cloned(skb) || headroom < 0) &&
-	    pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
+	if (!skb_header_cloned(skb) &&
+	    !skb_cloned(skb) &&
+	    (headroom + tailroom) >= 8) {
+		if (headroom < 8) {
+			skb->data = memmove(skb->head + 8, skb->data, skb->len);
+			skb_set_tail_pointer(skb, skb->len);
+		}
+	} else {
+		struct sk_buff *skb2;
+
+		skb2 = skb_copy_expand(skb, 8, 0, flags);
 		dev_kfree_skb_any(skb);
-		return NULL;
+		skb = skb2;
+		if (!skb)
+			return NULL;
 	}

 	skb_push(skb, 4);

[-- Attachment #2: 51_ax88179_178a_revert_3.12_lockups.patch --]
[-- Type: text/x-patch, Size: 1063 bytes --]

--- linux/drivers/net/usb/ax88179_178a.c.orig	2013-11-03 18:41:51.000000000 -0500
+++ linux/drivers/net/usb/ax88179_178a.c	2013-11-17 13:23:39.525734277 -0500
@@ -1177,18 +1177,31 @@
 	int frame_size = dev->maxpacket;
 	int mss = skb_shinfo(skb)->gso_size;
 	int headroom;
+	int tailroom;
 
 	tx_hdr1 = skb->len;
 	tx_hdr2 = mss;
 	if (((skb->len + 8) % frame_size) == 0)
 		tx_hdr2 |= 0x80008000;	/* Enable padding */
 
-	headroom = skb_headroom(skb) - 8;
+	headroom = skb_headroom(skb);
+	tailroom = skb_tailroom(skb);
 
-	if ((skb_header_cloned(skb) || headroom < 0) &&
-	    pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
+	if (!skb_header_cloned(skb) &&
+	    !skb_cloned(skb) &&
+	    (headroom + tailroom) >= 8) {
+		if (headroom < 8) {
+			skb->data = memmove(skb->head + 8, skb->data, skb->len);
+			skb_set_tail_pointer(skb, skb->len);
+		}
+	} else {
+		struct sk_buff *skb2;
+
+		skb2 = skb_copy_expand(skb, 8, 0, flags);
 		dev_kfree_skb_any(skb);
-		return NULL;
+		skb = skb2;
+		if (!skb)
+			return NULL;
 	}
 
 	skb_push(skb, 4);

Re: [GIT PULL] xhci: Regression fix for 3.13.

[Greg Kroah-Hartman]

On Fri, Dec 06, 2013 at 12:55:23AM -0500, Mark Lord wrote:
> On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
> > On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
> >> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
> >>
> >>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-linus (2013-11-27 09:49:03 -0800)
> >>
> >> are available in the git repository at:
> >>
> >>
> >>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
> > 
> > Pulled and pushed out, thanks.
> 
> Did this commit make it into linux-3.12.3 ?

There's this cool tool called 'git' where you can look these things
up... :)

No, it's not there yet, as it's not in Linus's tree yet, give me a
chance...

greg k-h

RE: [GIT PULL] xhci: Regression fix for 3.13.

[David Laight]

> From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> On Fri, Dec 06, 2013 at 12:55:23AM -0500, Mark Lord wrote:
> > On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
> > > On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
> > >> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
> > >>
> > >>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into
> usb-linus (2013-11-27 09:49:03 -0800)
> > >>
> > >> are available in the git repository at:
> > >>
> > >>
> > >>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
> > >
> > > Pulled and pushed out, thanks.
> >
> > Did this commit make it into linux-3.12.3 ?
> 
> There's this cool tool called 'git' where you can look these things
> up... :)

If you are really desperate to find what is in a specific kernel use
objdump -d.

	David

Re: [GIT PULL] xhci: Regression fix for 3.13.

[Mark Lord]

On 13-12-06 10:25 AM, Greg Kroah-Hartman wrote:
> On Fri, Dec 06, 2013 at 12:55:23AM -0500, Mark Lord wrote:
>> On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
>>> On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
>>>> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
>>>>
>>>>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-linus (2013-11-27 09:49:03 -0800)
>>>>
>>>> are available in the git repository at:
>>>>
>>>>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
>>>
>>> Pulled and pushed out, thanks.
>>
>> Did this commit make it into linux-3.12.3 ?
>
> No, it's not there yet, as it's not in Linus's tree yet, give me a chance...

Okay, good to see it moving along.

I asked about 3.12.3, because something new in that release broke
Davids workaround for me here.

But I'm with the rest of you: let's let what's in the pipeline
make it out there (3.12.4 ?) and then I'll retest and repatch that
if necessary to make things work here again.

Thanks
-- 
Mark Lord
Real-Time Remedies Inc.
mlord-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] xhci: Regression fix for 3.13.

[Sarah Sharp]

On Fri, Dec 06, 2013 at 11:10:29PM -0500, Mark Lord wrote:
> On 13-12-06 10:25 AM, Greg Kroah-Hartman wrote:
> > On Fri, Dec 06, 2013 at 12:55:23AM -0500, Mark Lord wrote:
> >> On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
> >>> On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
> >>>> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
> >>>>
> >>>>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-linus (2013-11-27 09:49:03 -0800)
> >>>>
> >>>> are available in the git repository at:
> >>>>
> >>>>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
> >>>
> >>> Pulled and pushed out, thanks.
> >>
> >> Did this commit make it into linux-3.12.3 ?
> >
> > No, it's not there yet, as it's not in Linus's tree yet, give me a chance...
> 
> Okay, good to see it moving along.
> 
> I asked about 3.12.3, because something new in that release broke
> Davids workaround for me here.
> 
> But I'm with the rest of you: let's let what's in the pipeline
> make it out there (3.12.4 ?) and then I'll retest and repatch that
> if necessary to make things work here again.

Hi Mark,

3.13-rc4 is out now, and it includes David's work-around.  Can you
please test with that kernel and confirm it fixes your issues with the
USB ethernet adapter?

Thanks,
Sarah Sharp

Re: [GIT PULL] xhci: Regression fix for 3.13.

[Mark Lord]

On 13-12-16 04:21 PM, Sarah Sharp wrote:
> On Fri, Dec 06, 2013 at 11:10:29PM -0500, Mark Lord wrote:
>> On 13-12-06 10:25 AM, Greg Kroah-Hartman wrote:
>>> On Fri, Dec 06, 2013 at 12:55:23AM -0500, Mark Lord wrote:
>>>> On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
>>>>> On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
>>>>>> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
>>>>>>
>>>>>>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-linus (2013-11-27 09:49:03 -0800)
>>>>>>
>>>>>> are available in the git repository at:
>>>>>>
>>>>>>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
>>>>>
>>>>> Pulled and pushed out, thanks.
>>>>
>>>> Did this commit make it into linux-3.12.3 ?
>>>
>>> No, it's not there yet, as it's not in Linus's tree yet, give me a chance...
>>
>> Okay, good to see it moving along.
>>
>> I asked about 3.12.3, because something new in that release broke
>> Davids workaround for me here.
>>
>> But I'm with the rest of you: let's let what's in the pipeline
>> make it out there (3.12.4 ?) and then I'll retest and repatch that
>> if necessary to make things work here again.
> 
> Hi Mark,
> 
> 3.13-rc4 is out now, and it includes David's work-around.  Can you
> please test with that kernel and confirm it fixes your issues with the
> USB ethernet adapter?

Just giving it a whirl here tonight.  Looks good, thanks.
I won't have any further opportunity to thrash it until the New Year.

Thanks all!
-- 
Mark Lord
Real-Time Remedies Inc.
mlord-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] xhci: Regression fix for 3.13.

[Sarah Sharp]

On Fri, Dec 06, 2013 at 12:55:23AM -0500, Mark Lord wrote:
> On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
> > On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
> >> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
> >>
> >>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-linus (2013-11-27 09:49:03 -0800)
> >>
> >> are available in the git repository at:
> >>
> >>
> >>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
> > 
> > Pulled and pushed out, thanks.
> 
> Did this commit make it into linux-3.12.3 ?
> 
> I ask, because the NIC still locks up with that kernel,
> and even with the patch I had been using from David Laight.
> 
> Reverting the change that originally broke it still works though.
> Could we please get this reverted until such time as a reworked
> patch can be prepared for it?

Greg, can you please revert David's patch (commit 35773dac5f86 from your
usb-linus branch)?

David said the patch fixed this lock up issue with his USB ethernet
adapter, but Mark says it doesn't fix the issue.  The patch touches some
hairy ring code in the xHCI driver, and I would rather not make any
changes to that code unless we can prove there is an issue with that
code.  We can always add the patch back later if it does turn out to fix
an issue.

David, can you please revert any local changes you've made, revert the
offending commit Mark is talking about (I think it's
http://patchwork.ozlabs.org/patch/264021/, which is commit
f27070158d6754765f2f5fd1617e8e42a0ea2318), and retest?  I want to see if
reverting that commit without making changes to the xHCI ring handling
code fixes your issues.

Thanks,
Sarah Sharp

> 
> This is what I am reverting locally to make it all work
> as it did prior to linux-3.12 was released.  Unmangled copy also attached:
> 
> --- linux/drivers/net/usb/ax88179_178a.c.orig	2013-11-03 18:41:51.000000000 -0500
> +++ linux/drivers/net/usb/ax88179_178a.c	2013-11-17 13:23:39.525734277 -0500
> @@ -1177,18 +1177,31 @@
>  	int frame_size = dev->maxpacket;
>  	int mss = skb_shinfo(skb)->gso_size;
>  	int headroom;
> +	int tailroom;
> 
>  	tx_hdr1 = skb->len;
>  	tx_hdr2 = mss;
>  	if (((skb->len + 8) % frame_size) == 0)
>  		tx_hdr2 |= 0x80008000;	/* Enable padding */
> 
> -	headroom = skb_headroom(skb) - 8;
> +	headroom = skb_headroom(skb);
> +	tailroom = skb_tailroom(skb);
> 
> -	if ((skb_header_cloned(skb) || headroom < 0) &&
> -	    pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
> +	if (!skb_header_cloned(skb) &&
> +	    !skb_cloned(skb) &&
> +	    (headroom + tailroom) >= 8) {
> +		if (headroom < 8) {
> +			skb->data = memmove(skb->head + 8, skb->data, skb->len);
> +			skb_set_tail_pointer(skb, skb->len);
> +		}
> +	} else {
> +		struct sk_buff *skb2;
> +
> +		skb2 = skb_copy_expand(skb, 8, 0, flags);
>  		dev_kfree_skb_any(skb);
> -		return NULL;
> +		skb = skb2;
> +		if (!skb)
> +			return NULL;
>  	}
> 
>  	skb_push(skb, 4);

> --- linux/drivers/net/usb/ax88179_178a.c.orig	2013-11-03 18:41:51.000000000 -0500
> +++ linux/drivers/net/usb/ax88179_178a.c	2013-11-17 13:23:39.525734277 -0500
> @@ -1177,18 +1177,31 @@
>  	int frame_size = dev->maxpacket;
>  	int mss = skb_shinfo(skb)->gso_size;
>  	int headroom;
> +	int tailroom;
>  
>  	tx_hdr1 = skb->len;
>  	tx_hdr2 = mss;
>  	if (((skb->len + 8) % frame_size) == 0)
>  		tx_hdr2 |= 0x80008000;	/* Enable padding */
>  
> -	headroom = skb_headroom(skb) - 8;
> +	headroom = skb_headroom(skb);
> +	tailroom = skb_tailroom(skb);
>  
> -	if ((skb_header_cloned(skb) || headroom < 0) &&
> -	    pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
> +	if (!skb_header_cloned(skb) &&
> +	    !skb_cloned(skb) &&
> +	    (headroom + tailroom) >= 8) {
> +		if (headroom < 8) {
> +			skb->data = memmove(skb->head + 8, skb->data, skb->len);
> +			skb_set_tail_pointer(skb, skb->len);
> +		}
> +	} else {
> +		struct sk_buff *skb2;
> +
> +		skb2 = skb_copy_expand(skb, 8, 0, flags);
>  		dev_kfree_skb_any(skb);
> -		return NULL;
> +		skb = skb2;
> +		if (!skb)
> +			return NULL;
>  	}
>  
>  	skb_push(skb, 4);

RE: [GIT PULL] xhci: Regression fix for 3.13.

[David Laight]

> From: Sarah Sharp
> Sent: 06 December 2013 17:03
> To: Mark Lord
> Cc: Greg Kroah-Hartman; linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; David Laight; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [GIT PULL] xhci: Regression fix for 3.13.
> 
> On Fri, Dec 06, 2013 at 12:55:23AM -0500, Mark Lord wrote:
> > On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
> > > On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
> > >> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
> > >>
> > >>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into
> usb-linus (2013-11-27 09:49:03 -0800)
> > >>
> > >> are available in the git repository at:
> > >>
> > >>
> > >>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
> > >
> > > Pulled and pushed out, thanks.
> >
> > Did this commit make it into linux-3.12.3 ?
> >
> > I ask, because the NIC still locks up with that kernel,
> > and even with the patch I had been using from David Laight.
> >
> > Reverting the change that originally broke it still works though.
> > Could we please get this reverted until such time as a reworked
> > patch can be prepared for it?
> 
> Greg, can you please revert David's patch (commit 35773dac5f86 from your
> usb-linus branch)?
> 
> David said the patch fixed this lock up issue with his USB ethernet
> adapter, but Mark says it doesn't fix the issue.  The patch touches some
> hairy ring code in the xHCI driver, and I would rather not make any
> changes to that code unless we can prove there is an issue with that
> code.  We can always add the patch back later if it does turn out to fix
> an issue.

I thought Mark had said that my patch fixed things for him with the earlier
kernel.  It certainly makes a massive difference on my system.
And I ran with enough diagnostics to work out when the tx side stopped.
The last packet for which an ack was received was the one before the one
that straddled the ring end.

If you want to change the ax179 driver you need to stop it using
SG and segmentation offload.
The patch that mark is talking about isn't really the one that makes a difference.
I think it just forces the skb be linearised at that point. Which mostly
means that there won't be any fragments.

	David



--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] xhci: Regression fix for 3.13.

[David Miller]

Sarah, calm down.

Let Greg merge the XHCI fix and let's have people test it.

The whole holdup is because the USB subsystem is taking forever
to merge the XHCI patch.

I would have merged it in 24-48 hours if it were my subsystem. :)

Re: [GIT PULL] xhci: Regression fix for 3.13.

[Greg Kroah-Hartman]

On Fri, Dec 06, 2013 at 09:03:25AM -0800, Sarah Sharp wrote:
> On Fri, Dec 06, 2013 at 12:55:23AM -0500, Mark Lord wrote:
> > On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
> > > On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
> > >> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
> > >>
> > >>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-linus (2013-11-27 09:49:03 -0800)
> > >>
> > >> are available in the git repository at:
> > >>
> > >>
> > >>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
> > > 
> > > Pulled and pushed out, thanks.
> > 
> > Did this commit make it into linux-3.12.3 ?
> > 
> > I ask, because the NIC still locks up with that kernel,
> > and even with the patch I had been using from David Laight.
> > 
> > Reverting the change that originally broke it still works though.
> > Could we please get this reverted until such time as a reworked
> > patch can be prepared for it?
> 
> Greg, can you please revert David's patch (commit 35773dac5f86 from your
> usb-linus branch)?

Now reverted.

thanks,

greg k-h

Re: [GIT PULL] xhci: Regression fix for 3.13.

[David Miller]

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Fri, 6 Dec 2013 09:29:47 -0800

> On Fri, Dec 06, 2013 at 09:03:25AM -0800, Sarah Sharp wrote:
>> On Fri, Dec 06, 2013 at 12:55:23AM -0500, Mark Lord wrote:
>> > On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
>> > > On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
>> > >> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
>> > >>
>> > >>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-linus (2013-11-27 09:49:03 -0800)
>> > >>
>> > >> are available in the git repository at:
>> > >>
>> > >>
>> > >>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
>> > > 
>> > > Pulled and pushed out, thanks.
>> > 
>> > Did this commit make it into linux-3.12.3 ?
>> > 
>> > I ask, because the NIC still locks up with that kernel,
>> > and even with the patch I had been using from David Laight.
>> > 
>> > Reverting the change that originally broke it still works though.
>> > Could we please get this reverted until such time as a reworked
>> > patch can be prepared for it?
>> 
>> Greg, can you please revert David's patch (commit 35773dac5f86 from your
>> usb-linus branch)?
> 
> Now reverted.

Please NO!

Merge the XHCI change, the change you have reverted actually fixes
things for some people and the XHCI change gets rid of the stated
regression.

RE: [GIT PULL] xhci: Regression fix for 3.13.

[David Laight]

> From: David Miller [mailto:davem@davemloft.net]
> > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Date: Fri, 6 Dec 2013 09:29:47 -0800
> 
> > On Fri, Dec 06, 2013 at 09:03:25AM -0800, Sarah Sharp wrote:
> >> On Fri, Dec 06, 2013 at 12:55:23AM -0500, Mark Lord wrote:
> >> > On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
> >> > > On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
> >> > >> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
> >> > >>
> >> > >>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb
> into usb-linus (2013-11-27 09:49:03 -0800)
> >> > >>
> >> > >> are available in the git repository at:
> >> > >>
> >> > >>
> >> > >>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
> >> > >
> >> > > Pulled and pushed out, thanks.
> >> >
> >> > Did this commit make it into linux-3.12.3 ?
> >> >
> >> > I ask, because the NIC still locks up with that kernel,
> >> > and even with the patch I had been using from David Laight.
> >> >
> >> > Reverting the change that originally broke it still works though.
> >> > Could we please get this reverted until such time as a reworked
> >> > patch can be prepared for it?
> >>
> >> Greg, can you please revert David's patch (commit 35773dac5f86 from your
> >> usb-linus branch)?
> >
> > Now reverted.
> 
> Please NO!
> 
> Merge the XHCI change, the change you have reverted actually fixes
> things for some people and the XHCI change gets rid of the stated
> regression.

I know that some of the xhci ring code is rather complex [1].
But that particular patch should be completely safe as it just adds
some extra nop commands into the ring.

Possibly Mark is seeing some additional failure caused by something else.

Without my fix the xhci code carries on sending data to the usb hardware,
but no ethernet frames get transmitted.

[1] I've some thoughts on how to simplify some of the horridness, it also
ought to be possible to transmit from the USB Ge card with a similar
amount of cpu overhead as a reasonable (single queue) ethernet card.
At the moment the USB code has for horrid code paths.
But I'm not looking at that code at the moment because Sarah needs to
catch up with her patch queue.

	David

Re: [GIT PULL] xhci: Regression fix for 3.13.

[Sarah Sharp]

On Fri, Dec 06, 2013 at 05:46:40PM -0000, David Laight wrote:
> > From: David Miller [mailto:davem@davemloft.net]
> > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Date: Fri, 6 Dec 2013 09:29:47 -0800
> > 
> > > On Fri, Dec 06, 2013 at 09:03:25AM -0800, Sarah Sharp wrote:
> > >> On Fri, Dec 06, 2013 at 12:55:23AM -0500, Mark Lord wrote:
> > >> > On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
> > >> > > On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
> > >> > >> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
> > >> > >>
> > >> > >>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb
> > into usb-linus (2013-11-27 09:49:03 -0800)
> > >> > >>
> > >> > >> are available in the git repository at:
> > >> > >>
> > >> > >>
> > >> > >>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
> > >> > >
> > >> > > Pulled and pushed out, thanks.
> > >> >
> > >> > Did this commit make it into linux-3.12.3 ?
> > >> >
> > >> > I ask, because the NIC still locks up with that kernel,
> > >> > and even with the patch I had been using from David Laight.
> > >> >
> > >> > Reverting the change that originally broke it still works though.
> > >> > Could we please get this reverted until such time as a reworked
> > >> > patch can be prepared for it?
> > >>
> > >> Greg, can you please revert David's patch (commit 35773dac5f86 from your
> > >> usb-linus branch)?
> > >
> > > Now reverted.
> > 
> > Please NO!
> > 
> > Merge the XHCI change, the change you have reverted actually fixes
> > things for some people and the XHCI change gets rid of the stated
> > regression.
> 
> I thought Mark had said that my patch fixed things for him with the
> earlier kernel.  It certainly makes a massive difference on my system.
> And I ran with enough diagnostics to work out when the tx side
> stopped.  The last packet for which an ack was received was the one
> before the one that straddled the ring end.
>
> If you want to change the ax179 driver you need to stop it using
> SG and segmentation offload.
> The patch that mark is talking about isn't really the one that makes a
> difference.
> I think it just forces the skb be linearised at that point. Which mostly
> means that there won't be any fragments.

Ok, so David verifies the xHCI driver does have an issue, and the patch
does fix the issue as stated.  Fine, let's merge that patch and see if
it helps.  Greg, can you fix this?  Sorry for the churn.

> I know that some of the xhci ring code is rather complex [1].
> But that particular patch should be completely safe as it just adds
> some extra nop commands into the ring.
> 
> Possibly Mark is seeing some additional failure caused by something else.

All right, let's get this patch into linus' tree, and track down exactly
what's causing Mark's issues after it's merged.

Sarah Sharp

Re: [GIT PULL] xhci: Regression fix for 3.13.

[Greg KH]

On Fri, Dec 06, 2013 at 10:19:47AM -0800, Sarah Sharp wrote:
> On Fri, Dec 06, 2013 at 05:46:40PM -0000, David Laight wrote:
> > > From: David Miller [mailto:davem@davemloft.net]
> > > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Date: Fri, 6 Dec 2013 09:29:47 -0800
> > > 
> > > > On Fri, Dec 06, 2013 at 09:03:25AM -0800, Sarah Sharp wrote:
> > > >> On Fri, Dec 06, 2013 at 12:55:23AM -0500, Mark Lord wrote:
> > > >> > On 13-12-02 04:42 PM, Greg Kroah-Hartman wrote:
> > > >> > > On Mon, Dec 02, 2013 at 12:49:08PM -0800, Sarah Sharp wrote:
> > > >> > >> The following changes since commit c24cb6c8b501ebdf1aacec7960110a9741a45ced:
> > > >> > >>
> > > >> > >>   Merge tag 'fixes-for-v3.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb
> > > into usb-linus (2013-11-27 09:49:03 -0800)
> > > >> > >>
> > > >> > >> are available in the git repository at:
> > > >> > >>
> > > >> > >>
> > > >> > >>   git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git tags/for-usb-linus-2013-12-02
> > > >> > >
> > > >> > > Pulled and pushed out, thanks.
> > > >> >
> > > >> > Did this commit make it into linux-3.12.3 ?
> > > >> >
> > > >> > I ask, because the NIC still locks up with that kernel,
> > > >> > and even with the patch I had been using from David Laight.
> > > >> >
> > > >> > Reverting the change that originally broke it still works though.
> > > >> > Could we please get this reverted until such time as a reworked
> > > >> > patch can be prepared for it?
> > > >>
> > > >> Greg, can you please revert David's patch (commit 35773dac5f86 from your
> > > >> usb-linus branch)?
> > > >
> > > > Now reverted.
> > > 
> > > Please NO!
> > > 
> > > Merge the XHCI change, the change you have reverted actually fixes
> > > things for some people and the XHCI change gets rid of the stated
> > > regression.
> > 
> > I thought Mark had said that my patch fixed things for him with the
> > earlier kernel.  It certainly makes a massive difference on my system.
> > And I ran with enough diagnostics to work out when the tx side
> > stopped.  The last packet for which an ack was received was the one
> > before the one that straddled the ring end.
> >
> > If you want to change the ax179 driver you need to stop it using
> > SG and segmentation offload.
> > The patch that mark is talking about isn't really the one that makes a
> > difference.
> > I think it just forces the skb be linearised at that point. Which mostly
> > means that there won't be any fragments.
> 
> Ok, so David verifies the xHCI driver does have an issue, and the patch
> does fix the issue as stated.  Fine, let's merge that patch and see if
> it helps.  Greg, can you fix this?  Sorry for the churn.

Yes, I've pushed it to Linus now, not including the revert.

greg k-h