From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Michal Pecio <michal.pecio@gmail.com>,
Mathias Nyman <mathias.nyman@linux.intel.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Sasha Levin <sashal@kernel.org>,
mathias.nyman@intel.com, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.18-6.12] usb: xhci: Improve Soft Retries after short transfers
Date: Mon, 31 Aug 2026 09:26:11 -0400 [thread overview]
Message-ID: <20260831133314.4125787-343-sashal@kernel.org> (raw)
In-Reply-To: <20260831133314.4125787-1-sashal@kernel.org>
From: Michal Pecio <michal.pecio@gmail.com>
[ Upstream commit e765ab012f73717238c95ab9c34bfc3c767fa48c ]
A short transfer is a successful one, so reset the error count.
Otherwise, endpoints which always complete short are limited to
three retries per endpoint life rather than per URB.
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://patch.msgid.link/20260603091132.1110849-7-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit Message Forensics
**Step 1.1 – Subject line**
Record: `[usb: xhci]` `[Improve]` – Improve soft-retry error counting
after short bulk/interrupt transfers.
**Step 1.2 – Tags**
Record:
- `Signed-off-by: Michal Pecio <michal.pecio@gmail.com>` (author)
- `Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>` (xHCI
maintainer)
- `Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>` (USB
maintainer)
- `Link: https://patch.msgid.link/20260603091132.1110849-7-
mathias.nyman@linux.intel.com` (patch 7/7 in a series)
- No `Fixes:`, `Reported-by:`, `Cc: stable`, `Tested-by:`, or `Reviewed-
by:` tags
**Step 1.3 – Body analysis**
Record:
- **Bug:** `ep->err_count` is not cleared on `COMP_SHORT_PACKET`, even
though a short transfer is a successful completion.
- **Symptom:** Endpoints that routinely complete short are limited to
three soft retries over the endpoint’s lifetime, not per URB/transfer.
- **Root cause:** `err_count` is reset on `COMP_SUCCESS` but not on
`COMP_SHORT_PACKET`, so successful short transfers do not reset the
counter.
- **Version info:** None in the message.
**Step 1.4 – Hidden bug fix?**
Record: **Yes.** Although the subject says “Improve,” this is a
correctness bug in xHCI soft-retry error accounting, not a cosmetic
cleanup.
---
## Phase 2: Diff Analysis
**Step 2.1 – Inventory**
Record:
- **Files:** `drivers/usb/host/xhci-ring.c` (+1 line)
- **Function:** `process_bulk_intr_td()`
- **Scope:** Single-file, one-line surgical fix
**Step 2.2 – Code flow change**
Record:
- **Before:** `COMP_SHORT_PACKET` sets `td->status = 0` only;
`ep->err_count` is unchanged.
- **After:** `COMP_SHORT_PACKET` also sets `ep->err_count = 0`, matching
`COMP_SUCCESS`.
- **Path:** Bulk/interrupt transfer completion in
`process_bulk_intr_td()`, called from `handle_tx_event()` for non-
control, non-isoc endpoints.
**Step 2.3 – Bug mechanism**
Record: **Logic / error-recovery bug.** `MAX_SOFT_RETRY` is 3. On
`COMP_USB_TRANSACTION_ERROR`, `ep->err_count` is incremented; if it
exceeds 3, soft retry is skipped and the error path proceeds without
`xhci_handle_halted_endpoint(..., EP_SOFT_RESET)`. Successful
completions should reset the counter, but short completions did not.
**Step 2.4 – Fix quality**
Record:
- **Obviously correct:** Short packet is a successful transfer
(`td->status = 0`); resetting `err_count` matches `COMP_SUCCESS` and
the design intent.
- **Minimal:** One line.
- **Regression risk:** Very low; only resets a counter on a success
path.
---
## Phase 3: Git History Investigation
**Step 3.1 – Blame**
Record:
- Soft retry introduced in `f8f80be501aa2` (2018): `ep_ring->err_count =
0` on `COMP_SUCCESS`.
- `err_count` moved to endpoint in `a1575120972ec` (Nov 2022, backported
with `Cc: stable`).
- `COMP_SHORT_PACKET` handling dates to 2017; never reset `err_count`.
- Bug present since soft-retry accounting was added (~2018).
**Step 3.2 – Fixes: tag**
Record: N/A – no `Fixes:` tag.
**Step 3.3 – Related file history**
Record:
- Related short-packet work by Michal Pecio: `9e3a28793d2fd` (“Fix Short
Packet handling rework ignoring errors”), affecting UAC, UVC, serial,
UAS.
- This fix is standalone; no dependency on other series patches for
correctness.
**Step 3.4 – Author context**
Record: Michal Pecio is an active xHCI contributor; Mathias Nyman is the
xHCI maintainer. Both signed off.
**Step 3.5 – Dependencies**
Record: **Standalone.** One-line addition; no new symbols or structures.
Patch 7/7 in submission, but this hunk is self-contained.
---
## Phase 4: Mailing List and External Research
**Step 4.1 – Original discussion**
Record: Link points to patch 7/7 in a Mathias Nyman series (2026-06-03).
`b4 dig` did not find the committed hash (commit not in this tree).
`patch.msgid.link` and `lore.kernel.org` were blocked (403/Anubis).
Discussion content could not be fetched.
**Step 4.2 – Reviewers**
Record: UNVERIFIED from lore. Commit has SOBs from author, xHCI
maintainer, and USB maintainer.
**Step 4.3 – Bug report**
Record: No external bug report or syzbot link. Bug inferred from code
analysis and commit message.
**Step 4.4 – Series context**
Record: Patch 7/7; this change does not appear to require earlier series
patches.
**Step 4.5 – Stable list**
Record: UNVERIFIED – could not search lore stable archives due to access
restrictions.
---
## Phase 5: Code Semantic Analysis
**Step 5.1 – Key functions**
Record: `process_bulk_intr_td()`, caller `handle_tx_event()`.
**Step 5.2 – Callers**
Record: `handle_tx_event()` is the main xHCI transfer-event handler,
invoked from the xhci interrupt path for every bulk/interrupt transfer
completion. High-traffic, common path.
**Step 5.3 – Callees**
Record: On transaction error with `err_count <= MAX_SOFT_RETRY`, calls
`xhci_handle_halted_endpoint(..., EP_SOFT_RESET)`. When limit exceeded,
soft retry is skipped and `finish_td()` runs with `-EPROTO`.
**Step 5.4 – Reachability**
Record: **Highly reachable.** Any bulk/interrupt endpoint can hit this.
Critically, in `handle_tx_event()`:
```2707:2712:drivers/usb/host/xhci-ring.c
case COMP_SUCCESS:
if (EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) !=
0) {
trb_comp_code = COMP_SHORT_PACKET;
xhci_dbg(xhci, "Successful completion on short
TX for slot %u ep %u with last td comp code %d\n",
slot_id, ep_index,
ep_ring->old_trb_comp_code);
}
```
Short transfers reported as `COMP_SUCCESS` are converted to
`COMP_SHORT_PACKET` before `process_bulk_intr_td()` runs. So the
`COMP_SUCCESS` `err_count` reset does **not** apply to short transfers
on typical hosts; they go through `COMP_SHORT_PACKET` without resetting
the counter.
**Step 5.5 – Similar patterns**
Record: `ep->err_count = 0` exists only on `COMP_SUCCESS` in
`process_bulk_intr_td()`. `handle_transferless_tx_event()` increments
`err_count` on stream transaction errors but never resets it on success.
This fix addresses the bulk/intr path only.
---
## Phase 6: Cross-Reference Against Local Tree
**Step 6.1 – Buggy code in this tree?**
Record: **Yes.** Local tree is **v6.18.44** (`git describe HEAD`, `make
kernelversion`). At lines 2572–2574, `COMP_SHORT_PACKET` does not reset
`ep->err_count`. The candidate commit is not yet applied.
**Step 6.2 – Backport complications**
Record: **Clean apply expected** – single line in a stable function with
no recent churn at that hunk.
**Step 6.3 – Related fixes already present?**
Record: `git log --grep='Soft Retries'` returned nothing. No equivalent
fix in this tree.
---
## Phase 7: Subsystem and Maintainer Context
**Step 7.1 – Subsystem**
Record: `drivers/usb/host/` – USB xHCI host controller driver.
**Criticality: IMPORTANT** (core USB path for most modern systems).
**Step 7.2 – Activity**
Record: Active subsystem; recent xhci fixes include HCE interrupt storm,
memory leaks, and short-packet handling.
---
## Phase 8: Impact and Risk Assessment
**Step 8.1 – Who is affected**
Record: Users of xHCI with bulk/interrupt endpoints that complete short
(serial, UVC, UAC, storage status pipes, interrupt IN with fixed
buffers). Essentially all xHCI users with typical USB devices.
**Step 8.2 – Trigger conditions**
Record:
- Endpoint sees successful short transfers (very common).
- Plus occasional `COMP_USB_TRANSACTION_ERROR` (transient bus errors).
- After 3 such errors, `err_count` stays elevated because short
successes never reset it.
- **Likelihood:** Moderate for long-lived endpoints with occasional bus
noise.
**Step 8.3 – Failure mode severity**
Record:
- Soft retry stops after 3 transaction errors over endpoint lifetime.
- Subsequent errors skip `EP_SOFT_RESET` and proceed to error completion
/ harder recovery.
- **Severity: MEDIUM-HIGH** – device malfunction, transfer failures,
possible disconnect; not a kernel oops, but real user-visible USB
breakage.
**Step 8.4 – Risk vs benefit**
Record:
- **Benefit:** HIGH for affected endpoints – restores intended per-
transfer error accounting.
- **Risk:** VERY LOW – one line, mirrors existing `COMP_SUCCESS`
behavior.
- **Ratio:** Strong benefit, minimal risk.
---
## Phase 9: Final Synthesis
**Step 9.1 – Evidence**
**FOR:**
- Real, long-standing logic bug in soft-retry accounting
- Affects a very common path (short bulk/interrupt transfers)
- `handle_tx_event()` converts success-with-remaining to
`COMP_SHORT_PACKET`, so most short transfers hit the buggy path
- After 3 transaction errors, error recovery degrades permanently for
that endpoint
- One-line, obviously correct fix
- Signed off by xHCI and USB maintainers
- Bug present in v6.18.44; fix applies cleanly
**AGAINST:**
- No syzbot report or user bug report in commit message
- Failure mode is degraded recovery, not kernel crash
- Lore discussion could not be verified
**UNRESOLVED:**
- Full mailing-list review thread inaccessible
- No explicit stable nomination found
**Step 9.2 – Stable rules checklist**
1. Obviously correct and tested? **PASS** – logic is clear; maintainer
SOBs present; no runtime test cited.
2. Fixes a real bug affecting users? **PASS** – short transfers are
routine; error accounting is wrong.
3. Important issue? **PASS (MEDIUM-HIGH)** – USB communication failures
on common device classes.
4. Small and contained? **PASS** – 1 line, 1 file.
5. No new features or APIs? **PASS**.
6. Can apply to local tree? **PASS** – buggy code confirmed in v6.18.44.
**Step 9.3 – Exception categories**
Record: None (not device ID, quirk, DT, build, or docs). Standard bug
fix.
**Step 9.4 – Decision rationale**
This is a clear correctness bug in xHCI error recovery. Because
`handle_tx_event()` rewrites success-with-short-data to
`COMP_SHORT_PACKET`, the existing `err_count` reset on `COMP_SUCCESS`
does not cover the common short-transfer case. Endpoints that routinely
short-packet can exhaust their three soft retries permanently after only
three transient transaction errors, causing degraded USB error recovery.
The fix is minimal, obviously correct, and low risk.
---
## Verification
- **[Phase 1]** Parsed subject, tags, and body from provided commit
message.
- **[Phase 2]** Confirmed diff: +1 line `ep->err_count = 0` in
`COMP_SHORT_PACKET` case.
- **[Phase 3]** `git blame -L 2560,2595 drivers/usb/host/xhci-ring.c`:
soft retry from 2018, `err_count` on endpoint from 2022.
- **[Phase 3]** `git show f8f80be501aa2`: original soft retry added
`err_count` reset on `COMP_SUCCESS` only.
- **[Phase 3]** `git show a1575120972ec`: moved `err_count` to endpoint;
had `Cc: stable`.
- **[Phase 3]** `git log --oneline -20 -- drivers/usb/host/xhci-ring.c`:
recent xhci activity confirmed.
- **[Phase 4]** `b4 dig`: no match (commit not in tree).
Lore/patch.msgid.link: blocked (403/Anubis). **UNVERIFIED:** review
thread content.
- **[Phase 5]** `grep process_bulk_intr_td`: called from
`handle_tx_event()` line 2982.
- **[Phase 5]** Read `handle_tx_event()` lines 2707–2712: `COMP_SUCCESS`
with remaining length converted to `COMP_SHORT_PACKET`.
- **[Phase 5]** `grep err_count drivers/usb/host/`: only reset on
`COMP_SUCCESS` in xhci-ring.c.
- **[Phase 6]** `git describe HEAD` → v6.18.44; `make kernelversion` →
6.18.44.
- **[Phase 6]** Read lines 2572–2574: bug present (no reset on
`COMP_SHORT_PACKET`).
- **[Phase 6]** `git log --grep='Soft Retries'`: no existing fix in
tree.
- **[Phase 8]** `MAX_SOFT_RETRY` = 3 in `drivers/usb/host/xhci.h` line
1272.
- **[Phase 8]** Read `COMP_USB_TRANSACTION_ERROR` path: when `err_count
> 3`, soft retry skipped.
**YES**
drivers/usb/host/xhci-ring.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 2bb7569c95054..3f1a6089a01ce 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2570,6 +2570,7 @@ static void process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep,
td->status = 0;
break;
case COMP_SHORT_PACKET:
+ ep->err_count = 0;
td->status = 0;
break;
case COMP_STOPPED_SHORT_PACKET:
--
2.53.0
next prev parent reply other threads:[~2026-08-31 13:44 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20260831133314.4125787-1-sashal@kernel.org>
2026-08-31 13:21 ` [PATCH AUTOSEL 6.18] usb: xhci: remove legacy 'num_trbs_free' tracking Sasha Levin
2026-08-31 13:21 ` [PATCH AUTOSEL 6.18-5.10] xhci: Prevent queuing new commands if xhci is inaccessible Sasha Levin
2026-08-31 13:22 ` [PATCH AUTOSEL 6.18-6.6] thunderbolt: Don't access path config space on Lane 1 adapters in tb_switch_reset_host() Sasha Levin
2026-08-31 13:23 ` [PATCH AUTOSEL 6.18-5.10] thunderbolt: Keep XDomain reference during the lifetime of a service Sasha Levin
2026-08-31 13:23 ` [PATCH AUTOSEL 6.18-6.1] usb: gadget: aspeed_udc: avoid past-the-end iterator in dequeue Sasha Levin
2026-08-31 13:25 ` [PATCH AUTOSEL 6.18-6.1] usb: gadget: udc: skip pullup() if already connected Sasha Levin
2026-08-31 13:25 ` [PATCH AUTOSEL 6.18-5.10] usb: core: hcd: fix possible deadlock in rh control transfers Sasha Levin
2026-08-31 13:26 ` Sasha Levin [this message]
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-6.6] thunderbolt: Verify Router Ready bit is set after router enumeration Sasha Levin
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-6.12] thunderbolt: Avoid reserved fields in path config space for USB4 routers Sasha Levin
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-6.6] thunderbolt: Improve multi-display DisplayPort tunnel allocation Sasha Levin
2026-08-31 13:27 ` [PATCH AUTOSEL 6.18-6.1] thunderbolt: Don't create multiple DMA tunnels on firmware connection manager Sasha Levin
2026-08-31 13:27 ` [PATCH AUTOSEL 6.18-5.10] usb: gadget: goku_udc: avoid NULL deref of dev->driver in INT_USBRESET log Sasha Levin
2026-08-31 13:27 ` [PATCH AUTOSEL 6.18] thunderbolt: Verify PCIe adapter in detect state before tunnel setup Sasha Levin
2026-08-31 13:28 ` [PATCH AUTOSEL 6.18-5.10] usbip: vhci_hcd: fix NULL deref in status_show_vhci Sasha Levin
2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-5.10] thunderbolt: Set tb->root_switch to NULL when domain is stopped Sasha Levin
2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-6.12] thunderbolt: Don't disable lane adapter if XDomain lane bonding isn't possible Sasha Levin
2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-5.10] usb: host: add ARCH_AIROHA in XHCI MTK dependency Sasha Levin
2026-08-31 13:30 ` [PATCH AUTOSEL 6.18-6.1] USB: cdc-acm: start bulk-IN polling when ALWAYS_POLL_CTRL is set Sasha Levin
2026-08-31 13:30 ` [PATCH AUTOSEL 6.18-5.10] net: usb: qmi_wwan: add MeiG SRM813Q Sasha Levin
2026-08-31 13:31 ` [PATCH AUTOSEL 6.18-5.10] thunderbolt: Keep the domain reference while processing hotplug Sasha Levin
2026-08-31 13:31 ` [PATCH AUTOSEL 6.18-6.6] thunderbolt: Increase timeout for Configuration Ready bit Sasha Levin
2026-08-31 13:31 ` [PATCH AUTOSEL 6.18-5.10] thunderbolt: Release request if tb_cfg_request() fails in __tb_xdomain_response() Sasha Levin
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=20260831133314.4125787-343-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
--cc=mathias.nyman@linux.intel.com \
--cc=michal.pecio@gmail.com \
--cc=patches@lists.linux.dev \
--cc=stable@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox