usb/cdc: fix RP2 USB CDC TX race with cores scheduler#5391
Open
rdon-key wants to merge 1 commit into
Open
Conversation
7dc5a7b to
ce5c04a
Compare
ce5c04a to
c059707
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5377.
This PR fixes a USB CDC TX race on RP2 targets when using
-scheduler=cores.The issue was reproduced on Raspberry Pi Pico / RP2040 with TinyGo 0.41.1.
RP2350 may be affected as well because it shares the RP2 USB backend and the
same USB CDC TX state machine.
Cause
USB CDC TX is driven from two places:
Write()viakickTx()txhandler()The previous code used
inflightboth as:That is not sufficient on multicore targets.
With
-scheduler=cores,Write()may run on one core while the USB TXcompletion handler runs on another core. In that case,
kickTx()can observe theTX path as idle while
txhandler()is still processing a completed packet andchaining the next one.
That can allow concurrent or re-entrant calls into
sendFromRing()/machine.SendUSBInPacket()for the same USB IN endpoint, which can corrupt USBCDC output.
Fix
This PR adds a separate atomic
txActiveflag.txActiverepresents ownership of the USB CDC TX pump.inflightremains onlythe number of bytes currently submitted to the endpoint.
kickTx()now starts the TX pump only if it can acquiretxActivewith CAS.The TX completion handler keeps TX pump ownership while chaining packets, and
ownership is released only when the TX ring is empty.
A final ring re-check is used when releasing ownership to avoid a missed wakeup
if
Write()appends data whiletxActiveis still set.Manual test
Test program:
Commands:
Results:
The
-scheduler=corestest was repeated multiple times without observingdropped or corrupted USB CDC output.