On Thursday 27 July 2006 8:41 am, Alan Stern wrote: > > > The problem came up when there was a control URB running while the HCD > > > module was unloaded. hcd_endpoint_disable() didn't wait for the URB to > > > complete, the driver was unloaded from memory, and boom! > > > > Hmm, the if(1) should leave the urb in ep->urb_list so > > hcd_endpoint_disable() > > would go through the loop again, retrying until the urb completed. Or was > > there something else going on? (It's late, sigh.) > > Yes, the URB remained in ep->urb_list and hcd_endpoint_disable() did see > it when it went through the loop again. But urb->status had been changed > to -ESHUTDOWN, so hcd_endpoint_disable() simply skipped over it and did > not keep retrying to unlink it. (It would be a bug for usbcore to call a > dequeue method more than once for the same URB.)
So that'd be a bug; it should busy-wait until list_empty(ep->urb_list) somehow, maybe an msleep() would suffice. > The real problem was this: hcd_endpoint_disable() relies on > hcd->driver->endpoint_disable() to wait until all URBs for an endpoint > have completed. Normally that's fine, but it doesn't work with URBs for > endpoints on the root hub -- because these URBs never go through the > hcd->driver->enqueue() pathway to begin with. > > So we needed another way to wait for them. My solution was to wait in > usb_rh_urb_dequeue(). Maybe there's a better way. I still don't follow. If the root hub control URBs don't go through that code path, how would endpoint_disable() be able to notice them in the first place? > > > Nowadays we can prevent the crash with appropriate refcounting. But it > > > could still leave us calling hcd->driver->stop() while the control URB was > > > running. Do you think it would be safe, given how non-standardized those > > > HCD methods are? > > > > I'm missing something. Refcounting wouldn't solve that; the issue is that > > the URB wasn't yet given back, and thus is on ep->urb_list (with a refcount, > > sure, but _because_ it's on the list and thus still busy). So stop() > > wouldn't > > be called ... if HCDs don't behave the same, that's clearly trouble. > > No, you're confused. (It must have been _very_ late.) > > Note that hcd_endpoint_disable() doesn't wait for ep->urb_list to be > empty. It merely waits for hcd->driver->endpoint_disable() to return. But _should_ it wait for urb_list to be empty, before calling the driver's endpoint_disable()? > Refcounting would solve the problem of the HCD's module being unloaded > from memory while its code was still in use. It would also solve the > related problem of the HCD's private data structure being freed while > still in use. > > But if hcd_endpoint_disable() fails to wait for root-hub URBs to complete, > then there is a possibility that stop() could be called while an URB was > still in progress. (Actually, improvements to the hub driver have made > this much less likely. Still, it's something to be aware of.) To the extent that root hub urbs -- control or status -- get special treatment, maybe there should just be a per-usb_device counter of how many are in flight, and that should go to zero before stop(). - Dave ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
