linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: xhci-ring: set all cancelled_td's cancel_status to TD_CLEARING_CACHE
@ 2021-08-13  2:43 Tao Wang
  2021-08-13  7:25 ` Ikjoon Jang
  0 siblings, 1 reply; 6+ messages in thread
From: Tao Wang @ 2021-08-13  2:43 UTC (permalink / raw)
  To: Mathias Nyman, Greg Kroah-Hartman, open list:USB XHCI DRIVER, open list
  Cc: Tao Wang

USB SSD may fail to unmount if disconnect during data transferring.

it stuck in usb_kill_urb() due to urb use_count will not become zero,
this means urb giveback is not happen.
in xhci_handle_cmd_set_deq() will giveback urb if td's cancel_status
equal to TD_CLEARING_CACHE,
but in xhci_invalidate_cancelled_tds(), only last canceled td's
cancel_status change to TD_CLEARING_CACHE,
thus giveback only happen to last urb.

this change set all cancelled_td's cancel_status to TD_CLEARING_CACHE
rather than the last one, so all urb can giveback.

Signed-off-by: Tao Wang <wat@codeaurora.org>
---
 drivers/usb/host/xhci-ring.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 8fea44b..c7dd7c0 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -960,19 +960,19 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
 			td_to_noop(xhci, ring, td, false);
 			td->cancel_status = TD_CLEARED;
 		}
-	}
-	if (cached_td) {
-		cached_td->cancel_status = TD_CLEARING_CACHE;
-
-		err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index,
-						cached_td->urb->stream_id,
-						cached_td);
-		/* Failed to move past cached td, try just setting it noop */
-		if (err) {
-			td_to_noop(xhci, ring, cached_td, false);
-			cached_td->cancel_status = TD_CLEARED;
+		if (cached_td) {
+			cached_td->cancel_status = TD_CLEARING_CACHE;
+
+			err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index,
+							cached_td->urb->stream_id,
+							cached_td);
+			/* Failed to move past cached td, try just setting it noop */
+			if (err) {
+				td_to_noop(xhci, ring, cached_td, false);
+				cached_td->cancel_status = TD_CLEARED;
+			}
+			cached_td = NULL;
 		}
-		cached_td = NULL;
 	}
 	return 0;
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: xhci-ring: set all cancelled_td's cancel_status to TD_CLEARING_CACHE
  2021-08-13  2:43 [PATCH] usb: xhci-ring: set all cancelled_td's cancel_status to TD_CLEARING_CACHE Tao Wang
@ 2021-08-13  7:25 ` Ikjoon Jang
  2021-08-13  8:44   ` wat
  0 siblings, 1 reply; 6+ messages in thread
From: Ikjoon Jang @ 2021-08-13  7:25 UTC (permalink / raw)
  To: Tao Wang
  Cc: Mathias Nyman, Greg Kroah-Hartman, open list:USB XHCI DRIVER, open list

Hi,

On Fri, Aug 13, 2021 at 10:44 AM Tao Wang <wat@codeaurora.org> wrote:
>
> USB SSD may fail to unmount if disconnect during data transferring.
>
> it stuck in usb_kill_urb() due to urb use_count will not become zero,
> this means urb giveback is not happen.
> in xhci_handle_cmd_set_deq() will giveback urb if td's cancel_status
> equal to TD_CLEARING_CACHE,
> but in xhci_invalidate_cancelled_tds(), only last canceled td's
> cancel_status change to TD_CLEARING_CACHE,
> thus giveback only happen to last urb.
>
> this change set all cancelled_td's cancel_status to TD_CLEARING_CACHE
> rather than the last one, so all urb can giveback.
>
> Signed-off-by: Tao Wang <wat@codeaurora.org>
> ---
>  drivers/usb/host/xhci-ring.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 8fea44b..c7dd7c0 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -960,19 +960,19 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
>                         td_to_noop(xhci, ring, td, false);
>                         td->cancel_status = TD_CLEARED;
>                 }
> -       }
> -       if (cached_td) {
> -               cached_td->cancel_status = TD_CLEARING_CACHE;
> -
> -               err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index,
> -                                               cached_td->urb->stream_id,
> -                                               cached_td);
> -               /* Failed to move past cached td, try just setting it noop */
> -               if (err) {
> -                       td_to_noop(xhci, ring, cached_td, false);
> -                       cached_td->cancel_status = TD_CLEARED;
> +               if (cached_td) {
> +                       cached_td->cancel_status = TD_CLEARING_CACHE;
> +
> +                       err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index,
> +                                                       cached_td->urb->stream_id,
> +                                                       cached_td);
> +                       /* Failed to move past cached td, try just setting it noop */
> +                       if (err) {
> +                               td_to_noop(xhci, ring, cached_td, false);
> +                               cached_td->cancel_status = TD_CLEARED;
> +                       }
> +                       cached_td = NULL;
>                 }
> -               cached_td = NULL;

I think we can call xhci_move_dequeue_past_td() just once to
the last halted && cancelled TD in a ring.

But that might need to compare two TDs to see which one is
the latter, I'm not sure how to do this well. :-/

if (!cached_td || cached_td < td)
  cached_td = td;

>         }
>         return 0;
>  }
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: xhci-ring: set all cancelled_td's cancel_status to TD_CLEARING_CACHE
  2021-08-13  7:25 ` Ikjoon Jang
@ 2021-08-13  8:44   ` wat
  2021-08-13  9:09     ` Mathias Nyman
  0 siblings, 1 reply; 6+ messages in thread
From: wat @ 2021-08-13  8:44 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Mathias Nyman, Greg Kroah-Hartman, open list:USB XHCI DRIVER, open list

On 2021-08-13 15:25, Ikjoon Jang wrote:
> Hi,
> 
> On Fri, Aug 13, 2021 at 10:44 AM Tao Wang <wat@codeaurora.org> wrote:
>> 
>> USB SSD may fail to unmount if disconnect during data transferring.
>> 
>> it stuck in usb_kill_urb() due to urb use_count will not become zero,
>> this means urb giveback is not happen.
>> in xhci_handle_cmd_set_deq() will giveback urb if td's cancel_status
>> equal to TD_CLEARING_CACHE,
>> but in xhci_invalidate_cancelled_tds(), only last canceled td's
>> cancel_status change to TD_CLEARING_CACHE,
>> thus giveback only happen to last urb.
>> 
>> this change set all cancelled_td's cancel_status to TD_CLEARING_CACHE
>> rather than the last one, so all urb can giveback.
>> 
>> Signed-off-by: Tao Wang <wat@codeaurora.org>
>> ---
>>  drivers/usb/host/xhci-ring.c | 24 ++++++++++++------------
>>  1 file changed, 12 insertions(+), 12 deletions(-)
>> 
>> diff --git a/drivers/usb/host/xhci-ring.c 
>> b/drivers/usb/host/xhci-ring.c
>> index 8fea44b..c7dd7c0 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -960,19 +960,19 @@ static int xhci_invalidate_cancelled_tds(struct 
>> xhci_virt_ep *ep)
>>                         td_to_noop(xhci, ring, td, false);
>>                         td->cancel_status = TD_CLEARED;
>>                 }
>> -       }
>> -       if (cached_td) {
>> -               cached_td->cancel_status = TD_CLEARING_CACHE;
>> -
>> -               err = xhci_move_dequeue_past_td(xhci, slot_id, 
>> ep->ep_index,
>> -                                               
>> cached_td->urb->stream_id,
>> -                                               cached_td);
>> -               /* Failed to move past cached td, try just setting it 
>> noop */
>> -               if (err) {
>> -                       td_to_noop(xhci, ring, cached_td, false);
>> -                       cached_td->cancel_status = TD_CLEARED;
>> +               if (cached_td) {
>> +                       cached_td->cancel_status = TD_CLEARING_CACHE;
>> +
>> +                       err = xhci_move_dequeue_past_td(xhci, slot_id, 
>> ep->ep_index,
>> +                                                       
>> cached_td->urb->stream_id,
>> +                                                       cached_td);
>> +                       /* Failed to move past cached td, try just 
>> setting it noop */
>> +                       if (err) {
>> +                               td_to_noop(xhci, ring, cached_td, 
>> false);
>> +                               cached_td->cancel_status = TD_CLEARED;
>> +                       }
>> +                       cached_td = NULL;
>>                 }
>> -               cached_td = NULL;
> 
> I think we can call xhci_move_dequeue_past_td() just once to
> the last halted && cancelled TD in a ring.
> 
> But that might need to compare two TDs to see which one is
> the latter, I'm not sure how to do this well. :-/
> 
> if (!cached_td || cached_td < td)
>   cached_td = td;
> 

thanks, I think you are correct that we can call 
xhci_move_dequeue_past_td() just once to
  the last halted && cancelled TD in a ring,
but the set status "cached_td->cancel_status = TD_CLEARING_CACHE;" 
should be every cancelled TD.
I am not very good at td and ring, I have a question why we need to
compare two TDs to see which one is the latter.

>>         }
>>         return 0;
>>  }
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>> a Linux Foundation Collaborative Project
>> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: xhci-ring: set all cancelled_td's cancel_status to TD_CLEARING_CACHE
  2021-08-13  8:44   ` wat
@ 2021-08-13  9:09     ` Mathias Nyman
  2021-08-13 10:00       ` wat
  2021-08-13 13:47       ` [RFT PATCH] xhci: fix failure to give back some cached cancelled URBs Mathias Nyman
  0 siblings, 2 replies; 6+ messages in thread
