Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions .github/workflows/host-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,39 @@ jobs:
exit 1
fi

# **The balance path's push half, which is a candidate and not shipped.**
# `cpu::Balance::PushOnSurplus` rests on one CPU observing another's
# SLEEPING bit while that CPU observes its surplus — the store-buffer
# litmus test — and `cpu::balance_fence`'s `SeqCst` fence is what forbids
# both sides missing. `push-fence-relaxed` weakens it to a release fence,
# which orders stores against stores and nothing against a later load, and
# `loom_push` must red.
#
# It is wired here rather than left to whoever adopts the policy, for the
# reason `no-preempt-guard` and `doorbell-kick-relaxed` were: a control
# nothing runs is a control nobody can trust on the day it matters. This
# model's panic unwinds cleanly — nothing is queued in it — so unlike the
# step above it does print a `FAILED` line; the verdict is still grepped
# by the assertion's own words, because a non-zero exit alone would accept
# a compile error.
- name: toyos-sched-loom push-on-surplus fence has teeth
run: |
set -o pipefail
echo "=== toyos-sched-loom --features push-fence-relaxed --test loom_push (must red)" \
| tee -a /tmp/host.log
if cargo test -p toyos-sched-loom \
--features push-fence-relaxed --test loom_push 2>&1 \
| tee /tmp/push-fence-relaxed.log | tee -a /tmp/host.log; then
echo "loom_push passed with the balance fence relaxed: the model has no teeth" \
| tee -a /tmp/host.log
exit 1
fi
if ! grep -q 'published and no push behind it' /tmp/push-fence-relaxed.log; then
echo "the fence-relaxed build failed without a_cpu_that_halts_without_seeing_the_surplus_was_pushed reaching its verdict: this step proved nothing" \
| tee -a /tmp/host.log
exit 1
fi

# `userland/.cargo/config.toml` cross-compiles by default, so a userland
# crate's host tests have to name the host. `calc` is here for the reason
# the pure crates above are: its arithmetic is decided without a guest,
Expand Down Expand Up @@ -606,9 +639,10 @@ jobs:
| tee -a /tmp/host.log

# The root package's `--lib`, one `--workspace` over the other host-tested
# members, `kernel-loom`'s no-loom command, eight negative-control runs
# members, `kernel-loom`'s no-loom command, nine negative-control runs
# (`wake-fence-off` plus the five `kernel-loom` gained 2026-08-17, and
# `toyos-sched-loom`'s `no-preempt-guard` and `doorbell-kick-relaxed`),
# `toyos-sched-loom`'s `no-preempt-guard`, `doorbell-kick-relaxed` and
# `push-fence-relaxed`),
# the three userland crates and the SDK. A red is still one line somewhere among them,
# and `--workspace` prints a `Running ...` line naming the binary rather
# than the `=== <crate>` the other steps echo, so the summary matches both
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
status: open
kind: finding
status: owner
kind: question
opened: 2026-08-22
---

Expand Down Expand Up @@ -51,3 +51,79 @@ and the fix is a design decision the balance path does not currently make: a
push on surplus, or a bounded re-arm of the probe, both of which cost an idle
CPU wakes it is currently right to be without
(`kernel/CLAUDE.md`: anything added to the idle loop is an audio change).

## Decision owed: which cure, if either, the balance path takes

Both cures are now implemented in `toyos-sched` behind `cpu::Balance`, which the
simulator selects and `kernel::sched::driver::env` does not: it selects
`Balance::Pull` and the kernel's behaviour is unchanged. The measurements are
`toyos-sched/sim/tests/policy.rs`'s three new cases, 20 seeds per point
(`cargo test -p toyos-sched-sim --test policy -- --nocapture`).

**`Balance::PullWithRearm { every_ns, times }`** — a CPU that halts with nothing
to run arms its one-shot timer `every_ns` ahead and probes again when it fires,
up to `times` times per idle period, the allowance refilling at every dispatch.
It observes nothing and needs nobody's cooperation.

