[Japabu]

Problem

A SYS_COUNTERS read issued within 10 ms of the last round's issue was served that round. It could therefore get a reading taken up to 10 ms before it asked, and two reads 5 ms apart got one reading twice. #1073's group-start diagnostic lost 13 of its 17 clock windows to it.

Behavior now

• Every reading is taken after its read was called (kernel/src/counters.rs).
  • A read joins a round only if that round was issued strictly after its call began.
  • Inside INTERVAL of the last issue, a read rests until the interval has passed and then issues its own round. Readers resting together wake at the same deadline: the first issues the round and the rest join it.
  • The wait is the round rate limit, which has a known end. It is not a delay standing in for an event.
  • The kick rate is unchanged: at most one per CPU per interval.
  • toyos_abi::counters::INTERVAL_NS is the one constant both the kernel and osbench name. The ABI header says a read may wait up to 10 ms.
  • JOIN is renamed INTERVAL, since joining no longer depends on it.
• osbench never sleeps before a measured row. around::read hands back this process's last reading when a fresh read would fall inside the interval.
  • Without that, osbench alone would sleep up to 10 ms right before a measured slice: between consecutive crunch workloads, and before tail's diary run. Linux's side never sleeps there.
  • One reading now bounds both rows beside it. The window rule still holds, because a row's span runs from its first reading's call.
• osbench_rules::window is strict. A window must lie between the row less its reads' cost and the row itself. JOIN_NS is gone.
• Counter::DeepIdleNanos is deleted, along with its kernel accounting. Its only reader was inspect's rendering, kernel.cpu.N.deep_idle_nanos. The kernel credited a period's time only when the period ended, so that time was no fraction of any window. DeepIdles stays.
• The consumers that worked around the join are simplified:
  • counters_read now requires the very next read after an answered one to give every CPU a new stamp.
  • acpi_mediated's Counted is now a plain function.
• issues/a-counters-read-within-10-ms-of-another-returns-the-older-reading-as-new.md is closed. Its rule now lives in the counters.rs header and in toyos_abi::counters.

Records

• The AMD deflate finding compares MPERF against MPERF over the same window: 0 thousandths on every other CPU against the thread's 999 (APM Vol. 2 §17.3).
• The idle track holds D1-D3's findings.
  • Switching the history off left the count inside the spread, so no idle-policy change is earned.
  • Mem's 3789 is still open. The fixed counters are the next instrument, and no predictor is tuned before they have read it.
• The exit costs are in µs, from same-build pairs: two A* boots against two B3 boots at f6510defd, with B7 beside them.
  • T14: a 10 ms sleep lands 10.5 to 10.6 µs later. Storms drain 88 to 91 µs later (w4) and 68 to 87 µs later (w16; B7: +52).
  • AMD: a 1 ms sleep lands 27 µs sooner, and nothing else moves.
  • Consequences for interactive, audio and normal workloads are stated as inference, as a tradeoff to bring to the architect and not as accepted.
• The track's exit: cpu.clock at ratio 42 is described as a diagnostic, not the product target.
  • The architect's wording asked only for that. It did not ask for a crunch.single criterion, so the one I added in r1 is dropped.
• acpiserver's processor _OSC commit is correct as written. A commit that fails, is refused or comes back with bits masked gates nothing.
  • cst::declare takes nothing from the negotiation, and each _CST state is checked on its own terms.
  • osc.rs now states this.

Decisions

• No kernel host test. The guest test and the restore-join mutation hold the property, which is end to end across the IPI round.
  • A host seam was offered as optional. I declined it: the seam would need either a raw accessor on time::Instant, which deliberately has none, or minting Instants outside clock.rs.
• Where counters_read runs:
  • On x86-64 it runs only on the T14's shared testcases boot.
  • On QEMU it runs only on AArch64, in virt_smp (EL2, SMC) and virt_el1_smp (EL1, HVC, TCG).

Checks

