diff --git a/ggml/src/ggml-hrx/ggml-hrx.cpp b/ggml/src/ggml-hrx/ggml-hrx.cpp index 5f01aa3002e6..204fb72fd72c 100644 --- a/ggml/src/ggml-hrx/ggml-hrx.cpp +++ b/ggml/src/ggml-hrx/ggml-hrx.cpp @@ -1636,8 +1636,9 @@ static std::unique_ptr 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; diff --git a/ggml/src/ggml-hrx/hrx-1bit-safety.cpp b/ggml/src/ggml-hrx/hrx-1bit-safety.cpp index 04ae93e239d5..da9b5152b9fe 100644 --- a/ggml/src/ggml-hrx/hrx-1bit-safety.cpp +++ b/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 " diff --git a/ggml/src/ggml-hrx/hrx-1bit-safety.h b/ggml/src/ggml-hrx/hrx-1bit-safety.h index 959e030e90da..7dcb2988ae25 100644 --- a/ggml/src/ggml-hrx/hrx-1bit-safety.h +++ b/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 diff --git a/ggml/src/ggml-hrx/runtime/host-memory-available.cpp b/ggml/src/ggml-hrx/runtime/host-memory-available.cpp index cc94655da4eb..41afa8393fd9 100644 --- a/ggml/src/ggml-hrx/runtime/host-memory-available.cpp +++ b/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::max() / 1024) { - return 0; + return false; } - return static_cast(kib) * 1024; + out = static_cast(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 diff --git a/ggml/src/ggml-hrx/runtime/host-memory-available.h b/ggml/src/ggml-hrx/runtime/host-memory-available.h index cbfb15a1e6a4..05aa93548d2e 100644 --- a/ggml/src/ggml-hrx/runtime/host-memory-available.h +++ b/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 diff --git a/tests/test-hrx-host-memory-available.cpp b/tests/test-hrx-host-memory-available.cpp index e36f7d4038fc..aa9f3dadd888 100644 --- a/tests/test-hrx-host-memory-available.cpp +++ b/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)); diff --git a/tests/test-hrx-pool-refusal-budget.cpp b/tests/test-hrx-pool-refusal-budget.cpp index 1bbcd2f7e7dc..0c26c4dade17 100644 --- a/tests/test-hrx-pool-refusal-budget.cpp +++ b/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;