usb: device_next: cdc_acm: bound poll_out backpressure wait - #2
usb: device_next: cdc_acm: bound poll_out backpressure wait#2Mikefly123 wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem: cdc_acm_poll_out() sleep-retries in an unbounded loop while the TX ring buffer is full and flow control is active. When a host session is attached but stalled (e.g. a host-side USB driver that stops issuing IN tokens for an extended period without disconnecting), this loop blocks for the full duration of the stall on every console byte written. Any thread that logs while this is happening backs up behind the stalled writer, and if enough producers share the same downstream queues, the backpressure cascades into a system-wide livelock rather than staying contained to the console path. Fix: cap the retry loop at 20 iterations of the existing 1 ms sleep (~20 ms total). Once the retry budget is exhausted, treat the still-attached-but-unresponsive session the same as the already-handled detached case: log once and discard the pending byte instead of continuing to block. This keeps the console best-effort under sustained backpressure while leaving the normal (non-stalled) flow-controlled path unaffected, since the loop still exits immediately once the ring buffer drains. Signed-off-by: Michael Pham <phamlongmichael@gmail.com>
3c87e09 to
1ce6608
Compare
|
Stack position 2/2 (stack zephyrproject-rtos#4, top of stack, targets #1's branch |
Pruning this PR (changeset blast-radius trim, 2026-08-01)Closing as part of the pre-merge trim of the USP radio changeset. The reason is The added bound is behind a condition that is always false. The patched if (k_is_in_isr() || !data->flow_ctrl || retries-- <= 0)C short-circuit evaluation means
Pre-patch, This also revises the root-cause story. A secondary reason not to carry it: Nothing is stacked on this PR, and no test or CI job asserts the bound. Branch |
|
Closed as pruned — see the rationale comment above. The added retry bound sits behind data->flow_ctrl, which is false on the flight image from both devicetree and the explicit UART_CFG_FLOW_CTRL_NONE configure, so the code is unreachable. PR #1 is unaffected and stays. Branch retained for recoverability. |
Base branch note
This fork's
maintracks upstreamzephyrproject-rtos/zephyrmain, which has divergedsubstantially from the v4.4.1 release this organization currently pins. To keep this PR's
diff scoped to the actual change, it targets a new
v4.4.1-basebranch pushed to point atthe upstream
v4.4.1tag commit (1f6485eca25431b5ff27ce9a754218c9e559bbbb).What
Bounds the sleep-retry loop in
cdc_acm_poll_out()to ~20 ms (20 retries of theexisting 1 ms sleep) when the TX ring buffer is full and flow control is active, and
falls back to the existing discard-and-warn behavior once the budget is exhausted.
Why
cdc_acm_poll_out()previously sleep-retried in an unbounded loop while the TXring buffer stayed full under active flow control. This is fine for a host session
that is genuinely draining the buffer, but an attached-but-stalled host (e.g. a
host-side USB driver that stops issuing IN tokens for an extended period without
disconnecting) turns this into a block for the full duration of the stall on every
single console byte written. Any additional thread that tries to log while blocked
this way backs up behind the stalled writer, and shared downstream queues can
cascade the stall into a system-wide livelock rather than staying contained to the
console path.
Capping the wait at ~20 ms and then discarding — the same fallback already used for
the detached (
!flow_ctrl) case — keeps the console best-effort under sustainedbackpressure without changing behavior in the normal case, since the loop still
exits immediately once the ring buffer drains.
This has been carried and HWIL-proven as a downstream patch on PROVES FCB v5e. It
was captured during an HWIL soak where a macOS host ceased IN polling for extended
periods; the unbounded retry loop cascaded into a com-stack livelock, later
root-caused to this unbounded wait. Companion HWIL/CI verification:
Open-Source-Space-Foundation/proves-core-reference#439
Not intended for merge into upstream
zephyrproject-rtos/zephyrfrom this fork.