On Tue, 2 Mar 2004, David Brownell wrote:
p.s. Alan, notice how the "unlink during submit" case works. It's got to giveback(), as promised by the unlink logic.
I'm not familiar enough with the OHCI driver to say much about it (see below), but I think the current code in UHCI does the Right Thing.
After looking at the issue, I don't -- hence the postscript. See below ...
"Unlink during submit" means that during the brief interval after
hcd_submit_urb() has released the hcd_data_lock and before the HCD's
enqueue() method has committed to accepting the URB, hcd_unlink_urb() manages to run. There's a race between the HCD enqueue() and dequeue() methods, and with "unlink during submit" dequeue() wins.
Actually, there's no race there. The dequeue() method just kicks the HCD; as soon as hcd_unlink_urb() drops the hcd_data_lock, then usb_unlink_urb() is guaranteed to succeed. Which means that the HCD _will_ giveback() ... that's what success means.
The dequeue() method doesn't have to queue the URB for giveback because the enqueue is about to fail. (Making that failure happen is important, though!) All the enqueue() method has to do is return an error code.
No, that's why I highlighted this. Since usb_unlink_urb() succeeded in marking the urb as having been unlinked, the driver is expecting to get a completion callback.
But returning an error code means that there will NOT be a completion callback ... as there would if this had been detected even a slight bit later, if dequeue() had been called after the request had been passed off to the hardware.
In fact, I don't see why your patch needed to acquire urb->lock. As in the UHCI driver, the enqueue() and dequeue() methods guarantee mutual exclusion through the use of ohci->lock. Once enqueue() owns that lock it
They guarantee mutual exclusion at the level of the HCD.
But urb->status is under control of urb->lock, so whenever it's tested or modified between submit() and complete(), that lock must be held.
You maybe right that in this case it's OK to break that locking policy, but I prefer not to do that. If for no other reason than to prevent scheduling this urb to the hardware... :)
- Dave
doesn't matter if someone attempts an unlink-during-submit; it can't happen because dequeue() is blocked. It would suffice to test urb->status without worrying about urb->lock.
------------------------------------------------------- SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with a free DVD software kit from IBM. Click Now! http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
