Date: Mon, 1 Sep 2025 08:48:58 +0200
From: Michal Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@intel.com>, Niklas Neronin
 <niklas.neronin@linux.intel.com>
Cc: linux-usb@vger.kernel.org
Subject: [PATCH RFC 2/2] usb: xhci: Drop the TD_CLEARING_CACHE_DEFERRED
 cancel
Message-ID: <20250901084858.27f91ab7.michal.pecio@gmail.com>
In-Reply-To: <20250901084642.2b42c0e7.michal.pecio@gmail.com>
References: <20250901084642.2b42c0e7.michal.pecio@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

The last remaining use is inside xhci_invalidate_cancelled_tds() for
destroying deferred TDs under certain failure conditions.

Instead of violating the spec by crudely erasing those deferred TDs,
we may give them one last chance to get their Set TR Dequeue. Maybe
the prior failure was caused by some corruption of one stream's ring
and other streams are fine, maybe it was a transient -ENOMEM.

Or maybe we want to cover this obscure case with less extra code.

Remove the search for TD_CLEARING_CACHE_DEFERRED TDs, nuke the one
cached TD we tried and failed to skip, and jump back to the main
loop to handle any TDs still in TD_DIRTY or TD_HALTED state. These
are our deferred TDs - all other dirty TDs have been given back.

If we fail again, the deferred TD will be nuked and so on, until we
run out of TDs. We are under the lock so nobody can keep submitting
and cancelling them forever.

Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---
 drivers/usb/host/xhci-ring.c | 26 +++++++++-----------------
 drivers/usb/host/xhci.h      |  1 -
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index fd0f8a9196e2..91a9aa6aec77 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1021,6 +1021,7 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
 
 	xhci = ep->xhci;
 
+again:
 	list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, cancelled_td_list) {
 		xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
 			       "Removing canceled TD starting at 0x%llx (dma) in stream %u URB %p",
@@ -1050,14 +1051,12 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
 				break;
 			case TD_DIRTY: /* TD is cached, clear it */
 			case TD_HALTED:
-			case TD_CLEARING_CACHE_DEFERRED:
 				if (cached_td) {
 					if (cached_td->urb->stream_id != td->urb->stream_id) {
 						/* Multiple streams case, defer move dq */
 						xhci_dbg(xhci,
 							 "Move dq deferred: stream %u URB %p\n",
 							 td->urb->stream_id, td->urb);
-						td->cancel_status = TD_CLEARING_CACHE_DEFERRED;
 						break;
 					}
 
@@ -1091,22 +1090,15 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep)
 					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) {
-			/*
-			 * Deferred TDs need to have the deq pointer set after the above command
-			 * completes, so if that failed we just give up on all of them (and
-			 * complain loudly since this could cause issues due to caching).
-			 */
-			if (td->cancel_status != TD_CLEARING_CACHE &&
-			    td->cancel_status != TD_CLEARING_CACHE_DEFERRED)
-				continue;
-			xhci_warn(xhci, "Failed to clear cancelled cached URB %p, give back anyway\n",
-				  td->urb);
-			td_to_noop(td, false);
-			ring = xhci_urb_to_transfer_ring(xhci, td->urb);
-			xhci_td_cleanup(xhci, td, ring, td->status);
+		xhci_warn(xhci, "Failed to clear cancelled cached URB %p, give back anyway\n",
+				cached_td->urb);
+		td_to_noop(cached_td, false);
+		xhci_td_cleanup(xhci, cached_td, cached_ring, cached_td->status);
 
-		}
+		/* Try to clear any deferred TDs on other streams */
+		cached_ring = NULL;
+		cached_td = NULL;
+		goto again;
 	}
 	return 0;
 }
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 94394db271bf..1eaa70f7e62e 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1296,7 +1296,6 @@ enum xhci_cancelled_td_status {
 	TD_DIRTY = 0,
 	TD_HALTED,
 	TD_CLEARING_CACHE,
-	TD_CLEARING_CACHE_DEFERRED,
 };
 
 struct xhci_td {
-- 
2.48.1