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