This is a note to let you know that I've just added the patch titled
USB: EHCI: fix bug in iTD/siTD DMA pool allocation
to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the
next -rc kernel release.
If you have any questions about this process, please let me know.
>From 85ecd0322b9a1a9f451d9150e9460ab42fd17219 Mon Sep 17 00:00:00 2001
From: Soeren Moch <smoch@xxxxxx>
Date: Fri, 22 Mar 2013 12:16:52 -0400
Subject: USB: EHCI: fix bug in iTD/siTD DMA pool allocation
[Description written by Alan Stern]
Soeren tracked down a very difficult bug in ehci-hcd's DMA pool
management of iTD and siTD structures. Some background: ehci-hcd
gives each isochronous endpoint its own set of active and free itd's
(or sitd's for full-speed devices). When a new itd is needed, it is
taken from the head of the free list, if possible. However, itd's
must not be used twice in a single frame because the hardware
continues to access the data structure for the entire duration of a
frame. Therefore if the itd at the head of the free list has its
"frame" member equal to the current value of ehci->now_frame, it
cannot be reused and instead a new itd is allocated from the DMA pool.
The entries on the free list are not released back to the pool until
the endpoint is no longer in use.
The bug arises from the fact that sometimes an itd can be moved back
onto the free list before itd->frame has been set properly. In
Soeren's case, this happened because ehci-hcd can allocate one more
itd than it actually needs for an URB; the extra itd may or may not be
required depending on how the transfer aligns with a frame boundary.
For example, an URB with 8 isochronous packets will cause two itd's to
be allocated. If the URB is scheduled to start in microframe 3 of
frame N then it will require both itds: one for microframes 3 - 7 of
frame N and one for microframes 0 - 2 of frame N+1. But if the URB
had been scheduled to start in microframe 0 then it would require only
the first itd, which could cover microframes 0 - 7 of frame N. The
second itd would be returned to the end of the free list.
The itd allocation routine initializes the entire structure to 0, so
the extra itd ends up on the free list with itd->frame set to 0
instead of a meaningful value. After a while the itd reaches the head
of the list, and occasionally this happens when ehci->now_frame is
equal to 0. Then, even though it would be okay to reuse this itd, the
driver thinks it must get another itd from the DMA pool.
For as long as the isochronous endpoint remains in use, this flaw in
the mechanism causes more and more itd's to be taken slowly from the
DMA pool. Since none are released back, the pool eventually becomes
exhausted.
This reuslts in memory allocation failures, which typically show up
during a long-running audio stream. Video might suffer the same
effect.
The fix is very simple. To prevent allocations from the pool when
they aren't needed, make sure that itd's sent back to the free list
prematurely have itd->frame set to an invalid value which can never be
equal to ehci->now_frame.
This should be applied to -stable kernels going back to 3.6.
Signed-off-by: Soeren Moch <smoch@xxxxxx>
Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Cc: stable <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/usb/host/ehci-sched.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index b476daf..010f686 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1214,6 +1214,7 @@ itd_urb_transaction (
memset (itd, 0, sizeof *itd);
itd->itd_dma = itd_dma;
+ itd->frame = 9999; /* an invalid value */
list_add (&itd->itd_list, &sched->td_list);
}
spin_unlock_irqrestore (&ehci->lock, flags);
@@ -1915,6 +1916,7 @@ sitd_urb_transaction (
memset (sitd, 0, sizeof *sitd);
sitd->sitd_dma = sitd_dma;
+ sitd->frame = 9999; /* an invalid value */
list_add (&sitd->sitd_list, &iso_sched->td_list);
}
--
1.8.1.rc1.5.g7e0651a
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html