Evidence: r3 https://github.com/ToyOSOrg/evidence/tree/14c6006c49c0b9480d3bfce30cb8eb4d479d0a65/r3, r1 https://github.com/ToyOSOrg/evidence/tree/73830f45f7c3dcc3d71a836c97238e26054a82d2/r1, r2 https://github.com/ToyOSOrg/evidence/tree/06e8ebcd59d365cfcc43a7e17738fdea0170682c/r2

Check	Rev	Exit	Log
cargo run -- --ci host	7c9cf91	0, 104 steps	r2/logs
cargo test --test toyos-build -- virt_smp virt_el1_smp osbench_capability_groups_exit_0 acpi_mediated_access	7c9cf91	0, 4 passed, the counters_read marker in both SMP boots	r2/logs
Mutation restore-join (rejoin a round within the interval), virt_smp	7c9cf91 + patch	1: "cpu0's stamp did not move", reverted	r2/mutations
T14 session staged, then judged from its readbacks (above)	7c9cf91	staged 2; judged 1 (counters, unrelated)	r2/metal

The head, be8e971, adds only issue records after 7c9cf91, so every check above still applies.

Metal: one T14 session at 7c9cf91 (the orchestrator ran it; all three boots rc=0). Judged with cargo test --test toyos-build -- --metal --metal-readback <dir> osbench counters counters_read counters_silent, from one directory holding all three readbacks: exit 1, 3 passed, 1 failed (r3).

Boot	sha256	Verdict
benchcase	c7e4ef785b257dc36e67c8e9d5fd3b87a00f3a62320e317aac9647dfd9fe66db	PASS osbench (see below)
testcases	ec2dbdb8c56dfc90e23cf9356d50da98bbfac9beb3e2e0aebe469f889b780864	PASS counters_read (x86-64). FAIL counters, unrelated, see below
shared-debug	1d5405fbe1adeeff1c4f066fe3e2e77f195d8c026599dddcc9dc9c60999966f6	PASS counters_silent

• benchcase:
  • reads_us is 129 to 261 µs per row, so no read waited before a slice.
  • crunch.single p50 against Idle CPUs enter the deep states their firmware declares #1073's B7: deflate 54.60 ms (B7 54.60), sha256 0.555 (0.567), matmul 1.076 (1.075), parse 34.89 (35.26), at 4188-4189 MHz (B7 4188).
• The counters red is independent of this branch.
  • The row reds any line inside counters_metal's idle second. The EC took query 0x4f for the first time at 13.538 s, 0.87 s into that second.
  • counters_metal starts the second once the EC's first query is said. Here the first was 0x40 at 12.520 s, and this two-job image's log settled 152 ms later.
  • In the 32 earlier testcases boots on hand, the second began after every early query.
  • The read before the second took 22 µs, and the line is the embedded controller's, which this branch does not touch.
  • Recorded: issues/the-counters-rows-idle-second-can-hold-the-ecs-second-boot-query.md.
  • What counters would otherwise have checked of this branch, a read per CPU fresh and in place, counters_read establishes on the same boot.

Risks

• osbench reads that fall inside the interval reuse the last reading. A row's window can therefore start up to 10 ms before the row's own slice, which is printing time between rows and is within the window rule.
• A fresh read can still wait if another process read the counters within the last 10 ms. Nothing in benchcase does.
• Other back-to-back callers in the tree now wait up to 10 ms per read:
  • counters_metal's loaded phase. Each of its rounds is now new, so no round's kick lateness is counted twice.
  • counters_read's many-readers phase: 50 reads on each of twice as many threads as CPUs.
• The exit-cost tradeoff and mem's 3789 are open in the track.

🤖 Generated with Claude Code

[Japabu]

Review r1: PR #1084 at 74c60af

Architecture

The fix belongs in the kernel's counters::read. That code owns the round, and it is the only place that can tie a reading to the moment its caller asked. The fix makes the reading's contract true at the source. Before, every consumer had to work around the join: Counted, the stamp-equality loop in counters_read, and JOIN_NS in osbench. All three are deleted, and the ABI header now states the contract. That deletes mechanisms rather than adding one.

DeepIdleNanos is removed from every layer: the kernel accounting, the ABI with COUNT 14 → 13, the inspect fixtures and the ABI fixtures. No source reference is left. Its only consumer was inspect rendering it.