**`Balance::PushOnSurplus { threshold }`** — a pass that publishes a surplus of
`threshold` or more rings the doorbell of one CPU that reads SLEEPING, walking
them in turn. No task moves on the ring; the woken CPU posts an ordinary probe.

Recovery on the lopsided machine, at `every_ns = QUANTUM_NS` and `threshold = 2`
— seeds reaching every CPU, and the worst time a halted CPU sat beside a
published surplus with no probe outstanding:

| cpus/threads | pull | re-arm ×1 | re-arm ×4 | push ≥2 |
|---|---|---|---|---|
| 2 / 8 | 9/20, 480 ms | 20/20, 10 ms | 20/20, 10 ms | 20/20, 1 ms |
| 4 / 16 | 2/20, 960 ms | 20/20, 10 ms | 20/20, 10 ms | 20/20, 22 ms |
| 4 / 64 | 2/20, 3.84 s | 20/20, 10 ms | 20/20, 10 ms | 20/20, 22 ms |
| 8 / 64 | 0/20, 3.84 s | 20/20, 10 ms | 20/20, 10 ms | 20/20, 66 ms |

Cost, in wakes of a halted CPU that found nothing to do — the whole sweep's
count, and the worst single run's rate against simulated time:

| workload | pull | re-arm ×1 | re-arm ×4 | push ≥2 |
|---|---|---|---|---|
| `interactive_mix(2,4)` | 0, 0.00/s | 44, 31.58/s | 171, 88.00/s | 9, 11.76/s |
| `interactive_mix(2,16)` | 0, 0.00/s | 49, 11.94/s | 174, 30.14/s | 2, 3.08/s |
| `interactive_mix(4,16)` | 0, 0.00/s | 104, 20.90/s | 363, 63.01/s | 53, 24.62/s |
| `wakeup_storm(4,16)` | 0, 0.00/s | 251, 92.13/s | 1004, 306.22/s | 80, 97.56/s |
| `wakeup_storm(8,64)` | 0, 0.00/s | 521, 150.67/s | 2075, 534.78/s | 748, 380.95/s |
| `audio_pipeline(4)` | 0, 0.00/s | 111, 153.85/s | 351, 243.90/s | **0, 0.00/s** |

The audio-relevant latency does not move: the watched thread's worst wake is the
same nanosecond under all four policies at every point but `wakeup_storm(8,64)`,
where the push's 50,750,000 ns beats the shipped path's 57,000,000 ns, and the
means move by under 2%. The model charges a pass zero nanoseconds, so it counts
an extra wake and cannot bill one — **what a wake costs the CPU it wakes is a
kernel measurement nobody has taken.** That is the one number this decision is
short of.

**The push costs an ordering the pull path does not have.** It is the
store-buffer litmus test: the pusher stores its surplus and loads the sleeper's
SLEEPING bit while the sleeper stores SLEEPING and loads the surplus, and without
a `SeqCst` fence between each side's store and its load both may miss — the same
defect in a window nanoseconds wide. `cpu::balance_fence` is that fence, an
`mfence` on the exit of every pass that has surplus, and
`toyos-sched/loom/tests/loom_push.rs` is the model;
`cargo test -p toyos-sched-loom --features push-fence-relaxed --test loom_push`
weakens it and the model reds. The re-arm needs no such edge at all: a timer
fires whether or not anybody published anything.

**Recommendation: `PushOnSurplus { threshold: 2 }`, or nothing.** It recovers the
machine at every width, it costs the audio pipeline exactly zero extra idle
wakes because a machine with no surplus never pushes, and it is self-limiting in
the direction that matters — the wake only ever happens where there is work to
come and get. The re-arm buys the same recovery with a periodic tick on every
idle CPU, including on a machine that has nothing at all to hand out; on the
audio workload that is 154 wakes per second bought for nothing. What the push
costs instead is an `mfence` on the pass exit of a CPU with surplus and a second
read of the surplus on the idle path — both off the audio pipeline's hot path,
because that workload never has surplus. Doing nothing also remains defensible:
no shipped workload has been shown to reach the state, and `kernel/CLAUDE.md`'s
rule is that the idle path stays empty until something forces it open.

