> From: Felipe Balbi [mailto:[email protected]]
> Sent: Wednesday, March 06, 2013 12:05 AM
...
> looking at that loop, is it really necessary ? I mean, what you do is:
>
> spin_lock_irqsave();
>
> while (!list_empty(list) && retry--) {
> if (!retry) {
> spin_unlock_irqrestore();
> boom();
> }
>
> spin_unlock_irqrestore();
> usleep_range();
> spin_lock_irqrestore();
> }
>
> are you really relying on that race to cleanup your qH list ? Doesn't
> look to me that usleep is at all necessary here.
Hi Felipe,
ehci-hcd, ohci-hcd, and uhci-hcd all spin in their endpoint_disable
functions too, so yes, I'm pretty sure it's necessary ;)
> BTW, you _do_ have a race because you don't check ep->hcpriv when
> ->urb_enqueue() is called. This means that you could queue to an
> endpoint which is in the process of getting disabled. The urb would
> either be lost or completed before being started (if I read the code
> correctly).
Good spotting. Yes, I need to recheck ep->hcpriv after reacquiring
the spinlock, thanks. Interestingly, ehci-hcd and ohci-hcd do that,
but uhci-hcd does not. I wonder if that's a bug?
--
Paul
--
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
