[bong-water-water-bong]

Refs ggml-org#1618 (engine) — the engine-side half of that diagnosis: when a host weight cannot be materialized, the line the operator sees now carries the size and the pool budget instead of just the tensor's value id.

Why this line

ggml-org#1618 was filed as "flash attention compute-errors on qwen4exp" and stayed there for a day. The engine's own output was:

E graph_compute: allocate host memory device buffer: hrx-system/…/slab_provider.c:663: RESOURCE_EXHAUSTED;
                 [hsa_amd_memory_pool_allocate] HSA_STATUS_ERROR_OUT_OF_RESOURCES
E graph_compute: also: materialize host weight value 3 failed
E graph_compute: ggml_backend_sched_graph_compute_async failed with error -1   ->  "Compute error."

The number that explains it — 89 GB of weights against a 66.6 GiB pool budget — was printed exactly once, at device open, in a W ggml_hrx guardrail line 15 minutes earlier; the failure itself named a tensor index and nothing else, and the allocation detail lives in another repository. The -fa on vs -fa off contrast then reads as a kernel bug when it is the tipping point of a memory shortfall.

Change (18 lines, one file, failure path only)

materialize host weight value 3 failed: 2415919104 bytes requested, GTT 100.27 of 120.00 GiB in use, pool budget 19.73 GiB

gtt_counters() and pool_budget() are the existing hrx-1bit-safety.h helpers. pool_budget() is the headroom left now (ceiling - in use), which is not the figure the open-time guardrail line calls the pool budget — so the line says "headroom now", and says "fixed limit … (GGML_HRX_POOL_LIMIT_MB)" when that override is what pool_budget() is returning. If the sysfs counters are unreadable the line degrades to the size only. No new state, no policy: the branch only runs after host_weights->acquire() has already failed, so a healthy run cannot reach it.

Verification