Two things would change the answer, and neither is measured yet: what one wake
actually costs a woken CPU in the kernel (the model cannot say), and how often a
real boot produces the composition in "Reachable in the kernel" above.
18 changes: 12 additions & 6 deletions kernel/src/sched/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use core::cell::UnsafeCell;
use core::ptr;
use core::sync::atomic::{AtomicBool, AtomicPtr, AtomicU64, Ordering};

use toyos_sched::cpu::{Action, CpuHandle, CpuHandles, CpuSched, Env, SchedPass};
use toyos_sched::cpu::{Action, Balance, CpuHandle, CpuHandles, CpuSched, Env, SchedPass};
use toyos_sched::fair::Frontier;
use toyos_sched::hw::{CpuId, Hw, Kicker, Machine, Nanos};
use toyos_sched::mailbox::{mailbox, Kick, PreemptGuard, Urgency};
Expand Down Expand Up @@ -661,10 +661,16 @@ pub enum Dispose {

/// The environment every pass runs against.
///
/// `steal` is the one policy bit in it, and it is on: an idle pass probes the
/// busiest CPU for work and a loaded pass answers probes from surplus (spec
/// §7.7, §9.4's pull half). Without it a task woken onto a busy CPU waits
/// there until the owner yields.
/// `balance` is the one policy value in it, and it is [`Balance::Pull`]: an idle
/// pass probes the busiest CPU for work and a loaded pass answers probes from
/// surplus (spec §7.7, §9.4's pull half). Without it a task woken onto a busy
/// CPU waits there until the owner yields.
///
/// The other settings of that knob are the simulator's, and deliberately not
/// this line's: [`Balance::PullWithRearm`] and [`Balance::PushOnSurplus`] both
/// add wakes to the idle path, which `kernel/CLAUDE.md` makes an audio change,
/// and what they buy and cost is measured in `toyos-sched/sim/tests/policy.rs`
/// rather than decided here.
///
/// The guard comes in by reference because its lifetime is the pass's and it
/// belongs to the caller that raised the count.
Expand All @@ -674,7 +680,7 @@ fn env(preempt: &PreemptOff) -> Env<'_, crate::hw::KernelHw, PreemptOff> {
cpus: cpus(),
frontier: &FRONTIER,
preempt,
steal: true,
balance: Balance::Pull,
}
}

Expand Down
8 changes: 8 additions & 0 deletions toyos-sched/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,11 @@ loom = []
# negative control (spec §7.3, §7.5). No kernel build reaches it; the crate
# declares it only so `cfg` checking knows the name.
doorbell-kick-relaxed = []
# Declared, never enabled here: `toyos-sched-loom` turns this on to weaken the
# `SeqCst` fence `cpu.rs`'s `balance_fence` puts between the push half's surplus
# store and its read of a target's SLEEPING bit, and
# `loom/tests/loom_push.rs` must red when it does — `Balance::PushOnSurplus`'s
# own negative control. No kernel build reaches it, and no kernel build selects
# that policy either; the crate declares the name only so `cfg` checking knows
# it.
push-fence-relaxed = []
10 changes: 10 additions & 0 deletions toyos-sched/loom/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ no-preempt-guard = []
# Never on by default and never reachable from a kernel build, which declares
# the same name only so `cfg` checking knows it.
doorbell-kick-relaxed = []
# The negative control for `Balance::PushOnSurplus`'s ordering. It weakens
# `cpu.rs`'s `balance_fence` from a `SeqCst` fence to a release one, which
# orders stores against stores and nothing against a later load — so the push's
# surplus store and the sleeper's SLEEPING store may each be missed by the
# other's read, and the CPU halts with a surplus published and no probe
# outstanding. `loom_push.rs` must red under it:
#
# cargo test -p toyos-sched-loom --features push-fence-relaxed --test loom_push
#
push-fence-relaxed = []