Net change is +149 / −161. The code part is small: the kernel gains about 20 lines, and the tests and ABI lose more than that.

Correctness of the wait and the join (checked independently)

• No reader gets a round issued before its call. called is read before the reader's first LAST section. An issuer records now, which it reads under the lock before ROUNDS.issue. A reader therefore joins only a round recorded strictly after its call. A CPU's answer is sampled after its serve reads the current generation, so it comes after that issue. A CPU that serves a later generation also samples after the issue.
  • The strict > is the right side of the boundary. An equal reading cannot be ordered, so the reader waits once more instead of joining.
  • Clock skew between CPUs is nanoseconds. The gap between a call and its answers is microseconds, so the skew cannot make an early round look valid.
• No starvation. A round issued after a reader's first lock section always satisfies issued > called. Each pass through the loop therefore either breaks, or waits until issued + JOIN and then breaks by joining or issuing. The wait is about JOIN plus wake latency, or 2·JOIN in the equal-clock-reading case. The rule of first waker issues and the rest join holds.
• The kick bound holds. Only answer(true) kicks, and only the reader that issued a round calls it. Under the lock, issued advances by at least JOIN each round. The kick itself happens after the lock is released, so two kicks can land closer than JOIN if an issuer is delayed. That was already true before this PR.
• Kill mid-wait. wait_until returns Cancelled and the read returns Gone. No lock is held across either wait. A round already issued still completes for the readers that joined it.
• CPUs going offline. smp::cpu_count only grows. There is no offline path, so this case does not arise.
• Spurious posts. A post to the thread's own watch, as thread_exit does for joiners, just parks again under || false. This matches sys_nanosleep.
• The no-fixed-delay rule. The wait is a deadline at the instant a policy allows the next round, and no event marks that instant. It is the same shape as sys_nanosleep. It is bounded, cancellable and commented as such. I accept the argument.
• The osbench_rules::window upper bound of exactly row_ns. It is sound. The stamps and the row use the same tick conversion (period_fs), and the slack (syscall entry plus kick) is far above the skew.

Records

• Exit-cost table. I checked every per-boot p50 against pr-1073 r3/readback-notes/{t14,amd}-{1,3}-As and -{2,4}-B3, and against r8/readback-notes/*-B7. B7 is at 18fe4e150 (r7/README). All values match.
• AMD deflate. "0 against 999" matches amd-1-As (cpu4 999, every other CPU 0) and amd-B4-benchcase (cpu10 999, the rest 0).
• _OSC. cst::declare takes nothing from negotiated, and negotiate_all (main.rs:463) runs before cst::processors (main.rs:483). So "_CST is read either way, each state checked on its own terms" is what the code does.

Findings

BLOCKER

1. tests/guest/osbench/src/around.rs:66-68: a stale comment. "the kernel credits a period's time when it ends, so its time is no fraction of the window" justified printing periods instead of the period time. This PR deletes the period time, so the clause now explains a choice that no longer exists. Delete it and keep "Deep idle is counted as the periods that ended in the window."

2. The PR's Risks section is wrong about which callers now wait, and one of them is a benchmark. The section says the back-to-back readers "in the tree" are counters_metal loaded and counters_read many-readers.

   • osbench's crunch loop (tests/guest/osbench/src/crunch.rs:221-223, inside for workload in &WORKLOADS) calls around.end and then, after result(), the next workload's around::begin(). That gap is a few prints, likely under 10 ms. So on ToyOS only, every crunch row after the first in each phase now sleeps up to 10 ms right before its measured slice.
   • Before this PR it joined the previous round and began at once.
   • This changes what the measured CPU does just before crunch.single.*. The idle track's reworded exit now holds those rows to Linux's figures, and Linux's side does no such read.
   • mem is not affected: clock::settle spins at least 10 ms before its begin.
   • Resolution needs no code change. Correct the Risks section, and have the B1 judgement compare the crunch.* p50 and mhz against the latest T14 benchcase at the base. If a row moves, the wait before the measurement is the cause to remove.

SUGGESTION

• kernel/src/counters.rs:44: the name JOIN. Joining no longer depends on it: a reader inside it waits, and joins only rounds issued after its call, whenever they come. A name for what it now is, the minimum interval between rounds, would read truer. Its Cadence text already says so.
• A host test seam exists. The decision is (last, called, now) → join | wait until | issue over plain nanoseconds. kernel/pure already uses u64 nanoseconds (sched::hw::Nanos). So "would add a time abstraction there" overstates the cost.
  • Three host cases would hold the strict-equality boundary and the join-after-call arm under arbitrary interleavings. The guest test sees neither.
  • The guest test plus the restore-join mutation does hold the serial property. I do not require it.
• counters_silent drives the stale path of the changed function, and it runs only on the actuator shared boot. If the orchestrator's T14 boot list includes that boot, it is worth batching there.
• issues/idle-cpus-enter-the-c-states-cst-names.md:
  • wake.storm.drain.w16 delta. "+52 to +87" takes its lower end from B7, a different build, while the other deltas come from the same-build A*/B3 pairs. Same-build is +68 to +87, with B7 at +52.
  • time.sleep_late.10ms delta. "+10" is +10.5 to +10.6.
  • History in a track. "(since fixed: …)" is history, which issues/README.md keeps out of tracks.
