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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jul 1, 2013 at 1:35 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> 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 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.
>
> The urb_list_lock in hcd.c serves to synchronize changes to
> ep->enabled.  An URB might be submitted after usb_hcd_flush_endpoint()
> completes, but the submission will fail in usb_hcd_link_urb_to_ep().

But the lock isn't held when setting and clearing ep->enabled in
usb_enable_endpoint and usb_disable_endpoint, is it?

Also looks there is the similar problem about 'dev->can_submit'
usage too.

>
>> So how about not adding the WARN_ON(!list_empty(&qh->qtd_list)
>> now when qh->qh_state is QH_STATE_LINKED?
>
> I think we should add the WARN_ON.  Or jump to the default case in that
> switch statement -- maybe the error message there should include a
> WARN_ON.

How about just adding WARN_ON(!list_empty(&qh->qtd_list) but
continue unlinking the qh like current code, which should be
harmless and avoid the QH leak?

>
>> 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.
>
> The only way for a QH to be in the IDLE state with a non-empty qtd_list
> is if qh->clearing_tt is set.  The code already checks for that.

OK.

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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux