On 2012-04-12 19:51, Michael Grzeschik wrote:
> Hi Arvid,
>
> On Tue, Apr 10, 2012 at 11:41:24PM +0000, Arvid Brodin wrote:
>> On 2012-04-05 14:56, Michael Grzeschik wrote:
>>> Its possible that there are RETIRED qtds in the
>>> the qtd_list, which have been dequeued. Make sure
>>> to iterate through the whole list.
>>>
>>> This for instance fixes hanging serial devices, which wait
>>> for dequeued urbs to properly close their device node.
>>>
>>> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
>>> ---
>>> drivers/usb/host/isp1760-hcd.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c
>>> index 6599616..6e0bd48 100644
>>> --- a/drivers/usb/host/isp1760-hcd.c
>>> +++ b/drivers/usb/host/isp1760-hcd.c
>>> @@ -800,7 +800,7 @@ static void collect_qtds(struct usb_hcd *hcd, struct isp1760_qh *qh,
>>>
>>> list_for_each_entry_safe(qtd, qtd_next, &qh->qtd_list, qtd_list) {
>>> if (qtd->status < QTD_XFER_COMPLETE)
>>> - break;
>>> + continue;
>>>
>>> last_qtd = last_qtd_of_urb(qtd, qh);
>>>
>>
>> Ok, so this happens when
>>
>> 1) several urbs are queued to an endpoint
>> 2) an urb (not the first in the list) is dequeued
>> and
>> 3) the device for some reason won't handle/finish the earlier urbs?
>
> Yes, to all three points.
>
> Especially point three is tricky. The caller [1] of usb_kill_urb
> triggers a wait_event, which will hang until somebody calls
> isp1760_urb_done.
>
> This will only happen when we collect all of the enqueud QTDs
> and look for QTDs which have the status QTD_RETIRE.
>
> What is the condition to finish the earlier urb in the queue of an
> endpoint? Has the collect_qtds to be in a serial order?
>
> This situation happens reproducible when i try to establish a pppd
> connection with my sierra serial usb modem which then does not get a
> link. The pppd afterwards hangs while trying to close the ttyUSB
> devicenode.
>
> [1] In my special case its: sierra_stop_rx_urbs called by sierra_close.
>
> Cheers,
> Michael
>
I've been thinking about this some more. The reason I don't really like this patch is that
this loop is nested and is run in interrupt context holding a spinlock. The idea here is
to check only the head of the queue and handle only the qtds that are ready to be handled,
instead of interating though the whole queue on every interrupt.
What about handling this on urb dequeueing instead, by moving the qtds belonging to the
dequeued urb to the front of the queue?
--
Arvid Brodin
Enea Services Stockholm AB - since February 16 a part of Xdin in the Alten Group. Soon we
will be working under the common brand name Xdin. Read more at www.xdin.com.--
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