[dependencies]
loom = "0.7"
Expand Down
187 changes: 187 additions & 0 deletions toyos-sched/loom/tests/loom_push.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
//! Loom: the publish/observe edge `Balance::PushOnSurplus` is made of.
//!
//! The pull half of the balance path needs no ordering between two CPUs at all:
//! an idle CPU reads a surplus, and if it reads a stale one it simply does not
//! probe. The push half is different in kind, because it is a *liveness* claim
//! that rests on one CPU observing another — the CPU that gained surplus reads
//! the SLEEPING bit of the CPU that is going to sleep, and decides from it
//! whether anybody needs telling.
//!
//! That makes it the store-buffer litmus test:
//!
//! ```text
//! pusher sleeper
//! ------ -------
//! surplus.store(2) doorbell.fetch_or(SLEEPING)
//! balance_fence() balance_fence()
//! doorbell.load() -> asleep? surplus.load() -> anything to steal?
//! ```
//!
//! Each side stores to one location and loads the other, and with anything
//! weaker than a `SeqCst` fence between the two, **both loads may miss**: the
//! pusher decides nobody is asleep, the sleeper decides there is nothing to come
//! for, and the machine is back in
//! `issues/kernel/an-idle-cpu-that-slept-before-the-surplus-is-never-probed.md`
//! in a window nanoseconds wide instead of milliseconds. Nothing in a guest test
//! can find that — on x86 the sleeper's side of it is a `lock or`, a full fence
//! already, so only the pusher's half is even reorderable and only under a
//! store-buffer delay no test can schedule.
//!
//! The property below is the one the push actually needs: **a CPU that halts
//! without having seen the surplus has a poke coming.** It is the same shape as
//! `loom_sleep.rs`'s I2 — halted with work queued implies an IPI in flight —
//! asked of the push's own pair rather than of the mailbox's.
//!
//! The negative control is a cargo feature rather than a comment:
//!
//! ```text
//! cargo test -p toyos-sched-loom --features push-fence-relaxed --test loom_push
//! ```
//!
//! makes `cpu::balance_fence` a release fence — which orders stores against
//! stores and nothing against a later load — and this file must red at
//! [`a_cpu_that_halts_without_seeing_the_surplus_was_pushed`]. Verified
//! 2026-08-22, both ways round.

use loom::sync::atomic::{AtomicUsize, Ordering};
use loom::sync::Arc;
use toyos_sched_loom::cpu::{balance_fence, CpuHandle};
use toyos_sched_loom::hw::CpuId;
use toyos_sched_loom::mailbox::{mailbox, Kick, MailboxConsumer};
use toyos_sched_loom::model::{model, Msg};

/// The surplus at which a pass pushes and at which an idle pass probes — one
/// inequality, `SchedPass::best_victim`'s.
const THRESHOLD: u32 = 2;

/// Two CPUs' shared faces: the one that gains surplus, and the one going to
/// sleep. The `CpuHandle` is the real one, so `publish_surplus`, `surplus`,
/// `doorbell` and `poke` are the shipped code and not a restatement of it.
struct World {
/// The CPU that gains surplus. Only its `surplus` matters here.
victim: CpuHandle<Msg>,
/// The CPU going to sleep. Only its doorbell matters here.
thief: CpuHandle<Msg>,
/// Targeted IPIs the push sent. As in `loom_sleep.rs`, an IPI's only
/// relevant effect is "the target cannot stay halted".
ipis: AtomicUsize,
/// Doorbell rings the push made, kicked or coalesced. A ring that elides its
/// IPI is still an observation — it can only elide because an IPI is already
/// on its way — so the property counts rings and not kicks.
pokes: AtomicUsize,
}

