On Sun, Jun 30, 2013 at 10:05 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > On Sun, 30 Jun 2013, Ming Lei wrote: > >> > Come to think of it, we never should see an endpoint being disabled >> > while there are active URBs. It might be a better idea to put a >> >> Right. >> >> > WARN_ON there and simply return, without unlinking anything. If this >> > ever triggers, it means there's a bug in usbcore. >> >> But that isn't always true, see below case: >> >> - the last URB in the endpoint completes without resubmitting in its >> completion handler >> >> - then the corresponding qh is kept in linked state, but wait for being unlinked >> >> - usb_disable_endpoint() is called before the qh is put into unlinked state, and >> no URBs are unlinked in usb_hcd_flush_endpoint() >> >> - ehci_endpoint_disable() still see qh in QH_STATE_LINKED state. >> >> So I think we should unlink here to speed up the procedure as suggested >> in your previous email. > > You are right. If the QH's qtd_list isn't empty then we should WARN_ON > and return without doing anything -- just leak the QH. Otherwise, Yes, we can add the WARN_ON() because caller should have unlinked all requests, but looks we can handle the unlink here too without obvious side-effect. Currently, URB might be probably submitted to HCD too even after usb_hcd_flush_endpoint() completes since both accesses to dev->ep_in[epnum] and ep->enabled aren't protected by effective locks. So how about not adding the WARN_ON(!list_empty(&qh->qtd_list) now when qh->qh_state is QH_STATE_LINKED? Also I am not sure if we should return without doing anything under the situation, at least when the QH is in IDLE and QH's qtd_list isn't empty, ep->hcpriv is cleared, then return. > start the unlink immediately. > > (By the way, in your suggested patch, I would not define the eptype > variable. Simply calculate the value where it is used. Also, you > removed all usages of "tmp", so the declaration should be removed as > well.) OK, I will follow your suggest. Thanks, -- Ming Lei -- 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
