On Mon, 8 Dec 2003, Oliver Neukum wrote: > > My patch does this (submissions made before the synchronous unlink returns > > will fail). > > I've changed my mind on this. Outright failure is a change of semantics. > It is not needed. Simply set a flag in this case and return success. > After the completion handler has returned, check the flag and recall it > with the error code for synchronous unlink in urb->status.
> > > Not quite so trivial to implement without races, but there are _no_ > > > changes to existing semantics. > Yes. That's the point. The essential point here is that usb_unlink_urb() > has to wait for the completion handler. The rest is sugar coating. > As I have pointed out you can do it with zero change to the API. I think you have underestimated the problems here. Suppose the completion handler resubmits and the resubmitted URB manages to complete before the original completion handler returns? Then giveback_urb would be invoked reentrantly. Which invocation should unlink_urb() then wait for? What if the resubmitted completion was the result of another synchronous unlink? Furthermore, in stating your essential point you have contradicted yourself. "usb_unlink_urb() has to wait for the completion handler". But the API currently does not make that guarantee. If you add it in, you change the API. > > I like that idea. In fact, why not make usb_unlink_urb() be purely async? > > Shouldn't the common case of synchronous unlink be made simple? To some extent I sympathize. But there is a precedent for not making the common case simple. Consider fork-exec. The most common use is for spawning new processes. But separating the two operations, thereby making spawn less simple, provided new and important capabilities. From the USB core's point of view, asynchronous unlink is the natural atomic operation. > > 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). > > > > These state changes should be protected by urb->lock. But they aren't, > > and we currently can't distinguish between idle and finishing up. > > > > My proposal would make thise categories reliably detectable: > > > > idle: urb->usage_count == 0, > > in progress: urb->usage_count > 0 and urb->status = -EINPROGRESS, > > finishing up: urb->usage_count > 0 and > > urb->status != -EINPROGRESS. > > > > What exactly does this buy? > usb_submit_urb() and usb_unlink_urb() don't change their behaviour based > on this states #1 and #3, except for usb_unlink_urb() waiting for a transition > out of #3 Waiting for a transition out of state #3 is important for synchronous unlinking. Also, even though the difference between state #2 on the one hand and states #1 and #3 on the other can currently be checked by looking at urb->status, doing so isn't reliable. urb->status is set to -EINPROGRESS in usb_submit_urb() long before the HCD has actually linked it in (and hence before an unlink call can succeed). > Having seperate operations for synchronous and asynchronous unlinking > is independent from that and a good idea, but 2.7 stuff. As far as I'm concerned, this is all 2.7 stuff. Alan Stern ------------------------------------------------------- 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
