hrx: a measured zero host figure refuses again, not 'unreadable' (review of #259)

ggml/src/ggml-hrx/ggml-hrx.cpp

@@ -1636,8 +1636,9 @@ static std::unique_ptr<ggml_backend_hrx_reg_context> create_registry_context() {
         device_free_total(device_ctx.get(), device_free, device_total);
         // A failed /proc read is "unknown", not "zero bytes left": kHostUnknown disables the term, while a measured zero must
         // refuse - a host with nothing left is the worst moment to admit a launch (engine #1502).
-        const size_t host_raw       = ggml::hrx::host_memory_available_bytes();
-        const size_t host_available = host_raw == 0 ? ggml::hrx::onebit::kHostUnknown : host_raw;
+        size_t       host_raw       = 0;
+        const size_t host_available =
+            ggml::hrx::onebit::pool_host_term(host_raw, ggml::hrx::host_memory_available_bytes(host_raw));
         const ggml::hrx::onebit::pool_budget_terms_t terms =
             ggml::hrx::onebit::pool_budget_terms(device_ctx->memory_total, device_free, host_available);
         device_ctx->memory_budget = terms.budget;

ggml/src/ggml-hrx/hrx-1bit-safety.cpp

@@ -84,6 +84,10 @@ bool gtt_counters(size_t & total, size_t & used) {
     return false;
 }
 
+size_t pool_host_term(size_t raw, bool readable) {
+    return readable ? raw : kHostUnknown;
+}
+
 bool pool_budget_is_fixed_limit() {
     const char * env = std::getenv("GGML_HRX_POOL_LIMIT_MB");
     return env != nullptr && std::atoll(env) > 0;
@@ -267,8 +271,8 @@ bool pool_reserve(size_t budget, size_t size, const char * what) {
         size_t in_use = 0;
         if (pool_live_counters(total, in_use)) {
             const size_t margin = pool_live_margin_bytes();
-            const size_t host_raw       = host_memory_available_bytes();
-            const size_t host_available = host_raw == 0 ? kHostUnknown : host_raw;
+            size_t       host_raw     = 0;
+            const size_t host_available = pool_host_term(host_raw, host_memory_available_bytes(host_raw));
             if (!pool_live_fits(size, total, in_use, margin, host_available)) {
                 const size_t live_free = in_use >= total ? 0 : total - in_use;
                 GGML_LOG_ERROR("%s: HRX pool limit: %s of %.1f MiB does not fit the live device budget "

ggml/src/ggml-hrx/hrx-1bit-safety.h

@@ -50,6 +50,12 @@ size_t pool_budget(size_t memory_total, size_t memory_free, size_t memory_availa
 // remainder, so a caller that prints the figure has to word it differently.
 bool   pool_budget_is_fixed_limit();
 
+// engine #1502/#259: the host term as the pool arithmetic wants it. `readable` says whether the figure was actually
+// taken; a *measured* zero is passed through as 0 (a host with nothing left must refuse), while an unreadable one
+// becomes kHostUnknown so the term is dropped rather than refusing everything. The two call sites that used
+// `raw == 0 ? kHostUnknown : raw` collapsed those two cases - see tests/test-hrx-pool-refusal-budget.cpp.
+size_t pool_host_term(size_t raw, bool readable);
+
 // The budget after an allocation the allocator refused, given the size it refused (engine #1502).
 //
 // The guardrail can only estimate what the pool will serve - ROCr exposes no "available" query, so pool_budget() comes from the

ggml/src/ggml-hrx/runtime/host-memory-available.cpp

@@ -26,40 +26,51 @@ constexpr size_t kMeminfoBytes = 16 * 1024;
 
 }  // namespace
 
-size_t mem_available_from_meminfo(const char * meminfo) {
+bool mem_available_from_meminfo(const char * meminfo, size_t & out) {
     if (meminfo == nullptr) {
-        return 0;
+        return false;
     }
     for (const char * line = meminfo; *line != '\0';) {
         if (std::strncmp(line, "MemAvailable:", sizeof("MemAvailable:") - 1) == 0) {
             unsigned long long kib = 0;
             if (std::sscanf(line + sizeof("MemAvailable:") - 1, " %llu", &kib) != 1) {
-                return 0;
+                return false;   // the field is there but unreadable: unknown, not zero
             }
             if (kib > std::numeric_limits<size_t>::max() / 1024) {
-                return 0;
+                return false;
             }
-            return static_cast<size_t>(kib) * 1024;
+            out = static_cast<size_t>(kib) * 1024;
+            return true;        // "MemAvailable: 0 kB" is a reading, not an unknown
         }
         const char * next = std::strchr(line, '\n');
         if (next == nullptr) {
-            return 0;
+            return false;
         }
         line = next + 1;
     }
-    return 0;
+    return false;
 }
 
-size_t host_memory_available_bytes() {
+size_t mem_available_from_meminfo(const char * meminfo) {
+    size_t out = 0;
+    return mem_available_from_meminfo(meminfo, out) ? out : 0;
+}
+
+bool host_memory_available_bytes(size_t & out) {
     FILE * file = std::fopen("/proc/meminfo", "rb");
     if (file == nullptr) {
-        return 0;
+        return false;
     }
     char   body[kMeminfoBytes] = {};
     const size_t read  = std::fread(body, 1, sizeof(body) - 1, file);
     std::fclose(file);
     body[read]         = '\0';
-    return mem_available_from_meminfo(body);
+    return mem_available_from_meminfo(body, out);
+}
+
+size_t host_memory_available_bytes() {
+    size_t out = 0;
+    return host_memory_available_bytes(out) ? out : 0;
 }
 
 }  // namespace ggml::hrx

ggml/src/ggml-hrx/runtime/host-memory-available.h

@@ -23,8 +23,18 @@ namespace ggml::hrx {
 // Split from the read so the warning in host-memory.cpp can be tested without a device or a live /proc.
 size_t mem_available_from_meminfo(const char * meminfo);
 
+// The same parse, saying whether a figure was taken: false when the field is absent, the body is truncated or the
+// number is unparsable. "MemAvailable: 0 kB" returns true with 0 - a fact, not an unknown (see the reader below).
+bool mem_available_from_meminfo(const char * meminfo, size_t & out);
+
 // Live MemAvailable on this host; 0 when /proc/meminfo cannot be read. 0 means "unknown": a caller that wants to warn
 // about memory must stay quiet rather than guess a threshold (engine #669).
 size_t host_memory_available_bytes();
 
+// The same read, but saying whether a figure was actually taken: true and *out filled when MemAvailable was parsed,
+// false (and *out untouched) when /proc/meminfo or the field is missing. A *measured* zero returns true with 0 in *out -
+// a host with nothing left is a fact, not an unknown, and engine #1502's admission check must refuse on it rather than
+// drop the host term (the size_t form above cannot tell the two apart).
+bool host_memory_available_bytes(size_t & out);
+
 }  // namespace ggml::hrx

tests/test-hrx-host-memory-available.cpp

@@ -63,6 +63,18 @@ int main() {
     REQUIRE(mem_available_from_meminfo(nullptr) == 0);
     REQUIRE(mem_available_from_meminfo("") == 0);
 
+    // engine #1502/#259: the read-or-not form. "0 kB" is a reading (an exhausted host, which must REFUSE), while an
+    // absent or unparsable field is an unknown (the term is dropped). The size_t form above cannot tell them apart,
+    // which is how both pool call sites ended up mapping a measured zero to "unknown".
+    size_t parsed = 0;
+    REQUIRE(mem_available_from_meminfo("MemAvailable: 0 kB\n", parsed) && parsed == 0);
+    parsed = 1234;
+    REQUIRE(!mem_available_from_meminfo("MemFree: 512 kB\n", parsed) && parsed == 1234);      // untouched on failure
+    REQUIRE(!mem_available_from_meminfo("MemAvailable: banana kB\n", parsed));
+    REQUIRE(!mem_available_from_meminfo("", parsed));
+    REQUIRE(!mem_available_from_meminfo(nullptr, parsed));
+    REQUIRE(mem_available_from_meminfo("MemAvailable: 4096 kB\n", parsed) && parsed == 4096 * 1024);
+
     // The live reader on this host: whoever calls it must get either a plausible number or 0 ("unknown", stay quiet).
     const size_t live = host_memory_available_bytes();
     std::printf("host_memory_available_bytes() = %zu bytes (%.1f MiB)\n", live, double(live) / double(MiB));

tests/test-hrx-pool-refusal-budget.cpp

@@ -181,6 +181,19 @@ int main() {
     expect_true("and the terms say the host bound it", pool_budget_terms(total, issue_free, 0).host_bound);
     expect_eq("unknown device stays unfettered", pool_budget(0, 0, host_tight), 0);
 
+    // ---- the host term's ENCODING (review of fork #259) ----------------------------------------------------------
+    // Both call sites used to write `raw == 0 ? kHostUnknown : raw`, which cannot tell an unreadable /proc from a host
+    // with nothing left - and the second is the case where admitting is worst, as the comment above them says.
+    // pool_host_term() takes the reader's own answer instead.
+    using ggml::hrx::onebit::pool_host_term;
+    expect_eq("a measured host figure passes through", pool_host_term(64 * GiB, true), 64 * GiB);
+    expect_eq("a MEASURED ZERO stays zero, so the term refuses", pool_host_term(0, true), 0);
+    expect_eq("an unreadable /proc becomes unknown, so the term is dropped", pool_host_term(0, false), kHostUnknown);
+    expect_true("a measured zero refuses through the live check",
+                !pool_live_fits(1 * MiB, gtt, gtt - 45 * GiB, margin_256, pool_host_term(0, true)));
+    expect_true("an unreadable /proc still admits on the GTT term alone",
+                pool_live_fits(1 * MiB, gtt, gtt - 45 * GiB, margin_256, pool_host_term(0, false)));
+
     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;