From: Mathias Nyman @ 2021-08-13  9:09 UTC (permalink / raw)
  To: wat, Ikjoon Jang
  Cc: Mathias Nyman, Greg Kroah-Hartman, open list:USB XHCI DRIVER, open list

On 13.8.2021 11.44, wat@codeaurora.org wrote:
> On 2021-08-13 15:25, Ikjoon Jang wrote:
>> Hi,
>>
>> On Fri, Aug 13, 2021 at 10:44 AM Tao Wang <wat@codeaurora.org> wrote:
>>>
>>> USB SSD may fail to unmount if disconnect during data transferring.
>>>
>>> it stuck in usb_kill_urb() due to urb use_count will not become zero,
>>> this means urb giveback is not happen.
>>> in xhci_handle_cmd_set_deq() will giveback urb if td's cancel_status
>>> equal to TD_CLEARING_CACHE,
>>> but in xhci_invalidate_cancelled_tds(), only last canceled td's
>>> cancel_status change to TD_CLEARING_CACHE,
>>> thus giveback only happen to last urb.
>>>
>>> this change set all cancelled_td's cancel_status to TD_CLEARING_CACHE
>>> rather than the last one, so all urb can giveback.
>>>
>>> Signed-off-by: Tao Wang <wat@codeaurora.org>
>>> ---
>>>  drivers/usb/host/xhci-ring.c | 24 ++++++++++++------------
>>>  1 file changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>>> index 8fea44b..c7dd7c0 100644
>>> --- a/drivers/usb/host/xhci-ring.c
>>> +++ b/drivers/usb/host/xhci-ring.c
>>> @@ -960,19 +960,19 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
>>>                         td_to_noop(xhci, ring, td, false);
>>>                         td->cancel_status = TD_CLEARED;
>>>                 }
>>> -       }
>>> -       if (cached_td) {
>>> -               cached_td->cancel_status = TD_CLEARING_CACHE;
>>> -
>>> -               err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index,
>>> -                                               cached_td->urb->stream_id,
>>> -                                               cached_td);
>>> -               /* Failed to move past cached td, try just setting it noop */
>>> -               if (err) {
>>> -                       td_to_noop(xhci, ring, cached_td, false);
>>> -                       cached_td->cancel_status = TD_CLEARED;
>>> +               if (cached_td) {
>>> +                       cached_td->cancel_status = TD_CLEARING_CACHE;
>>> +
>>> +                       err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index,
>>> +                                                       cached_td->urb->stream_id,
>>> +                                                       cached_td);
>>> +                       /* Failed to move past cached td, try just setting it noop */
>>> +                       if (err) {
>>> +                               td_to_noop(xhci, ring, cached_td, false);
>>> +                               cached_td->cancel_status = TD_CLEARED;
>>> +                       }
>>> +                       cached_td = NULL;
>>>                 }
>>> -               cached_td = NULL;
>>
>> I think we can call xhci_move_dequeue_past_td() just once to
>> the last halted && cancelled TD in a ring.
>>
>> But that might need to compare two TDs to see which one is
>> the latter, I'm not sure how to do this well. :-/
>>
>> if (!cached_td || cached_td < td)
>>   cached_td = td;
>>
> 
> thanks, I think you are correct that we can call xhci_move_dequeue_past_td() just once to
>  the last halted && cancelled TD in a ring,
> but the set status "cached_td->cancel_status = TD_CLEARING_CACHE;" should be every cancelled TD.
> I am not very good at td and ring, I have a question why we need to
> compare two TDs to see which one is the latter.