• For the orchestrator, on the PR body:
  • "On QEMU, the x86-64 counters_read runs only on the T14's shared boot" is garbled. On x86-64 it runs only on the metal shared boot, and on QEMU only on AArch64.
  • DeepIdleNanos "nothing read it" is not quite true: inspect rendered it as kernel.cpu.N.deep_idle_nanos.
  • The track's exit now adds "crunch.single at Linux's on the same T14". The body describes that as "reworded". Confirm the new criterion is the architect's, and say so.

Evidence

• Host CI at 27bf960: exit 0, 104 steps.
• QEMU virt_smp, virt_el1_smp, osbench_capability_groups_exit_0 and acpi_mediated_access: exit 0, with the counters_read marker in both SMP boots.
• Restore-join mutation: exit 1, "cpu0's stamp did not move", reverted. The tree it ran on carried uncommitted record edits only, which does not affect the result.
• 74c60af changes records and one comment, so the reuse is valid.
• Outstanding, both with the orchestrator:
  • x86-64 counters_read on the T14 testcases boot. That boot also carries counters_metal, whose judge holds nothing the new wait can break.
  • The B1 benchcase, including the crunch comparison above.

SEND BACK

[Japabu]

Review r1 response (head 7c9cf91):

• B1: the stale clause in around.rs is deleted. The comment now ends at "Deep idle is counted as the periods that ended in the window."
• B2: fixed in osbench rather than only in the Risks section. around::read returns this process's last reading when a fresh read would fall within toyos_abi::counters::INTERVAL_NS of it.
  • So no measured row starts straight after a rate-limited wait: not between the crunch workloads, not in mem, and not before tail's diary run, which also followed a read within ms.
  • One reading bounds both adjacent rows. osbench_rules::window still holds, because a row's span runs from its first reading's call.
  • The staged benchcase is there to confirm it: every crunch row's reads_us should be far below 10 000. Its crunch p50 and mhz compare against Idle CPUs enter the deep states their firmware declares #1073's B7 T14 readings (deflate 54.60 ms, sha256 0.567, matmul 1.075, parse 35.26, mhz 4188).
