And the usage count thing seems to be designed to facilitate something that I think usbcore should avoid.
I'm not sure what you're referring to. The particular reason I introduced it was to handle the case of reentrant calls to giveback_urb. I don't see how the core can avoid them if the proper conditions come up.
There is no such thing as a reentrant call to giveback_urb(); that'd mean that somehow the same URB got queued twice. If nothing else, the data structures don't support such a notion ... only one queue! :)
Another important area where better synchronization is needed in the core is submission/unlinking. An URB should be either:
idle (not used at all or just beginning the submission process but not yet linked by the HCD), in progress (linked by the HCD, no errors encountered yet, not unlinked, not completed), or finishing up (error, unlink, or in completion handler).
Why should the model visible to drivers be any more complex than the simple two-state model I thought we had today?
- URB is inactive. Device driver handles any nuances like never-used, just-completed, etc. Device driver may submit it, if it's properly initialized.
- URB is submitted. HCD controls it, and manages nuances like whether the hardware sees it, whether it's being canceled, whether completion status is known, etc. HCD will give it back (state becomes inactive). Device driver may unlink, but otherwise may not access that URB.
That is, your "finishing up" is just some HCD-internal variant of "linked" -- except for "in completion handler", which is the first not-linked state on today's return path.
The problem with this simple two-state model is that it does not facilitate an implementation of synchronous unlinking that waits until the URB is idle. Of course, for anyone who believes that synchronous unlink only means waiting until the completion handler returns, this doesn't matter. But for device drivers that want to reuse an URB it does matter.
But the "urb is idle" state is clearly a driver policy. All usbcore can know is whether it's queued for the endpoint. When an urb isn't queued, it doesn't (shouldn't!) care why ... there could be dozens of reasons why it's not queued (on an un-imaginative day). I can't see any reason why usbcore should even care about one of them.
You've talked a lot about this resubmission stuff, and I've never understood why you think this should matter to anyone except the device driver. Maybe you should explain that to me ... :)
Anyway, the existing code doesn't follow this model exactly: usb_unlink_urb checks whether urb->status is -EINPROGRESS before calling the HC driver. According to the model it shouldn't do that; instead it should check whether the URB is submitted (active). But we currently have no way of doing this, since still-linked URBs can have status other than -EINPROGRESS and non-linked URBs can have status equal to -EINPROGRESS.
Curious -- I just posted a patch that tests whether the urb is on the queue. Mostly since looking at the AIO code reminded me that'd be a good idea, so I updated the gadget controller drivers to do that on their unlink paths and finally sent in the host-side version. That's what you said we "have no way of doing", right?
The -EINPROGRESS test is hard to avoid, unless you change the semantics of unlink to discard status, either the "real completion" status or the "request canceled" status. Discarding either of those makes for trouble. Regardless, that's still a "submitted" state ... it's just that unlinks can't be done without trashing real completion status.
These state changes should be protected by urb->lock. But they aren't, and we currently can't distinguish between idle and finishing up.
I don't follow. Today, state transitions are made by only two function calls -- usb_submit_urb, usb_hcd_giveback_urb -- since there are only two transitions. And urb->lock is used to make the usbcore/hcd boundary handle unlinking -- no more.
Exactly -- no more. It doesn't protect the submission transition.
That's because the caller/submitter guarantees the urb isn't in use. By definition, there's nothing to protect, no need to lock anything.
- Dave
------------------------------------------------------- This SF.net email is sponsored by: IBM Linux Tutorials. Become an expert in LINUX or just sharpen your skills. Sign up for IBM's Free Linux Tutorials. Learn everything from the bash shell to sys admin. Click now! http://ads.osdn.com/?ad_id=1278&alloc_id=3371&op=click _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
