diff mbox

[4.2.y-ckt,009/211] xhci: don't finish a TD if we get a short transfer event mid TD

Message ID 1452023119-25647-10-git-send-email-kamal@canonical.com
State New
Headers show

Commit Message

Kamal Mostafa Jan. 5, 2016, 7:41 p.m. UTC
4.2.8-ckt1 -stable review patch.  If anyone has any objections, please let me know.

------------------

From: Mathias Nyman <mathias.nyman@linux.intel.com>

commit e210c422b6fdd2dc123bedc588f399aefd8bf9de upstream.

If the difference is big enough between the bytes asked and received
in a bulk transfer we can get a short transfer event pointing to a TRB in
the middle of the TD. We don't want to handle the TD yet as we will anyway
receive a new event for the last TRB in the TD.

Hold off from finishing the TD and removing it from the list until we
receive an event for the last TRB in the TD

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[ kamal: backport to 4.2-stable: context ]
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
---
 drivers/usb/host/xhci-ring.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Ben Hutchings Jan. 6, 2016, 5:05 p.m. UTC | #1
On Tue, 2016-01-05 at 11:41 -0800, Kamal Mostafa wrote:
> 4.2.8-ckt1 -stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Mathias Nyman <mathias.nyman@linux.intel.com>
> 
> commit e210c422b6fdd2dc123bedc588f399aefd8bf9de upstream.
> 
> If the difference is big enough between the bytes asked and received
> in a bulk transfer we can get a short transfer event pointing to a TRB in
> the middle of the TD. We don't want to handle the TD yet as we will anyway
> receive a new event for the last TRB in the TD.
> 
> Hold off from finishing the TD and removing it from the list until we
> receive an event for the last TRB in the TD
> 
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> [ kamal: backport to 4.2-stable: context ]
> Signed-off-by: Kamal Mostafa <kamal@canonical.com>
[...]

This causes regressions (see https://bugs.debian.org/808602 and
https://bugs.debian.org/808953 ) so please hold off until there's a
complete fix upstream.

Ben.
Kamal Mostafa Jan. 6, 2016, 7:55 p.m. UTC | #2
On Wed, 2016-01-06 at 17:05 +0000, Ben Hutchings wrote:
> On Tue, 2016-01-05 at 11:41 -0800, Kamal Mostafa wrote:
> > 4.2.8-ckt1 -stable review patch.  If anyone has any objections, please let me know.
> > 
> > ------------------
> > 
> > From: Mathias Nyman <mathias.nyman@linux.intel.com>
> > 
> > commit e210c422b6fdd2dc123bedc588f399aefd8bf9de upstream.
> > 
> > If the difference is big enough between the bytes asked and received
> > in a bulk transfer we can get a short transfer event pointing to a TRB in
> > the middle of the TD. We don't want to handle the TD yet as we will anyway
> > receive a new event for the last TRB in the TD.
> > 
> > Hold off from finishing the TD and removing it from the list until we
> > receive an event for the last TRB in the TD
> > 
> > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > [ kamal: backport to 4.2-stable: context ]
> > Signed-off-by: Kamal Mostafa <kamal@canonical.com>
> [...]
> 
> This causes regressions (see https://bugs.debian.org/808602 and
> https://bugs.debian.org/808953 ) so please hold off until there's a
> complete fix upstream.

Thanks for the heads-up, Ben.  I'll defer it for 4.2-stable.

I'm thinking that it should also be reverted from the stable kernels
that already carry it (3.2, 3.13, 3.16, 3.19), unless that complete
upstream fix is really imminent.  Is it?

 -Kamal
Ben Hutchings Jan. 7, 2016, 2:52 a.m. UTC | #3
On Wed, 2016-01-06 at 11:55 -0800, Kamal Mostafa wrote:
> On Wed, 2016-01-06 at 17:05 +0000, Ben Hutchings wrote:
> > On Tue, 2016-01-05 at 11:41 -0800, Kamal Mostafa wrote:
> > > 4.2.8-ckt1 -stable review patch.  If anyone has any objections, please let me know.
> > > 
> > > ------------------
> > > 
> > > From: Mathias Nyman <mathias.nyman@linux.intel.com>
> > > 
> > > commit e210c422b6fdd2dc123bedc588f399aefd8bf9de upstream.
> > > 
> > > If the difference is big enough between the bytes asked and received
> > > in a bulk transfer we can get a short transfer event pointing to a TRB in
> > > the middle of the TD. We don't want to handle the TD yet as we will anyway
> > > receive a new event for the last TRB in the TD.
> > > 
> > > Hold off from finishing the TD and removing it from the list until we
> > > receive an event for the last TRB in the TD
> > > 
> > > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > [ kamal: backport to 4.2-stable: context ]
> > > Signed-off-by: Kamal Mostafa <kamal@canonical.com>
> > [...]
> > 
> > This causes regressions (see https://bugs.debian.org/808602 and
> > https://bugs.debian.org/808953 ) so please hold off until there's a
> > complete fix upstream.
> 
> Thanks for the heads-up, Ben.  I'll defer it for 4.2-stable.
> 
> I'm thinking that it should also be reverted from the stable kernels
> that already carry it (3.2, 3.13, 3.16, 3.19), unless that complete
> upstream fix is really imminent.  Is it?

Normally if there's a regression that affects both mainline and stable
branches, we wait for it to be fixed in mainline first.

Ben.
diff mbox

Patch

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 63041c1..d08d1f1 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2156,6 +2156,10 @@  static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td,
 				EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)));
 	/* Fast path - was this the last TRB in the TD for this URB? */
 	if (event_trb == td->last_trb) {
+		if (td->urb_length_set && trb_comp_code == COMP_SHORT_TX)
+			return finish_td(xhci, td, event_trb, event, ep,
+					 status, false);
+
 		if (EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) != 0) {
 			td->urb->actual_length =
 				td->urb->transfer_buffer_length -
@@ -2207,6 +2211,12 @@  static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td,
 			td->urb->actual_length +=
 				TRB_LEN(le32_to_cpu(cur_trb->generic.field[2])) -
 				EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
+
+		if (trb_comp_code == COMP_SHORT_TX) {
+			xhci_dbg(xhci, "mid bulk/intr SP, wait for last TRB event\n");
+			td->urb_length_set = true;
+			return 0;
+		}
 	}
 
 	return finish_td(xhci, td, event_trb, event, ep, status, false);