From b7480aad7ae0e532bac9ce921fa2abed135ee0ac Mon Sep 17 00:00:00 2001 From: Joel Teply Date: Tue, 6 Oct 2026 09:52:20 -0500 Subject: [PATCH 1/2] metal + opt: a lookup that finds no buffer fails the graph, and a failed graph fails the training job The Metal OUT_PROD bug (#41) corrupted every LoRA backward on Apple silicon for ten days while saying so in one log line per op ("tensor '' buffer is nil") that nobody read: a lookup that finds no buffer hands the kernel a nil buffer, the op does nothing it was meant to, and the graph carries on with stale memory. A warning that names a corrupted kernel input must not be a warning (BigMama on #41). - ggml-metal: every such lookup is counted process-wide; ggml_metal_graph_compute returns GGML_STATUS_FAILED for a graph during whose encode the count rose - ggml-opt: ggml_opt_eval no longer ignores the compute's status; a failure becomes the context's refusal (the reason #35 already carries to llama_opt_failure) - llama: a failed eval stops the epoch through the same path as a refused graph, so the server reports the reason and writes no adapter (serving is unaffected) Known-positive on the M5 (Qwen3.5-0.8B, Metal): with #41's fix reverted, /train ends in error "the backend failed to compute the training graph (GGML status: error (operation failed)) ... nothing was written", no adapter file, /health ok; with the fix, the run completes and writes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo --- ggml/src/ggml-metal/ggml-metal-context.m | 10 ++++++++++ ggml/src/ggml-metal/ggml-metal-device.h | 2 ++ ggml/src/ggml-metal/ggml-metal-device.m | 11 +++++++++++ ggml/src/ggml-opt.cpp | 9 ++++++++- src/llama-context.cpp | 9 +++++++++ 5 files changed, 40 insertions(+), 1 deletion(-) diff --git a/ggml/src/ggml-metal/ggml-metal-context.m b/ggml/src/ggml-metal/ggml-metal-context.m index b45e72093600..13e181ebf814 100644 --- a/ggml/src/ggml-metal/ggml-metal-context.m +++ b/ggml/src/ggml-metal/ggml-metal-context.m @@ -441,6 +441,10 @@ enum ggml_status ggml_metal_graph_compute(ggml_metal_t ctx, struct ggml_cgraph * return GGML_STATUS_FAILED; } + // a lookup that finds no buffer during this graph's encode means some op ran on the wrong + // memory: the graph is reported failed, never as a success computed on stale bytes + const uint64_t nil_lookups_0 = ggml_metal_nil_lookups(); + // number of nodes encoded by the main thread (empirically determined) const int n_main = MAX(64, 0.1*gf->n_nodes); @@ -611,6 +615,12 @@ enum ggml_status ggml_metal_graph_compute(ggml_metal_t ctx, struct ggml_cgraph * } } + if (ggml_metal_nil_lookups() != nil_lookups_0) { + GGML_LOG_ERROR("%s: %llu tensor lookup(s) found no buffer while encoding this graph: its result is not trusted\n", + __func__, (unsigned long long) (ggml_metal_nil_lookups() - nil_lookups_0)); + return GGML_STATUS_FAILED; + } + return GGML_STATUS_SUCCESS; } diff --git a/ggml/src/ggml-metal/ggml-metal-device.h b/ggml/src/ggml-metal/ggml-metal-device.h index 51f563fa2020..8c80ffaf98a6 100644 --- a/ggml/src/ggml-metal/ggml-metal-device.h +++ b/ggml/src/ggml-metal/ggml-metal-device.h @@ -346,6 +346,8 @@ void ggml_metal_buffer_clear (ggml_metal_buffer_t buf, uint8_t value); // Metal buffer based on the host memory pointer // struct ggml_metal_buffer_id ggml_metal_buffer_get_id(ggml_metal_buffer_t buf, const struct ggml_tensor * t); +// lookups that found no buffer holding the tensor, process-wide (see ggml_metal_buffer_get_id) +uint64_t ggml_metal_nil_lookups(void); // [MOE-GATHER #23] GPU virtual address of a host pointer inside this Metal buffer (MTLBuffer.gpuAddress // + intra-buffer delta), or 0 when the pointer is not covered / the OS lacks gpuAddress. Cross-buffer diff --git a/ggml/src/ggml-metal/ggml-metal-device.m b/ggml/src/ggml-metal/ggml-metal-device.m index 82215837c8fa..872a2b9f5ed8 100644 --- a/ggml/src/ggml-metal/ggml-metal-device.m +++ b/ggml/src/ggml-metal/ggml-metal-device.m @@ -2378,6 +2378,16 @@ void ggml_metal_buffer_clear(ggml_metal_buffer_t buf, uint8_t value) { } } +// Every lookup that found no buffer holding a tensor's bytes, process-wide. Such a lookup hands +// the kernel a nil buffer: the op reads or writes nothing it was meant to, and the graph goes on +// with stale memory (Metal OUT_PROD read stale scratch in every LoRA backward that way, only a +// log line saying so). ggml_metal_graph_compute fails a graph during which this count rose. +static atomic_uint_fast64_t g_ggml_metal_nil_lookups = 0; + +uint64_t ggml_metal_nil_lookups(void) { + return (uint64_t) atomic_load(&g_ggml_metal_nil_lookups); +} + struct ggml_metal_buffer_id ggml_metal_buffer_get_id(ggml_metal_buffer_t buf, const struct ggml_tensor * t) { struct ggml_metal_buffer_id res = { nil, 0 }; @@ -2399,6 +2409,7 @@ struct ggml_metal_buffer_id ggml_metal_buffer_get_id(ggml_metal_buffer_t buf, co } GGML_LOG_ERROR("%s: error: tensor '%s' buffer is nil\n", __func__, t->name); + atomic_fetch_add(&g_ggml_metal_nil_lookups, 1); return res; } diff --git a/ggml/src/ggml-opt.cpp b/ggml/src/ggml-opt.cpp index cfd2d51cbd64..5220ec5f65b4 100644 --- a/ggml/src/ggml-opt.cpp +++ b/ggml/src/ggml-opt.cpp @@ -1090,7 +1090,14 @@ void ggml_opt_eval(ggml_opt_context_t opt_ctx, ggml_opt_result_t result) { } } - ggml_backend_sched_graph_compute(opt_ctx->backend_sched, opt_ctx->allocated_graph_copy); + const enum ggml_status status = ggml_backend_sched_graph_compute(opt_ctx->backend_sched, opt_ctx->allocated_graph_copy); + if (status != GGML_STATUS_SUCCESS) { + // the step's gradients (and the optimizer update inside this graph) were computed on + // memory the backend says it did not have: the run must stop, never continue on them + opt_ctx->refusal = std::string("the backend failed to compute the training graph (") + ggml_status_to_string(status) + + "): this step's gradients are not trusted, so the run stopped and nothing was written"; + GGML_LOG_ERROR("%s: %s\n", __func__, opt_ctx->refusal.c_str()); + } opt_ctx->iter += opt_ctx->allocated_graph == opt_ctx->gb_opt; opt_ctx->opt_i = (opt_ctx->opt_i + 1) % opt_ctx->opt_period; diff --git a/src/llama-context.cpp b/src/llama-context.cpp index e360daac87ca..27a26d0948c9 100644 --- a/src/llama-context.cpp +++ b/src/llama-context.cpp @@ -3537,6 +3537,15 @@ void llama_context::opt_epoch_iter( GGML_ASSERT(row == n_outputs); } ggml_opt_eval(opt_ctx, result); + if (const char * why = ggml_opt_refusal(opt_ctx); why[0] != '\0') { + // the backend failed the graph (ggml_opt_eval): the run fails, as a refused graph does + opt_failure = why; + LLAMA_LOG_ERROR("%s: %s: stopping the epoch\n", __func__, why); + ggml_free(ctx_compute_opt); + opt_alloc_failed.store(true); + opt_stop_requested.store(true); + return; + } if (callback) { callback(train, opt_ctx, dataset, result, idata_in_loop + (pos_ctx + pos_batch)/n_ubatch + 1, ndata_in_loop, t_loop_start); } From 35bd2662c890d082a615a5dc7b07cd9041b04795 Mon Sep 17 00:00:00 2001 From: Joel Teply Date: Tue, 6 Oct 2026 10:03:35 -0500 Subject: [PATCH 2/2] metal: nil lookups count against the graph compute that encoded them, never process-wide (Cormac on #42) A process-wide counter let a training graph's nil lookup fail a decode graph encoding beside it, which costs her a turn for a fault that was not hers. Each Metal context now owns its count: every encode block (the main thread's and the dispatched ones) points a thread-local sink at its context's counter while it encodes and clears it after, a nil lookup increments the current sink, and ggml_metal_graph_compute zeroes its own count on entry and fails only its own graph. Measured on the M5 (Qwen3.5-0.8B, Metal, 2 slots decoding): - serving only, fix in: 29 decode requests ok, 0 failed, 0 "buffer is nil" - #41's fix reverted, training beside decoding: the training job fails on its first graph ("found no buffer while encoding this graph"), the decode requests in flight with it all succeed (2 ok, 0 failed: a thin sample, one failing graph) - fix in, training beside decoding: training done, 216 decode requests ok, 0 failed, 0 "buffer is nil" Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo --- ggml/src/ggml-metal/ggml-metal-context.m | 12 +++++++++--- ggml/src/ggml-metal/ggml-metal-device.h | 5 +++-- ggml/src/ggml-metal/ggml-metal-device.m | 19 +++++++++++-------- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/ggml/src/ggml-metal/ggml-metal-context.m b/ggml/src/ggml-metal/ggml-metal-context.m index 13e181ebf814..41b6b776bdef 100644 --- a/ggml/src/ggml-metal/ggml-metal-context.m +++ b/ggml/src/ggml-metal/ggml-metal-context.m @@ -79,6 +79,10 @@ // error state - set when a command buffer fails during synchronize // once set, graph_compute will return GGML_STATUS_FAILED until the backend is recreated bool has_error; + + // lookups that found no buffer while THIS context encoded its current graph (counted by the + // encode blocks through ggml_metal_nil_sink_set; read once every block has finished) + uint64_t nil_lookups; }; ggml_metal_t ggml_metal_init(ggml_metal_device_t dev) { @@ -443,7 +447,7 @@ enum ggml_status ggml_metal_graph_compute(ggml_metal_t ctx, struct ggml_cgraph * // a lookup that finds no buffer during this graph's encode means some op ran on the wrong // memory: the graph is reported failed, never as a success computed on stale bytes - const uint64_t nil_lookups_0 = ggml_metal_nil_lookups(); + __atomic_store_n(&ctx->nil_lookups, 0, __ATOMIC_RELAXED); // number of nodes encoded by the main thread (empirically determined) const int n_main = MAX(64, 0.1*gf->n_nodes); @@ -615,9 +619,9 @@ enum ggml_status ggml_metal_graph_compute(ggml_metal_t ctx, struct ggml_cgraph * } } - if (ggml_metal_nil_lookups() != nil_lookups_0) { + if (__atomic_load_n(&ctx->nil_lookups, __ATOMIC_RELAXED) != 0) { GGML_LOG_ERROR("%s: %llu tensor lookup(s) found no buffer while encoding this graph: its result is not trusted\n", - __func__, (unsigned long long) (ggml_metal_nil_lookups() - nil_lookups_0)); + __func__, (unsigned long long) __atomic_load_n(&ctx->nil_lookups, __ATOMIC_RELAXED)); return GGML_STATUS_FAILED; } @@ -714,6 +718,7 @@ void ggml_metal_set_n_cb(ggml_metal_t ctx, int n_cb) { ctx->debug_graph, ctx->debug_fusion); + ggml_metal_nil_sink_set(&ctx->nil_lookups); // this block's lookups count against this graph for (int idx = 0; idx < ggml_metal_op_n_nodes(ctx_op); ++idx) { const int res = ggml_metal_op_encode(ctx_op, idx); if (res == 0) { @@ -722,6 +727,7 @@ void ggml_metal_set_n_cb(ggml_metal_t ctx, int n_cb) { idx += res - 1; } + ggml_metal_nil_sink_set(NULL); ggml_metal_op_free(ctx_op); diff --git a/ggml/src/ggml-metal/ggml-metal-device.h b/ggml/src/ggml-metal/ggml-metal-device.h index 8c80ffaf98a6..d4ed162acbbf 100644 --- a/ggml/src/ggml-metal/ggml-metal-device.h +++ b/ggml/src/ggml-metal/ggml-metal-device.h @@ -346,8 +346,9 @@ void ggml_metal_buffer_clear (ggml_metal_buffer_t buf, uint8_t value); // Metal buffer based on the host memory pointer // struct ggml_metal_buffer_id ggml_metal_buffer_get_id(ggml_metal_buffer_t buf, const struct ggml_tensor * t); -// lookups that found no buffer holding the tensor, process-wide (see ggml_metal_buffer_get_id) -uint64_t ggml_metal_nil_lookups(void); +// where this thread's lookups that find no buffer are counted (the graph compute it encodes +// for; NULL: not counted). See ggml_metal_buffer_get_id. +void ggml_metal_nil_sink_set(uint64_t * sink); // [MOE-GATHER #23] GPU virtual address of a host pointer inside this Metal buffer (MTLBuffer.gpuAddress // + intra-buffer delta), or 0 when the pointer is not covered / the OS lacks gpuAddress. Cross-buffer diff --git a/ggml/src/ggml-metal/ggml-metal-device.m b/ggml/src/ggml-metal/ggml-metal-device.m index 872a2b9f5ed8..c916ca8a1657 100644 --- a/ggml/src/ggml-metal/ggml-metal-device.m +++ b/ggml/src/ggml-metal/ggml-metal-device.m @@ -2378,14 +2378,15 @@ void ggml_metal_buffer_clear(ggml_metal_buffer_t buf, uint8_t value) { } } -// Every lookup that found no buffer holding a tensor's bytes, process-wide. Such a lookup hands -// the kernel a nil buffer: the op reads or writes nothing it was meant to, and the graph goes on -// with stale memory (Metal OUT_PROD read stale scratch in every LoRA backward that way, only a -// log line saying so). ggml_metal_graph_compute fails a graph during which this count rose. -static atomic_uint_fast64_t g_ggml_metal_nil_lookups = 0; +// Lookups that found no buffer holding a tensor's bytes are counted into the graph compute +// that is encoding on this thread (ggml_metal_nil_sink_set): such a lookup hands the kernel a +// nil buffer, the op reads or writes nothing it was meant to, and the graph goes on with stale +// memory (Metal OUT_PROD read stale scratch in every LoRA backward that way, one log line each). +// Per compute, never process-wide: a training graph's lookup must not fail a decode beside it. +static _Thread_local uint64_t * g_ggml_metal_nil_sink = NULL; -uint64_t ggml_metal_nil_lookups(void) { - return (uint64_t) atomic_load(&g_ggml_metal_nil_lookups); +void ggml_metal_nil_sink_set(uint64_t * sink) { + g_ggml_metal_nil_sink = sink; } struct ggml_metal_buffer_id ggml_metal_buffer_get_id(ggml_metal_buffer_t buf, const struct ggml_tensor * t) { @@ -2409,7 +2410,9 @@ struct ggml_metal_buffer_id ggml_metal_buffer_get_id(ggml_metal_buffer_t buf, co } GGML_LOG_ERROR("%s: error: tensor '%s' buffer is nil\n", __func__, t->name); - atomic_fetch_add(&g_ggml_metal_nil_lookups, 1); + if (g_ggml_metal_nil_sink != NULL) { + __atomic_fetch_add(g_ggml_metal_nil_sink, 1, __ATOMIC_RELAXED); + } return res; }