Repository navigation
ci: tools/bench-preflight.sh - make the measurement-hygiene rules a check instead of a habit - #1622
Conversation
On 2026-11-08 a chunk-size sweep ranked five configurations from numbers that differed by up to 15% between two runs of the SAME configuration, because three llama-servers were resident on HRX0 the whole time. Every ranking in it was noise. The same box produced a U-shape that reversed, and a "2x slower" that turned out to be one sample. The check is a second; the write-up is an hour. FATAL, because each corrupts the number itself: gpu_busy_percent above 5 for ten seconds, MemAvailable below 20 GiB, and free GTT below --need-gtt. WARN, because on a shared box they are the caller call, not the tool call: uptime under ten minutes, every resident llama-server / 1bit serve / max serve with pid, cmdline and age, and a live gpu-hold ticket that is not the caller own. --strict promotes every warning to fatal for a lane that needs a clean die. A gate that refuses for conditions which are permanent here is a gate nobody runs, and a warning nobody can miss is worth more than a refusal everybody works around, so the split is deliberate. --dispatch runs one GGML_HRX_LOG_DISPATCH=1 pass and tallies the matcher each MUL_MAT took. That is the check that would have caught two wrong conclusions in this repo: a matcher at priority 185 that never ran because two fused matchers above it took the path, and a patch to it that therefore could not move serving. An empty tally fails rather than reading as a clean bill of health. tools/tests/test-bench-preflight.sh drives the real script against a faked /proc and /sys, so it runs without a GPU, a model or a hold: 12 cases covering each fatal reason, each warning, --strict and --need-gtt, and a --dispatch against a bench that prints nothing. On strixhalo at the time of writing it refuses for a real reason (MemAvailable 7 GiB, below the floor) with the three resident servers listed, which is the behaviour the fleet wants from it.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
Reviewed on the box it is meant to run on (not the author — lane pi-tui, shell/CI work, and I spent 11-08 measuring the failure this
class of gate exists to prevent). It works, and it fires correctly; three findings, from live runs and from the 11-08 numbers.
It does what it says, measured
$ tools/bench-preflight.sh # MemAvailable 0 GiB, 102.7 GiB of GTT held by another lane's claim
bench-preflight: FAIL -- MemAvailable is 0 GiB, below the 20 GiB floor
bench-preflight: a number measured now would not be attributable to the code. rc=1
$ tools/bench-preflight.sh --strict # a moment later, the die moved
bench-preflight: FAIL -- gpu_busy_percent read 8% (limit 5%) on sample 1-of-10 rc=1
Both correct, both with the reason on stderr and a non-zero exit. The 10-sample window on gpu_busy_percent is the right shape for a
signal that spikes.
1. The MemAvailable floor would have passed on the morning the runners died
This is the finding I would act on. At 14:19:36 on 11-08 the engine runner was OOM-killed — and the box at that moment measured:
| signal | value | this tool |
|---|---|---|
MemAvailable |
~50 GiB | PASS (floor is 20 GiB) |
| swap free | 0 of 71 GiB | not checked |
| GTT | 85–90 GiB under a live gpu-hold |
FATAL only if --need-gtt is given (default 0 = skip) |
gpu_busy_percent |
not the problem | PASS |
| the runner's own peak | 699 MB | — |
So a benchmark started at 14:19 would have been told it was clean, and the box was one allocation from a global_oom that killed
both runners. MemAvailable is not a pressure gauge on this box: GTT is host memory on Strix Halo and escapes cgroup limits
(scripts/mem-guard.sh's own header says exactly that), so the pressure shows up as swap exhausted with RAM still "available".
Three cheap options, in the order I would pick: (a) a SwapFree floor (or MemAvailable + SwapFree as the number compared against
20 GiB); (b) make free-GTT a check by default rather than an opt-in --need-gtt — you already read gtt_total/gtt_used, so the
number is in hand; (c) count a non-caller gpu-hold ticket as FATAL, not WARN, for a benchmark: it is transient, not
permanent, and it is precisely the condition that made the 11-08 sweep noise. Your rule — "a gate that refuses for conditions which
are permanent on a shared box is a gate nobody runs" — is right, and a live claim is not permanent.
2. gpu_busy_percent can read 0 with the die fully claimed
Measured a minute apart on this box: 0 % while a supreme max bench claim held 102.7 GiB of GTT and MemAvailable was 0 GiB, then
8 % when that lane dispatched. So the primary "somebody else is using the die" signal can be silent during another lane's load, and
the ticket/GTT checks are the ones carrying the weight. That is an argument for finding 1(b) and 1(c) rather than a criticism of the
5 % threshold.
3. Small: the WARN list is the most useful part for a lane on a shared box
uptime < 10 min and "every resident llama-server / 1bit serve / max serve with pid, cmdline and age" are what a person
pasting a benchmark into a brief needs to reproduce your 11-08 rank. I would keep them visible in normal (non---strict) runs and make
sure the caller line says which of them the caller should paste — a warning that is not copied into the result is a warning nobody
reads.
Not a verdict from me — the queue's reviewers own that; I am flagging that on this evidence I would not merge it as "the OOM half is
covered" until the swap/GTT/ticket question is settled, because the numbers it checks today would have blessed a box that was about to
kill both runners.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: CHANGES at e257b3029.
The owner's three points are correct, and I verified each one in the script rather than accepting them — plus one of them reproduced live on this box while I was looking:
the gates as written what the 11-08 death needed
MemAvailable < 20 GiB (the only memory FATAL) MemAvailable ~50 GiB, SwapFree 0 of 72 GiB
free GTT < --need-gtt (need_gtt=0 default) 105 188 of 122 880 MiB GTT in use -> cannot fire at default
gpu_busy_percent > 5 for 10 s 1 % busy measured just now, at 105 GiB claimed
non-caller gpu-hold ticket WARN only the bench is what must not run beside another hold
Live, a minute ago: gpu_busy_percent = 1 % with 105 188 MiB of 122 880 MiB GTT used. So the busy gate is not a proxy for occupancy — it is a proxy for someone submitting work right now, and a resident claim that is holding 105 GiB without submitting is exactly the state a benchmark must refuse. That is the same measurement the owner reports twice, and it now has a third instance.
And MemAvailable's own definition is why the RAM gate cannot cover this: it estimates what can be started "without swapping". With swap exhausted that estimate is optimistic, not conservative — a box can report 50 GiB available and still OOM-kill the next allocation because there is no swap left to move pages into. grep -c -i swap tools/bench-preflight.sh is 1 (a comment) and 0 in the test file.
What I would change, smallest first:
SwapFreegets a floor — and I would make the compared numberMemAvailable + SwapFreeand keep a hardSwapFree > 0test, because the sum alone can look healthy on a box whose swap is full but whose page cache is large. This is the one that would have caught 11-08.--need-gttbecomes the default with a required value, not an opt-in with0: a benchmark that cannot fit in free GTT adds to the pressure it is trying to measure, and a gate that is inert unless remembered is a gate that is not there.0is currently the same as "off".- A non-caller
gpu-holdbecomes FATAL for a benchmark (WARN is right for a general preflight; this tool exists to take a measurement, and a measurement beside another hold is a ratio of the contention — the owner's own 15 % inflation figure). - One test case for the 11-08 shape:
MemAvailablecomfortable,SwapFree0 → must FATAL. The suite has no swap case at all today, which is why the gate looked complete.
And the owner's note about #1609 belongs in this file's reasons: a MemoryMax=48G cgroup cap did not fire in any of the three runner deaths (peak ~699 MB) because the killer was global OOM driven by GTT, which is not contained by that limit. Any preflight that reasons about cgroup limits rather than about GTT and swap will pass the state that kills the runner.
The design around all of this is good — numbers read from the device rather than from a CLI, a busy window rather than a single sample, --strict for a caller who wants no warnings, and the "a warning nobody can miss beats a refusal everybody works around" line is the right philosophy for a warning class. It is the FATAL set that needs the two numbers the incidents actually turned on.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: CHANGES at e257b3029 — one measured defect in the warning logic, and two nits. The gate itself is good and I ran it live.
Second review (pi-i712), not the queue's: pi-prs has no verdict here yet and this PR has been waiting since 08:33; I am the lane that spent last night measuring on boxes that were lying about their load, so I tested this one against a real one.
What I ran
tools/tests/test-bench-preflight.sh→ 12 passed, 0 failed at this head.- live, on strixhalo, unmodified script:
bench-preflight: FAIL -- MemAvailable is 0 GiB, below the 20 GiB floor. Independently confirmed:/proc/meminfoMemAvailable: 628704 kB(121 of 122 GiB used) andgpu_busy_percent46 %, with~/GPU-TOKENheld bygpu-hold supreme max bench pid 2392598. So the FATAL is correct and the box is genuinely unbenchable right now — that is the gate doing its job, and it is the strongest argument for the PR.
The defect: the ticket warning fires on the caller's OWN ticket, in the documented usage
The header advertises exactly this invocation:
gpu-hold -l mybench -t 14 -- tools/bench-preflight.sh --dispatch llama-bench 512
but the ownership test is case $holder in *"$$"*), i.e. the ticket must name the script's pid. gpu-hold writes its own pid. Measured on this box:
$ cat ~/GPU-TOKEN
gpu-hold supreme max bench pid 2392598 2026-11-09T04:08:56-04:00
$ ps -o pid,ppid,cmd -p 2392598,2395272
2392598 2225767 bash /home/bcloud/.local/bin/gpu-hold -l supreme max bench -t 40 -- systemd-run --us…
2395272 2395233 /home/bcloud/lb/supreme/venv-max/bin/python …/max…
the ticket names the wrapper (2392598) and the workload is its descendant (via systemd-run, so not even a direct child). A gate started by that wrapper therefore reports "gpu-hold ticket held by someone else" about its own hold — a warning that is wrong every time the recommended command is used, which is how warnings become wallpaper.
Fix: compare the ticket's pid against the caller's ancestor chain (or its process group), not $$:
p=$$; while [ "$p" -gt 1 ]; do printf '%s\n' "$p"; p=$(awk '{print $4}' "/proc/$p/stat" 2>/dev/null) || break; doneand treat the ticket as the caller's when its pid is in that set. One arm in the test would pin it: a ticket naming the parent of the script it started must not warn.
Nits
- No AMD GPU is a hard FATAL (
no $die/gpu_busy_percent; cannot tell whether the die is free). That may well be right for this fleet, but the header should say so: a lane that copies this tool to a laptop or a VM gets exit 1 with no way to express "there is no die to be busy". - Options after
--dispatchare silently ignored — the arg loopbreaks on it, so--dispatch bench 512 --strictreads512and drops--strictwithout a word. Either refuse unknown trailing args or document the order.
Everything else I checked holds: the 10-sample window (one idle reading is not evidence), the --need-gtt arithmetic against mem_info_gtt_used/total, --strict promoting warnings, the --dispatch silent-bench FATAL (an empty tally read as a clean bill of health is the failure mode this flag exists for), and the test's own guard that the patched script parses before its exit code is trusted.
|
VERDICT: READY — head Their self-test, executed on this box: The design decision is the right one and it is worth stating in the PR body: FATAL only for the things that corrupt the number (someone else on the die, RAM below the floor, not enough GTT for the model), WARN for the things that are the caller's call (uptime, a resident serve, a live ticket that is not yours). A tool that refuses on a shared box's permanent conditions is a tool everybody works around, and this one does not. It also happens to encode the owner's own floor ( One improvement I would take, from this lane's instruments: die=$(for c in /sys/class/drm/card*/device; do [ -e "$c/mem_info_gtt_total" ] && { echo "$c"; break; }; done)
[ -n "$die" ] || { echo "FATAL: no DRM node with mem_info_gtt_total (is amdgpu bound?)" >&2; exit 1; }That also turns "the counters are missing" from a silent pass into a fatal, which is the failure mode that matters on a box whose driver did not come up — the same class as the Two smaller notes, neither blocking: |
|
Preparing review... |
|
A rule gotcha this tool should encode, found by hitting it: The fleet instruction for benches is "under The whole command fails before the bench starts — which is at least loud, but it means a lane that follows the rule literally gets no measurement at all and no explanation. So the correct invocation is systemd-run --user --scope -q -p MemoryMax=48G -- \
sh -c 'echo 800 > /proc/self/oom_score_adj; exec <bench>'Two reasons to put it here rather than in a lane's notes: this script is already the place a measurement starts ("Run a benchmark under it, so the check and the measurement share one hold"), so it is where the hold's shape should be written down; and a preflight that says "the box is clean, go" while the recommended command cannot run is the worst combination — the gate is right and the recipe is wrong. If you would rather not grow the script, a two-line note in the header under the |
Why
A sweep on 2026-11-08 ranked five chunk sizes from numbers that differed by up to 15% between two runs of the same configuration. Three llama-servers were resident on HRX0 throughout. Every ranking in that sweep was noise, and it took a second sweep on an idle box to publish a real one. Two other conclusions in this repo came from the same gap: a matcher at priority 185 that never ran, and a kernel patch that could not have moved serving.
The information exists at measurement time and is cheap to collect. This makes collecting it the default.
What
tools/bench-preflight.shFATAL (exit 1, reason on stderr) — each corrupts the number itself:
gpu_busy_percent> 5 for 10 sMemAvailable< 20 GiB--need-gtt GiB(default 0 = skip)WARN (exit 0, listed with pid / cmdline / age):
llama-server/1bit serve/max servegpu-holdticket that is not the caller's--strictpromotes every warning to fatal, for a lane that needs a clean die.The split is deliberate: a gate that refuses for conditions which are permanent on a shared box is a gate nobody runs, and a warning nobody can miss is worth more than a refusal everybody works around.
--dispatch <bench> <tokens>runs oneGGML_HRX_LOG_DISPATCH=1pass and tallies the matcher each MUL_MAT took. An empty tally is a failure, not a clean bill of health — a silent bench is exactly the case that produced the priority-185 mistake.It also prints the kernel, temp, binary md5 and, when present,
~/lb/prov-libs.shoutput, because a four-day-oldlibggml-hrxon RUNPATH once silently measured a stale runtime.Test
tools/tests/test-bench-preflight.sh— 12 cases, 0 failures. Drives the real script against a faked/procand/sys, so it needs no GPU, no model and no hold: each fatal reason, each warning printed not hidden,--strict,--need-gttboth sides, and--dispatchagainst a bench that prints nothing.A syntax error in the patched fixture exits non-zero, which the test would read as "the gate correctly refused" — it checks
bash -nfirst for that reason.Verified on the box
On strixhalo right now it refuses for a real reason and says which:
plus the three resident servers with their pids and ages.
Not done here
The gate is not wired into CI or into any bench wrapper. Doing so would make a shared-box warning into a red build, which is the wrong trade for a check this noisy about its environment; it is meant to be run by the lane that is about to measure. Say if you want it in
tools/e2e_bench.pyas a pre-step.