mitosis: fix vtime drift for borrowed and cross-cell-pinned tasks#3593
Open
likewhatevs wants to merge 2 commits into
Open
mitosis: fix vtime drift for borrowed and cross-cell-pinned tasks#3593likewhatevs wants to merge 2 commits into
likewhatevs wants to merge 2 commits into
Conversation
c36eb8f to
9ea7a3a
Compare
d1a6cc1 to
6098d30
Compare
6098d30 to
618ec74
Compare
Add ktstr (KVM-backed) reproducers for the scx_mitosis vtime drift
bugs, the --vtime-borrow-fixes flag they gate on, and the ktstr
dev-dependency. The flag is declared here as a no-op; the follow-up
commit gives it the behavior that fixes the bugs, so these tests
reproduce the bugs (fail) at this commit and pass after it.
Two failure modes, each reachable from two paths:
vtime-too-far-ahead error (ktstr_mitosis_init_stall_tests.rs): the
mitosis_enqueue "vtime too far ahead" scx_bpf_error fires when a task's
dsq_vtime exceeds basis_vtime by more than 8192 * slice_ns (~164s at the
default 20ms slice).
- via_borrowing: SCHED_IDLE workers ride borrowed CPUs; dsq_vtime grows
at 100x but their cell's vtime_now does not, so the enqueue basis
check trips.
- via_cross_cell_pin: a SCHED_IDLE worker pinned to a CPU outside its
home cell never advances that CPU's vtime, so dsq_vtime drifts past
the per-CPU basis without bound.
Runnable-task stall (ktstr_mitosis_vtime_runnable_stall_tests.rs): a
SCHED_IDLE drifter loses every vtime-ordered dispatch until the watchdog
fires (SCX_EXIT_ERROR_STALL).
- via_borrowing: the runaway dsq_vtime poisons the home cell domain, so
a sibling task can no longer be dispatched once borrowing is cut.
- via_cross_cell_pin: the drifted dsq_vtime sits far above the foreign
cell's tasks that mitosis_dispatch compares it against.
ktstr is gated behind the ktstr-tests cargo feature.
Signed-off-by: Pat Somaru <patso@likewhatevs.io>
Two related vtime drift bugs, both gated behind --vtime-borrow-fixes (default off, so this is a no-op until the flag is set). A task's dsq_vtime is charged on every stop, but its cell's vtime_now -- the basis the next enqueue compares against and the value mitosis_dispatch orders by -- was only advanced for in-cell, un-borrowed runs. Runtime spent borrowing, or pinned outside the home cell, was hidden from every cell, so dsq_vtime drifted away from every basis it is checked against: it tripped the "vtime too far ahead" error, or, once the gap was large enough, lost every dispatch into a runnable-task stall (watchdog SCX_EXIT_ERROR_STALL). Borrowed runtime: mitosis_stopping now advances the task's charge-cell vtime regardless of borrow / CPU retag, so a borrowing task keeps its home cell's vtime tracking the runtime it spent on loan. Cross-cell pin: a task pinned to a CPU outside its home cell is charged to the cell of the CPU it actually runs on -- vtime_charge_cell is set from the run CPU in mitosis_select_cpu and mitosis_enqueue, not from the home cell. That cell's and that CPU's vtime_now then track it through the existing stopping gates, so cctx->vtime_now is a live enqueue basis and its dsq_vtime stays in the domain the foreign cell's tasks -- and mitosis_dispatch -- compare against. It stays dispatchable instead of drifting until it loses every dispatch. With the flag set, the four reproducers from the previous commit pass. Signed-off-by: Pat Somaru <patso@likewhatevs.io>
67c22aa to
3574b98
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.
Problem
scx_mitosis charges each task's
dsq_vtimeon every stop, but onlyadvanced the owning cell's
vtime_nowfor in-cell, un-borrowed runs.Runtime a task spent borrowing an idle CPU from another cell, or
pinned to a CPU outside its home cell, was charged to
dsq_vtimebut hidden from every cell's
vtime_now— the basis the nextmitosis_enqueuecompares against and the valuemitosis_dispatchorders by.
So
dsq_vtimedrifts away from every basis it is ever checked against.For a SCHED_IDLE task (weight 1 → charged at 100×) the gap grows fast
and surfaces two ways:
dsq_vtimeexceeds the basis by morethan
8192 * slice_ns(~164s at the 20ms default), somitosis_enqueuecalls
scx_bpf_errorand the scheduler exits.task already loses every vtime-ordered dispatch and the watchdog fires
(
SCX_EXIT_ERROR_STALL).Fix
Both behaviors are gated behind
--vtime-borrow-fixes(default off, sothis is a no-op until the flag is set).
mitosis_stoppingadvances the task'scharge-cell
vtime_nowregardless of borrow / CPU retag, so aborrowing task keeps its home cell tracking the runtime it spent on loan.
charged to the cell of the CPU it actually runs on —
vtime_charge_cellis taken from the run CPU in
mitosis_select_cpuandmitosis_enqueue,not from the home cell. That cell and that CPU's
vtime_nowthen trackit, so its
dsq_vtimestays in the domain the foreign cell's tasks —and
mitosis_dispatch— compare against, and it stays dispatchable.Commits
KVM-VM reproducers (error + stall, each via borrowing and via
cross-cell pin), the
--vtime-borrow-fixesflag (declared as a no-ophere), and the ktstr dev-dependency behind the
ktstr-testsfeature.With the flag a no-op, all four reproduce the bugs (fail) at this commit.
the
vtime_borrow_fixesbehavior; with the flag set, the fourreproducers pass.
Test plan
Each reproducer was run against both commits (
--kernel ../linux,-E 'test(mitosis_vtime)'):note: stall_via_borrowing is flaky, 1/4 runs errantly passes without fix. this is a hard bug to reliably reproduce.