On Thu, 6 Dec 2007, Pete Zaitcev wrote: > On Wed, 5 Dec 2007 17:31:20 -0500 (EST), Alan Stern <[EMAIL PROTECTED]> wrote: > > > [...] In fact every HCD _must_ keep interrupts disabled > > while doing most of its IRQ processing. This is because an interrupt > > could cause some random driver to submit an URB -- and URB submission > > can't be done concurrently with IRQ processing since they both alter > > the same data structures. > > Fair enough, but this is why the HCD has to do spin_lock_irqsave > across the time when lists are being modified or otherwise inconsistent. > It should not need to keep interrupts closed across executing callbacks > for complete URBs. IMHO.
Well, I tend to agree about that. However in practice it is unavoidable, since the callbacks are made from a subroutine called by the code that modifies the lists. Unless I'm mistaken, you can't pass the flags argument to a subroutine -- it's only guaranteed to work in the same stack frame where it was set. In addition the USB stack's driver interface specifies that the completion callbacks will be made with interrupts disabled. > In other message you wrote that: > > > However usb_hcd_unlink_urb_from_ep is documented as requiring > > that it be called with interrupts disabled. > > I continue to think that methods with such requirements must not > exist (at least not at the level we operate in USB stack; I can see > a need for something like that in the architecture support code). > It is unduly onerous to make sure the requirement is met while the > code changes, and it leads to unnecessary expansion of time spent with > interrupts off. In this case it's the natural thing to do. Interrupts have to be disabled anyway at the point where usb_hcd_unlink_urb_from_ep is called, so there's no harm in requiring it to be called with interrupts disabled. Particularly since the routine would just have to disable them itself. Alan Stern - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
