Am Montag, 8. Dezember 2003 20:07 schrieb Alan Stern:
> 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?
Why are these new problems?
The newly submitted URB cannot complete because the hardware or SMP code
will prevent an IRQ handler from being reentered.
You yourself have solved the problem or you couldn't fail resubmission
under these circumstances.
Could you elaborate?
> 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.
No, or at least not visibly ;-)
The current code will return either before after, while or before the
completion handler has run.
If we change the code so that the completion handler will always have run
we behave in a way the current code can behave. We just do so always.
Correct code will not be affected.
The same is true for racing with resubmission in the completion handler.
The current code can either error or unlink the newly submitted URB.
If we make sure the latter case always wins the race, it's still not
an outright change of 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.
OK, I'll consider it a matter of personal taste then :-)
> > > 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).
I see.
> > 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.
Well, you are saying that there are numerous bugs in the drivers concerning
usb_unlink_urb(). Shouldn't something be done?
Regards
Oliver
-------------------------------------------------------
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
