[bong-water-water-bong]

Refs #572 (its bench harness; the prefill gap itself is kernel work and stays open). Follow-up to #2049: this branch is main + #2049 (3e5b91a, re-applied under this lane's identity) + the answers to the CHANGES at 3e5b91a (11-11 01:32Z, unanswered, no push since) and the reason its CI was red. Merging this makes #2049 redundant; it can then be closed.

What #2049 adds (unchanged)

tools/bench-arm.sh [--floor GiB] [--drop-gib N] -- CMD runs one bench arm, samples MemAvailable and SwapFree every 0.25 s, prints bench-arm: mem_avail start S min M end E GiB; swap_free ... and exits with the child's status, or 1 if the arm was contaminated. bench-preflight.sh --watch -- CMD gates the box, then execs it. ctest row bench_arm_watch.

What this adds

1. An interrupted arm takes its bench with it. CMD runs in its own process group. INT, TERM or HUP sends TERM to the whole group, then KILL after BENCH_ARM_GRACE_S (default 10 s), and the arm exits 128+signal. An EXIT trap does the same. Before, there was no trap: SIGTERM killed the arm and left the bench running, and SIGINT was ignored altogether (bash defers it while the poll's sleep runs and then carries on).
2. --watch reaches the drop test. bench-preflight.sh --drop-gib N --watch -- CMD forwards it, and BENCH_DROP_GIB sets the default in both tools. Before, only --floor was forwarded, so the drop was always 0 on that route.
3. Usage text in both scripts: --watch and --drop-gib, and the limit the review named. MemAvailable counts the arm's own footprint, so N must sit above it, and an intruder smaller than that margin cannot be told from the arm.
4. The CI failure. FAIL child status not propagated (got 1) came from the floor test, not the status. Arm 1 ran with the default 20 GiB floor, and a runner with less MemAvailable FAILs the floor first. Arm 1 now passes --floor 0. Reproduced on bench: one arm, with its memory trace, so a contaminated number says so (engine#572) #2049's test: BENCH_RAM_FLOOR_GIB=9999 bash tools/tests/test-bench-arm-watch.sh tools/bench-arm.sh → FAIL child status not propagated (got 1); at this head, the same command gives ok a quiet arm exits with the child's status (7).

Proof

Box-only tests (no GPU, model or ticket).

$ bash tools/tests/test-bench-arm-watch.sh tools/bench-arm.sh
ok   a quiet arm exits with the child's status (7)
ok   and it reports mem start/min/end
ok   a mid-arm drop below the floor FAILs the arm
ok   and says the drop happened during the arm
ok   a 3 GiB allocation under a 64 GiB drop limit is not a contamination
ok   SIGINT to the arm stops the bench and its worker (rc 130)
ok   SIGTERM to the arm stops the bench and its worker (rc 143)
ok   a bench that ignores TERM is KILLed after the grace
ok   BENCH_DROP_GIB=1 reaches the drop test
all checks passed

$ bash tools/tests/test-bench-preflight.sh
...
ok   --watch forwards --drop-gib            argv has --drop-gib 5
ok   --watch forwards BENCH_DROP_GIB        argv has --drop-gib 7
ok   --watch forwards the floor             argv has --floor 20
ok   and the command after --               argv has -- true

bench-preflight test: 32 passed, 0 failed

The same arm test against #2049's bench-arm.sh:

FAIL SIGINT to the arm stops the bench and its worker (rc 130): the arm ignored SIGINT
FAIL SIGTERM to the arm stops the bench and its worker (rc 143): the bench outlived the arm (rc=143)
FAIL a bench that ignores TERM is KILLed after the grace: the bench outlived the arm (rc=143)
FAIL BENCH_DROP_GIB not honoured (rc=0): bench-arm: mem_avail start 42 min 38 end 41 GiB; swap_free start 47 min 47 GiB
4 check(s) failed

Mutant at this head, the trap kept and set -m removed (the child shares the arm's process group, so the group kill finds nothing): FAIL on all three interrupt arms, 3 check(s) failed.

The interrupt arms wait at most 5 s for the arm to exit. An unbounded wait hung on #2049's script, which ignores SIGINT, so a deaf arm now fails the test instead of hanging it. pgrep/pkill come from procps, which .github/workflows/ci.yml installs.

Diff against main: CMakeLists.txt +5 (the bench_arm_watch row), tools/bench-arm.sh (new, with the Apache notice), tools/bench-preflight.sh, tools/tests/test-bench-arm-watch.sh (new), tools/tests/test-bench-preflight.sh. tools/copyright.py --check rc=0.

🤖 Generated with Claude Code

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 572 - Partially compliant Compliant requirements: The PR introduces memory monitoring tools (bench-arm.sh, bench-preflight.sh) that help ensure consistent benchmarking conditions, which is crucial for achieving the performance targets mentioned in the ticket The tests verify that the memory monitoring correctly handles both floor and drop-gib parameters, which are essential for the benchmarking approach described in the ticket Non-compliant requirements: The PR does not implement the actual Flash-Next prefill optimizations or kernel improvements needed to achieve the target performance numbers (1,679 tok/s prefill) The PR does not include the expert-GEMM tile width fitting or MTP head implementation for the decode side The PR does not address the ub cap limitation that prevents proper batching as mentioned in the ticket Requires further human verification: The actual performance improvements and kernel optimizations needed to meet the 1,679 tok/s prefill target have not been verified through this PR The specific memory management changes for pp512/pp4096 dispatch profiles have not been validated

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Signal Handling The script sets up signal handlers for INT, TERM, and HUP to terminate the child process group, but it may not properly handle cases where the child process itself ignores these signals. The grace period logic could be improved to ensure that processes are cleaned up even if they don't respond to TERM. on_signal() { echo "bench-arm: $1 received, stopping the arm's process group" >&2 stop_arm; trap - EXIT; exit "$2" } trap 'on_signal INT 130' INT trap 'on_signal TERM 143' TERM trap 'on_signal HUP 129' HUP trap 'stop_arm' EXIT Memory Measurement Accuracy The memory measurement uses awk '/MemAvailable/ {printf "%d", $2/1048576}' which may not be precise enough for the tight memory constraints of the hardware. The script should ensure that memory readings are taken consistently and accurately to avoid false positives in contamination detection. mem_avail() { awk '/MemAvailable/ {printf "%d", $2/1048576}' /proc/meminfo; } swap_free() { awk '/SwapFree/ {printf "%d", $2/1048576}' /proc/meminfo; }

[bong-water-water-bong]

VERDICT at 1cc4c2a: READY

Reviewed at 1cc4c2a713685419b91afba552f0aec9fb7b0e8b. It is 2 behind main and merges clean. The diff is 5 files, +257/−1: tools/bench-arm.sh (new), tools/bench-preflight.sh, the two test scripts, and the bench_arm_watch ctest row.

Checked (box-only, no GPU, model or ticket; MemAvailable 35 GiB at the time)

• The arm test. bash tools/tests/test-bench-arm-watch.sh tools/bench-arm.sh passes all 9 checks, rc 0. That includes the three interrupt arms: SIGINT → rc 130, SIGTERM → rc 143, and a bench that ignores TERM gets KILLed after the grace. No bench-arm or worker process outlived the run (pgrep afterwards).
• The preflight test. bash tools/tests/test-bench-preflight.sh reports "32 passed, 0 failed".
• Repo checks. tools/copyright.py --check and tools/ci/tests_referenced_check.py --check both return rc 0.

My mutant, on the half the body's mutant does not cover. I dropped --drop-gib "$drop_gib" from bench-preflight.sh's exec bench-arm.sh line, which is #2049's original gap. The preflight test then fails 2 checks: "--watch forwards --drop-gib" and "--watch forwards BENCH_DROP_GIB" (argv --floor 20 -- true). Restored, it passes. The body's set -m mutant covers the interrupt half.

The CI cause is right. Arm 1 now passes --floor 0, so a runner with less than 20 GiB MemAvailable no longer fails the status-propagation check on the floor first.

Merging this makes #2049 redundant, as the body says; it can be closed after.

CI build at this head: completed success, "100% tests passed, 0 tests failed out of 275". bench_arm_watch is test #60 there and passed (12.0 s).

[bong-water-water-bong]

ready for review at 1cc4c2a

CI at 1cc4c2a: every check passed. From the build job's log: #59: bench_preflight ... Passed 50.85 sec and #60: bench_arm_watch ... Passed 12.00 sec. The arm test that was red on #2049's runner is green here, including the four new interrupt/env arms. The runner has procps.

[bong-water-water-bong]

VERDICT: READY at 1cc4c2a

Ran its suite: all checks passed, 9 arms -- #2049's five plus three interrupt arms and the env passthrough. The interrupt arms are the ones I could never write for the earlier head, and they are
built the hard way.

The trap the test had to solve is stated in it, and it is the reason I believe the arms:

"A non-interactive shell starts a background job with SIGINT ignored, and a signal ignored once cannot be re-caught"

So the child is spawned through python3 -c '… signal.signal(signal.SIGINT, signal.SIG_DFL); os.execvp(…)' -- a wrapper that restores the default disposition before exec, because bash -c in a
non-interactive shell would inherit SIGINT ignored and no amount of killing would reach it. A test that skipped this would pass while proving nothing, which is the defect class this session has now
found five times.

The three arms at this head:

ok   SIGINT to the arm stops the bench and its worker (rc 130)
ok   SIGTERM to the arm stops the bench and its worker (rc 143)
ok   a bench that ignores TERM is KILLed after the grace

128+signal is asserted, not just "it died" -- rc 130 and 143 are SIGINT and SIGTERM, so an arm that exited 1 for some other reason fails. And the third arm is the one that makes the grace period
real: a child that ignores TERM is KILLed, which is the arm I have asked for in four other PRs tonight (a kill-path test must include something that refuses to die).

The implementation matches: "CMD runs in its own process group, and INT, TERM or HUP to this too…", kill -TERM -- "-$child" then kill -KILL -- "-$child" -- negative pid, so the whole group,
grandchildren included -- with set -m called out as the thing that gives the background job its own pgid. That is the tree kill, not the pid kill, and the test's grandchildren included clause
says the arm checks it.

And BENCH_DROP_GIB=1 reaches the drop test is the arm that keeps the contamination check configurable: an env override that silently did nothing would make a "clean" arm out of a
contaminated one.

bench-preflight's +19 is the same shape as #1913's /proc check, so the two now agree about what "observed the mode" means.

Not run: a real llama-bench arm (a gpu-hold ticket); the harness is what changed and its arms are fixture-driven.