• Suggestions:
  • Taken: JOIN is renamed INTERVAL, and its value now comes from the new ABI constant.
  • Taken: the delta figures are corrected (w16 +68 to +87 with B7 +52; 10 ms +10.5 to +10.6; AMD 10 ms 0 to +1).
  • Taken: the history clause is removed from the track.
  • Taken: the PR body is corrected (where counters_read runs, and inspect as DeepIdleNanos's reader).
  • Declined: the pure host seam, as the PR body explains.
• The crunch.single exit criterion: it was not the architect's wording, which asked only that ratio 42 not be treated as the product target. It is dropped.
• Checks at 7c9cf91:
  • cargo run -- --ci host exit 0.
  • The four QEMU tests exit 0.
  • The restore-join mutation is red again (exit 1).
  • One T14 session is staged: benchcase, testcases (counters_metal and counters_read) and shared-debug (counters_silent).
  • Evidence: https://github.com/ToyOSOrg/evidence/tree/06e8ebcd59d365cfcc43a7e17738fdea0170682c/r2

[Japabu]

Review r2: PR #1084 at 7c9cf91

This round covers r1's two blockers and the one new commit, 7c9cf91: +37/−23 over the kernel, the ABI, osbench and the idle track. I did not reopen points r1 settled.

Architecture

• The reuse belongs in osbench. The kernel's rate limit protects every CPU from a stream of kicks, and any reader can cause them. So the rate limit stays. The program that must not sleep before a measured slice is the one that adapts to it.
  • Putting the reuse in around::read covers every osbench read path with one rule. Passing a reading from row to row would have to be threaded through each call site.
• INTERVAL_NS belongs in toyos_abi::counters. The wait is behaviour a program can observe at the syscall, and the ABI header already promised it. A program can avoid the wait only if it knows the interval. The kernel takes its Cadence from the same constant, so there is one value and no private copy in osbench.
• The JOIN to INTERVAL rename is complete. git grep finds no JOIN or JOIN_NS left in the tree. The kernel header and the Cadence doc now describe a minimum gap between rounds, which is what the constant is.

Is the reuse sound? (checked independently)

• The window rule holds. Take a reused reading R, called at t_R at cost c_R, and the next row's end reading A.
  • Every stamp in R lies in [t_R, t_R + c_R], and every stamp in A lies in [A.called, A.called + A.cost].
  • cpus computes row_ns from R's own called, so it equals A.called − t_R + A.cost.
  • So window ≤ row_ns and window ≥ row_ns − (c_R + A.cost) = row_ns − reads_ns.
  • This is what separates the reuse from the old join. The old join paired an old reading with the second caller's call time, so it needed JOIN_NS slack. Here the reading keeps its own call time, so the strict bounds still hold.
• No counts move between rows. Row N ends at R, and row N+1 begins at R. The gap between the rows (row N's result() prints) is counted only in N+1, and N+1's printed row_us includes that gap.
  • A reading taken before the row's own begin() call is the documented contract ("One reading then bounds both rows beside it"). It is not a hidden pre-call round.
• The predicate is conservative in the right direction. The kernel's issued lies in [called, called + cost] for the reading. That holds whether the read issued the round or joined a round issued after its call.
  • So ns_since(called) ≥ cost + INTERVAL_NS implies kernel now ≥ issued + INTERVAL. When osbench decides to read fresh, its own last round never makes that read wait.
  • Reuse can also happen up to cost early, which costs nothing.
• Failure handling is correct. A failed read is not cached. A stale CPU in a reused reading is refused by name again in the next row, which is what a joined round did before.
  • If a slice were shorter than the interval, its end read would reuse its begin reading. That is refused as OneReading, which is honest.
• The mutex is held across the syscall. That is fine: only the coordinating thread reads, and the lock is what keeps LAST coherent.
• Every pre-slice read path is covered or far from any earlier read.
  • crunch: rows inside a phase, and the first all row when cool returns at once. Both are covered by the reuse.
  • mem: round() takes 200 ms and settle at least 10 ms between rows.
  • tail: the 60 s run's first read comes before its 100 ms warmup. The diary run's read comes after stolen's 10 s sleep.
  • The ToyOS job list runs each group as its own process. The groups that read (mem, tail, crunch) are whole groups apart.
  • The only other reader in the tree is inspect, and benchcase does not run it.
• The benchcase's reads_us check is the right oracle. A fresh read's cost_ns includes any wait in the kernel, so a wait before a slice would show as reads_us of several thousand. Nothing cheaper establishes that on metal. QEMU's osbench_capability_groups_exit_0 runs only identity and fair, so it never executes this path.

B1 and B2

• B1 is resolved. around.rs:80-81 ends at "Deep idle is counted as the periods that ended in the window."
• B2 is resolved in code, which goes beyond what r1 asked. On ToyOS, no osbench read that its own earlier read could delay now sleeps before a slice. The Risks section now names the reuse and the other-process case correctly.

Records

• The deltas match the table's values.
  • w16: the same-build pairs give 87.0, 67.9, 83.5 and 71.4, so +68 to +87. B7 gives 130.1 − 77.6 = +52.5.
  • The 10 ms sleep gives +10.5.
  • AMD 10 ms: the cross pairs give 0 to +1.0.
  • "about 70 to 90 µs" covers w4 at +88 to +91 and w16 at +68 to +87.
• The crunch base figures match. r2/metal/crunch-B7-t14.txt gives deflate 54604410, sha256 567191, matmul 1075089 and parse 35264393 ns, all at mhz 4188.
  • B7 18fe4e150 is an ancestor of the merge base b0164b262. Between them, osbench and the kernel differ only in five lines of osbench/src/sys/toyos.rs, so B7 stands for the base.
  • On that boot the all-core rows were unmeasured, because there was no package sensor. Only crunch.single.* compares.
• The crunch.single criterion is dropped. The exit no longer holds cpu.clock to ratio 42, and it is now described as a diagnostic.
  • The architect's wording is in neither the PR nor my sources, so I cannot confirm it. The orchestrator should confirm that the architect's wording is what the body says.
• The history clause is gone. "were served one round" records D1's finding without the fix's history.

Findings

BLOCKER

None.

SUGGESTION

• sdk/abi/src/counters.rs:7-8 and kernel/src/counters.rs:46: both repeat "10 ms" beside the constant they now name. The ABI line could read "so a read may wait up to that long", so it cannot go stale. The Cadence reason string is the kernel's justification text, so leaving it is defensible.
• tests/guest/osbench/src/around.rs:56-57: the parenthetical (toyos_abi::counters::INTERVAL_NS) repeats the constant the code names seven lines below. Without it the doc reads tighter.

For the orchestrator (PR body; required for truth, not a code blocker)

• The diary-run claim is false. "Without that, osbench alone would sleep up to 10 ms right before a measured slice: between consecutive crunch workloads, and before tail's diary run." The commit message and the r1 response say the same, adding "which also followed a read within ms".
  • tail::suite runs stolen(Duration::from_secs(10)) between the 60 s run's counters_over and diary_run's first read (tail.rs:54-67), so the reads are 10 s apart.
  • Only the crunch rows were affected. Strike the tail clause. The commit message is pushed history and can stay.
• The reason given for declining the host seam is inaccurate. A seam needs neither an Instant accessor nor minting Instants.
  • A kernel/pure decision can be generic over the time type. For example, fn step<T: Ord + Copy>(last: Option<(G, T)>, called: T, now: T, reopens: impl Fn(T) -> T) -> Step<G, T>, which the kernel would instantiate with Instant and |t| t + INTERVAL.duration(), and host tests with u64.
  • Declining it is still acceptable. The decision is three arms under one lock, readable at the site, and the guest test with the restore-join mutation holds the property.
  • The body should give that as the reason instead of the claimed impossibility.

Evidence

• Host CI at 7c9cf91: exit 0 (r2/logs/ci-host-2.{exit,rev}).
• QEMU at 7c9cf91: exit 0. "4 passed", with two counters_read markers from the AArch64 SMP boots (r2/logs/qemu-3.log). None of these runs executes the osbench reuse; its compile is covered.
• Restore-join mutation at 7c9cf91: the patch rejoins a round inside the interval. Exit 1, and the tree was clean after reverting (r2/mutations/restore-join-r2.run).
• Staged T14 session: rev 7c9cf91, and the three image hashes match the body.
• Outstanding, with the orchestrator, in the staged T14 session:
  • benchcase: no *.cpus line refused, crunch reads_us far below 10 000, and crunch.single.* p50 and mhz against B7.
  • testcases: counters_metal and x86-64 counters_read.
  • shared-debug: counters_silent.
  • A moved crunch row or a reads_us in the milliseconds would reopen B2.

LAND