I'm debugging the exact same issue.
For normal endpoints (no streams) it should be enough to set cancel_td->cancel_status = TD_CLEARING_CACHE
in the TD_DIRTY and TD_HALTED case.

We don't need to move the dq past the last cancelled TD as other cancelled TDs are set to no-op, and
the command to move the dq will flush the xHC controllers TD cache and read the no-ops.
(just make sure we call xhci_move_dequeue_past_td() _after_ overwriting cancelled TDs with no-op)

Streams get trickier as each endpoint has several rings, and we might need to move the dq pointer for
many stream rings on that endpoint. This needs more work as we shouldn't start the endpoint before all
the all move dq commands complete. i.e. the current  ep->ep_state &= ~SET_DEQ_PENDING isn't enough.

-Mathias

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: xhci-ring: set all cancelled_td's cancel_status to TD_CLEARING_CACHE
  2021-08-13  9:09     ` Mathias Nyman
@ 2021-08-13 10:00       ` wat
  2021-08-13 13:47       ` [RFT PATCH] xhci: fix failure to give back some cached cancelled URBs Mathias Nyman
  1 sibling, 0 replies; 6+ messages in thread
From: wat @ 2021-08-13 10:00 UTC (permalink / raw)


On 2021-08-13 17:09, Mathias Nyman wrote:
> On 13.8.2021 11.44, wat@codeaurora.org wrote:
>> On 2021-08-13 15:25, Ikjoon Jang wrote:
>>> Hi,
>>> 
>>> On Fri, Aug 13, 2021 at 10:44 AM Tao Wang <wat@codeaurora.org> wrote:
>>>> 
>>>> USB SSD may fail to unmount if disconnect during data transferring.
>>>> 
>>>> it stuck in usb_kill_urb() due to urb use_count will not become 
>>>> zero,
>>>> this means urb giveback is not happen.
>>>> in xhci_handle_cmd_set_deq() will giveback urb if td's cancel_status
>>>> equal to TD_CLEARING_CACHE,
>>>> but in xhci_invalidate_cancelled_tds(), only last canceled td's
>>>> cancel_status change to TD_CLEARING_CACHE,
>>>> thus giveback only happen to last urb.
>>>> 
>>>> this change set all cancelled_td's cancel_status to 
>>>> TD_CLEARING_CACHE
>>>> rather than the last one, so all urb can giveback.
>>>> 
>>>> Signed-off-by: Tao Wang <wat@codeaurora.org>
>>>> ---
>>>>  drivers/usb/host/xhci-ring.c | 24 ++++++++++++------------
>>>>  1 file changed, 12 insertions(+), 12 deletions(-)
>>>> 
>>>> diff --git a/drivers/usb/host/xhci-ring.c 
>>>> b/drivers/usb/host/xhci-ring.c
>>>> index 8fea44b..c7dd7c0 100644
>>>> --- a/drivers/usb/host/xhci-ring.c
>>>> +++ b/drivers/usb/host/xhci-ring.c
>>>> @@ -960,19 +960,19 @@ static int 
>>>> xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
>>>>                         td_to_noop(xhci, ring, td, false);
>>>>                         td->cancel_status = TD_CLEARED;
>>>>                 }
>>>> -       }
>>>> -       if (cached_td) {
>>>> -               cached_td->cancel_status = TD_CLEARING_CACHE;
>>>> -
>>>> -               err = xhci_move_dequeue_past_td(xhci, slot_id, 
>>>> ep->ep_index,
>>>> -                                               
>>>> cached_td->urb->stream_id,
>>>> -                                               cached_td);
>>>> -               /* Failed to move past cached td, try just setting 
>>>> it noop */
>>>> -               if (err) {
>>>> -                       td_to_noop(xhci, ring, cached_td, false);
>>>> -                       cached_td->cancel_status = TD_CLEARED;
>>>> +               if (cached_td) {
>>>> +                       cached_td->cancel_status = 
>>>> TD_CLEARING_CACHE;
>>>> +
>>>> +                       err = xhci_move_dequeue_past_td(xhci, 
>>>> slot_id, ep->ep_index,
>>>> +                                                       
>>>> cached_td->urb->stream_id,
>>>> +                                                       cached_td);
>>>> +                       /* Failed to move past cached td, try just 
>>>> setting it noop */
>>>> +                       if (err) {
>>>> +                               td_to_noop(xhci, ring, cached_td, 
>>>> false);
>>>> +                               cached_td->cancel_status = 
>>>> TD_CLEARED;
>>>> +                       }
>>>> +                       cached_td = NULL;
>>>>                 }
>>>> -               cached_td = NULL;
>>> 
>>> I think we can call xhci_move_dequeue_past_td() just once to
>>> the last halted && cancelled TD in a ring.
>>> 
>>> But that might need to compare two TDs to see which one is
>>> the latter, I'm not sure how to do this well. :-/
>>> 
>>> if (!cached_td || cached_td < td)
>>>   cached_td = td;
>>> 
>> 
>> thanks, I think you are correct that we can call 
>> xhci_move_dequeue_past_td() just once to
>>  the last halted && cancelled TD in a ring,
>> but the set status "cached_td->cancel_status = TD_CLEARING_CACHE;" 
>> should be every cancelled TD.
>> I am not very good at td and ring, I have a question why we need to
>> compare two TDs to see which one is the latter.
> 
> I'm debugging the exact same issue.
> For normal endpoints (no streams) it should be enough to set
> cancel_td->cancel_status = TD_CLEARING_CACHE
> in the TD_DIRTY and TD_HALTED case.
> 
> We don't need to move the dq past the last cancelled TD as other
> cancelled TDs are set to no-op, and
> the command to move the dq will flush the xHC controllers TD cache and
> read the no-ops.
> (just make sure we call xhci_move_dequeue_past_td() _after_
> overwriting cancelled TDs with no-op)
> 
> Streams get trickier as each endpoint has several rings, and we might
> need to move the dq pointer for
> many stream rings on that endpoint. This needs more work as we
> shouldn't start the endpoint before all
> the all move dq commands complete. i.e. the current  ep->ep_state &=
> ~SET_DEQ_PENDING isn't enough.
> 
> -Mathias
ok, thanks, please tell me if you have a great solution after debugging, 
I still need to learn from you.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFT PATCH] xhci: fix failure to give back some cached cancelled URBs.
  2021-08-13  9:09     ` Mathias Nyman
  2021-08-13 10:00       ` wat
@ 2021-08-13 13:47       ` Mathias Nyman
  1 sibling, 0 replies; 6+ messages in thread