impl World {
/// `SchedPass::push_on_surplus`, as the pass runs it: publish, fence, look
/// for a sleeper, ring it.
fn publish_and_push(&self) {
self.victim.publish_surplus(THRESHOLD);
balance_fence();
if self.thief.doorbell().sleeping() {
self.pokes.fetch_add(1, Ordering::AcqRel);
if self.thief.poke() == Kick::Send {
self.ipis.fetch_add(1, Ordering::AcqRel);
}
}
}

/// `SchedPass::try_sleep` under `Balance::PushOnSurplus`, with the part that
/// matters: the probe's own read of the surplus found nothing, SLEEPING is
/// published, and the re-read behind the fence is the load that pairs with
/// the pusher's read of that bit.
///
/// Returns `true` if the CPU halted **without** having seen the surplus —
/// the state the push has to make impossible without a poke behind it.
fn try_sleep(&self, rx: &MailboxConsumer<Msg>) -> bool {
self.thief.doorbell().begin_pass();
// `post_steal_probe`'s read, before SLEEPING is published. A stale
// answer here is exactly what the re-read below exists to catch.
let probed = self.victim.surplus() >= THRESHOLD;
let arm = self.thief.doorbell().arm_sleep();
balance_fence();
if self.victim.surplus() >= THRESHOLD {
// Observed: the pass goes round again and posts a probe.
arm.abandon();
return false;
}
let halted = arm.confirm(rx).is_ok();
halted && !probed
}
}

/// The push's liveness: a CPU cannot halt believing there is nothing to steal
/// while another CPU has already published a surplus and believes nobody is
/// asleep.
///
/// One of the two has to observe the other, and the `SeqCst` fence in
/// [`balance_fence`] is the whole of why. The assertion is stated on the poke
/// and not on the IPI for the reason `Doorbell::ring` gives: a ring that returns
/// `Kick::Elide` did so because the kick edge was already raised, i.e. because a
/// kick is already in flight.
#[test]
fn a_cpu_that_halts_without_seeing_the_surplus_was_pushed() {
model(|| {
let (victim_tx, _victim_rx) = mailbox::<Msg>();
let (thief_tx, thief_rx) = mailbox::<Msg>();
let world = Arc::new(World {
victim: CpuHandle::new(CpuId(0), victim_tx),
thief: CpuHandle::new(CpuId(1), thief_tx),
ipis: AtomicUsize::new(0),
pokes: AtomicUsize::new(0),
});

let pusher = {
let world = world.clone();
loom::thread::spawn(move || world.publish_and_push())
};
let blind = world.try_sleep(&thief_rx);
pusher.join().unwrap();

assert!(
!blind || world.pokes.load(Ordering::SeqCst) >= 1,
"cpu1 halted with cpu0's surplus of {THRESHOLD} published and no push behind it — \
both sides of the store-buffer pair missed, which is \
`an-idle-cpu-that-slept-before-the-surplus-is-never-probed` in a window a fence \
would have closed",
);
});
}

/// The other direction, and the one that says the fence is not simply making
/// everything look asleep: the push does not ring a CPU that is awake.
///
/// A ring on a busy target sets the kick edge with no message behind it, which
/// costs that target's next idle disposition a spurious extra pass — harmless,
/// and exactly what the push is allowed to cost. What it may not do is ring a
/// CPU that never armed a sleep at all, because then the count this policy is
/// priced by is measuring something that is not a wake.
#[test]
fn a_push_rings_nobody_when_nobody_is_asleep() {
model(|| {
let (victim_tx, _victim_rx) = mailbox::<Msg>();
let (thief_tx, _thief_rx) = mailbox::<Msg>();
let world = Arc::new(World {
victim: CpuHandle::new(CpuId(0), victim_tx),
thief: CpuHandle::new(CpuId(1), thief_tx),
ipis: AtomicUsize::new(0),
pokes: AtomicUsize::new(0),
});

// The target runs a pass and stays awake: `begin_pass` clears SLEEPING
// and nothing arms one.
let target = {
let world = world.clone();
loom::thread::spawn(move || world.thief.doorbell().begin_pass())
};
world.publish_and_push();
target.join().unwrap();

assert_eq!(
world.pokes.load(Ordering::SeqCst),
0,
"the push rang a CPU that never published SLEEPING — a wake charged to a policy \
that did not need to send it",
);
assert_eq!(world.ipis.load(Ordering::SeqCst), 0, "and no IPI either");
});
}
Loading
Loading