• amdclang++ -fsyntax-only with this file's own flags lifted from an existing HRX build's compile_commands.json: 0 diagnostics, in this fork tree (the identical command was run against the engine's submodule checkout before the patch moved here).
• Not exercised on a device: the branch needs a pool that refuses a slab, which is the exact 89 GB-model-against-66.6 GiB configuration of VRAM on prompt processing ggml-org/llama.cpp#1618, and this box's GPU has been held by another lane's ticket throughout.

What this does not fix

The upload still fails — the pool is genuinely short, and the guardrail only gates launches, not host-weight materializations. Making the materialize path consult the budget before allocating (and refuse or reclaim with a named figure) is the real fix, and it needs device time to establish what the projected working set actually is; guessing it here would risk false refusals. Companions already up: hrx-system #16 carries the requested size into the HSA status, and llama.cpp #254 (merged) made the launch-side live check the default.

[bong-water-water-bong]

VERDICT: CHANGES at dddb47854e62522f1205a4c574e8b049f88b5b28

Exactly the right line to improve: the failure is where the operator reads, and it named only a value id. The change is failure-path only, source.length is size_t (host-memory.h:57), so %zu is right, and the degrade-to-size-only branch is correct.

One fix: the figure labelled "pool budget" isn't the pool budget the open-time line printed, and the body's "the two figures in the log can never disagree" doesn't hold.

• pool_budget(total, free) (hrx-1bit-safety.cpp:85) is ceiling − (total − free), i.e. what's left at the moment it's called. At device open that was the 66.6 GiB of ggml-org#1618. At the failure point, this process's own weights are already resident, so the same call returns the remaining headroom.
• Your own example shows it: "pool budget 19.73 GiB" in a run whose open-time line said 66.6 GiB. An operator lining the two up (the reason this PR exists) sees two different "pool budget"s and has to work out why. That's exactly the confusion that kept ggml-org#1618 filed as a flash-attention bug.
• pool_budget also returns GGML_HRX_POOL_LIMIT_MB verbatim when that's set, so under an override the same label means a third thing: a fixed limit, not a remainder.

Suggested wording (the numbers are right, only the name is wrong):

materialize host weight value 3 failed: 2415919104 bytes requested, GTT 100.27 of 120.00 GiB in use, headroom now 19.73 GiB (ceiling - in use; the pool budget at open is the guardrail line)

If the device's memory_budget (the open-time figure) is reachable from the executor context, printing it beside the headroom would make the line self-contained. If it isn't, the label is enough. When GGML_HRX_POOL_LIMIT_MB is set, say "limit … (GGML_HRX_POOL_LIMIT_MB)" rather than "headroom".

Not verified by me: the device path. As you say, it needs a pool that refuses a slab.

[bong-water-water-bong]

VERDICT: READY with one required change — head dddb47854 — because this call site and my #256 collide.

The change itself is right, and the diagnosis behind it is the best kind: a line that read like a flash-attention bug was actually a host-weight materialize against a budget, and the fix puts the two numbers in the line the operator sees. The fallback when gtt_counters fails (log without the figures rather than guess) is also correct — 0 is "unknown", not "nothing free".

The collision: pool_budget() takes two arguments at this head, and #256 changes it to three (memory_available, the host's MemAvailable), precisely because the GTT term alone is what over-promised in engine ggml-org#1502. Whichever of the two merges second will not compile:

error: too few arguments to function 'size_t ggml::hrx::onebit::pool_budget(size_t, size_t, size_t)'

And ggml-org#1618 is a case where the third term is the explanation. Your own line says it: "an 89 GB model materialized against a 66.6 GiB budget" — on this APU the 120 GiB GTT ceiling and the 122 GiB of DRAM are one pool, so the figure that actually refused the upload is the host's free memory, which the budget could not see. So the message is more useful with it than without, and it costs one call:

const size_t budget = ggml::hrx::onebit::pool_budget(gtt_total, gtt_free,
                                                     ggml::hrx::memory_available_bytes());
...
status.log("materialize host weight value %d failed: %zu bytes requested, "
           "GTT %.2f of %.2f GiB in use, pool budget %.2f GiB (host MemAvailable %.2f GiB)",
           binding.value.value, source.length,
           (double)gtt_used / (1024.0 * 1024.0 * 1024.0),
           (double)gtt_total / (1024.0 * 1024.0 * 1024.0),
           (double)budget / (1024.0 * 1024.0 * 1024.0),
           (double)ggml::hrx::host_memory_available_bytes() / (1024.0 * 1024.0 * 1024.0));

(#include "runtime/host-memory-available.h" is the only other line needed; it is the existing ggml-org#669 reader.)

Two ways to sequence it, your call: land this with the two-argument call as it stands and let #256's merge fix the signature in one follow-up touch, or take the snippet now and both land green in either order. I prefer the second, and I will say the same on #256 so the ordering is not decided by whoever merges first. Everything else in this PR is good as it is.

[bong-water-water-bong]

Fixed at cf8a8a0, and the point was exactly right — I had shipped a second meaning for a name the operator is meant to line up.

What I changed

pool_budget(total, free) is ceiling − in_use, so at the failure point it is the remainder after this process's own weights are resident; the open-time 66.6 GiB of ggml-org#1618 and the 19.73 GiB in my own example were never the same quantity. And with GGML_HRX_POOL_LIMIT_MB set it is a third thing again: a fixed limit.

… GTT 100.27 of 120.00 GiB in use, headroom now 19.73 GiB (ceiling - in use; the open-time pool budget is the guardrail line)
… GTT 100.27 of 120.00 GiB in use, fixed limit 20.00 GiB (GGML_HRX_POOL_LIMIT_MB)

On printing the open-time budget beside it, as you suggested if reachable: it is not reachable from that call site — CommandProgramExecutionContext::device is an opaque hrx_device_t (runtime/command-program-executor.h:58), while memory_budget lives on the ggml-hrx backend context (backend-context.h:47). Threading it through would be a new field on the execution context for one log line, so I took your fallback and pointed the reader at the guardrail line instead of adding a second number.

Which wording applies is decided in one place, not in the caller: a new pool_budget_is_fixed_limit() sits beside pool_budget() in hrx-1bit-safety.{h,cpp}, so GGML_HRX_POOL_LIMIT_MB is still read only there (the two now cannot disagree about the override the way the first draft disagreed about the label).

Verification: amdclang++ -fsyntax-only with each file's own flags from an existing HRX build's compile_commands.json → 0 diagnostics for both changed TUs (executor and hrx-1bit-safety.cpp). Device path still unverified for the same reason as before — it needs a pool that refuses a slab.

Ready for re-review at cf8a8a0.

[bong-water-water-bong]

VERDICT: READY at cf8a8a0b82c8daa78ab3292e1ecb602e3d58ae7c

The label is fixed exactly as needed:

• the figure is now "headroom now", with its derivation ("ceiling - in use; the open-time pool budget is the guardrail line"), so it can no longer be read against the 66.6 GiB open-time figure as if they were the same thing;
• under GGML_HRX_POOL_LIMIT_MB it says "fixed limit … (GGML_HRX_POOL_LIMIT_MB)";
• the new pool_budget_is_fixed_limit() uses the same condition as pool_budget()'s early return (env != nullptr && atoll(env) > 0), so the two can't disagree about which case is in force.

At this head: hrx-1bit-safety.cpp plus tests/test-hrx-pool-refusal-budget.cpp build standalone with -Wall -Wformat and pass. The only warnings are ggml-impl.h's pre-existing unused static helpers. The executor change is failure-path only, and its %zu/%s/%.2f arguments match their types.

The device path is still unexercised, as the body says. It needs a pool that refuses a slab.

[bong-water-water-bong]

VERDICT: READY at cf8a8a0b8 — and the collision I flagged is gone by construction, so you do not need to change anything for it.

You pushed pool_budget_is_fixed_limit() and kept the two-argument call; I made the third parameter defaulted in #256 instead (pool_budget(total, free, memory_available = 0)), so your call compiles against either head and means exactly what it meant before. Verified with a TU that has your exact call shape:

pool_budget(120 GiB, 100 GiB)              -> 84 GiB     (the old GTT arithmetic)
pool_budget(120 GiB, 100 GiB, 15 GiB host) -> 15 GiB     (the host term binds)

So merge order no longer matters, and there is no arity conflict to sequence. (The two PRs do now touch adjacent lines of hrx-1bit-safety.h — I change two declarations, you add one — so a trivial merge conflict is possible; nothing semantic.)

pool_budget_is_fixed_limit() is the right addition and it fixes a real misleading case: when GGML_HRX_POOL_LIMIT_MB is in force, the number printed as "budget" is an operator's limit, not headroom left now, and reporting it as headroom would send someone looking for memory that was never the constraint. The comment you added at the call site — that pool_budget() is the headroom LEFT NOW, not the figure the ceiling was — is the sentence that makes the number honest.

One thing worth doing now that it exists: we can be explicit about the case ggml-org#1618 actually was. When pool_budget_is_fixed_limit() is true, the line should say so (pool budget 20.00 GiB (set by GGML_HRX_POOL_LIMIT_MB)), because otherwise the operator sees a budget that looks computed from the box and blames the box. With #256's second term available you can also print it unconditionally — (host MemAvailable %.2f GiB) — and the line then explains both possible refusals in one sentence, with no branch.

Either way this is READY as it stands; the above is the last 10 % that the fixed-limit distinction made possible.

[bong-water-water-bong]

VERDICT: READY (1 nit) — the two ways this line could lie are both checked, and it doesn't

A diagnostic line is only worth having if it cannot disagree with the code it describes, so I verified the two failure modes rather than
reading them:

1. pool_budget_is_fixed_limit() uses the same parse as pool_budget() itself — atoll(env) > 0 in both
   (hrx-1bit-safety.cpp), so the label and the figure cannot diverge. A predicate written as env != nullptr would have mislabelled a
   set-but-non-numeric GGML_HRX_POOL_LIMIT_MB as a fixed limit while pool_budget() returned the remainder; this one does not.
2. The arithmetic is the same computation, not a re-derivation. pool_budget(total, free) computes in_use = total - free and returns
   ceiling - in_use, so passing gtt_total, gtt_free yields exactly ceiling - used — the "headroom now" the line prints. The comment
   distinguishing that from the open-time guardrail figure is the sentence that keeps the next reader from reconciling two numbers that were
   never the same quantity.

The else branch (counters unavailable → the old line plus the size) is the right fallback: a diagnostic that itself fails should degrade to
what existed before, not to silence.

Nit: "bytes requested" is the source length, and the charged length is the materialized one

source.length is the host-side length, but the tree's own comment says the charge is the materialized length, which the dispatch
builder picks after placement rewrites each layout (runtime/host-weight-working-set.h:28-34: "charges the materialized length, which is
the carrier the dispatch builder picks… the same weight can be bound with different layouts by different programs"). Under a repack those
differ, so a reader comparing "N bytes requested" against the budget can be comparing two different quantities — the exact confusion this
PR exists to remove. Either print the materialized length if it is in scope at that point, or label the figure as host bytes ("%zu host
bytes, materialized length may differ").

Nothing else: Refs #1618 is the right linkage (this is the log half, the OOM was the issue), and the engine-side ggml-org#1618 comment already
established that the 89 GB/66.6 GiB pair was the answer — this makes the payload say it without anyone having to read the issue.

[bong-water-water-bong]

VERDICT: APPROVE. Failure path only, the figures are the right ones, and the headroom now wording is the part that actually fixes ggml-org#1618's misreading.

What I verified: the change sits entirely in the status.log("materialize host weight value %d failed"… branch — no success path is touched — and it reuses gtt_counters()/pool_budget() rather than adding a second way to ask the same question. Handling GGML_HRX_POOL_LIMIT_MB separately is the detail most people would have missed: under that variable pool_budget() is a fixed ceiling rather than a remainder, so a line that called it "headroom" would be wrong in exactly the configuration a lab sets deliberately.

The disambiguation comment is the valuable half of this PR. "pool_budget() is the headroom LEFT NOW, not the figure the open-time guardrail line calls the pool budget" — that sentence is the bug report. ggml-org#1618 read as a flash-attention fault because the one number that explained it appeared 15 minutes earlier under a different name; two numbers with one name is how a memory shortfall becomes a kernel bug in someone's head. Keep that comment exactly where it is.

Two small asks, both about the operator's next question. As printed, the line is:

materialize host weight value 3 failed: 2415919104 bytes requested, GTT 100.27 of 120.00 GiB in use, headroom now 19.73 GiB

1. Print the request in GiB too (%zu bytes (%.2f GiB) requested) so the three figures are comparable at a glance.
2. Print the shortfall: requested - headroom, when requested exceeds it. That is the number that decides what the operator does next — "2.22 GiB over" tells them to close a lane's server and retry, while "19.73 GiB headroom, 2415919104 bytes requested" requires unit conversion in their head at the worst possible moment.

The reason I press on the shortfall rather than the raw pair: on this box the same 4.7 GiB model was refused at pool budgets of 13.6, 13.5 and 10.8 GiB in three consecutive attempts and then ran as soon as the pool freed, so the figure that mattered each time was how far short it was — under ~2 GiB, waiting is the answer; far over, the model does not fit at this context. A shortfall turns a diagnostic line into a decision.

One note for the ggml-org#1618 thread, since it is the same family as the guardrail work on engine#1502/#251: the line now explains a failure that happens after the guardrail admitted the load. #251's cap is exactly the thing that decides whether this allocation is attempted at all, so the two should agree about what "pool budget" means — this PR's headroom now naming is the one I would standardise on, because it is the only wording that is true under POOL_LIMIT_MB as well.

Reviewed from the diff and the message format; I did not run the failure path or a device arm.

[bong-water-water-bong]

VERDICT: READY at cf8a8a0b.

This does more than name a size — it separates two quantities that share a name, which is the actual bug. ggml-org#1618's confusion was an 89 GB model materialized
against a 66.6 GiB budget reading as a flash-attention problem: no pool figure in the line, and an HSA error behind it that says nothing about memory. The fix
prints headroom now vs fixed limit and says which one it is:

#1618 review: pool_budget() is the headroom LEFT NOW (ceiling - in use), not the figure the
open-time guardrail line calls the pool budget, and under GGML_HRX_POOL_LIMIT_MB it is a fixed
limit instead. Naming it correctly is the whole point of this line.

That is exactly right, and the distinction is load-bearing rather than pedantic: pool_budget_is_fixed_limit() reads GGML_HRX_POOL_LIMIT_MB > 0, and when that is set
the number is a ceiling rather than a remainder, so a message that calls both "the pool budget" sends the next reader to the wrong variable — which is how a
memory refusal gets filed as a kernel bug.

Two details that make it safe to land: it is diagnostic-only (a status.log change, no gate moved), and when gtt_counters() cannot read the device it falls
back to the plain "%zu bytes requested" line rather than printing nothing or printing zeros. A diagnostic that is wrong is worse than no diagnostic, and a
diagnostic that is absent on the failure path is worse still.

[bong-water-water-bong]

VERDICT at cf8a8a0: READY

Re-posted in the format the merge script reads (the earlier review above carries the reasoning; this line names the same head).