From: Mathias Nyman @ 2021-08-13 13:47 UTC (permalink / raw)
  To: wat, ikjn; +Cc: gregkh, linux-usb, linux-kernel, mathias.nyman

Only TDs with status TD_CLEARING_CACHE will be given back after
cache is cleared with a set TR deq command.

xhci_invalidate_cached_td() failed to set the TD_CLEARING_CACHE status
for some cancelled TDs as it assumed an endpoint only needs to clear the
TD it stopped on from cache. There are some cases this isn't true.

For example with streams as an endpoint may have several
stream rings, each stopping on different TDs.

* FIXME *, explain that streams case isn't fully solved by this, but it's
 x100 better than before as we give back the URBs and thread won't hang.
 Some of the streams TRB cache still isn't cleared, and may point to data
 in URBs we alrady gave back. xHC controller may touch this.

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 40 ++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d0faa67a689d..9017986241f5 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -942,17 +942,21 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
 					 td->urb->stream_id);
 		hw_deq &= ~0xf;
 
-		if (td->cancel_status == TD_HALTED) {
-			cached_td = td;
-		} else if (trb_in_td(xhci, td->start_seg, td->first_trb,
-			      td->last_trb, hw_deq, false)) {
+		if (td->cancel_status == TD_HALTED ||
+		    trb_in_td(xhci, td->start_seg, td->first_trb, td->last_trb, hw_deq, false)) {
 			switch (td->cancel_status) {
 			case TD_CLEARED: /* TD is already no-op */
 			case TD_CLEARING_CACHE: /* set TR deq command already queued */
 				break;
 			case TD_DIRTY: /* TD is cached, clear it */
 			case TD_HALTED:
-				/* FIXME  stream case, several stopped rings */
+				td->cancel_status = TD_CLEARING_CACHE;
+				if (cached_td)
+					/* FIXME  stream case, several stopped rings */
+					xhci_dbg(xhci,
+						 "Move dq past stream %u URB %p instead of stream %u URB %p\n",
+						 td->urb->stream_id, td->urb,
+						 cached_td->urb->stream_id, cached_td->urb);
 				cached_td = td;
 				break;
 			}
@@ -961,18 +965,24 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
 			td->cancel_status = TD_CLEARED;
 		}
 	}
