> From: Felipe Balbi [mailto:balbi@xxxxxx] > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
