[bong-water-water-bong]

What this fixes

memory_budget is consulted in exactly one place — the backend's buffer-type allocator (ggml-hrx.cpp:411) — so the transient arena, the scratch a launch grows into, is
allocated uncharged. "A launch that needs more is refused here" (ggml-org#680) was therefore true of one of the two ways a launch can run out of pool memory, which is why engine
ggml-org#1417 is a segfault rather than a refusal: the arena grows into memory the guardrail never counted.

This charges the arena to the same ledger, through the same pool_reserve/pool_release the allocator uses.

The change

• runtime/transient-arena-charge.h (new, pure): plan_arena_charge(required, capacity, retires_old) → {reserve_bytes, release_bytes}.
• TransientArena::ensure_capacity(…, size_t pool_budget = 0) — the charge is taken before the superseded buffer is touched, so a refusal leaves the arena with the capacity
  it already had; 0 means the device size is unknown, which keeps the previous uncharged behaviour (and pool_reserve would not have added to the ledger either, so a paired
  release would unbalance it).
• CommandProgramExecutionContext carries the device's pool_budget; graph-executor.cpp fills it from context_.device->memory_budget, the only place that dereferences the
  ggml device struct. The executor's growth call site (command-program-executor.cpp:1918) passes it through — it is the one place that knows commands.transients.arena_size
  and the arena's current capacity.
• clear()/the destructor return everything charged, retired diagnostic buffers included.

The trap this is written around

In diagnostic_fresh mode the arena retires a superseded buffer into diagnostic_retired_buffers_ instead of releasing it, so the arena's resident memory is retired +
current, not its capacity. A charge written as "reserve the delta, release the old capacity" would therefore under-report in exactly the mode used to diagnose
out-of-memory — the ledger would look comfortable while the device is full. The plan keeps a retired buffer's charge, and charged_bytes_ tracks the total so teardown returns
the right amount.

Test

tests/test-hrx-transient-arena-charge.cpp (new, no device, no libhrx — the helper is pure, the same division as host-memory-refusal.h), five arms, all passing:

ok   first allocation charges its whole size
ok   growth charges the delta and releases the old
ok   diagnostic growth keeps the retired buffer charged      <- the arm a naive version fails
ok   a request that fits charges nothing
ok   a smaller request charges nothing

Built and run at this head (cmake RC=0, ninja RC=0, test-hrx-transient-arena-charge RC=0), and llama-server/llama-cli link clean with the change in place.

Not in this PR, said out loud

Host staging and the command-program buffers have the same shape (allocate, no budget consult) but allocate at different sites; they get the same reserve/release pair next
rather than one call site that pretends to cover all three. If one of them is allocated before the device context exists, the honest answer there is "this guardrail cannot
charge it" and that belongs in the docs rather than in a workaround.

Stacking

Based on 585ad55e (llama.cpp #260), because the pin as it stands on 1bit/hrx-vulkan-patched does not compile (ambiguating new declaration of 'bool …device_free_total').
Merge #260 first; this is otherwise independent of it.

[bong-water-water-bong]

VERDICT: READY — the pure plan is right and the call site implements all four orderings; the orderings themselves are the untested part

The diagnosis is the good kind: memory_budget was consulted in exactly one place (ggml-hrx.cpp:411, the buffer-type allocator), so the transient arena — the scratch a launch grows into — was allocated uncharged, and ggml-org#680's promise ("a launch that needs more is refused here") covered tensors only. That is why ggml-org#1417 is a segfault rather than a refusal: the arena grew into memory the guardrail never counted. Charging it to the same ledger through the same pool_reserve/pool_release is the right closure, and extracting the two numbers into a pure header is the right shape for testing.

The pure plan gets the case a naive version gets wrong, and it is the case its own test asserts: when the superseded buffer is retired (diagnostic mode keeps it resident) its charge must stay, because releasing it there under-reports by exactly the buffers retained to diagnose memory problems — the ledger comfortable while the device is full. plan_arena_charge returns release_bytes = 0 for that case, and test-hrx-transient-arena-charge.cpp pins it alongside first-allocation, growth, fits and smaller. Good.

I read the call site rather than the description, because the orderings are where this goes wrong — and all four are right:

path	what it does
refusal	pool_reserve first, return before buffer_ is touched → the arena keeps the capacity it already had
growth, non-diagnostic	synchronise, hrx_buffer_release(buffer_), then pool_release(charge.release_bytes) — the charge goes only after the buffer is gone
growth, diagnostic	buffer pushed to diagnostic_retired_buffers_, no release — charge stays with it
allocation fails after the charge	pool_release(charge.reserve_bytes), and charged_bytes_ unwound with the same underflow guard as everywhere else

charged_bytes_'s decrements all use >= ? : guards, so a ledger imbalance cannot underflow into a huge number. And the pool_budget == 0 escape (device size unknown) skips both the reserve and the release, which is the only consistent choice — a paired release without a reserve would unbalance the ledger.

One note: the riskiest part is the part with no test

The new test covers plan_arena_charge only — the four orderings above are code, not assertions. They are exactly where a later refactor (or a reordering "for clarity") would silently reintroduce the bug class: a refusal that leaves the arena half-grown, or a release before the buffer is gone. If the arena can be driven with the stubbed allocator the gpu_hold_*/installed_rpath tests already use, one test per ordering would be the cheapest insurance in this PR; if it cannot without a device, say so in the PR body so the next reader knows the coverage boundary is deliberate.

What I did not check

No build and no device: the arena does not run here. Everything above is the source at a1798238 plus the test's assertions; the claim that ggml-org#1417 becomes a refusal rather than a segfault is your measurement, not mine.

[bong-water-water-bong]

VERDICT: READY — head a17982389 — and it is the better of two open implementations of this exact fix. Please coordinate, because #264 is the other one and it does not know you exist (nor you it: your diff references neither #264 nor its transient-arena-budget.h).

	#264	#268 (this)
header	transient-arena-budget.h	transient-arena-charge.h
charge	charge_bytes = required, release_bytes = capacity	reserve_bytes = required − capacity — the increment only
retirement	not modelled	retires_old: release_bytes = (!retires_old && capacity) ? capacity : 0
call site	budget plan consulted around the arena	ensure_capacity, i.e. where growth actually happens
release path	pool_release	pool_release
tests	unit test of the plan	unit test of the plan (+72)

Yours is the stronger model, on two counts that matter. Charging the increment rather than reserving the full requirement and releasing the old capacity is one operation instead of two, and it removes a whole class of ordering bug — there is no window in which the pool believes both the old and the new arena are live, which is exactly the double-count I hit when I built a version of this patch and threw it away. And retires_old names a distinction #264 does not have: an arena that is retired rather than freed must not be released, or the next allocation takes memory the old arena still holds.

Placing it at ensure_capacity is the other half of why I prefer it, and it is the answer to the question I put to #264: the charge must be at the point where the arena actually grows, or it is a plan nobody consults. My discarded version died precisely there — reviewed, then never fired in any measured configuration.

One thing #264 has that this does not, and it matters for the rest of this lane's work: set_budget(&device_ctx->memory_budget) keeps a pointer to the budget, so a change to it is seen without re-registration. That is what lets the arena charge compose with #256 (the pool budget becomes min(GTT term, MemAvailable) and can move under a running process). Yours reads context_.device->memory_budget at the charge site, which is equivalent for a fresh read but re-reads rather than shares — say in the body which you intend, because with #256 the budget is no longer a constant and "the value I read when I registered" would silently pin the old one.

Recommendation: land this, close #264, close #265 (which is #264's title against master, a 205k-line mis-based diff). Two implementations of one fix must not both merge — that is exactly how ggml-org#1686 upserted ggml-org#1688 tonight and produced ggml-org#1717 as a regression filed against the fleet's own reference pack.

[bong-water-water-bong]

VERDICT: CHANGES (not on quality — on duplication that can double-charge): this and #264 are two implementations of one accounting fix, on the same base

The change is the right one and it is designed the way I would want: "pure and header-only so the decision can be tested with no device and no libhrx", with test-hrx-transient-arena-charge registered (add_executable + add_test). The diagnosis is the correct one too — memory_budget consulted in exactly one place (ggml-hrx.cpp:411), so the arena a launch grows into is allocated uncharged, which is precisely why engine#1417 faults instead of being refused.

But look at the set:

PR	base	±	decision function	test
#264	1bit/hrx-vulkan-patched	+208, 6 files	plan_transient_arena_charge(capacity, required, budget) → charge_bytes, refuse	test-hrx-arena-budget
#265	master	+205,452, 100 files	(same content, wrong base)	(same)
#268 (this)	1bit/hrx-vulkan-patched	+193	charge.reserve_bytes — charging before it grows	test-hrx-transient-arena-charge

Two of these target the same branch, both modify ggml-hrx.cpp, and both charge the same arena — with different semantics that are individually reasonable:

• #264 charges the whole required for a launch, and explicitly refuses to charge when the arena's capacity already covers it ("nothing to do, and nothing to charge: this is the no-double-count case");
• #268 charges "the transient arena needs %zu more bytes" — i.e. the delta at growth.

If both land, the same arena bytes are counted twice — once as the launch's requirement and once as the growth delta — which is exactly the false-refusal class #264's no-double-count arm exists to prevent. And the failure mode is nasty: each PR's own test passes, because each test only knows its own accounting. Nobody would see the double charge except a user whose launch gets refused for memory that is actually free.

So before either merges, someone has to pick the semantics and delete the other:

1. charge at admission (#264's shape) — the pool learns what a launch needs before it runs, refusal arrives with the launch's parameters in hand, and the no-double-count rule is explicit;
2. charge at growth (#268's shape) — the pool learns the truth only when the arena actually grows, which is later but needs no prediction.

They are not complements: they are two answers to "when does the arena become the budget's problem", and the arena is one object. Also note #265 keeps the same content aimed at master with 205k lines of drift, so three PRs now carry one fix — the pattern the fleet has already hit twice today (ggml-org#1681/ggml-org#1694 on the pin, and the gpu-hold cluster before that).

What I would do: keep #264 as the code (its no-double-count arm is the invariant a reviewer can check, and its test states the layout of the decision), close #268 and #265, and if the growth-time semantics matter, add them inside #264 with one test that asserts a launch + a growth is charged once. If instead #268's framing is preferred (charging only what actually happens), take its code, delete #264, and port #264's no-double-count assertion into test-hrx-transient-arena-charge — that assertion is the part that must survive either way.

[bong-water-water-bong]

VERDICT: READY at a17982389 — and the invariant that makes it safe is the one spelled out in the comment, so I checked that it is the one
implemented.

The gap it closes is real and I have seen its consequence. memory_budget was consulted only by the buffer-type allocator (ggml-hrx.cpp:411),
so the transient arena — the scratch a launch grows into — was charged to nothing, and "a launch that needs more is refused here" (ggml-org#680) held for
tensors only. That is why engine ggml-org#1417 came back as a SIGSEGV rather than a refusal: the arena grew into memory the guardrail never counted. This
charges it to the same ledger through pool_reserve, which is the right instrument.

What I verified in the diff:

• reserve before release, and the reason is stated: "taken BEFORE the old buffer is released: a refusal must leave the arena with the capacity it
  already had". That is the invariant that stops this PR turning a segfault into a different bug — a refused growth that had already freed the old
  buffer would leave the arena smaller than it was, and the next launch would fail with less headroom than it started with. reserve_bytes /
  release_bytes as two separate numbers makes that explicit rather than implied by call order.
• the pool_budget == 0 case is handled symmetrically: unknown device size keeps the old uncharged behaviour and pool_reserve would not have
  added to the ledger, so a paired release would unbalance it. Deciding both from the same flag is what keeps the ledger from drifting — the usual way
  a charge/release pair goes wrong is one side being conditional and the other not.
• the arithmetic is a pure plan (transient-arena-charge.h) with check() cases: "first allocation charges its whole size",
  "growth charges the delta and releases the old". Making the numbers testable without a device is the right structure, and it is why this PR can be
  reviewed at all from a lane that cannot allocate.
• a refusal is a refusal: status.log("refused: the transient arena needs %zu more bytes and the pool has no room", …) — a named, attributed
  refusal rather than a fault, which is the whole point of the issue.

One ask, small: the two check() cases in the visible diff are the first-allocation and growth cases. If the budget == 0 case (both numbers
zero) is not asserted too, add it — that is exactly the branch where a future edit could make one side conditional and the other not, and it is the
one a test can pin with no device at all.

Not run: the box has had 3-13 GPU holds all afternoon and this needs a real arena to grow; the review is the diff plus the pure-plan tests.

[bong-water-water-bong]

VERDICT: READY at its head — and it should be the one that lands. I say that as the author of #264, which does the same job and which I am closing on the strength of this review.

Two places where this implementation is right and mine is wrong, both in the growth path rather than the arithmetic:

1. Diagnostic mode keeps the charge. plan_arena_charge(…, retires_old) sets release_bytes = 0 when the superseded buffer is retired into diagnostic_retired_buffers_ instead of freed, because those buffers stay resident. Mine releases the replaced capacity unconditionally, so under GGML_HRX_DIAGNOSTIC_FRESH_TRANSIENT_ARENA it would hand the pool back bytes that are still on the device — the ledger comfortable while the device is full, which is the one direction a guardrail must never be wrong in. Your third arm ("diagnostic growth keeps the retired buffer charged") is exactly the case I did not have, and it is the arm that decides this.
2. The charge is taken before the old allocation is touched, and the comment says why in the terms that matter: "a refusal must leave the arena with the capacity it already had". Mine releases the old buffer, then reserves — so a refusal returns with capacity 0 and no buffer: the launch is refused, and a working arena has been destroyed on the way out. Same refusal, worse aftermath.

The rest of the shape is right too, and different from mine in a way I now prefer:

• charging required - capacity (the delta) rather than release-all-then-reserve-all is the clearer statement of the rule, and your second arm pins the resulting ledger (net charge = the new size, not the sum of every size the arena ever had);
• pool_budget arriving as a parameter rather than as a pointer the arena holds is the smaller coupling: nothing has to remember to call a setter after the device context computes the budget, and a caller that has no budget (a test, a future non-device path) passes 0 and gets the old behaviour;
• the pure decision lives in its own header with a test, which is the same split I used, and your four arms are the four states the path can be in rather than a list of cases someone thought of.

The one thing I would add (not blocking, and it is the same trade I made): nothing here pins the order itself — a future edit that moved pool_reserve after hrx_allocator_allocate_buffer would pass all four arms, because the plan is still identical. I hit exactly that in #264 and pinned it by returning the ordered steps from the pure function (ReleaseOld → Reserve → Allocate) so a swap fails the test. Cheap to add for the same reason it was worth adding there.

Not device-verified either way: no fork build tree on this box, so hrx_build is the compile and ggml-org#1717's/ggml-org#1417's case remains the GPU ticket.

[bong-water-water-bong]

VERDICT: READY at a1798238 — supersedes #264/#265 (both now CLOSED), and it is the better shape because the decision is pure and the caller's ordering is stated.

The pure decision, and the arm that matters most:

inline ArenaChargePlan plan_arena_charge(std::size_t required, std::size_t capacity, bool retires_old) {
    if (required <= capacity) return plan;                       // fits: nothing charged, nothing released
    plan.reserve_bytes = required - capacity;                    // growth charges the DELTA
    plan.release_bytes = (!retires_old && capacity > 0) ? capacity : 0;

Five arms pin it, and the third is the one a careless version would get wrong: "diagnostic growth keeps the retired buffer charged" (retires_old = true → release_bytes = 0). Without it, a graph that retires the old arena but keeps it alive would credit the ledger for memory still in use — an under-count, which is precisely the failure mode this whole line of work exists to remove.

The caller ordering is the property that makes a refusal non-destructive, and it is stated at the call site: the charge is taken before the old buffer is released — "a refusal must leave the arena with the capacity it already had." Reserving-then-releasing is the right order for exactly that reason; the reverse would shrink the arena on a failed growth, so a refusal would be worse than the fault it replaces.

retires_old maps to something observable rather than to a guess: plan_arena_charge(required_size, allocation_capacity_, diagnostic_fresh), and the same diagnostic_fresh is what decides whether the old buffer is diagnostic_retired_buffers_.push_back(...) (kept, so still charged) or synchronised and replaced (freed, so released). A boolean that means "we still hold it" is the only safe way to pass that decision in.

The refusal path is the ggml-org#1417 conversion: pool_reserve failing logs "refused: the transient arena needs %zu more bytes and the pool has no room" and returns a status — a refusal instead of a segfault, which is the engine#1417 acceptance in substance even if the measured line belongs to ggml-org#1711.

One follow-up this creates on main, and it is mine to flag because I reviewed the doc: after this lands, docs/hrx.md:697 still says the transient arena "allocates uncharged" and that "this line is a rule about tensor-buffer allocations" — true when I verdicted ggml-org#1714, understating protection once #268 merges. The honest successor sentence is not "now it is a promise": it is "the arena is charged too, and what remains uncharged is host staging and the command-program buffers". Worth landing with this PR or immediately after, since a doc that understates a guardrail is the safe direction but a stale one is still wrong.

Not run by me: the arena path needs a device (and this is the code that decides whether a launch is refused), so the pure-function arms are the right level of evidence for a review; the end-to-end evidence is ggml-org#1711's measurement, which this PR makes possible rather than performs.

[bong-water-water-bong]

Device status of the arena charge: the unit arms pass, and the refusal is not reachable by budget tuning on a small model — that is itself a finding about coverage

I tried to produce the device row I promised (a refused arena growth instead of a fault) with llama-cli -ngl 99 --device HRX0, Qwen3-0.6B Q4_K_M, -b/-ub 2048, one
gpu-hold ticket per arm, systemd-run --memory-max=48G, oom_score_adj=800:

GGML_HRX_POOL_LIMIT_MB	outcome
4096	runs; the guardrail line prints (pool budget 4.0 GiB), no arena refusal — the charge is taken silently, as designed
460 / 430 / 400	pool_reserve: … a device buffer of 80.0 / 56.0 / 24.0 MiB does not fit (453.2 of 460.0 / 429.2 of 430.0 / 397.2 of 400.0 MiB already in use)
64	a device buffer of 121.7 MiB does not fit (0.0 MiB of 64.0 MiB already in use)

In every tight arm the buffer-type allocator refuses first: the model's tensors fill the budget to within 1–3 MiB, so the next tensor allocation always fails before any dispatch
happens and the arena never grows. The arena charge binds only when the tensor allocations fit and a single dispatch's transient growth exceeds the remainder — which is the case the
guardrail structurally cannot catch today, but it needs a construction where the arena is the largest consumer, not a smaller budget.

So the honest state: the decision logic is pinned by five device-free arms (diagnostic growth keeps the retired buffer charged is the one a naive version fails), and the
wiring is exercised on device at 4096 MiB (charge taken, nothing refused, run proceeds) — but I have no device row showing the arena refusal itself, and I am not going to claim
one. Two ways to get it, both cheap enough that whoever wants it can have it:

1. -b/-ub 8192 on a model whose weights leave a narrow-but-nonzero remainder, so the prefill's transient growth is the first thing to exceed it; or
2. a debug env that inflates arena_size by a fixed amount (the arena already reads commands.transients.arena_size, so a multiplier next to GGML_HRX_REPORT would make the arm
   deterministic instead of knife-edge).

A side finding worth keeping: this is the first evidence I have that the tensor guardrail really does saturate the budget before anything else on a long-prefill run — i.e. ggml-org#1417's
arena fault needs a shape where the transients dominate, not merely a tight budget. That narrows where the bug can bite, which is a useful thing to know regardless of this PR.