On Tue, 25 Jul 2006, David Brownell wrote:
> Plus, this trace clearly shows the issue is with UHCI, not OHCI!
No, not really. The trace says that the OHCI operations caused
hcd_data_lock to be acquired in hcd_endpoint_disabled() with interrupts
enabled, which is inconsistent with the earlier behavior when UHCI caused
it to be acquired in urb_unlink() (called during giveback) with interrupts
disabled. Of course, that lock should never be held with interrupts
enabled.
> > It could have happened any time a USB controller was turned off.
>
> Right, and I've seen the warning with with OHCI and EHCI, as well as
> (now) with the UHCI driver as shown in this trace. The URB seems to
> be the root hub status URB, every time I've seen the message.
>
> The message is just a warning, and isn't actually reflecting any
> corruption ... every time I've seen it ("rmmmod xxx-hcd"), things
> worked just fine. So the short response is to ignore it for now, or
> not turn on those (new and not wholly trustworthy) lock debug tools.
But it is a warning that we need to take seriously. It demonstrates a
real problem that needs to be fixed. It's not an error in the lock debug
tools.
> > Does the following patch help?
>
> The theory behind the patch being ... what?
Oh, sorry, I forgot to explain it...
In hcd_endpoint_disable(), while interrupts are disabled we call unlink1()
which in turn may call usb_rh_urb_dequeue() -- in fact, _will_ call it for
root-hub status URBs. Note, however, this comment at the top of
usb_rh_urb_dequeue():
/* Asynchronous unlinks of root-hub control URBs are legal, but they
* don't do anything. Status URB unlinks must be made in process context
* with interrupts enabled.
*/
I no longer remember the exact reason, but apparently the second sentence
refers to the fact that del_timer_sync() is used when unlinking status
URBs. No doubt that's a holdover from old code; in any case it clearly
could be removed if we changed all the HCDs over to the new root-hub
polling scheme since it is conditional on (!hcd->uses_new_polling). A
few other changes would be needed as well, nothing serious.
At any rate, it's obvious that the problem was caused when
usb_rh_urb_dequeue() unconditionally re-enabled interrupts. My patch
makes sure that it is called with interrupts already enabled, so the
problem goes away.
Alan Stern
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