-	if (cached_td) {
-		cached_td->cancel_status = TD_CLEARING_CACHE;
 
-		err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index,
-						cached_td->urb->stream_id,
-						cached_td);
-		/* Failed to move past cached td, try just setting it noop */
-		if (err) {
-			td_to_noop(xhci, ring, cached_td, false);
-			cached_td->cancel_status = TD_CLEARED;
+	/* If there's no need to move the dequeue pointer then we're done */
+	if (!cached_td)
+		return 0;
+
+	err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index,
+					cached_td->urb->stream_id,
+					cached_td);
+	if (err) {
+		/* Failed to move past cached td, just set cached TDs to no-op */
+		list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, cancelled_td_list) {
+			if (td->cancel_status != TD_CLEARING_CACHE)
+				continue;
+			xhci_dbg(xhci, "Failed to clear cancelled cached URB %p, mark clear anyway\n",
+				 td->urb);
+			td_to_noop(xhci, ring, td, false);
+			td->cancel_status = TD_CLEARED;
 		}
-		cached_td = NULL;
 	}
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-08-13 13:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-13  2:43 [PATCH] usb: xhci-ring: set all cancelled_td's cancel_status to TD_CLEARING_CACHE Tao Wang
2021-08-13  7:25 ` Ikjoon Jang
2021-08-13  8:44   ` wat
2021-08-13  9:09     ` Mathias Nyman
2021-08-13 10:00       ` wat
2021-08-13 13:47       ` [RFT PATCH] xhci: fix failure to give back some cached cancelled URBs Mathias Nyman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).