On Tue, 14 Jan 2003, Oliver Neukum wrote:
> Hi,
>
> thanks for your suggestion about a spinlock.
> Here it is again, after a redesign based on your suggestion.
>
> I'll start testing this, when I'm back home.
>
> Regards
> Oliver
Your patch looks like a pretty good start to me. It guarantees that an
unlink during a completion handler will prevent a resubmission, and it
insures that usb_unlink_urb() won't return before the handler is done.
You need to add the spinlocking and internal_state stuff to
unlink_complete() in hcd.c.
There's a little problem with usb_hcd_giveback_urb(). It boils down to
this: who owns the urb while the completion handler is running? Since the
driver needs to able to resubmit from within the handler, the driver
should be the owner. But, the core needs to be able to set the
internal_state back to IDLE after the handler has finished (assuming no
resubmission), so the core needs to retain ownership of internal_state.
This problem shows up where you set internal_state back to IDLE. You
should only set it to IDLE if the state currently is COMPLETING. After
all, at that point if the urb has been resubmitted, you don't want to
change the state from whatever it is -- probably SUBMITTED but maybe
CANCELED.
A related problem is this. If an urb is cancelled before it finishes,
while the handler is running should internal_state be CANCELED or
COMPLETING? Since the handler needs to be able to resubmit, I think it
should be set to COMPLETING. That has to be added to your patch.
This leads to a third problem. Suppose the handler resubmits, and the urb
either finishes normally or gets cancelled, all before the handler
returns. The handler_lock will prevent the handler from being re-entered,
but the state will still get set to COMPLETING, which will confuse things
upon return from the initial call of the handler. To prevent that from
happening, internal_state should be set to COMPLETING only while the
handler_lock is held. (This may lead to more problems, since
internal_state must be under the control of both lock and handler_lock.)
So the lifecycle now looks like this:
IDLE --> SUBMITTED [--> CANCELED] -->* COMPLETING { -->* IDLE
{ --> SUBMITTED
where the transitions marked with * can only take place under control of
handler_lock.
I think this is pretty much how it should work. I'm still bothered by one
thought. There's an inevitable race between completion notification and
unlinking. What is the deciding factor: how should we distinguish between
an unlink affecting the current urb and an unlink preventing a
resubmission? That line has to be crossed before invoking the completion
handler; I'm just not sure that the scheme outlined above does it
correctly.
Alan Stern
-------------------------------------------------------
This SF.NET email is sponsored by: FREE SSL Guide from Thawte
are you planning your Web Server Security? Click here to get a FREE
Thawte SSL guide and find the answers to all your SSL security issues.
http://ads.sourceforge.net/cgi-bin/redirect.pl?thaw0026en
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
