Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-07-01 Thread Alan Stern
On Mon, 1 Jul 2013, Ming Lei wrote: 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. The urb_list_lock in hcd.c serves to synchronize

Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-07-01 Thread Ming Lei
On Mon, Jul 1, 2013 at 10:44 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: 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

Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-07-01 Thread Alan Stern
On Mon, 1 Jul 2013, Ming Lei wrote: On Mon, Jul 1, 2013 at 10:44 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: Currently, URB might be probably submitted to HCD too even after usb_hcd_flush_endpoint() completes since both accesses to

Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-07-01 Thread Ming Lei
On Tue, Jul 2, 2013 at 12:02 AM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: On Mon, Jul 1, 2013 at 10:44 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: Currently, URB might be probably submitted to HCD too even

Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-07-01 Thread Alan Stern
On Tue, 2 Jul 2013, Ming Lei wrote: I am wondering if holding the lock in usb_hcd_flush_endpoint() can avoid the race completely. Suppose usb_hcd_link_urb_to_ep() in submit path runs on CPU1 just when usb_hcd_flush_endpoint()(called from usb_disable_endpoint()) completes on CPU0, there is

Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-07-01 Thread Ming Lei
On Mon, Jul 1, 2013 at 10:44 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: However, this does still leave one possible race: On the first submission to an isochronous endpoint, itd_submit() and sitd_submit() will allocate a new ehci_iso_stream before

Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-06-30 Thread Ming Lei
On Sun, Jun 30, 2013 at 2:35 AM, Alan Stern st...@rowland.harvard.edu wrote: On Sat, 29 Jun 2013, Ming Lei wrote: The ehci_endpoint_disable() routine can be improved. In the QH_STATE_LINNKED or QH_STATE_COMPLETING case, we now need to handle interrupt QHs -- the comment about periodic qh

Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-06-30 Thread Alan Stern
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

Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-06-30 Thread Ming Lei
On Sun, Jun 30, 2013 at 10:05 PM, Alan Stern st...@rowland.harvard.edu 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,

Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-06-30 Thread Alan Stern
On Mon, 1 Jul 2013, Ming Lei wrote: 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

Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-06-30 Thread Ming Lei
On Mon, Jul 1, 2013 at 1:35 AM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: 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

Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-06-29 Thread Ming Lei
On Sat, Jun 29, 2013 at 1:36 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 28 Jun 2013, Ming Lei wrote: ehci-hcd currently unlinks an interrupt QH when it becomes empty, that is, after its last URB completes. This works well because in almost all cases, the completion handler for

[PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-06-28 Thread Ming Lei
ehci-hcd currently unlinks an interrupt QH when it becomes empty, that is, after its last URB completes. This works well because in almost all cases, the completion handler for an interrupt URB resubmits the URB; therefore the QH doesn't become empty and doesn't get unlinked. When we start using

Re: [PATCH v3 3/4] USB: EHCI: improve interrupt qh unlink

2013-06-28 Thread Alan Stern
On Fri, 28 Jun 2013, Ming Lei wrote: ehci-hcd currently unlinks an interrupt QH when it becomes empty, that is, after its last URB completes. This works well because in almost all cases, the completion handler for an interrupt URB resubmits the URB; therefore the QH doesn't become empty and