> > I thought 2.4 already has a fix for this. The correct test here is > > whether urb->dev is non-null, yes? Host controllers clear that > > when they're really done with an URB. > > This is most subtle. IMHO too subtle.
It's a side effect of a fix for some related races, as I recall. But let me correct myself: even that's not good enough. Drivers always need to explicitly manage URB reuse from completion handlers ... if they expect usbcore to mark URBs as reusable, there's ALWAYS a race on SMPs between completion handler finishing with that URB, and other driver code reusing it. One way to achieve that synchronization is never to reuse URBs. Another is for drivers to keep a pool using urb->urb_list, and for the "last completion" for an URB to put it there when processing truly finishes. (That's available for driver use when an HCD does not own the URB.) > Couldn't we have a clearly > named flag for that and kill EINPROGRESS outright ? See above -- no such flag could be maintained by usbcore, because URB reusability is really driver state. The attatched PNG (from last January, I vaguely recall :) summarizes the URB lifecycle. It's not quite perfect, it's really OK to unlink periodic urbs in callbacks, but it's the best we have today (and is still basically right). Notice two key transitions trigger in_interrupt, AND out of the control of usbcore+hcd: the completion finishes, for "unlink" or "resubmit" states. On the other hand, I would like to see some explicit way to record that the HCD owns the URB, so that it's easier for HCDs to do periodic urb manipulation (notably unlink) from completion callbacks to do the right thing. - Dave
usb-urb.png
Description: PNG image
