On Monday 19 January 2009, Alan Stern wrote: > On Mon, 19 Jan 2009, Oliver Neukum wrote: > > > > The general policy with locking is that a given procedure > > > should always work the same. If there are two different > > > locking policies, there should be two different entry points. > > > > Basically, I don't think this is doable. I don't follow that comment. It's obviously doable in the general case... > > We've made the basic decision > > that the completion handler shall always be called, for whatever usbcore > > returns the URB. So the HCD decides which context the completion > > handler is called in. > > Not exactly. The HCD doesn't know anything about the > usb_unlink_urb()'s caller's context; it only knows about its own > context. Right. > > Now I see three options. > > > > 1. Drivers need to cope with any context and any locks held. > > This seems to be hardly practical to me. We'd beg for bugs. > > It's the situation we're in right now. So far there haven't been very > many bugs. Haven't been many reports mostly by chance. Mass storage could hit similar issues after scatterlist cleanup, ISTR. > I still wonder if it isn't possible to change the driver in > question so that it drops its own lock before unlinking an URB. That's what the SMP-unsafe patch I sent does. But the lock is needed, so long as the completion handler always touches the list ... the SMP-unsafety comes from having one bit of code walk that list while another removes things from it. A notion that just landed in my greycells is to change the completion handler. If it sees a "purging" flag is set, it could just mark the SKB as "handled" and quickly exit, but not update that list ... the code that purges the queue would set a flag on each SKB (holding current lock), unlink, wait for all the urbs to have completed, then clean up. I'll let that thought percolate a bit more. There are a few details to sort. Maybe someone else can do a patch befor I sort them. ;) - Dave > > 2. We disallow HCDs calling the completion handler of any caller > > calling into usbcore (both usb_unlink_urb() and usb_submit_urb() > > suffer from this issue). This is very unelegant. It forces HCDs into > > using work queues needlessly. > > True. > > > 3. We tell completion handlers which context they run in. > > It would be cleaner to do this with separate entry points, but how? > > It can't be done. > > Suppose driver X calls usb_unlink_urb from two different places, and in > one of them it holds a private lock but in the other it doesn't. > There's no way the HCD can propagate this information down to the > callback routine. Even if there were multiple entry points, the HCD > wouldn't know which one to use. > > Alan Stern > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
