From 5d92ecb61260912160cafe202e27b693c3fb4dda Mon Sep 17 00:00:00 2001 From: agent Date: Tue, 10 Nov 2026 20:53:41 -0400 Subject: [PATCH] hrx: a host-bound refusal says the host bound it, not that GTT is full (engine#2069) `ggml-hrx.cpp` refuses the device when the pool budget is 0 with a message about GTT total/free/ceiling and the advice "free GTT or lower the reserve". But `pool_budget_terms()` can make the budget 0 because the **host** term was the smaller one -- and since #1942/#259 a *measured* zero host figure refuses by design -- so the line names the term that was not binding and sends the operator to the wrong resource. Reported twice: from my #259 review (the same defect one layer down in the live check's line), and by a lane who hit it while reproducing #1926: `GTT total 120.0 GiB with 78.0 GiB free` cannot explain a refusal when 78 GiB against a 104 GiB ceiling leaves ~62 GiB of headroom. The wording is now a **pure function**, `pool_refusal_note(terms, total, free, device)`, so both branches are asserted without a device -- the whole product of a guardrail is this line, and it had no test: ``` PASS a measured-zero host refusal says HOST, not the GTT ceiling PASS and it does not send the operator to GTT PASS and it names the measured zero and its likely cause PASS a tight-host refusal also says HOST PASS and does NOT claim a measured zero PASS a GTT-bound refusal still names the ceiling and the reserve PASS and does not claim the host bound it test-hrx-pool-refusal-budget: all requirements passed ``` The GTT branch keeps its familiar sentence, so an operator who has seen that line before still recognises it; only the host-bound case gains new words, and it names the two terms and the numbers that decided. **Not compiled here**: `ggml-hrx.cpp` needs the private HRX headers, which this checkout does not carry, so the call-site change is checked by signature (`pool_refusal_note(terms, size_t, size_t, const char*)`, a syntax-only compile of exactly that expression) rather than by a build. The pure function and its arms are compiled and run above (`ggml` from the same tree). `hrx-1bit-safety.cpp` already includes ``, so the `std::snprintf` adds no include. Refs: engine#2069 (this), engine#1926 (the report that re-found it), llama.cpp#259 (my review that found the second site). --- ggml/src/ggml-hrx/ggml-hrx.cpp | 11 ++++------ ggml/src/ggml-hrx/hrx-1bit-safety.cpp | 30 ++++++++++++++++++++++++++ ggml/src/ggml-hrx/hrx-1bit-safety.h | 8 +++++++ tests/test-hrx-pool-refusal-budget.cpp | 28 ++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 7 deletions(-) diff --git a/ggml/src/ggml-hrx/ggml-hrx.cpp b/ggml/src/ggml-hrx/ggml-hrx.cpp index d70ceece75b8..4a99f3b8dc38 100644 --- a/ggml/src/ggml-hrx/ggml-hrx.cpp +++ b/ggml/src/ggml-hrx/ggml-hrx.cpp @@ -1654,13 +1654,10 @@ static std::unique_ptr create_registry_context() { if (device_ctx->memory_budget == 0 && device_ctx->memory_total != 0) { // Nothing fits under the ceiling: refuse the device here, loudly, instead of planning a launch that can // reach the fault line (#680). Report only, do not try to be clever about it. - const size_t reserve = ggml::hrx::onebit::pool_reserve_bytes(); - GGML_LOG_ERROR("ggml_hrx: %s: refusing to use the device: GTT total %.1f GiB with %.1f GiB free leaves " - "nothing under the %.1f GiB ceiling (reserve GGML_HRX_GTT_RESERVE_MB, override " - "GGML_HRX_POOL_LIMIT_MB); free GTT or lower the reserve (engine #680)\n", - device_ctx->name.c_str(), device_total / (1024.0 * 1024.0 * 1024.0), - device_free / (1024.0 * 1024.0 * 1024.0), - (device_total > reserve ? device_total - reserve : 0) / (1024.0 * 1024.0 * 1024.0)); + // engine #2069: the line names the term that made the budget zero -- a host-bound refusal that says "free GTT" + // sends the operator to the wrong resource. The wording is a pure function so both branches are tested. + GGML_LOG_ERROR("%s\n", ggml::hrx::onebit::pool_refusal_note(terms, device_total, device_free, + device_ctx->name.c_str()).c_str()); hrx_device_release(hrx_device); continue; } diff --git a/ggml/src/ggml-hrx/hrx-1bit-safety.cpp b/ggml/src/ggml-hrx/hrx-1bit-safety.cpp index 3e280184b557..011e84950d90 100644 --- a/ggml/src/ggml-hrx/hrx-1bit-safety.cpp +++ b/ggml/src/ggml-hrx/hrx-1bit-safety.cpp @@ -136,6 +136,36 @@ pool_budget_terms_t pool_budget_terms(size_t memory_total, size_t memory_free, s return t; } +// engine #2069 (llama.cpp #259's review): the refusal has to name the term that bound it. `budget == 0` is reached two ways +// that look identical and have opposite remedies -- the GTT ceiling leaving nothing under the reserve, or the host figure +// being the smaller term (a measured zero refuses by design since #1942/#259, and a tiny MemAvailable does the same). The +// numbers printed are the ones the decision used, not the ones that happen to be nearby. +std::string pool_refusal_note(const pool_budget_terms_t & terms, size_t memory_total, size_t memory_free, + const char * device_name) { + char line[512]; + const double gib = 1024.0 * 1024.0 * 1024.0; + if (terms.host_bound) { + // The host term was the smaller one and it is what made the budget zero. + std::snprintf(line, sizeof line, + "ggml_hrx: %s: refusing to use the device: the HOST term bound the pool - host MemAvailable %.1f GiB " + "against a GTT term of %.1f GiB (GTT total %.1f GiB with %.1f GiB free). This is not the GTT ceiling: " + "freeing GTT will not help while host memory is the smaller term%s (engine #1502, #2069)", + device_name != nullptr ? device_name : "HRX", terms.host_available / gib, terms.gtt_term / gib, + memory_total / gib, memory_free / gib, + terms.host_available == 0 ? " -- a measured zero refuses by design, so check that /proc/meminfo is readable (#1942)" + : ""); + return std::string(line); + } + const size_t reserve = pool_reserve_bytes(); + std::snprintf(line, sizeof line, + "ggml_hrx: %s: refusing to use the device: GTT total %.1f GiB with %.1f GiB free leaves nothing under the " + "%.1f GiB ceiling (reserve GGML_HRX_GTT_RESERVE_MB, override GGML_HRX_POOL_LIMIT_MB); free GTT or lower the " + "reserve (engine #680)", + device_name != nullptr ? device_name : "HRX", memory_total / gib, memory_free / gib, + (memory_total > reserve ? memory_total - reserve : 0) / gib); + return std::string(line); +} + size_t pool_budget(size_t memory_total, size_t memory_free, size_t memory_available) { return pool_budget_terms(memory_total, memory_free, memory_available).budget; } diff --git a/ggml/src/ggml-hrx/hrx-1bit-safety.h b/ggml/src/ggml-hrx/hrx-1bit-safety.h index 3c709d5485d3..de7bb0aa7dd4 100644 --- a/ggml/src/ggml-hrx/hrx-1bit-safety.h +++ b/ggml/src/ggml-hrx/hrx-1bit-safety.h @@ -19,6 +19,8 @@ #pragma once +#include + #include #include @@ -46,6 +48,12 @@ inline constexpr size_t kHostUnknown = static_cast(-1); pool_budget_terms_t pool_budget_terms(size_t memory_total, size_t memory_free, size_t memory_available = kHostUnknown); size_t pool_budget(size_t memory_total, size_t memory_free, size_t memory_available = kHostUnknown); +// The refusal line, as text (engine #2069, llama.cpp #259's review). A budget of 0 means "nothing fits", and WHICH term made +// it zero decides what the operator should free: the GTT ceiling leaves GTT and the reserve as the knobs, while a host-bound +// budget leaves host memory, and "free GTT" would send them to the wrong one. Pure, so the two wordings are pinned by +// test-hrx-pool-refusal-budget.cpp without a device -- the whole product of a guardrail is this line. +std::string pool_refusal_note(const pool_budget_terms_t & terms, size_t memory_total, size_t memory_free, const char * device_name); + // true when GGML_HRX_POOL_LIMIT_MB is in force: pool_budget() then returns that fixed limit rather than a // remainder, so a caller that prints the figure has to word it differently. bool pool_budget_is_fixed_limit(); diff --git a/tests/test-hrx-pool-refusal-budget.cpp b/tests/test-hrx-pool-refusal-budget.cpp index 81269da7f99d..e4bc71c4710a 100644 --- a/tests/test-hrx-pool-refusal-budget.cpp +++ b/tests/test-hrx-pool-refusal-budget.cpp @@ -232,6 +232,34 @@ int main() { expect_eq("and a stale caller value is not reported as a reading", raw, 0); expect_eq("no reader at all is unknown too", pool_host_term_read(nullptr, raw), kHostUnknown); + // ---- the refusal WORDING names the term that bound it (engine #2069) ------------------------------------------- + // + // The budget is 0 in two situations with opposite remedies, and the old line printed GTT numbers for both: "free GTT or + // lower the reserve" when the HOST term was the smaller one sends the operator to the wrong resource. Both wordings are + // pure, so this is device-free -- and the line is the whole product of a guardrail. + { + const size_t total = 120 * GiB; + const size_t free_ = 78 * GiB; // 62 GiB of GTT headroom cannot explain a refusal + const auto host0 = pool_budget_terms(total, free_, 0); // a MEASURED zero refuses by design + const std::string note0 = pool_refusal_note(host0, total, free_, "HRX0"); + expect_true("a measured-zero host refusal says HOST, not the GTT ceiling", note0.find("HOST term") != std::string::npos); + expect_true("and it does not send the operator to GTT", note0.find("free GTT or lower the reserve") == std::string::npos); + expect_true("and it names the measured zero and its likely cause", note0.find("measured zero refuses by design") != std::string::npos); + + const auto host_tight = pool_budget_terms(total, free_, 1 * GiB); + const std::string note1 = pool_refusal_note(host_tight, total, free_, "HRX0"); + expect_true("a tight-host refusal also says HOST", note1.find("HOST term") != std::string::npos); + expect_true("and does NOT claim a measured zero", note1.find("measured zero") == std::string::npos); + + // A GTT refusal keeps its familiar advice: there the ceiling and the reserve really are the knobs. + const size_t in_use = 100 * GiB; + const auto gtt = pool_budget_terms(total, total - in_use, kHostUnknown); + const std::string note2 = pool_refusal_note(gtt, total, total - in_use, "HRX0"); + expect_true("a GTT-bound refusal still names the ceiling and the reserve", + note2.find("free GTT or lower the reserve") != std::string::npos); + expect_true("and does not claim the host bound it", note2.find("HOST term") == std::string::npos); + } + printf(failures == 0 ? "test-hrx-pool-refusal-budget: all requirements passed\n" : "test-hrx-pool-refusal-budget: %d FAILED\n", failures); return failures == 0 ? 0 : 1;