Re: USB storage crash report in 2.6 SMP

Olaf Hering
2004-10-02T18:10:03Z

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

On Thu, Sep 30, Hubert Tonneau wrote:

> Copying a large amount of datas (several gigabytes) between two USB 2.0 attached
> disks will crash any Linux 2.6 SMP kernel, including 2.6.9-rc3.
>
> The stack report is:
> qh_completions 0x7B/0x118 [ehci_hcd]

this was fixed a while ago, but not yet synced with Linus.

...
Maybe the call chain is something like this:

ehci_irq
spin_lock (&ehci->lock)
ehci_work ehci_watchdog
end_unlink_async
qh_completions
ehci_urb_done
spin_unlock (&ehci->lock)
usb_hcd_giveback_urb spin_lock (&ehci->lock)
ehci_work

now ehci_watchdog could proceed until usb_hcd_giveback_urb returns, then
ehci_urb_done must wait until the watchdog is done. both seem to operate
on the same list. I cant test it right now, box crashed.
...

--
USB is for mice, FireWire is for men!

sUse lINUX ag, nÜRNBERG

[-- Attachment #2: ehci-0802.patch --]
[-- Type: text/plain, Size: 1508 bytes --]

This addresses an SMP-only issue with the EHCI driver, where only one CPU
should scan the schedule at a time (scanning is not re-entrant) but either
the IRQ handler or a watchdog timer could end up starting it. Many thanks
to Olaf Hering for isolating the failure mode!

Once once CPU starts scanning, any other might as well finish right
away. This fix just adds a flag to detect that case.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

--- 1.93/drivers/usb/host/ehci-hcd.c Mon Aug 23 16:48:53 2004
+++ edited/ehci-hcd.c Thu Sep 2 16:05:47 2004
@@ -695,9 +695,18 @@
timer_action_done (ehci, TIMER_IO_WATCHDOG);
if (ehci->reclaim_ready)
end_unlink_async (ehci, regs);
+
+ /* another CPU may drop ehci->lock during a schedule scan while
+ * it reports urb completions. this flag guards against bogus
+ * attempts at re-entrant schedule scanning.
+ */
+ if (ehci->scanning)
+ return;
+ ehci->scanning = 1;
scan_async (ehci, regs);
if (ehci->next_uframe != -1)
scan_periodic (ehci, regs);
+ ehci->scanning = 0;

/* the IO watchdog guards against hardware or driver bugs that
* misplace IRQs, and should let us run completely without IRQs.
--- 1.43/drivers/usb/host/ehci.h Tue Aug 24 12:18:34 2004
+++ edited/ehci.h Thu Sep 2 15:32:30 2004
@@ -53,6 +53,7 @@
struct ehci_qh *async;
struct ehci_qh *reclaim;
unsigned reclaim_ready : 1;
+ unsigned scanning : 1;

/* periodic schedule support */
#define DEFAULT_I_TDPS 1024 /* some HCs can do less */