serial: Add TX backpressure to prevent guest soft lockup#5865
Closed
JackThomson2 wants to merge 2 commits into
Closed
serial: Add TX backpressure to prevent guest soft lockup#5865JackThomson2 wants to merge 2 commits into
JackThomson2 wants to merge 2 commits into
Conversation
A guest tight loop writing to /dev/ttyS0 (e.g. `cat /dev/zero >
/dev/ttyS0`) saturates the vCPU thread in MMIO-exit handling and starves
the other vCPU, producing soft-lockup, workqueue-lockup and RCU-stall
warnings such as:
BUG: workqueue lockup - pool cpus=0-1 ... stuck for 38s!
watchdog: BUG: soft lockup - CPU#1 stuck for 86s! [swapper/1:0]
rcu: INFO: rcu_preempt detected stalls on CPUs/tasks
The underlying vm-superio Serial keeps LSR_THR_EMPTY|LSR_IDLE set
unconditionally (the upstream comment is "we should always be ready to
receive more data"), so the guest 8250 driver never throttles and writes
each byte through MMIO. The vCPU thread then performs a 1-byte
write(2)+flush+eventfd-write per byte; with FC's O_NONBLOCK stdout the
host-side pipe buffer no longer paces it either.
Wrap a soft TX FIFO in front of vm-superio:
- Guest data writes are pushed onto a bounded VecDeque (no syscall on
the vCPU thread) and a TimerFd is armed for one drain interval.
- LSR reads mask THR_EMPTY|IDLE while the queue is at the modelled
FIFO depth, so the guest sees a busy port and waits.
- A drain runs on the event-manager thread (event subscriber on the
timerfd), pops up to one FIFO per tick, and feeds each byte through
the regular `Serial::write(DATA, b)` path (write+flush+IRQ raise).
- Loopback writes bypass the queue so vm-superio's synchronous
RX-FIFO routing and RDA interrupts stay in sync with the guest.
- Overrun beyond `TX_QUEUE_CAPACITY` drops bytes via the existing
`tx_lost_byte` event, matching real-hardware FIFO overrun.
Empirically a 60-second `cat /dev/zero > /dev/ttyS0` produces zero
soft-lockup messages; the flooding vCPU's host-side stime drops from
~76% to ~5%.
Signed-off-by: Jack Thomson <jackabt@amazon.com>
The soft TX FIFO added in the previous commit holds bytes the guest has
written to the data register but the host hasn't yet drained. With the
queue not part of `SerialState`, snapshot/restore (and live migration)
silently drops up to 64 KiB of pending console output — observable as
missing bytes in the destination's serial.log when a snapshot is taken
mid-burst.
Persist the queue:
- Add `SerialState::tx_queue: Vec<u8>` with `#[serde(default)]`, so
older snapshots restore as an empty queue and round-trip cleanly.
- Bump `SNAPSHOT_VERSION` from 10.0.0 to 10.1.0. The format is a
strict superset of v10.0.0; older Firecrackers reject the new
snapshot via the existing minor-version gate in `Snapshot::load`.
- Add `SerialWrapper::tx_queue_snapshot` and `restore_tx_queue` so
`DeviceManager::serial_state` and the x86 / aarch64 restore paths
can move the bytes through the persistence layer.
- Truncate on restore to the live `TX_QUEUE_CAPACITY` to keep the
runtime invariant `tx_queue.len() <= TX_QUEUE_CAPACITY` even if a
snapshot was taken with a larger cap. Re-arm the drain timer if
the restored queue is non-empty so bytes start flowing immediately
on the destination side.
Signed-off-by: Jack Thomson <jackabt@amazon.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5865 +/- ##
==========================================
+ Coverage 82.62% 82.80% +0.17%
==========================================
Files 275 277 +2
Lines 29750 29989 +239
==========================================
+ Hits 24581 24831 +250
+ Misses 5169 5158 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
1 participant
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.
Changes
...
Reason
...
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.PR Checklist
tools/devtool checkbuild --allto verify that the PR passesbuild checks on all supported architectures.
tools/devtool checkstyleto verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md.Runbook for Firecracker API changes.
integration tests.
TODO.rust-vmm.