Run a ktstr scenario under a simulator as well as a VM - #47
Open
rrnewton wants to merge 18 commits into
Open
Conversation
A ktstr test's workload is arbitrary Rust that happens to end in
`execute_steps(ctx, steps)`. The SHAPE of most of those bodies is
declarative — a list of steps — but nothing in the type system says
so, and nothing can recover the step list without booting a guest and
running the test. That is a shame for anything that wants to READ a
workload rather than execute it.
Introduce `ScenarioDef`: the step list (plus the optional `Assert`
override `execute_steps_with` takes) lifted out of the body into a
value that can be built, inspected and printed on the host with no VM
and no `&Ctx`. `ScenarioDef::run` dispatches to `execute_steps_with`,
so there is no second execution engine — a scenario runs down exactly
the path a hand-written body runs down.
Add `#[ktstr_scenario]` as the entrypoint that consumes one. It does
NOT fork `#[ktstr_test]`: it emits the author's function renamed, a
canonical `fn() -> ScenarioDef` builder that coerces the return value
via `Into` (so a body may return `ScenarioDef`, `Vec<Step>` or a bare
`Step`), a `ScenarioEntry` registration in the new `KTSTR_SCENARIOS`
slice, and a synthesized `fn <name>(ctx: &Ctx) -> Result<AssertResult>`
that it hands to `ktstr_test_impl` unchanged. Attribute parsing,
cross-attribute validation and every existing registration are
therefore literally the same code, and any future `#[ktstr_test]`
field is inherited by construction. `delegates_to_ktstr_test_verbatim`
pins that: the scenario expansion must CONTAIN `ktstr_test_impl`'s own
output for the runner, token for token.
The function is what is restricted, not the attribute grammar. No
parameters — in particular no `&Ctx`, which is what makes the body
host-buildable; no `async`, generics or `where` clause, which the
generated builder could not call; and no `post_vm` /
`post_vm_unconditional`, since a host-side callback is the arbitrary
Rust this entrypoint exists to exclude. Each rejection names its
reason and points at `#[ktstr_test]`.
The `&Ctx` restriction costs less than it looks. Nearly every `&Ctx`
use in an existing scenario body is `ctx.cgroup_def(n)`, which is
`CgroupDef::named(n).workers(ctx.workers_per_cgroup)` — and a
`CgroupDef` that leaves its worker count unset resolves through the
same default anyway (empty `works` -> `WorkSpec::default()` ->
`num_workers: None` -> `resolve_num_workers`'s
`unwrap_or(ctx.workers_per_cgroup)`). The two spell the same workload;
the ctx-free one just leaves it symbolic for the runner to bind, which
is the more useful form for a value that is meant to describe a
workload rather than one host's instance of it.
`Setup::Factory(fn(&Ctx) -> Vec<CgroupDef>)` remains the genuine
escape hatch, and a `ScenarioDef` may legally hold one. Rather than
pretend otherwise, `is_declarative()` reports it, so a consumer reading
scenarios as data skips such a scenario deliberately instead of
silently believing an empty cgroup list.
Port the five pure-scenario tests in `tests/ktstr_sched_tests.rs`
(`sched_basic_proportional`, `sched_cpuset_split`, `sched_dynamic_add`,
`sched_verifier_stats_populated`, `sched_perf_positive` — the last
exercising the checks path). Their attributes, topology, gates and
runtime behaviour are unchanged; the bodies lose 54 lines of `Step {
setup: .., ops: vec![], hold: .. }` boilerplate.
Also correct a stale note in that file claiming plain `#[test]`
functions in a `KtstrTestEntry`-registering binary are invisible to the
runner. `list_plain_tests` re-emits them; verified against this
binary's own `NEXTEST=1 --list` output, and the new
scenario-extraction tests at the end of the file are plain `#[test]`s
that do run.
Test: 7 `ScenarioDef` unit tests, 7 macro-expansion tests (incl. the
verbatim-delegation pin), 3 host-side extraction tests over the ported
set, 2 compile-fail fixtures. fmt / clippy (both feature sets) /
doc-strict / docs.rs-mode all clean; the full compile_fail suite passes
with no existing snapshot disturbed.
PR likewhatevs#43 made a test's workload a value. This makes it a value that can leave the process, so the same definition can drive the VM backend and the simulator instead of being written twice. `export_scenario` renders a `ScenarioDef` plus its `KtstrTestEntry` topology and duration as a `SourceScenario`-shaped JSON record — the input to scx-sim's `scxsim-workload-ir`, which lowers it to a restricted IR the simulator ingests. A new `export_registered_scenarios` test walks the real `KTSTR_SCENARIOS` registry and writes one record per scenario to `$KTSTR_SCENARIO_EXPORT_DIR`, doing nothing when that is unset. The records are therefore derived from the registry, not restated by hand, which is the only thing that makes a simulator run from one of them the SAME scenario. JSON rather than a Cargo dependency because ktstr and scx-sim are separate repositories and the dependency runs ktstr -> scx-sim; a path dependency between two checkouts would bake a host-specific path into a committed manifest. The schema is not maintained by agreement: the consumer deserialises with the real `SourceScenario` type, so an exporter that drifts produces a hard serde error naming the field rather than a workload that quietly means something else. Nothing is approximated here. A construct the record cannot carry faithfully is OMITTED and reported as an `ExportGap` naming what and why — unmapped work types, `Setup::Factory` setups, payloads, ops, and an `Assert` override (the record carries the workload, not the oracle). Approximation is the lowering's job, where it is classified and reported in the IR's fidelity report; an approximation invented at export time would be invisible to that machinery and would reach the simulator disguised as an exact reading of the test. `unmapped_work_type_is_a_gap_not_a_substitution` is the test that fails if someone later makes the exporter "more useful" by falling back to spin_wait. `workers_per_cgroup` is recorded explicitly rather than left implicit, because `num_workers: None` means "inherit" and two backends inheriting different defaults is exactly the divergence that makes a cross-backend comparison meaningless. The value 1 is `Ctx::builder`'s default, confirmed against a real VM run's stats sidecar. Test Results Summary: 7 new export unit tests, all pass. The dump test exports all 5 registered scenarios (4 clean, 1 reporting its Assert override as a gap). fmt and clippy clean on the workspace.
Adds `sched_basic_proportional_wprof`, a #[cfg(feature = "wprof")] sibling of
`sched_basic_proportional` that attaches wprof capture to the same workload.
A passing run writes {sidecar_dir}/{name}-{variant_hash:016x}.wprof.pb — a
Perfetto protobuf — beside the stats JSON the scx-sim calibration already
consumes, which is what makes a live-vs-simulated trace comparison possible.
Both tests now build their ScenarioDef from one shared
`sched_basic_proportional_scenario()` helper rather than restating it, so the
traced workload cannot drift from the calibrated one.
Kept as a cfg-gated sibling rather than adding `wprof` to the existing test:
the attribute requires the `wprof` cargo feature and the macro rejects it at
parse time when the feature is off, so putting it on the original would break
the default build of this test file for everyone who does not want a build
that clones and compiles wprof from GitHub.
The default -d 500 captures 500ms from guest init, which is boot, not the 12s hold. A measured default run produced a 0.495s trace containing only init/swapper/rcu/kworker and zero workload tasks.
wprof's --kthread 'Allow kernel tasks' and --idle 'Allow idle tasks' gate whether those tasks are traced. Without them the kthread accounting may be incomplete, which matters because quantifying unmodelled kernel-thread CPU time is the point of the capture.
Measured, not assumed: adding them dropped 24.0s of userspace on-CPU time from the trace and shrank it 587KB -> 189KB. Recorded at the call site so the next reader does not repeat the experiment.
Found while triaging the next tranche of tests to port. The port triage
counted what the IR lowering accepts -- 42 of 45 work types, 7 ops --
and concluded 7 tests port clean. But the lowering is not the first
gate. This exporter sits in front of it and was far narrower: three
work types, zero ops, and two fields dropped without a word.
THE TWO SILENT DROPS. This file's stated premise is that it is allowed
to be wrong but not allowed to be wrong quietly, and on these two it was
quietly wrong:
nice WorkSpec carries it, SourceWorkSpec has a field waiting
for it, and this exporter hardcoded null. Now carried.
The widths differ (ktstr i32, IR i8); Linux nice is
-20..=19 so every legal value fits, and an out-of-range
one is now a recorded gap rather than a truncation into
a different, plausible priority.
sched_policy SourceWorkSpec has no policy field at all, so this one
cannot be carried -- but it was not even mentioned,
which meant a policy-mixing scenario exported as though
every worker were SCHED_NORMAL. Now a recorded gap.
custom_sched_mixed is exactly that scenario: its whole
point is a Normal/Batch/Idle/FIFO mix.
Neither was noticed because every scenario ported so far leaves both at
their defaults, so the wrong answer was accidentally the right one.
WORK TYPES: 3 -> 9, the fieldless ones only. SourceWorkType mirrors this
enum name-for-name, which makes a mechanical 45-variant map tempting.
It would be wrong. The mirror is not exact -- PriorityInversion::pi_mode,
ProducerConsumerImbalance::queue_depth_target and Custom::{run, cfg}
have no home on the IR side -- so mapping by name would reintroduce
exactly the silent drop this commit removes. The nine fieldless variants
have nothing to lose and transfer verbatim. The other 36 want a
per-variant mapping that translates what it can and records a gap for
what it cannot; that is a judgement each time, not a loop.
IoSyncWrite is the one that matters today: it is what
custom_cgroup_io_compute_imbalance needs, and it was refused here even
though the IR has lowered it all along.
Three tests added, each verified to FAIL when its fix is reverted --
sabotaging the nice carry, the policy gap and the IoSyncWrite arm turns
exactly those three red and leaves the other seven green.
Found auditing what the five hand conversions dropped. The doc comment on WORKERS_PER_CGROUP said the value "is asserted below against the value observed in a real VM run". There is no such assertion -- the function's only one is the export count, and the body never mentions workers_per_cgroup. I wrote that comment, and then read it back a day later and repeated it to the owner as fact instead of reading the code under it. A comment describing a check nobody wrote is worse than no comment: it stops the next reader from looking. The comment now states what is true, why it matters, and why the assertion still is not here. WHY IT MATTERS. Once a scenario uses CgroupDef::named rather than ctx.cgroup_def -- which is exactly what the conversion at 85c72e1 did -- the backends resolve the worker count from different places. The VM resolves an unset num_workers through resolve_num_workers(work, ctx.workers_per_cgroup, ..), following Ctx. The record carries workers: null and the simulator binds it to this literal. Equal at 1, silently divergent at anything else -- and the cross-backend check compares CPU shares, so two runs at different worker counts can still agree. WHY NO ASSERTION YET, rather than a bodge. Reading Ctx's default needs a constructed Ctx, and TestTopology::synthetic is #[cfg(test)] so it is unreachable from an integration test. I tried twice, invented an accessor that does not exist both times, and stopped: the right fix is for the exporter to carry the RESOLVED count instead of inheriting a literal, which removes the coupling rather than pinning it. Tracked in the harness audit doc and deliberately left undone here. No behaviour change; the audit found the five scenarios ARE faithful to their originals on the VM path.
likewhatevs#43, likewhatevs#44 and likewhatevs#45 as one branch. They share the scenario-export base commit and diverge only in tests/ktstr_sched_tests.rs, where one side adds the wprof capture attributes and the other corrects the export test's comment.
Unformatted since the tests were added in 3db68f2 -- I ran cargo test on that change and never cargo fmt --check, so likewhatevs#45 has been sitting unformatted. Found by checking the combined branch, which is the first time these three have been built together. Whitespace only.
The first conversion out of the 95 `#[ktstr_test(scheduler = ...)]` candidates,
picked because it is the one scheduling-meaningful body among the eight that
open once the gate-A signature change is assumed. An IoSyncWrite cgroup against
a fully-subscribed SpinWait cgroup is a real scheduling question; most of the
cheap-looking alternatives are not (see below).
All three `&Ctx` reads resolve against the test's own declared attribute:
ctx.cgroup_def("cg_0") -> CgroupDef::named("cg_0")
ctx.topo.total_cpus() -> 4 (llcs = 1, cores = 4, threads = 1)
ctx.settled_hold(1.0) -> HoldSpec::FULL
The first is the equivalence the `#[ktstr_scenario]` docs state outright: the
step runner applies `ctx.workers_per_cgroup` to any `CgroupDef` that leaves its
worker count unset, so naming it and defaulting it are the same workload.
THE THIRD IS CONDITIONAL AND IS THE ONE TO WATCH. `settled_hold(1.0)` is
`Fixed(settle + duration)`; `FULL` is `Frac(1.0)`, which resolves to
`duration`. They agree only while settle is zero -- which it is, because nothing
outside `src/**/tests` and `test_support` calls `CtxBuilder::settle`, so a
`#[ktstr_test]` run takes the builder default of 0ms. Introduce a settle window
for those runs and this scenario silently drops it. That is not left to a
comment: `cgroup_io_compute_imbalance_conversion_is_faithful` asserts the hold
form directly, so the day the assumption stops holding is the day it goes red.
The guard reaches the workload through KTSTR_SCENARIOS rather than by calling
the function, because `#[ktstr_scenario]` replaces the item with a harness fn --
and going through the registry is the better check anyway, since it is the path
the exporter uses, so it asserts on the value a backend actually receives.
Mutation-tested rather than merely passing, because a conversion guard that
cannot fail is worse than none -- it certifies the rewrite it was supposed to
check. Each substitution was perturbed on the scenario side alone and the guard
went red for all three: worker count 4 -> 3, work type IoSyncWrite -> SpinWait,
hold FULL -> frac(0.5). Restored, it is green.
(One earlier perturbation appeared to survive. It had edited both the scenario
and the expected value -- the two lines are textually identical -- so it was an
invalid mutation, not a hole in the guard. Worth recording: a sed-based mutation
test on a file that contains its own expected value can silently prove nothing.)
Removes `custom_cgroup_io_compute_imbalance`, whose only caller this was.
Keeping it would leave two copies of one workload free to drift, and the copy
nothing calls is the one that drifts unnoticed.
NOT YET CROSS-CHECKED AGAINST THE VM. The comparison in sched-test's
`ktstr-scenario-replay` needs a committed per-scenario VM baseline, and the VM
path does not currently boot on this host: the already-ported, unmodified
`sched_basic_proportional` -- the exact test that produced the one committed
baseline -- fails 6/6 retries with "init script never started (kernel or mount
failure)" and `send_sys_rdy ... port_exists=false`. So this lands as the ktstr
half only. No record is added on the sched-test side, deliberately: adding one
without a baseline turns `every_record_has_a_baseline_and_a_declared_expectation`
red, and that assertion exists precisely to stop coverage narrowing silently.
…orts Getting `cover_cgroup_io_compute_imbalance` to actually RUN on the simulator found two things sitting between the conversion and the engine. Both silent. 1. THE EXPORTER REFUSED IT. `work_type()` mapped exactly three variants -- SpinWait, YieldHeavy, Mixed -- so IoSyncWrite fell to the `_ => None` arm, became an ExportGap, and cg_0's workspec was dropped from the record entirely. All five previously-ported scenarios are SpinWait, so nothing had ever exercised this path. The mapping is verbatim and rewrites nothing: `SourceWorkType::IoSyncWrite` is the same fieldless variant. What the simulator cannot model about it -- block device, queue depth, byte counts -- is discarded one layer down by the lowering, which records it. That is the division of labour this function's own doc comment describes, and the gap message said "not mapped yet". 2. THE EXPORT REGISTRY IS PER-TEST-BINARY. `export_registered_scenarios` walks KTSTR_SCENARIOS from inside `tests/ktstr_sched_tests.rs`. The scenario was converted in `tests/scenario_coverage.rs` -- a different test binary, so a different link unit -- and was therefore never exported and never reached the simulator. No error, no gap, no refusal: it simply did not appear downstream. That is the silent-narrowing shape one level further out than the check built to catch it. Moved into `ktstr_sched_tests.rs` alongside the other ports and added to PORTED_SCENARIOS. Smallest fix, and it matches the existing convention. It is NOT a general fix: the bulk of the ~90 remaining candidates live in `scenario_coverage.rs` and converting any of them hits this again. Giving each test binary its own export step is the real fix; not attempted here. Adds `io_compute_imbalance_topology_matches_the_hardcoded_worker_count`. Resolving `ctx.topo.total_cpus()` to a literal `4` is what conversion IS, and it turns a value that tracked the topology into one that no longer does -- change `cores = 4` to `cores = 8` and the scenario stops meaning "fully subscribed" while still running and still reporting a share. This makes that edit fail instead of passing quietly. Verified by running it, not by reading. Exports with 0 gaps; the record lowers, ingests and runs on the simulator: 5 tasks, 4 CPUs, 11.9s logical, 3832 slices, cg_0 1.52% / cg_1 98.48%, bit-identical across three runs. The VM half does NOT run on this host, and not because of this change: the unmodified, already-ported `sched_basic_proportional` fails identically, 6/6 retries, `send_sys_rdy ... port_exists=false` / "init script never started". Retried with KTSTR_NO_PERF_MODE=1 -- the documented fix for that exact diagnostic -- and got the same failure (elapsed_ms 10605 vs 10602). The cached kernel has CONFIG_VIRTIO_CONSOLE=y, so a missing virtio-console driver is ruled out. Cause not established beyond that. Test Results Summary: 3 passed of the directly affected ktstr_sched_tests (conversion guard, topology guard, ported-scenario registration); scenario export unit tests 7 passed; export_registered_scenarios writes 6 records with 0 gaps for the new one; cargo fmt clean.
…l loudly when it cannot
KTSTR_SCENARIOS is a linkme distributed slice, and a distributed slice
is PER LINK UNIT. Each tests/*.rs is its own binary with its own slice.
The exporter lived inside tests/ktstr_sched_tests.rs and iterated that
binary's slice, so a scenario declared anywhere else was invisible to
it: the conversion compiled, its own tests passed, and no record was
ever written.
IT REFUSED SILENTLY. Not a wrong record -- an absent one, with nothing
reporting the absence. Eight conversions were planned against candidates
that all live in other binaries before anyone noticed, and the one
conversion already made (cover_cgroup_io_compute_imbalance, in
tests/scenario_coverage.rs) is in that state today.
WHAT CHANGED
src/test_support/scenario_export.rs: the export loop, callable from any
binary. Each binary that declares a scenario calls it.
Alternatives weighed and rejected, recorded in the module docs: moving
the exporter to a binary that sees everything (there is none -- that is
the problem); aggregating slices across binaries (linkme cannot, and a
runner-level merge would still not make a MISSING record visible, which
is the actual defect); replacing linkme with a runtime registry (same
per-binary limit, and it loses registration-at-definition-site).
AND THE HALF THAT MATTERS MORE: every_scenario_binary_exports reads the
test sources and fails, BY FILENAME, if any file declares a
#[ktstr_scenario] without calling the exporter. Source-level on purpose
-- the property is "this binary calls the exporter", and a binary that
does not cannot report that, because the missing call is precisely the
code that is not there to run. The sources are the only vantage point
from which an absence is visible.
Two smaller silent-drop removals in the same area: the exporter now
refuses to overwrite an existing record, so two binaries registering the
same scenario name is an error rather than a lost file; and
DEFAULT_WORKERS_PER_CGROUP is one shared constant instead of a copy per
binary, since that number is exactly the cross-backend coupling audited
in ai_docs/KTSTR_CONVERSION_AUDIT_20260813.md and multiplying it would
multiply the hazard.
PROVED END TO END, not asserted. With a scenario temporarily added to
tests/scenario_coverage.rs, a non-exporter binary:
1. the guard FAILED naming scenario_coverage.rs;
2. after adding its export test, the exporter wrote SIX records, the
sixth from that binary;
3. that record compiled through the scx-sim replay path to a simulator
Scenario -- 2 tasks, 2 cgroups, fidelity exact.
The temporary scenario was removed; scenario_coverage.rs keeps its
export test so the conversion landing there needs no further work.
Verified: build clean, clippy 0 with -D warnings, fmt clean, and the lib
suite unchanged at 1 pre-existing upstream failure
(cache::housekeeping::clean_orphaned_tmp_dirs_double_dash_parses_as_positive_pid,
which reproduces at the merge-base with none of our commits).
…carrying work types
ONE conversion, not three. The pool of eight scheduler-property tests
yields exactly two reachable with bounded work, one of which is already
in flight; the rest need engine work or are unportable. Reported in the
task note.
THE CONVERSION IS A PURE RESHAPE. The body never dereferenced ctx -- it
only handed it to execute_defs -- and execute_defs(ctx, defs) is
execute_steps(ctx, vec![Step::with_defs(defs, HoldSpec::FULL)]) while
ScenarioDef::with_defs(defs) is new(vec![Step::with_defs(defs,
HoldSpec::FULL)]). Same step, same hold, same defs. No substitution, so
no condition to state. The oracle lives entirely in the attribute
(sustained_samples, max_keep_last_rate, max_fallback_rate) and the macro
carries it through untouched.
EXPORTER: FutexPingPong and CrossAffinityChurn mapped. Both are
field-carrying, and both were checked field-by-field against the IR
first -- each carries only spin_iters: u64 on both sides, so nothing is
dropped and no gap is recorded. That is the bar a field-carrying mapping
must meet; a variant whose ktstr fields have no IR home still records a
gap instead.
GUARD: a faithfulness test reaching the workload through KTSTR_SCENARIOS
-- the exporter's own path -- rather than by calling the builder, so it
asserts on the value the export walks. MUTATION-TESTED, five
perturbations, each verified to hit the SCENARIO SIDE ONLY: two of them
(spin_iters: 0 and cross_affinity_churn(0)) appear on both sides, so a
naive sed would have changed both and the guard would have survived for
the wrong reason. All five turn it red.
AND A FINDING BIGGER THAN THE CONVERSION. The record declares workers 8
and 2; the simulator runs FOUR tasks. FutexPingPong lowers to
Plan { tasks: 2 } -- hardcoded -- so eight declared workers become two.
The fidelity report says BlockingMechanism, which describes the wait/wake
MECHANISM and says nothing about the count, so the narrowing is
invisible in every artifact: the record still says workers: 8, and the
cross-backend check compares per-cgroup CPU SHARE against a single
cgroup, which is 1.0 on both sides however many tasks produced it. VM
would run 10, sim runs 4, and every instrument reports agreement.
Documented at the call site rather than left to the report.
An existing test, unmapped_work_type_is_a_gap_not_a_substitution, failed
when FutexPingPong stopped being unmapped. That is the test working. It
now uses MutexContention, with a note to pick another if that is mapped.
Verified: build clean, clippy 0 with -D warnings, fmt clean, export
gap-free for this scenario, and it compiles to a simulator Scenario.
Brings `cover_cgroup_io_compute_imbalance` -- converted to `#[ktstr_scenario]`, with `IoSyncWrite` mapped in the exporter -- onto the branch that already carries the per-binary export fix. The two were developed in parallel and the merge is not mechanical, because the conversion branch contains a WORKAROUND that this branch's fix removes. WHAT THE WORKAROUND WAS. `KTSTR_SCENARIOS` is a linkme distributed slice and a distributed slice is per link unit, so the exporter -- then living inside `tests/ktstr_sched_tests.rs` -- could only see scenarios declared in that one binary. The conversion's own home, `tests/scenario_coverage.rs`, was invisible to it. The conversion branch worked around that by MOVING the scenario into `ktstr_sched_tests.rs` alongside the exporter. WHY IT DOES NOT SURVIVE THE MERGE. `export_registered_scenarios` is now callable from any binary, and `scenario_coverage.rs` already calls it. So the scenario goes back where it belongs, next to the twenty-odd `cover_*` tests it is one of. That also settles a self-contradiction the mechanical merge would have shipped: the note left in `src/scenario/interaction.rs` says the caller lives in `tests/scenario_coverage.rs`, which was true when it was written and false after the move. Resolved by hand, not by taking a side: * `src/scenario/export.rs` -- both branches map `IoSyncWrite`. This branch also maps five more fieldless variants and two field-carrying ones, so its arm list is kept, with the conversion branch's explanation of WHY a fieldless mapping is safe promoted to a header comment over the whole fieldless block. The doc comment's count of unmapped variants was stale at 36 (45 total, 9 mapped); with two field-carrying variants also mapped it is 34. * `tests/ktstr_sched_tests.rs` -- the scenario, its two guards and its `PORTED_SCENARIOS` entry are removed. The mechanical merge had also spliced the scenario's doc comment onto the end of `PORTED_SCENARIOS`'s, leaving that constant undocumented. Its doc now says outright that the list is scoped to this binary and why. * `tests/scenario_coverage.rs` -- receives the scenario and both guards, unchanged except for one reference that pointed at the exporter's old address. Verified: `cargo build --all-targets` and `cargo clippy --all-targets -- -D warnings` clean, `cargo fmt --check` clean, and the eight registry/export guards across the three scenario-declaring binaries pass -- including `every_scenario_binary_exports`, which is what fails by filename if a binary declares a scenario and never exports it. An export run writes all seven records from three separate binaries.
This was referenced Aug 14, 2026
`just lint` is what CI's lint job runs, and its doc-strict leg has failed on this branch since the first export commit f5d0fce, seven days ago: error: unresolved link to `FidelityReport` --> src/scenario/export.rs:26:44 = note: `-D rustdoc::broken-intra-doc-links` implied by `-D warnings` error: could not document `ktstr` `FidelityReport` is a type in scx-sim's IR crate — a different crate in a different repository — so rustdoc cannot resolve it, and `-D warnings` makes the unresolved intra-doc link an error rather than a warning. The prose is right; the square brackets are not. Unbracketed, and the reason it cannot be a link is now stated, so the next person does not "fix" it back into a link. WHY NOBODY CAUGHT IT, which is the part worth recording. The PR body and the branch's handover note both say "fmt + clippy clean". That is true, and it is not the gate: `just lint` has SEVEN legs — fmt, check, check wprof+integration, clippy, clippy wprof+integration, doc-strict, and check docsrs-mode — and doc-strict is the sixth. A green measured on two of seven axes read as a green on all of them. Upstream CI would have caught it in one run, but the run has sat at `action_required` since the PR was opened (fork PRs need a maintainer to release them), so it never executed, and CI's 16 jobs are on self-hosted runners we cannot reproduce in the fork. Verified: all seven legs pass at this commit. Before: lint: 1 leg(s) FAILED: doc-strict / EXIT=1 After: lint: all legs passed / EXIT=0
`scenario_registry_matches_the_ported_set` asserts that KTSTR_SCENARIOS holds EXACTLY the names in `PORTED_SCENARIOS`. That is set equality in both directions, but `PORTED_SCENARIOS` was not cfg-aware while `sched_basic_proportional_wprof` is `#[cfg(feature = "wprof")]`. On any wprof build the registry has six entries and the list has five: left: [.., sched_basic_proportional_wprof, ..] (6) right: [..] (5) The list has to be gated by the same cfg as the scenarios it names -- it is not "the ports that exist somewhere", it is "the ports linked into THIS build". Why it survived: `just lint` DOES widen the feature axis, but only via `cargo check` and `cargo clippy`, which are compile-only. The scenario compiles fine under wprof; it is the runtime assertion that fails, and nothing ran the tests on that axis. Upstream would have caught it on six of the sixteen jobs -- test-x64 and test-arm64 both matrix over features ['', 'wprof'] and two kernels, plus coverage-x64 and coverage-arm64, which run `just coverage 7.1 wprof`. Also assert that the wprof sibling extracts the SAME workload as `sched_basic_proportional`. That identity is why it calls `sched_basic_proportional_scenario()` instead of restating the defs, and it is the whole basis for treating the traced run as the calibrated one -- but it was unasserted, so nothing stopped the two from drifting. Adding a name to `PORTED_SCENARIOS` without a shape check would also have made that const's own docstring false. Verified at both feature sets: the ten host-side scenario tests across ktstr_sched_tests, scenario_coverage and cross_affinity_churn_e2e pass under default features and under `wprof,integration`; before this commit the wprof run was 9 passed / 1 failed.
likewhatevs
requested changes
Aug 24, 2026
likewhatevs
left a comment
likewhatevs
left a comment
Owner
There was a problem hiding this comment.
Thanks for putting this together. I like the core goal: describe a scenario once, continue running it against a real kernel in the VM, and also make it available to a much faster simulator. That can improve iteration speed, ergonomics, and ktstr’s capabilities without giving up authoritative VM coverage. ScenarioDef also looks like a useful foundation for reducing test boilerplate and making workloads easier to inspect and reuse.
My concern is primarily architectural rather than about the obvious in-progress bugs or test failures. I think we should preserve the core idea while changing how ktstr and the simulator are connected.
ktstr should run the simulator as a library backend
As I understand the current implementation, ktstr does not depend on or call the simulator. It discovers annotated scenarios and emits JSON shaped for scx-sim to consume later. Something outside ktstr must trigger that export, locate the artifacts, invoke scx-sim, and associate the results with the original test.
That is useful as a prototype, but it is not the integration I would like ktstr to establish.
ktstr should depend on a published simulator library crate from crates.io and call it directly in-process. The simulator integration should fulfill ktstr’s typed backend interfaces so that scenarios, configuration, capabilities, results, and errors are compiler-checked rather than connected by convention.
The intended flow should be approximately:
- ktstr discovers and resolves the scenario.
- ktstr validates that the selected backend supports it.
- ktstr schedules the work through its existing infrastructure.
- ktstr calls the simulator library with typed inputs.
- The library returns typed results or a typed error.
- ktstr handles caching, lifecycle, diagnostics, and reporting.
This allows simulation to participate naturally in the systems ktstr has already built, including its CAS, cache identity, time-sharing, resource coordination, cancellation, diagnostics, and result reporting. It also preserves a single, understandable ktstr-facing invocation instead of requiring users or external tooling to coordinate an export command and a separate simulator command.
A dependency only on a workload-IR crate, followed by an external simulator invocation, would not fully address this. The simulator itself needs to expose a published Rust library API. Its public types may live in that crate or in another published shared crate, but ktstr should ultimately make a direct typed library call and receive a typed result.
If the required simulator library crate is not published yet, I would treat publishing that API on crates.io as a prerequisite for the integration rather than establishing the export bridge as ktstr’s permanent interface.
Keep the boundary fully typed
The current manual construction of serde_json::Value duplicates another project’s schema without Cargo being able to verify compatibility. The projects can drift while both continue to compile, with failures appearing later during export or execution.
ktstr should use the simulator crate’s actual public types. Scenario conversion, capability validation, simulator execution, and result handling should all return Result with meaningful typed errors.
If a scenario contains something the simulator cannot represent, ktstr should reject it explicitly before execution. It should not emit a partial workload while warning that portions were omitted. A successful simulator run should mean that the complete supported scenario was accepted and executed as intended.
JSON, environment variables, filesystem artifacts, and executable invocation should not form the primary integration boundary. Serialization can still be used privately—for example, as an implementation detail of ktstr’s CAS—but users and backend implementations should interact through typed Rust interfaces.
I do not see a new host-side simulator exec being added by this PR, which is good. The concern is that the export architecture requires some external invocation to complete the workflow. The wprof test appears to use an existing guest execution path; that should remain separate from the simulator interface.
Avoid a second registration system
linkme and a Cargo dependency solve different problems. linkme can aggregate registrations within one linked test binary, but it does not establish the contract between ktstr and the simulator, and it cannot discover scenarios across other test binaries.
ktstr already has registered KtstrTestEntry values. I think the scenario declaration should be attached to that existing entry instead of adding a parallel KTSTR_SCENARIOS distributed slice and reconnecting the two registries by string name.
The existing entry could contain an optional, fallible scenario builder, with the macro populating it. That would give ktstr one source of identity and discovery for both VM and simulator execution.
Keeping linkme internally for ktstr’s existing test registration may still be reasonable. I do not see a need for the additional scenario-specific registry, particularly once simulation is implemented as a normal ktstr backend. Removing it should also remove the need for per-binary exporter tests, string joins, and source-scanning checks.
Resolve the scenario once
ScenarioDef is an executable Rust declaration, not automatically a fixed or portable scenario. Its builders can potentially depend on environment variables, files, time, global state, callbacks, or generated values.
ktstr should resolve the declaration once into an exact typed scenario. The VM and simulator inputs should then be derived from that same resolved value rather than independently invoking the builder in different contexts.
The typed backend interface should make support explicit. It should cover, as appropriate:
- Resolved scenario and backend configuration
- Capability validation
- Simulator and schema/version identity for cache keys
- Scheduling and lifecycle integration
- Cancellation
- Typed measurements, results, and diagnostics
- Conversion and execution errors
This lets the simulator implementation or its ktstr adapter fulfill a real interface with compiler enforcement. Unsupported behavior becomes a capability decision or typed error rather than something silently omitted during JSON conversion.
Be precise about what simulation validates
The proposed representation appears to describe the workload, but not necessarily the complete test and all of its assertions. That is still valuable, but it means simulation may initially be a fast workload or calibration backend rather than an equivalent execution of the ktstr test.
Until the relevant oracle and assertions are also supported by the typed simulator interface, the VM should remain authoritative. Clear terminology here will let simulation improve the feedback loop without implying equivalent coverage where the two backends validate different things.
Ergonomics and maintainability
The attribute-based scenario declaration is an ergonomic improvement. The current workflow—environment variables, filesystem output, specially named tests in each binary, source scanning, JSON parsing, and a separate simulator invocation—feels more like useful prototype plumbing than the user-facing interface ktstr should establish.
A direct backend integration would provide a much cleaner experience: users choose or enable simulation through ktstr, and ktstr handles the rest. That avoids an increasingly arcane series of invocations as caching, variants, profiling, and additional backends are added.
I would also consider keeping the wprof changes separate from the scenario and simulator integration. The profiling support is useful, but combining scenario representation, external interchange, registration changes, test conversions, simulator work, and profiling lifecycle behavior makes the architectural boundary harder to evaluate and maintain.
Overall assessment
This work has several meaningful benefits:
- A faster development feedback loop.
- Less scenario-definition boilerplate.
- Better workload inspection and reuse.
- A foundation for simulator and profiling capabilities.
- Continued real-kernel VM execution.
The current export-oriented architecture could weaken some of ktstr’s longer-term qualities:
- Correctness: the simulator input is independently reconstructed and may be incomplete.
- Reliability: schema drift and unsupported features can fail late or produce partial artifacts.
- Portability: JSON moves data between programs, but does not provide a compiler-checked semantic contract.
- Maintainability: ktstr duplicates external schema knowledge and maintains parallel registries.
- Ergonomics: users or tooling must coordinate export and simulator invocations.
- Capability: running outside ktstr bypasses its CAS, time-sharing, lifecycle, diagnostics, and reporting infrastructure.
The linked main run completed fully green at this PR’s base commit, so I would not use the incomplete PR matrix as evidence that this work has already reduced runtime reliability. My concern is directional: the loosely coupled export boundary is likely to become less reliable and harder to use as ktstr and the simulator evolve independently.
I would be happy with the direction after restructuring it around:
- A published simulator library crate from crates.io.
- A direct, in-process call from ktstr to that library.
- Typed backend interfaces that the simulator integration fulfills.
- Typed scenario inputs, results, capabilities, and errors.
- One existing ktstr test/scenario registration path.
- A scenario resolved once before backend-specific conversion.
- Exhaustive, fail-closed capability validation.
- Integration with ktstr’s CAS, time-sharing, cancellation, lifecycle, diagnostics, and reporting.
- No JSON, environment-variable, filesystem, or subprocess boundary as the primary interface.
- Clear documentation of what simulation does and does not validate.
Export could remain as a secondary debugging or inspection feature if it is useful, but it should not define the simulator architecture or normal user workflow.
The central idea is strong and aligned with where I would like ktstr to go. I am requesting changes because I think a direct, typed, library-first simulator backend will make this feature substantially easier to trust, use, maintain, and extend.
Owner
|
To be clear, i don't mind monster PRs so long as commits are done in such a manner to enable understanding why things are as they are and commends/docs are sufficient (worst i have done, which made things soooo much nicer and more reliable: #37 ). |
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
2 participants
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.
Today a ktstr test's workload only exists while the test is running. It is built
inside the test function, handed to
execute_steps, and gone when the functionreturns. Nothing outside that function ever holds it, so there is no way to run
the same workload anywhere else, diff two tests against each other, or print
what a test is about to do without reading its body.
This change makes a workload a value: a
ScenarioDefyou can hold, passaround, inspect and serialise. That value runs in a VM exactly as today, and it
can also be written out and run by something else. The something-else we built
this for is a scheduler simulator, which runs the same workload in a fraction of
a second without booting a guest — but nothing here is specific to that
consumer, and ktstr gains no dependency on it.
Terms used below
cgroups to create and how long to hold them.
ScenarioDef— the Rust value that holds one. Same shapeexecute_stepsalready takes.
WorkTypeenum, the kind of load a workergenerates (
SpinWait,IoSyncWrite,FutexPingPong, …). 45 variants today.ScenarioDefexports to.the scenario could not be carried faithfully and was therefore left out.
What it adds
ScenarioDef— a scenario as data. A list ofSteps plus an optionalAssertoverride. It is the shapeexecute_stepsalready consumes, lifted outof the test function into a value.
#[ktstr_scenario]— an entry point for tests that are purely a workload.Where
#[ktstr_test]takesfn(&Ctx) -> Result<AssertResult>, this takesfn() -> ScenarioDef. It delegates toktstr_test's existing expansionverbatim rather than forking it; a macro-expansion test asserts the generated
tokens contain
ktstr_test_impl's output exactly, so the two cannot drift. Ascenario test runs in a VM identically to the
#[ktstr_test]it replaces.It rejects at compile time what it cannot honour, with a message saying so:
function parameters,
async, generics,whereclauses, and thepost_vm/post_vm_unconditionalcallbacks. Every rejection is tested — the two that auser is most likely to hit, a
&Ctxparameter andpost_vm, astrybuildfixtures with checked-in
.stderr, and the three uncallable signatures asdiagnostic-text assertions in the macro crate.
Export.
export_scenarioturns a registeredScenarioDefinto JSON. Theschema is not maintained by agreement between two projects: the consumer
deserialises into its own real type, so a mismatch is a hard
serdeerrornaming the field rather than a workload that quietly means something else.
Anything the record cannot carry faithfully becomes a gap, never an
approximation. That is the central design choice — this file is allowed to be
wrong, but not allowed to be wrong quietly. There are eight gap reasons today: a
work type with no counterpart; a cgroup list produced by an
fn(&Ctx)thatneeds a running guest; a payload that runs an external binary; a
niceoutside-20..=19; a non-default scheduling policy, which the record has no field for;a cpuset form with no counterpart; an
Assertoverride, since the recordcarries the workload and not the oracle; and ops, which are not exported at all
yet. The exporting test prints every gap it produced.
Of the 45 work types, 11 are mapped: the nine fieldless ones, which transfer
verbatim because they have nothing to drop, and two field-carrying ones
(
FutexPingPong,CrossAffinityChurn) checked field-by-field first — bothcarry only
spin_iters: u64on each side. The other 34 carry fields with nocounterpart, so they gap rather than get mapped by name.
Export reaches every test binary, and says so when it cannot.
KTSTR_SCENARIOSis alinkmedistributed slice, and a distributed slice isper link unit. Each
tests/*.rsis its own binary with its own slice, so anexporter that lives in one binary can only ever see that binary's scenarios —
a scenario declared anywhere else compiles, passes its own tests, and produces
no record, with nothing reporting the absence.
The export loop therefore lives in
src/test_support/, callable from anybinary, and each binary that declares a scenario calls it. The half that matters
more is the check:
every_scenario_binary_exportsreads the test sources andfails, by filename, if any file declares a
#[ktstr_scenario]withoutcalling the exporter. Source-level on purpose — a binary that does not call the
exporter cannot report that fact about itself, because the missing call is
exactly the code that is not there to run. Two smaller silent drops are closed
in the same area: the exporter refuses to overwrite an existing record, so two
binaries registering one scenario name is an error rather than a lost file, and
the default worker count is one shared constant rather than a copy per binary.
Seven scheduler tests converted to
#[ktstr_scenario], across three testbinaries, exercising the path end to end. They pass in a VM on a source-built
6.14.11.
The conversions are guarded, because a conversion is a hand rewrite. It
turns a
&Ctxbody into a constant expression, and the failure it invites is ascenario that runs something subtly different from the test it replaced. Every
guard reaches the scenario through the registry — the same path the exporter
walks — rather than calling the builder directly, so it asserts on the value a
backend would actually receive, not on a local copy that could agree while the
registry disagreed. There are two kinds:
hold fractions, and — for the one port that overrides
ctx.assert— that itsgates survive extraction, so it cannot silently end up running ungated.
&Ctxread into a literal.cover_cgroup_io_compute_imbalancehardcodes 4workers to mean "one per CPU", resolved from its declared
llcs = 1, cores = 4, threads = 1; a guard pins that literal to theattribute, so changing
coresfails a test instead of silently producing ahalf-subscribed cgroup.
A wprof capture on
sched_basic_proportional, so a scenario run can producea guest profile alongside its stats.
What it does not change
#[ktstr_test]changes behaviour.#[ktstr_scenario]isadditive, and is a thin wrapper over the old macro by construction rather than
by care.
KTSTR_SCENARIO_EXPORT_DIRis set, so an ordinary test run is unaffected.every_scenario_binary_exports—because a check that only runs when you remember to run it does not close the
gap it exists to close. It reads sources and needs no VM.
Scope and limits
A shared oracle has to be expressed against both backends' outputs, not
smuggled through a workload record — so a scenario carrying an
Assertoverride exports a gap for it.
an empty workload pretending to be complete.
lives in a different repository and the dependency runs one way: ktstr never
depends on it.
Supersedes #43, #44 and #45, which contain the same work split three
ways.