However, the hcd glue layer uses urb->status == -EINPROGRESS as a test for
It shouldn't use it. The clean way to fix this issue is to have an explicit state field.
But urb->status ** IS ** an explicit state field.
It holds the first fault experienced by that request. One of those fault codes is a software-induced "you're being unlinked" fault, which goes with an "unlink this asap" request from usbcore.
(Which I'll note does have synchronization around it ...)
Oliver responded:
Right, it holds information about the status of the io operation the URB describes. It doesn't hold information about the status of the URB itself, such as unlinkable, ununlinkable, running completion handler, unlinked These are not identical. We just have a loose connection of EINPROGRESS meaning unlinkable, which is not a clean way to do this.
So long as the urb hasn't been returned to the driver, it may be definition be unlinked until it starts a completion cycle.
Such a cycle starts by changing urb->status from EINPROGRESS, then either usbcore (in the unlink case) or the HCD (typical case) starts to push the URB through one of the paths ending in an urb giveback().
From that point on, unlinks must fail since the urb's status has been determined, and the URB is being returned; it's already being unlinked, and that can't be done twice for one submission. It's not a "loose" connection ... it's a tight one, reducing the number of ways that errors/bugs can appear.
If the completion handler is running, then unlink processing shouldn't even be able to start, since the urb will no longer be owned by either usbcore or the HCD.
And Pete also responded:
Oliver was misleading, I think. The problem is that peeking into any urb fields before HC driver released it is racy.
That's why only usbcore and the HCDs may do so, and they need to hold urb->lock before reading/modifying urb->status.
There is just no way to read urb->status between a call to usb_submit_urb and an enter to (*urb->complete) safely.
Then what's urb->lock for?
So the completion uses an extra field to indicate that urb was released by HC driver to the uppper level. It's not an explicit field, it's an extra field.
This really makes no sense to me. There's a field, with an SMP-safe lock. The protocol between one layer and the next involves holding that lock before changing that field. And you're saying that's racey. How?
- Dave
------------------------------------------------------- This SF.net email is sponsored by: VM Ware With VMware you can run multiple operating systems on a single machine. WITHOUT REBOOTING! Mix Linux / Windows / Novell virtual machines at the same time. Free trial click here: http://www.vmware.com/wl/offer/345/0 _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
