All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Tao Wang <wat@codeaurora.org>,
	Mathias Nyman <mathias.nyman@linux.intel.com>
Subject: [PATCH 5.13 20/22] xhci: Fix failure to give back some cached cancelled URBs.
Date: Fri, 10 Sep 2021 14:30:19 +0200	[thread overview]
Message-ID: <20210910122916.600202484@linuxfoundation.org> (raw)
In-Reply-To: <20210910122915.942645251@linuxfoundation.org>

From: Mathias Nyman <mathias.nyman@linux.intel.com>

commit 94f339147fc3eb9edef7ee4ef6e39c569c073753 upstream.

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.

This isn't always true. For example with streams enabled an endpoint may
have several stream rings, each stopping on a different TDs.

Note that if an endpoint has several stream rings, the current code
will still only clear the cache of the stream pointed to by the last
cancelled TD in the cancel list.

This patch only focus on making sure all canceled TDs are given back,
avoiding hung task after device removal.
Another fix to solve clearing the caches of all stream rings with
cancelled TDs is needed, but not as urgent.

This issue was simultanously discovered and debugged by
by Tao Wang, with a slightly different fix proposal.

Fixes: 674f8438c121 ("xhci: split handling halted endpoints into two steps")
Cc: <stable@vger.kernel.org> #5.12
Reported-by: Tao Wang <wat@codeaurora.org>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20210820123503.2605901-4-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/usb/host/xhci-ring.c |   40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -942,17 +942,21 @@ static int xhci_invalidate_cancelled_tds
 					 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
 			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;
 }



  parent reply	other threads:[~2021-09-10 12:33 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-10 12:29 [PATCH 5.13 00/22] 5.13.16-rc1 review Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 01/22] firmware: dmi: Move product_sku info to the end of the modalias Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 02/22] igmp: Add ip_mc_list lock in ip_check_mc_rcu Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 03/22] net: ll_temac: Remove left-over debug message Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 04/22] Revert "r8169: avoid link-up interrupt issue on RTL8106e if user enables ASPM" Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 05/22] blk-mq: fix kernel panic during iterating over flush request Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 06/22] blk-mq: fix is_flush_rq Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 07/22] blk-mq: clearing flush request reference in tags->rqs[] Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 08/22] ALSA: usb-audio: Add registration quirk for JBL Quantum 800 Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 09/22] Bluetooth: Add additional Bluetooth part for Realtek 8852AE Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 10/22] Bluetooth: btusb: Make the CSR clone chip force-suspend workaround more generic Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 11/22] usb: host: xhci-rcar: Dont reload firmware after the completion Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 12/22] usb: xhci-mtk: fix issue of out-of-bounds array access Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 13/22] usb: cdnsp: fix the wrong mult value for HS isoc or intr Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 14/22] usb: gadget: tegra-xudc: " Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 15/22] usb: mtu3: restore HS function when set SS/SSP Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 16/22] usb: mtu3: use @mult for HS isoc or intr Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 17/22] usb: mtu3: fix the wrong HS mult value Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 18/22] xhci: fix even more unsafe memory usage in xhci tracing Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 19/22] xhci: fix " Greg Kroah-Hartman
2021-09-10 12:30 ` Greg Kroah-Hartman [this message]
2021-09-10 12:30 ` [PATCH 5.13 21/22] x86/reboot: Limit Dell Optiplex 990 quirk to early BIOS versions Greg Kroah-Hartman
2021-09-10 12:30 ` [PATCH 5.13 22/22] PCI: Call Max Payload Size-related fixup quirks early Greg Kroah-Hartman
2021-09-10 18:38 ` [PATCH 5.13 00/22] 5.13.16-rc1 review Fox Chen
2021-09-10 19:43 ` Florian Fainelli
2021-09-10 23:17 ` Shuah Khan
2021-09-11 16:20 ` Justin Forbes
2021-09-11 19:36 ` Guenter Roeck
2021-09-12  0:48 ` Daniel Díaz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210910122916.600202484@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathias.nyman@linux.intel.com \
    --cc=stable@vger.kernel.org \
    --cc=wat@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.