[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#733826: [PATCH] xhci: Set scatter-gather limit to avoid failed block writes.



Dan, can you test this patch, on top of the other patch that Ben sent?
There's directions for building a custom kernel here, if you need it:

http://kernelnewbies.org/KernelBuild

I suggest either getting the Debian kernel source and patching that, or
patching 3.12.6 or later.

Sarah Sharp

8<------------------------------------------------------------------>8

Commit 35773dac5f862cb1c82ea151eba3e2f6de51ec3e "usb: xhci: Link TRB
must not occur within a USB payload burst" attempted to fix an issue
found with USB ethernet adapters, and inadvertently broke USB storage
devices.  The patch attempts to ensure that transfers never span a
segment, and rejects transfers that have more than 63 entries (or
possibly less, if some entries cross 64KB boundaries).

usb-storage limits the maximum transfer size to 120K, and we had assumed
the block layer would pass a scatter-gather list of 4K entries,
resulting in no more than 31 sglist entries:

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

That assumption was wrong, since we've seen the driver reject a write
that was 218 sectors long (of probably 512 bytes each):

Jan  1 07:04:49 jidanni5 kernel: [  559.624704] xhci_hcd 0000:00:14.0: Too many fragments 79, max 63
...
Jan  1 07:04:58 jidanni5 kernel: [  568.622583] Write(10): 2a 00 00 06 85 0e 00 00 da 00

Limit the number of scatter-gather entries to half a ring segment.  That
should be margin enough in case some entries cross 64KB boundaries.
Increase the number of TRBs per segment from 64 to 256, which should
result in ring segments fitting on a 4K page.

Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
---
 drivers/usb/host/xhci.c | 4 ++--
 drivers/usb/host/xhci.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4265b48856f6..d45a0d584daf 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4713,8 +4713,8 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 	struct device		*dev = hcd->self.controller;
 	int			retval;
 
-	/* Accept arbitrarily long scatter-gather lists */
-	hcd->self.sg_tablesize = ~0;
+	/* Limit the block layer scatter-gather lists to half a segment. */
+	hcd->self.sg_tablesize = TRBS_PER_SEGMENT / 2;
 
 	/* support to build packet from discontinuous buffers */
 	hcd->self.no_sg_constraint = 1;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 03c74b7965f8..c283cf183c48 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1260,7 +1260,7 @@ union xhci_trb {
  * since the command ring is 64-byte aligned.
  * It must also be greater than 16.
  */
-#define TRBS_PER_SEGMENT	64
+#define TRBS_PER_SEGMENT	256
 /* Allow two commands + a link TRB, along with any reserved command TRBs */
 #define MAX_RSVD_CMD_TRBS	(TRBS_PER_SEGMENT - 3)
 #define TRB_SEGMENT_SIZE	(TRBS_PER_SEGMENT*16)
-- 
1.8.3.3


Reply to: