On Wed, 15 Jan 2003, David Brownell wrote: > Alan Stern wrote: > > > This is where my lack of knowledge of the details underlying the usb core > > shows through. Okay, looking at hcd.c I see where urb_unlink() is called > > and how it releases the bandwidth. I'm surprised; is it really supposed > > to work this way? > > At this point, only "uhci-hcd" relies on that and I have a note somewhere > that it doesn't actually reserve such bandwidth correctly. The basic > problem is that the model of those bandwidth reservation calls is wrong. > > (Especially so for ISO ... consider that two queued ISO requests will > get accounted as twice the bandwidth of one, rather than just one > being used serially...)
Okay, let's forget about bandwidth reservation for the time being. > As for the other posts on this thread, I hope to have time to read > them later. Adding more spinlocks is (more often than not) a Bad Thing, > deadlock-prone. It's not clear to me why urb->lock wouldn't suffice, > and there's some other wierdness being proposed. I agree that using a spinlock to solve our problem is an unorthodox approach. We are taking care to avoid deadlocks -- not much of a problem here because the new spinlock is only used in two places in the code. urb->lock doesn't solve the problem at hand, because it is required to be unlocked while the completion handler runs (so that the handler can resubmit its urb) -- but that is precisely the time period that we need our lock to be held. On the face of it, since what we are trying to do is block a thread of execution until some subroutine finishes, a wait queue would appear to be the tool of choice. Unfortunately, it won't work here. First think of where the wait queue header would have to be stored: a synchronous call to usb_unlink_urb() needs to get on the queue, even if the completion handler has already started to run. Hence the only place to store the queue header is in the urb itself. (An alternative location would be on the stack of giveback_urb, but that's not possible once the handler has started.) Now consider the problem of an urb that is always resubmitted by its completion handler. When the time comes to unload the driver, the driver will make a synchronous call to usb_unlink_urb(). Suppose that call happens just after the handler resubmitted the urb and before the handler has returned. The unlink routine will put itself on the urb's wait queue, and hence will be awakened as soon as the resubmitting handler returns, _not_ after the handler for the resubmitted urb has run. I can't see any reasonable way to get around this using wait queues. But a spinlock solves the problem just fine. If usb_unlink_urb() is called for an urb that has not yet completed, then it chains itself into the completion routine and waits as usual. If it is called for a completed urb, it changes the internal_state so that resubmission will fail, and spins on the handler_lock -- which is released immediately after the completion handler returns. I think this is a rather simple solution to a complex problem, and while slightly unorthodox it does not seem at all wierd (IMHO). In fact, it solves a problem that, as far as I know, is completely unsolved in the Linux kernel at large -- namely, how can you tell when an interrupt routine, bottom-half handler, or kernel thread has exited? The routine can signal a struct completion or a struct semaphore to inform you it is about to exit, but for all you know it could then have been preempted before executing the Return instruction and hence still be alive. You can't allow your module to be unloaded until after that Return, but there's no way in general to know that it has occurred. Practically all existing drivers just rely on the fact that this kind of preemption is very rare and does not last long -- less time, at any rate, than unloading a module requires. The only sure-fire way to know is to have the callback routine's caller inform you, and that's what synchronous usb_unlink_urb() does. Alan Stern ------------------------------------------------------- This SF.NET email is sponsored by: Thawte.com Understand how to protect your customers personal information by implementing SSL on your Apache Web Server. Click here to get our FREE Thawte Apache Guide: http://ads.sourceforge.net/cgi-bin/redirect.pl?thaw0029en _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
