From 76870adcf837b659af83cdb85bdaa1ec24f5ac71 Mon Sep 17 00:00:00 2001 From: Aman Gupta Date: Mon, 27 Jul 2026 17:16:32 +0800 Subject: [PATCH 01/18] ggml: add a scheduler sanitizer --- ggml/src/CMakeLists.txt | 2 + ggml/src/ggml-backend-sanitize.cpp | 498 +++++++++++++++++++++++++++++ ggml/src/ggml-backend-sanitize.h | 36 +++ ggml/src/ggml-backend.cpp | 58 +++- 4 files changed, 590 insertions(+), 4 deletions(-) create mode 100644 ggml/src/ggml-backend-sanitize.cpp create mode 100644 ggml/src/ggml-backend-sanitize.h diff --git a/ggml/src/CMakeLists.txt b/ggml/src/CMakeLists.txt index 96535b49fa8..2d103c1e459 100644 --- a/ggml/src/CMakeLists.txt +++ b/ggml/src/CMakeLists.txt @@ -201,6 +201,8 @@ add_library(ggml-base ggml-alloc.c ggml-backend.cpp ggml-backend-meta.cpp + ggml-backend-sanitize.cpp + ggml-backend-sanitize.h ggml-opt.cpp ggml-threading.cpp ggml-threading.h diff --git a/ggml/src/ggml-backend-sanitize.cpp b/ggml/src/ggml-backend-sanitize.cpp new file mode 100644 index 00000000000..1cecb20ce49 --- /dev/null +++ b/ggml/src/ggml-backend-sanitize.cpp @@ -0,0 +1,498 @@ +#include "ggml-backend-sanitize.h" +#include "ggml-backend-impl.h" +#include "ggml-impl.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +int ggml_san_level(void) { + static const int level = []() { + const char * env = getenv("GGML_SCHED_SANITIZE"); + return env ? atoi(env) : 0; + }(); + return level; +} + +namespace { + +constexpr int SAN_MAX_ACTORS = 64; +constexpr size_t SAN_MAX_RANGES = 65536; + +using vclock = std::vector; + +struct san_access { + int actor = -1; + uint64_t clock = 0; + int split = -1; + const char * what = ""; // always a string literal + char tensor[GGML_MAX_NAME] = { 0 }; // copied: source tensor is recycled per graph +}; + +struct san_entry { + size_t end; + san_access last_write; + std::vector reads; // at most one per actor +}; + +struct san_state { + std::mutex mutex; + + std::unordered_map actors; + std::vector actor_names; + std::unordered_map name_counts; + std::vector vc; // vc[a][b] = a's knowledge of b's clock + + std::unordered_map> mem; + + std::unordered_map ev_vc; + std::unordered_map events; +}; + +san_state & state() { + static san_state * s = []() { + san_state * st = new san_state(); + st->actor_names.push_back("HOST"); + st->vc.emplace_back(SAN_MAX_ACTORS, 0); + return st; + }(); + return *s; +} + +// every helper below assumes the caller holds state().mutex + +int actor_id(san_state & s, ggml_backend_t backend) { + if (backend == nullptr) { + return 0; + } + + auto it = s.actors.find(backend); + if (it != s.actors.end()) { + return it->second; + } + + // two backends can share a device and therefore a name, disambiguate + std::string base = ggml_backend_name(backend); + const int idx = s.name_counts[base]++; + + const int id = (int) s.actor_names.size(); + GGML_ASSERT(id < SAN_MAX_ACTORS); + s.actor_names.push_back(idx == 0 ? base : base + "#" + std::to_string(idx)); + s.vc.emplace_back(SAN_MAX_ACTORS, 0); + s.actors[backend] = id; + return id; +} + +const char * actor_name(san_state & s, int id) { + return id >= 0 && id < (int) s.actor_names.size() ? s.actor_names[id].c_str() : "?"; +} + +void join(vclock & dst, const vclock & src) { + for (size_t i = 0; i < dst.size(); i++) { + dst[i] = std::max(dst[i], src[i]); + } +} + +uint64_t tick(san_state & s, int a) { + return ++s.vc[a][a]; +} + +uint64_t issue(san_state & s, int a) { + join(s.vc[a], s.vc[0]); + return tick(s, a); +} + +void maybe_flush(san_state & s) { + if (s.mem.empty()) { + return; + } + + // once the host knows every actor's current clock, the issue rule guarantees that any + // future operation joins vc[0] first, so the shadow state can no longer report a race + for (size_t b = 0; b < s.vc.size(); b++) { + if (s.vc[0][b] < s.vc[b][b]) { + return; + } + } + + s.mem.clear(); +} + +bool is_view_op(enum ggml_op op) { + return op == GGML_OP_VIEW || op == GGML_OP_RESHAPE || op == GGML_OP_PERMUTE || op == GGML_OP_TRANSPOSE; +} + +struct mem_range { + ggml_backend_buffer_t buf; + size_t off; + size_t len; +}; + +bool resolve(const ggml_tensor * t, size_t offset, size_t size, mem_range & out) { + if (t == nullptr || t->data == nullptr) { + return false; + } + + ggml_backend_buffer_t buf = t->view_src ? t->view_src->buffer : t->buffer; + if (buf == nullptr) { + return false; + } + + // weights are written once at load and never recycled by the allocator + if (ggml_backend_buffer_get_usage(buf) == GGML_BACKEND_BUFFER_USAGE_WEIGHTS) { + return false; + } + + void * base = ggml_backend_buffer_get_base(buf); + if (base == nullptr || size == 0) { + return false; + } + + out.buf = buf; + out.off = (size_t) ((const char *) t->data - (const char *) base) + offset; + out.len = size; + return true; +} + +thread_local int tl_split = -1; +thread_local int tl_split_actor = -1; + +void report(san_state & s, const mem_range & mr, const san_access & cur, const san_access & prev, const char * kind) { + fprintf(stderr, "\nggml-sched-sanitize: RACE (%s) on %s[%zu, %zu)\n", + kind, ggml_backend_buffer_name(mr.buf), mr.off, mr.off + mr.len); + fprintf(stderr, "ggml-sched-sanitize: %-5s %-12s @%-4llu split %-4d %s (%s)\n", + strcmp(kind, "write-after-read") == 0 ? "read" : "write", + actor_name(s, prev.actor), (unsigned long long) prev.clock, prev.split, prev.tensor, prev.what); + fprintf(stderr, "ggml-sched-sanitize: %-5s %-12s @%-4llu split %-4d %s (%s)\n", + strcmp(kind, "read-after-write") == 0 ? "read" : "write", + actor_name(s, cur.actor), (unsigned long long) cur.clock, cur.split, cur.tensor, cur.what); + fprintf(stderr, "ggml-sched-sanitize: no happens-before edge: %s knows %s@%llu, needs >=%llu\n\n", + actor_name(s, cur.actor), actor_name(s, prev.actor), + (unsigned long long) s.vc[cur.actor][prev.actor], (unsigned long long) prev.clock); + + GGML_ABORT("ggml-sched-sanitize: race detected"); +} + +// make sure a range boundary exists at pos, so [off,off+len) lands on whole entries +void split_at(std::map & m, size_t pos) { + auto it = m.upper_bound(pos); + if (it == m.begin()) { + return; + } + --it; + if (it->first < pos && it->second.end > pos) { + san_entry tail = it->second; + it->second.end = pos; + m.emplace(pos, std::move(tail)); + } +} + +void access_range(san_state & s, const mem_range & mr, bool write, const san_access & info) { + auto & m = s.mem[mr.buf]; + const int a = info.actor; + + // the shadow state is bounded by the work issued before the host observes every actor, + // not by run length. reaching this means maybe_flush stopped clearing it - + // most likely an operation was enqueued without going through issue(), which breaks + // the argument maybe_flush relies on. that is a sanitizer bug, not a memory limit. + GGML_ASSERT(m.size() <= SAN_MAX_RANGES && "shadow state is not being flushed - see issue()/maybe_flush()"); + + const size_t begin = mr.off; + const size_t end = mr.off + mr.len; + + split_at(m, begin); + split_at(m, end); + + // fill gaps so the whole span is covered by entries + size_t cur = begin; + while (cur < end) { + auto it = m.lower_bound(cur); + if (it == m.end() || it->first >= end) { + san_entry e; + e.end = end; + m.emplace(cur, std::move(e)); + break; + } + if (it->first > cur) { + san_entry e; + e.end = it->first; + m.emplace(cur, std::move(e)); + } + cur = it->second.end; + } + + for (auto it = m.lower_bound(begin); it != m.end() && it->first < end; ++it) { + san_entry & e = it->second; + + if (e.last_write.actor >= 0 && e.last_write.actor != a) { + if (e.last_write.clock > s.vc[a][e.last_write.actor]) { + report(s, mr, info, e.last_write, write ? "write-after-write" : "read-after-write"); + } + } + + if (write) { + for (const san_access & r : e.reads) { + if (r.actor != a && r.clock > s.vc[a][r.actor]) { + report(s, mr, info, r, "write-after-read"); + } + } + } + + if (write) { + e.last_write = info; + e.reads.clear(); + } else { + bool found = false; + for (san_access & r : e.reads) { + if (r.actor == a) { + r = info; + found = true; + break; + } + } + if (!found) { + e.reads.push_back(info); + } + } + } + +} + +// returns false if the tensor is not in tracked memory +bool touch(san_state & s, int a, uint64_t c, const ggml_tensor * t, + size_t offset, size_t size, bool write, const char * what) { + mem_range mr; + if (!resolve(t, offset, size, mr)) { + return false; + } + + san_access info; + info.actor = a; + info.clock = c; + info.what = what; + info.split = tl_split; + snprintf(info.tensor, sizeof(info.tensor), "%s", t->name); + + if (ggml_san_level() >= 2) { + GGML_LOG_DEBUG("san [split %3d %-10s] %-10s %-5s %s[%zu+%zu] %s (%s)\n", + tl_split, tl_split_actor >= 0 ? actor_name(s, tl_split_actor) : "-", actor_name(s, a), + write ? "write" : "read", + ggml_backend_buffer_name(mr.buf), mr.off, mr.len, t->name, what); + } + + access_range(s, mr, write, info); + return true; +} + +void trace(san_state & s, int a, const char * fmt, ...) { + if (ggml_san_level() < 2) { + return; + } + + char body[512]; + va_list ap; + va_start(ap, fmt); + vsnprintf(body, sizeof(body), fmt, ap); + va_end(ap); + + GGML_LOG_DEBUG("san [split %3d %-10s] %-10s %s", + tl_split, tl_split_actor >= 0 ? actor_name(s, tl_split_actor) : "-", actor_name(s, a), body); +} + +} // namespace + +void ggml_san_sync(ggml_backend_t backend) { + if (ggml_san_level() == 0) { + return; + } + san_state & s = state(); + std::lock_guard lk(s.mutex); + + const int a = actor_id(s, backend); + join(s.vc[0], s.vc[a]); // host now knows everything this backend knew + trace(s, a, "EDGE synchronize -> HOST\n"); + maybe_flush(s); +} + +void ggml_san_event_record(ggml_backend_event_t event, ggml_backend_t backend) { + if (ggml_san_level() == 0) { + return; + } + san_state & s = state(); + std::lock_guard lk(s.mutex); + + const int a = actor_id(s, backend); + s.ev_vc[event] = s.vc[a]; + trace(s, a, "EDGE event_record ev=%d\n", (int) s.events.emplace(event, (int) s.events.size()).first->second); +} + +void ggml_san_event_wait(ggml_backend_t backend, ggml_backend_event_t event) { + if (ggml_san_level() == 0) { + return; + } + san_state & s = state(); + std::lock_guard lk(s.mutex); + + const int a = actor_id(s, backend); + auto it = s.ev_vc.find(event); + if (it != s.ev_vc.end()) { + join(s.vc[a], it->second); + } + trace(s, a, "EDGE event_wait ev=%d\n", (int) s.events.emplace(event, (int) s.events.size()).first->second); +} + +void ggml_san_event_sync(ggml_backend_event_t event) { + if (ggml_san_level() == 0) { + return; + } + san_state & s = state(); + std::lock_guard lk(s.mutex); + + auto it = s.ev_vc.find(event); + if (it != s.ev_vc.end()) { + join(s.vc[0], it->second); + } + trace(s, 0, "EDGE event_sync ev=%d\n", (int) s.events.emplace(event, (int) s.events.size()).first->second); + maybe_flush(s); +} + +void ggml_san_compute(ggml_backend_t backend, const ggml_cgraph * cgraph) { + if (ggml_san_level() == 0 || cgraph == nullptr) { + return; + } + san_state & s = state(); + std::lock_guard lk(s.mutex); + + const int a = actor_id(s, backend); + + // a backend with no synchronize runs inline on the calling thread, so it both + // inherits and publishes the host's knowledge - without this the CPU backend's + // writes are never ordered against anything and no race can be detected + const bool synchronous = backend->iface.synchronize == nullptr; + + const uint64_t c = issue(s, a); + + size_t n_read = 0; + size_t n_write = 0; + for (int i = 0; i < cgraph->n_nodes; i++) { + ggml_tensor * node = cgraph->nodes[i]; + if (is_view_op(node->op)) { + continue; + } + for (int j = 0; j < GGML_MAX_SRC; j++) { + if (node->op == GGML_OP_CPY && j == 1) { + continue; + } + if (node->src[j] && touch(s, a, c, node->src[j], 0, ggml_nbytes(node->src[j]), false, "compute")) { + n_read++; + } + } + if (touch(s, a, c, node, 0, ggml_nbytes(node), true, "compute")) { + n_write++; + } + } + + if (synchronous) { + join(s.vc[0], s.vc[a]); + maybe_flush(s); + } + + trace(s, a, "compute %d nodes, r=%zu w=%zu ranges\n", cgraph->n_nodes, n_read, n_write); +} + +void ggml_san_access(ggml_backend_t backend, const ggml_tensor * tensor, + size_t offset, size_t size, bool write, const char * what) { + if (ggml_san_level() == 0) { + return; + } + san_state & s = state(); + std::lock_guard lk(s.mutex); + + const int a = actor_id(s, backend); + const uint64_t c = issue(s, a); + touch(s, a, c, tensor, offset, size, write, what); + + // a synchronous access has completed by the time the call returns + if (backend == nullptr || backend->iface.synchronize == nullptr) { + join(s.vc[0], s.vc[a]); + maybe_flush(s); + } +} + +void ggml_san_cpy_async(ggml_backend_t src_be, ggml_backend_t dst_be, + const ggml_tensor * src, const ggml_tensor * dst) { + if (ggml_san_level() == 0) { + return; + } + san_state & s = state(); + std::lock_guard lk(s.mutex); + + const int a_src = actor_id(s, src_be); + const int a_dst = actor_id(s, dst_be); + + // TODO: clarify in ggml + // which stream actually performs the read of src is backend specific. host memory has + // no queue of its own, so a host->device transfer runs on dst's queue - that is what + // makes the source racy against a later host write. a device->device transfer is + // issued on the src stream (see "copy on src stream" in ggml-cuda), where src's own + // later work is already ordered against it. + ggml_backend_buffer_t src_buf = src->view_src ? src->view_src->buffer : src->buffer; + const int read_actor = (src_buf && ggml_backend_buffer_is_host(src_buf)) ? a_dst : a_src; + + uint64_t read_clock; + uint64_t write_clock; + + if (read_actor == a_dst) { + // the dst queue performs both sides of the copy after src's pending work + join(s.vc[a_dst], s.vc[a_src]); + read_clock = write_clock = issue(s, a_dst); + } else { + // issue the src read first, then carry its completion into the dst write + read_clock = issue(s, read_actor); + join(s.vc[a_dst], s.vc[read_actor]); + write_clock = issue(s, a_dst); + } + + touch(s, read_actor, read_clock, src, 0, ggml_nbytes(src), false, "cpy_async src"); + touch(s, a_dst, write_clock, dst, 0, ggml_nbytes(dst), true, "cpy_async dst"); + + trace(s, a_dst, "cpy_async %s@%s -> %s\n", src->name, actor_name(s, read_actor), dst->name); +} + +void ggml_san_buffer_free(ggml_backend_buffer_t buffer) { + if (ggml_san_level() == 0) { + return; + } + san_state & s = state(); + std::lock_guard lk(s.mutex); + + s.mem.erase(buffer); +} + +void ggml_san_split(int split_id, ggml_backend_t backend, int n_inputs) { + if (ggml_san_level() == 0) { + return; + } + tl_split = split_id; + + if (split_id < 0) { + tl_split_actor = -1; + return; + } + + san_state & s = state(); + std::lock_guard lk(s.mutex); + const int a = actor_id(s, backend); + tl_split_actor = a; + + if (n_inputs == 0) { + trace(s, a, "split 0 inputs (no sync path)\n"); + } +} diff --git a/ggml/src/ggml-backend-sanitize.h b/ggml/src/ggml-backend-sanitize.h new file mode 100644 index 00000000000..9009f50e649 --- /dev/null +++ b/ggml/src/ggml-backend-sanitize.h @@ -0,0 +1,36 @@ +#pragma once + +// happens-before instrumentation for the ggml backend API +// +// GGML_SCHED_SANITIZE=1 detect happens-before violations +// GGML_SCHED_SANITIZE=2 also trace synchronization edges and memory ranges + +#include "ggml-backend.h" + +#ifdef __cplusplus +extern "C" { +#endif + + int ggml_san_level(void); + + void ggml_san_sync (ggml_backend_t backend); + void ggml_san_event_record(ggml_backend_event_t event, ggml_backend_t backend); + void ggml_san_event_wait (ggml_backend_t backend, ggml_backend_event_t event); + void ggml_san_event_sync (ggml_backend_event_t event); + + void ggml_san_compute (ggml_backend_t backend, const struct ggml_cgraph * cgraph); + + // backend == NULL means the access is performed by the host thread + void ggml_san_access (ggml_backend_t backend, const struct ggml_tensor * tensor, + size_t offset, size_t size, bool write, const char * what); + + void ggml_san_cpy_async (ggml_backend_t src_be, ggml_backend_t dst_be, + const struct ggml_tensor * src, const struct ggml_tensor * dst); + + void ggml_san_buffer_free (ggml_backend_buffer_t buffer); + + void ggml_san_split (int split_id, ggml_backend_t backend, int n_inputs); + +#ifdef __cplusplus +} +#endif diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index 3d6310f3ffe..ab7dc643abe 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -10,6 +10,7 @@ #include "ggml-backend.h" #include "ggml-backend-impl.h" +#include "ggml-backend-sanitize.h" #include "ggml-alloc.h" #include "ggml-impl.h" @@ -109,6 +110,8 @@ void ggml_backend_buffer_free(ggml_backend_buffer_t buffer) { return; } + ggml_san_buffer_free(buffer); + if (buffer->iface.free_buffer != NULL) { buffer->iface.free_buffer(buffer); } @@ -205,7 +208,12 @@ void ggml_backend_buffer_reset(ggml_backend_buffer_t buffer) { bool ggml_backend_buffer_copy_tensor(const struct ggml_tensor * src, struct ggml_tensor * dst) { ggml_backend_buffer_t dst_buf = dst->view_src ? dst->view_src->buffer : dst->buffer; if (dst_buf->iface.cpy_tensor) { - return dst_buf->iface.cpy_tensor(dst_buf, src, dst); + const bool copied = dst_buf->iface.cpy_tensor(dst_buf, src, dst); + if (copied) { + ggml_san_access(NULL, src, 0, ggml_nbytes(src), false, "buffer_copy_tensor src"); + ggml_san_access(NULL, dst, 0, ggml_nbytes(dst), true, "buffer_copy_tensor dst"); + } + return copied; } return false; } @@ -261,6 +269,7 @@ void ggml_backend_tensor_set_async(ggml_backend_t backend, struct ggml_tensor * ggml_backend_synchronize(backend); ggml_backend_tensor_set(tensor, data, offset, size); } else { + ggml_san_access(backend, tensor, offset, size, true, "set_async"); backend->iface.set_tensor_async(backend, tensor, data, offset, size); } } @@ -275,6 +284,7 @@ void ggml_backend_tensor_get_async(ggml_backend_t backend, const struct ggml_ten ggml_backend_synchronize(backend); ggml_backend_tensor_get(tensor, data, offset, size); } else { + ggml_san_access(backend, tensor, offset, size, false, "get_async"); backend->iface.get_tensor_async(backend, tensor, data, offset, size); } } @@ -297,6 +307,9 @@ void ggml_backend_tensor_set_2d_async(ggml_backend_t backend, struct ggml_tensor GGML_ASSERT(tensor->data != NULL && "tensor not allocated"); GGML_ASSERT(offset + (n_copies-1)*stride_tensor + size <= ggml_nbytes(tensor) && "tensor write out of bounds"); + for (size_t i = 0; i < n_copies; i++) { + ggml_san_access(backend, tensor, offset + i*stride_tensor, size, true, "set_2d_async"); + } backend->iface.set_tensor_2d_async(backend, tensor, data, offset, size, n_copies, stride_tensor, stride_data); } @@ -318,6 +331,9 @@ void ggml_backend_tensor_get_2d_async(ggml_backend_t backend, const struct ggml_ GGML_ASSERT(tensor->data != NULL && "tensor not allocated"); GGML_ASSERT(offset + (n_copies-1)*stride_tensor + size <= ggml_nbytes(tensor) && "tensor read out of bounds"); + for (size_t i = 0; i < n_copies; i++) { + ggml_san_access(backend, tensor, offset + i*stride_tensor, size, false, "get_2d_async"); + } backend->iface.get_tensor_2d_async(backend, tensor, data, offset, size, n_copies, stride_tensor, stride_data); } @@ -333,6 +349,7 @@ void ggml_backend_tensor_set(struct ggml_tensor * tensor, const void * data, siz GGML_ASSERT(tensor->data != NULL && "tensor not allocated"); GGML_ASSERT(offset + size <= ggml_nbytes(tensor) && "tensor write out of bounds"); + ggml_san_access(NULL, tensor, offset, size, true, "tensor_set"); buf->iface.set_tensor(buf, tensor, data, offset, size); } @@ -348,6 +365,7 @@ void ggml_backend_tensor_get(const struct ggml_tensor * tensor, void * data, siz GGML_ASSERT(tensor->data != NULL && "tensor not allocated"); GGML_ASSERT(offset + size <= ggml_nbytes(tensor) && "tensor read out of bounds"); + ggml_san_access(NULL, tensor, offset, size, false, "tensor_get"); buf->iface.get_tensor(buf, tensor, data, offset, size); } @@ -370,6 +388,9 @@ void ggml_backend_tensor_set_2d(struct ggml_tensor * tensor, const void * data, GGML_ASSERT(tensor->data != NULL && "tensor not allocated"); GGML_ASSERT(offset + (n_copies-1)*stride_tensor + size <= ggml_nbytes(tensor) && "tensor write out of bounds"); + for (size_t i = 0; i < n_copies; i++) { + ggml_san_access(NULL, tensor, offset + i*stride_tensor, size, true, "tensor_set_2d"); + } buf->iface.set_tensor_2d(buf, tensor, data, offset, size, n_copies, stride_tensor, stride_data); } @@ -392,6 +413,9 @@ void ggml_backend_tensor_get_2d(const struct ggml_tensor * tensor, void * data, GGML_ASSERT(tensor->data != NULL && "tensor not allocated"); GGML_ASSERT(offset + (n_copies-1)*stride_tensor + size <= ggml_nbytes(tensor) && "tensor read out of bounds"); + for (size_t i = 0; i < n_copies; i++) { + ggml_san_access(NULL, tensor, offset + i*stride_tensor, size, false, "tensor_get_2d"); + } buf->iface.get_tensor_2d(buf, tensor, data, offset, size, n_copies, stride_tensor, stride_data); } @@ -408,6 +432,7 @@ void ggml_backend_tensor_memset(struct ggml_tensor * tensor, uint8_t value, size GGML_ASSERT(offset + size <= ggml_nbytes(tensor) && "tensor write out of bounds"); GGML_ASSERT(buf->iface.memset_tensor != NULL && "memset not implemented by backend buffer"); + ggml_san_access(NULL, tensor, offset, size, true, "tensor_memset"); buf->iface.memset_tensor(buf, tensor, value, offset, size); } @@ -418,6 +443,7 @@ void ggml_backend_synchronize(ggml_backend_t backend) { } backend->iface.synchronize(backend); + ggml_san_sync(backend); } ggml_backend_graph_plan_t ggml_backend_graph_plan_create(ggml_backend_t backend, struct ggml_cgraph * cgraph) { @@ -449,7 +475,11 @@ enum ggml_status ggml_backend_graph_compute(ggml_backend_t backend, struct ggml_ enum ggml_status ggml_backend_graph_compute_async(ggml_backend_t backend, struct ggml_cgraph * cgraph) { GGML_ASSERT(backend); - return backend->iface.graph_compute(backend, cgraph); + const enum ggml_status status = backend->iface.graph_compute(backend, cgraph); + if (status == GGML_STATUS_SUCCESS) { + ggml_san_compute(backend, cgraph); + } + return status; } bool ggml_backend_supports_op(ggml_backend_t backend, const struct ggml_tensor * op) { @@ -482,9 +512,11 @@ void ggml_backend_tensor_copy(const struct ggml_tensor * src, struct ggml_tensor } if (ggml_backend_buffer_is_host(src->buffer)) { + ggml_san_access(NULL, src, 0, ggml_nbytes(src), false, "tensor_copy src"); ggml_backend_tensor_set(dst, src->data, 0, ggml_nbytes(src)); } else if (ggml_backend_buffer_is_host(dst->buffer)) { ggml_backend_tensor_get(src, dst->data, 0, ggml_nbytes(src)); + ggml_san_access(NULL, dst, 0, ggml_nbytes(src), true, "tensor_copy dst"); } else if (!ggml_backend_buffer_copy_tensor(src, dst)) { #ifndef NDEBUG GGML_LOG_DEBUG("%s: warning: slow copy from %s to %s\n", __func__, ggml_backend_buffer_name(src->buffer), ggml_backend_buffer_name(dst->buffer)); @@ -506,7 +538,9 @@ void ggml_backend_tensor_copy_async(ggml_backend_t backend_src, ggml_backend_t b GGML_ASSERT(backend_dst); if (backend_dst->iface.cpy_tensor_async != NULL) { - if (backend_dst->iface.cpy_tensor_async(backend_src, backend_dst, src, dst)) { + const bool accepted = backend_dst->iface.cpy_tensor_async(backend_src, backend_dst, src, dst); + if (accepted) { + ggml_san_cpy_async(backend_src, backend_dst, src, dst); return; } } @@ -540,6 +574,7 @@ void ggml_backend_event_record(ggml_backend_event_t event, ggml_backend_t backen GGML_ASSERT(backend->iface.event_record != NULL); backend->iface.event_record(backend, event); + ggml_san_event_record(event, backend); } void ggml_backend_event_synchronize(ggml_backend_event_t event) { @@ -547,6 +582,7 @@ void ggml_backend_event_synchronize(ggml_backend_event_t event) { GGML_ASSERT(event->device->iface.event_synchronize); event->device->iface.event_synchronize(event->device, event); + ggml_san_event_sync(event); } void ggml_backend_event_wait(ggml_backend_t backend, ggml_backend_event_t event) { @@ -554,6 +590,7 @@ void ggml_backend_event_wait(ggml_backend_t backend, ggml_backend_event_t event) GGML_ASSERT(backend->iface.event_wait != NULL); backend->iface.event_wait(backend, event); + ggml_san_event_wait(backend, event); } static void ggml_backend_graph_optimize(ggml_backend_t backend, struct ggml_cgraph * cgraph) { @@ -1616,6 +1653,8 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s } } + ggml_san_split(split_id, split_backend, split->n_inputs); + // copy the input tensors to the split backend for (int input_id = 0; input_id < split->n_inputs; input_id++) { ggml_backend_t input_backend = ggml_backend_sched_get_tensor_backend(sched, split->inputs[input_id]); @@ -1726,7 +1765,14 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s } else { // try async copy, but if not possible, we can still use a sync copy without synchronizing the dst backend, since we handle the synchronization here with multiple copies and events // TODO: add public function to facilitate this, since applications do not have direct access to the backend interface - if (!split_backend->iface.cpy_tensor_async || !split_backend->iface.cpy_tensor_async(input_backend, split_backend, input, input_cpy)) { + bool cpy_async_ok = false; + if (split_backend->iface.cpy_tensor_async) { + cpy_async_ok = split_backend->iface.cpy_tensor_async(input_backend, split_backend, input, input_cpy); + if (cpy_async_ok) { + ggml_san_cpy_async(input_backend, split_backend, input, input_cpy); + } + } + if (!cpy_async_ok) { ggml_backend_synchronize(input_backend); if (sched->events[split_backend_id][sched->cur_copy] != NULL) { ggml_backend_event_synchronize(sched->events[split_backend_id][sched->cur_copy]); @@ -1742,6 +1788,7 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s if (!sched->callback_eval) { enum ggml_status ec = ggml_backend_graph_compute_async(split_backend, &split->graph); if (ec != GGML_STATUS_SUCCESS) { + ggml_san_split(-1, NULL, 0); return ec; } } else { @@ -1764,6 +1811,7 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s enum ggml_status ec = ggml_backend_graph_compute_async(split_backend, &gv); if (ec != GGML_STATUS_SUCCESS) { + ggml_san_split(-1, NULL, 0); return ec; } @@ -1786,6 +1834,8 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s prev_backend_id = split_backend_id; } + ggml_san_split(-1, NULL, 0); + return GGML_STATUS_SUCCESS; } From 22994a6f0874e91b8803c440b0e18f4a85447d1d Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Mon, 17 Aug 2026 17:10:38 +0200 Subject: [PATCH 02/18] ggml: add a non-fatal mode to the scheduler sanitizer The sanitizer aborts on the first race, so enumerating the races in a workload takes one run per race. Add GGML_SCHED_SANITIZE_NONFATAL=1 to report each one and keep going, with a count printed at exit. This is safe because access_range() updates the shadow state after report(), so a range that has been reported does not keep re-firing. Assisted-By: Claude Opus 5 --- ggml/src/ggml-backend-sanitize.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/ggml/src/ggml-backend-sanitize.cpp b/ggml/src/ggml-backend-sanitize.cpp index 1cecb20ce49..624ef6c6c87 100644 --- a/ggml/src/ggml-backend-sanitize.cpp +++ b/ggml/src/ggml-backend-sanitize.cpp @@ -21,6 +21,17 @@ int ggml_san_level(void) { return level; } +// GGML_SCHED_SANITIZE_NONFATAL=1 reports every distinct race and keeps going, instead of +// aborting on the first one. the shadow state is updated after a report, so a reported +// range does not keep firing - this lets a single run enumerate all races in a workload. +static int ggml_san_nonfatal(void) { + static const int nonfatal = []() { + const char * env = getenv("GGML_SCHED_SANITIZE_NONFATAL"); + return env ? atoi(env) : 0; + }(); + return nonfatal; +} + namespace { constexpr int SAN_MAX_ACTORS = 64; @@ -54,6 +65,8 @@ struct san_state { std::unordered_map ev_vc; std::unordered_map events; + + size_t n_races = 0; // only counted in nonfatal mode }; san_state & state() { @@ -61,6 +74,12 @@ san_state & state() { san_state * st = new san_state(); st->actor_names.push_back("HOST"); st->vc.emplace_back(SAN_MAX_ACTORS, 0); + if (ggml_san_nonfatal()) { + static san_state * at_exit = st; + atexit([]() { + fprintf(stderr, "\nggml-sched-sanitize: %zu race(s) reported\n", at_exit->n_races); + }); + } return st; }(); return *s; @@ -177,6 +196,11 @@ void report(san_state & s, const mem_range & mr, const san_access & cur, const s actor_name(s, cur.actor), actor_name(s, prev.actor), (unsigned long long) s.vc[cur.actor][prev.actor], (unsigned long long) prev.clock); + if (ggml_san_nonfatal()) { + s.n_races++; + return; + } + GGML_ABORT("ggml-sched-sanitize: race detected"); } From 0051b4501ac9ba0c8b0fa1a0496166c9eff68e25 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Mon, 17 Aug 2026 17:10:48 +0200 Subject: [PATCH 03/18] ggml,llama: let the sanitizer see direct writes to host tensors llama.cpp writes several graph inputs straight through tensor->data after checking the buffer is host-visible, bypassing ggml_backend_tensor_set. The scheduler sanitizer only hooks the backend API, so it could not see those writes and silently missed every race against them. Add ggml_backend_tensor_set_direct(), a no-op unless GGML_SCHED_SANITIZE is set, to announce such a write. Wrap it in llama_host_write(), which also carries the is_host assertion, and use it in place of the bare assertion at all 22 direct-write sites. Folding the announcement into the idiom that already marked these sites keeps the two from drifting apart. Measured on a multi-ubatch prefill on an integrated GPU (gfx1151): the run reports 87 races, 69 of which were previously invisible. 40 are on attn_inp_kq_mask alone, which was entirely unseen before. Assisted-By: Claude Opus 5 --- ggml/include/ggml-backend.h | 6 ++++++ ggml/src/ggml-backend.cpp | 12 ++++++++++++ src/llama-graph.cpp | 24 ++++++++++++------------ src/llama-impl.h | 9 +++++++++ src/llama-kv-cache-msa.cpp | 6 +++--- src/llama-kv-cache.cpp | 14 +++++++------- 6 files changed, 49 insertions(+), 22 deletions(-) diff --git a/ggml/include/ggml-backend.h b/ggml/include/ggml-backend.h index cc3f8cd36e3..0e4d1d9b17c 100644 --- a/ggml/include/ggml-backend.h +++ b/ggml/include/ggml-backend.h @@ -95,6 +95,12 @@ extern "C" { GGML_API void ggml_backend_tensor_get_2d(const struct ggml_tensor * tensor, void * data, size_t offset, size_t size, size_t n_copies, size_t stride_tensor, size_t stride_data); GGML_API void ggml_backend_tensor_memset( struct ggml_tensor * tensor, uint8_t value, size_t offset, size_t size); + // declare an imminent direct write to tensor->data, bypassing ggml_backend_tensor_set. + // callers that write host-visible tensors in place must announce it here, otherwise the + // scheduler sanitizer cannot see the write and will not detect races against it. + // no-op unless GGML_SCHED_SANITIZE is set. + GGML_API void ggml_backend_tensor_set_direct(struct ggml_tensor * tensor, size_t offset, size_t size); + GGML_API void ggml_backend_synchronize(ggml_backend_t backend); GGML_API ggml_backend_graph_plan_t ggml_backend_graph_plan_create(ggml_backend_t backend, struct ggml_cgraph * cgraph); diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index ab7dc643abe..e68b0da5fff 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -436,6 +436,18 @@ void ggml_backend_tensor_memset(struct ggml_tensor * tensor, uint8_t value, size buf->iface.memset_tensor(buf, tensor, value, offset, size); } +void ggml_backend_tensor_set_direct(struct ggml_tensor * tensor, size_t offset, size_t size) { + if (ggml_san_level() == 0) { + return; + } + + GGML_ASSERT(tensor); + GGML_ASSERT(tensor->data != NULL && "tensor not allocated"); + GGML_ASSERT(offset + size <= ggml_nbytes(tensor) && "tensor write out of bounds"); + + ggml_san_access(NULL, tensor, offset, size, true, "set_direct"); +} + void ggml_backend_synchronize(ggml_backend_t backend) { GGML_ASSERT(backend); if (backend->iface.synchronize == NULL) { diff --git a/src/llama-graph.cpp b/src/llama-graph.cpp index 5212e19a256..1040346e758 100644 --- a/src/llama-graph.cpp +++ b/src/llama-graph.cpp @@ -176,7 +176,7 @@ void llm_graph_input_pos_bucket::set_input(const llama_ubatch * ubatch) { if (pos_bucket) { const int64_t n_tokens = ubatch->n_tokens; - GGML_ASSERT(ggml_backend_buffer_is_host(pos_bucket->buffer)); + llama_host_write(pos_bucket); GGML_ASSERT(!ubatch->equal_seqs()); // TODO: use ubatch->n_seqs instead of failing int32_t * data = (int32_t *) pos_bucket->data; @@ -200,7 +200,7 @@ void llm_graph_input_out_ids::set_input(const llama_ubatch * ubatch) { const int64_t n_tokens = ubatch->n_tokens; - GGML_ASSERT(ggml_backend_buffer_is_host(out_ids->buffer)); + llama_host_write(out_ids); int32_t * data = (int32_t *) out_ids->data; if (n_outputs == n_tokens) { @@ -240,7 +240,7 @@ void llm_graph_input_mean::set_input(const llama_ubatch * ubatch) { const int64_t n_seqs_unq = ubatch->n_seqs_unq; GGML_ASSERT(mean); - GGML_ASSERT(ggml_backend_buffer_is_host(mean->buffer)); + llama_host_write(mean); float * data = (float *) mean->data; memset(mean->data, 0, n_tokens*n_seqs_unq*ggml_element_size(mean)); @@ -286,7 +286,7 @@ void llm_graph_input_cls::set_input(const llama_ubatch * ubatch) { cparams.pooling_type == LLAMA_POOLING_TYPE_LAST )) { GGML_ASSERT(cls); - GGML_ASSERT(ggml_backend_buffer_is_host(cls->buffer)); + llama_host_write(cls); uint32_t * data = (uint32_t *) cls->data; memset(cls->data, 0, n_seqs_unq*ggml_element_size(cls)); @@ -331,7 +331,7 @@ void llm_graph_input_rs::set_input(const llama_ubatch * ubatch) { const int64_t n_rs = mctx->get_n_rs(); if (s_copy) { - GGML_ASSERT(ggml_backend_buffer_is_host(s_copy->buffer)); + llama_host_write(s_copy); int32_t * data = (int32_t *) s_copy->data; // assuming copy destinations ALWAYS happen ONLY on the cells between head and head+n @@ -448,7 +448,7 @@ void llm_graph_input_attn_no_cache::set_input(const llama_ubatch * ubatch) { }; GGML_ASSERT(self_kq_mask); - GGML_ASSERT(ggml_backend_buffer_is_host(self_kq_mask->buffer)); + llama_host_write(self_kq_mask); if (self_kq_mask->type == GGML_TYPE_F16) { fill_mask((ggml_fp16_t *) self_kq_mask->data, ggml_nelements(self_kq_mask), 0, LLAMA_SWA_TYPE_NONE); } else { @@ -457,7 +457,7 @@ void llm_graph_input_attn_no_cache::set_input(const llama_ubatch * ubatch) { if (hparams.swa_type != LLAMA_SWA_TYPE_NONE) { GGML_ASSERT(self_kq_mask_swa); - GGML_ASSERT(ggml_backend_buffer_is_host(self_kq_mask_swa->buffer)); + llama_host_write(self_kq_mask_swa); if (self_kq_mask_swa->type == GGML_TYPE_F16) { fill_mask((ggml_fp16_t *) self_kq_mask_swa->data, ggml_nelements(self_kq_mask_swa), hparams.n_swa, hparams.swa_type); } else { @@ -747,7 +747,7 @@ static void dsv4_set_kq_mask( GGML_ASSERT(dst->ne[2] == 1); GGML_ASSERT(dst->ne[3] == n_stream); GGML_ASSERT((int64_t) plan.n_visible.size() == (int64_t) n_tokens); - GGML_ASSERT(ggml_backend_buffer_is_host(dst->buffer)); + llama_host_write(dst); if (dst->type == GGML_TYPE_F32) { float * data = (float *) dst->data; @@ -1029,7 +1029,7 @@ void llm_graph_input_attn_cross::set_input(const llama_ubatch * ubatch) { const int64_t n_enc = cross_kq_mask->ne[0]; const int64_t n_tokens = ubatch->n_tokens; - GGML_ASSERT(ggml_backend_buffer_is_host(cross_kq_mask->buffer)); + llama_host_write(cross_kq_mask); GGML_ASSERT(!ubatch->equal_seqs()); // TODO: use ubatch->n_seqs instead of failing const auto fill_mask = [&](auto * data) { @@ -1076,7 +1076,7 @@ void llm_graph_input_mem_hybrid::set_input(const llama_ubatch * ubatch) { const int64_t n_rs = mctx->get_recr()->get_n_rs(); if (inp_rs->s_copy) { - GGML_ASSERT(ggml_backend_buffer_is_host(inp_rs->s_copy->buffer)); + llama_host_write(inp_rs->s_copy); int32_t * data = (int32_t *) inp_rs->s_copy->data; // assuming copy destinations ALWAYS happen ONLY on the cells between head and head+n @@ -1120,7 +1120,7 @@ void llm_graph_input_mem_hybrid_k::set_input(const llama_ubatch * ubatch) { const int64_t n_rs = mctx->get_recr()->get_n_rs(); if (inp_rs->s_copy) { - GGML_ASSERT(ggml_backend_buffer_is_host(inp_rs->s_copy->buffer)); + llama_host_write(inp_rs->s_copy); int32_t * data = (int32_t *) inp_rs->s_copy->data; // assuming copy destinations ALWAYS happen ONLY on the cells between head and head+n @@ -1194,7 +1194,7 @@ void llm_graph_input_mem_hybrid_iswa::set_input(const llama_ubatch * ubatch) { const int64_t n_rs = mctx->get_recr()->get_n_rs(); if (inp_rs->s_copy) { - GGML_ASSERT(ggml_backend_buffer_is_host(inp_rs->s_copy->buffer)); + llama_host_write(inp_rs->s_copy); int32_t * data = (int32_t *) inp_rs->s_copy->data; // assuming copy destinations ALWAYS happen ONLY on the cells between head and head+n diff --git a/src/llama-impl.h b/src/llama-impl.h index 4988b06d2ca..7202d8d9360 100644 --- a/src/llama-impl.h +++ b/src/llama-impl.h @@ -1,6 +1,7 @@ #pragma once #include "ggml.h" // for ggml_log_level +#include "ggml-backend.h" #include #include @@ -93,6 +94,14 @@ struct buffer_view { } }; +// announce that the whole of `t` is about to be written in place via t->data, bypassing +// ggml_backend_tensor_set. every direct write to a graph input must go through this, or the +// scheduler sanitizer cannot see the write and will silently miss races against it. +static inline void llama_host_write(struct ggml_tensor * t) { + GGML_ASSERT(ggml_backend_buffer_is_host(t->buffer)); + ggml_backend_tensor_set_direct(t, 0, ggml_nbytes(t)); +} + void replace_all(std::string & s, const std::string & search, const std::string & replace); // TODO: rename to llama_format ? diff --git a/src/llama-kv-cache-msa.cpp b/src/llama-kv-cache-msa.cpp index 55ef286cafa..24b25917efd 100644 --- a/src/llama-kv-cache-msa.cpp +++ b/src/llama-kv-cache-msa.cpp @@ -273,7 +273,7 @@ uint32_t llama_kv_cache_msa_context::get_n_pos() const { } void llama_kv_cache_msa_context::set_input_cell_pos(ggml_tensor * dst, const llama_ubatch * ubatch, int32_t div) const { - GGML_ASSERT(ggml_backend_buffer_is_host(dst->buffer)); + llama_host_write(dst); GGML_ASSERT(dst->type == GGML_TYPE_I32); GGML_ASSERT(div > 0); @@ -302,7 +302,7 @@ void llama_kv_cache_msa_context::set_input_cell_pos(ggml_tensor * dst, const lla } void llama_kv_cache_msa_context::set_input_pos_slot(ggml_tensor * dst, const llama_ubatch * ubatch) const { - GGML_ASSERT(ggml_backend_buffer_is_host(dst->buffer)); + llama_host_write(dst); GGML_ASSERT(dst->type == GGML_TYPE_I32 || dst->type == GGML_TYPE_F32); const int64_t n_tokens = ubatch->n_tokens; @@ -346,7 +346,7 @@ void llama_kv_cache_msa_context::set_input_pos_slot(ggml_tensor * dst, const lla } void llama_kv_cache_msa_context::set_input_pos_mask(ggml_tensor * dst, const llama_ubatch * ubatch) const { - GGML_ASSERT(ggml_backend_buffer_is_host(dst->buffer)); + llama_host_write(dst); GGML_ASSERT(dst->type == GGML_TYPE_F32); const int64_t n_tokens = ubatch->n_tokens; diff --git a/src/llama-kv-cache.cpp b/src/llama-kv-cache.cpp index 5382cd7266f..d33a2a16109 100644 --- a/src/llama-kv-cache.cpp +++ b/src/llama-kv-cache.cpp @@ -1460,7 +1460,7 @@ void llama_kv_cache::set_input_k_idxs(ggml_tensor * dst, const llama_ubatch * ub const uint32_t n_tokens = ubatch->n_tokens; GGML_ASSERT(n_tokens == (int64_t) sinfo.size()*sinfo.n_stream()); - GGML_ASSERT(ggml_backend_buffer_is_host(dst->buffer)); + llama_host_write(dst); int64_t * data = (int64_t *) dst->data; for (uint32_t s = 0; s < sinfo.n_stream(); ++s) { @@ -1476,7 +1476,7 @@ void llama_kv_cache::set_input_v_idxs(ggml_tensor * dst, const llama_ubatch * ub const uint32_t n_tokens = ubatch->n_tokens; GGML_ASSERT(n_tokens == (int64_t) sinfo.size()*sinfo.n_stream()); - GGML_ASSERT(ggml_backend_buffer_is_host(dst->buffer)); + llama_host_write(dst); int64_t * data = (int64_t *) dst->data; if (!v_trans) { @@ -1506,7 +1506,7 @@ void llama_kv_cache::set_input_v_idxs(ggml_tensor * dst, const llama_ubatch * ub } void llama_kv_cache::set_input_k_shift(ggml_tensor * dst) const { - GGML_ASSERT(ggml_backend_buffer_is_host(dst->buffer)); + llama_host_write(dst); int32_t * data = (int32_t *) dst->data; @@ -1725,7 +1725,7 @@ static void set_input_kq_mask_impl(const args_set_input_kq_mask & args, T * data void llama_kv_cache::set_input_kq_mask(ggml_tensor * dst, const llama_ubatch * ubatch, bool causal_attn) const { const uint32_t n_tokens = ubatch->n_tokens; - GGML_ASSERT(ggml_backend_buffer_is_host(dst->buffer)); + llama_host_write(dst); const int64_t n_kv = dst->ne[0]; const int64_t n_stream = dst->ne[3]; // num streams in the current ubatch @@ -1766,7 +1766,7 @@ void llama_kv_cache::set_input_pos_bucket(ggml_tensor * dst, const llama_ubatch GGML_ASSERT(n_stream == 1 && "TODO: support multiple streams"); const auto & cells = v_cells[0]; - GGML_ASSERT(ggml_backend_buffer_is_host(dst->buffer)); + llama_host_write(dst); GGML_ASSERT(!ubatch->equal_seqs()); // TODO: use ubatch->n_seqs instead of failing int32_t * data = (int32_t *) dst->data; @@ -1786,7 +1786,7 @@ void llama_kv_cache::set_input_pos_bucket(ggml_tensor * dst, const llama_ubatch } void llama_kv_cache::set_input_k_rot(ggml_tensor * dst) const { - GGML_ASSERT(ggml_backend_buffer_is_host(dst->buffer)); + llama_host_write(dst); const auto n_rot = dst->ne[0]; GGML_ASSERT(attn_rot_hadamard.count(dst->ne[0])); @@ -1795,7 +1795,7 @@ void llama_kv_cache::set_input_k_rot(ggml_tensor * dst) const { } void llama_kv_cache::set_input_v_rot(ggml_tensor * dst) const { - GGML_ASSERT(ggml_backend_buffer_is_host(dst->buffer)); + llama_host_write(dst); const auto n_rot = dst->ne[0]; GGML_ASSERT(attn_rot_hadamard.count(dst->ne[0])); From 292887a740eb003c496bc2405ee64c0d5f1b7552 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Mon, 17 Aug 2026 17:16:05 +0200 Subject: [PATCH 04/18] llama: give every graph input a name Most graph inputs were never named, so they showed up as leaf_N in GGML_SCHED_DEBUG output, in scheduler sanitizer reports and in any other tooling that identifies tensors by name. Of the 24 inputs, 15 were anonymous. Name them, following the existing conventions: cb() where the builder is an llm_graph_context method, ggml_set_name() in the free functions and memory-module methods that have no callback to hand. The kq_mask helper also stamped the same "attn_inp_kq_mask" on every variant, so the base, SWA, MLA and LID masks were indistinguishable from one another. Give it a name parameter and pass a distinct name at each call site. No functional change: a prefill that reports 87 sanitizer races reports the same 87 before and after, with every tensor now identified. Assisted-By: Claude Opus 5 --- src/llama-graph.cpp | 28 ++++++++++++++++++++-------- src/llama-kv-cache-dsv4.cpp | 1 + src/llama-kv-cache.cpp | 3 +++ 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/llama-graph.cpp b/src/llama-graph.cpp index 1040346e758..8e57c3202ae 100644 --- a/src/llama-graph.cpp +++ b/src/llama-graph.cpp @@ -29,7 +29,8 @@ static ggml_tensor * build_attn_inp_kq_mask( ggml_context * ctx, const llama_kv_cache_context * mctx, const llama_ubatch & ubatch, - const llama_cparams & cparams) { + const llama_cparams & cparams, + const char * name = "attn_inp_kq_mask") { const auto n_kv = mctx->get_n_kv(); const auto n_tokens = ubatch.n_tokens; const auto n_stream = cparams.kv_unified ? 1 : ubatch.n_seqs_unq; @@ -39,7 +40,7 @@ static ggml_tensor * build_attn_inp_kq_mask( ggml_tensor * res = ggml_new_tensor_4d(ctx, type, n_kv, n_tokens/n_stream, 1, n_stream); ggml_set_input(res); - ggml_set_name(res, "attn_inp_kq_mask"); + ggml_set_name(res, name); return res; } @@ -790,7 +791,7 @@ static ggml_tensor * dsv4_build_raw_kq_mask( ggml_tensor * res = ggml_new_tensor_4d(ctx, type, n_kv, n_tokens/n_stream, 1, n_stream); ggml_set_input(res); - ggml_set_name(res, "attn_inp_kq_mask"); + ggml_set_name(res, "dsv4_raw_attn_inp_kq_mask"); return res; } @@ -2377,6 +2378,7 @@ ggml_tensor * llm_graph_context::build_inp_pos() const { cur = ggml_new_tensor_1d(ctx0, GGML_TYPE_I32, (int64_t)n_tokens*hparams.n_pos_per_embd()); ggml_set_input(cur); + cb(cur, "inp_pos", -1); res->add_input(std::move(inp)); @@ -2413,6 +2415,7 @@ ggml_tensor * llm_graph_context::build_inp_out_ids() const { cur = ggml_new_tensor_1d(ctx0, GGML_TYPE_I32, n_outputs); ggml_set_input(cur); + cb(cur, "inp_out_ids", -1); res->add_input(std::move(inp)); @@ -2426,6 +2429,7 @@ ggml_tensor * llm_graph_context::build_inp_mean() const { cur = ggml_new_tensor_2d(ctx0, GGML_TYPE_F32, n_tokens, ubatch.n_seqs_unq); ggml_set_input(cur); + cb(cur, "inp_mean", -1); res->add_input(std::move(inp)); @@ -2439,6 +2443,7 @@ ggml_tensor * llm_graph_context::build_inp_cls() const { cur = ggml_new_tensor_1d(ctx0, GGML_TYPE_I32, ubatch.n_seqs_unq); ggml_set_input(cur); + cb(cur, "inp_cls", -1); res->add_input(std::move(inp)); @@ -2463,6 +2468,7 @@ ggml_tensor * llm_graph_context::build_inp_cross_embd() const { cur = ggml_new_tensor_2d(ctx0, GGML_TYPE_F32, n_embd, n_enc); ggml_set_input(cur); + cb(cur, "inp_cross_embd", -1); res->add_input(std::move(inp)); @@ -2476,6 +2482,7 @@ ggml_tensor * llm_graph_context::build_inp_pos_bucket_enc() const { cur = ggml_new_tensor_2d(ctx0, GGML_TYPE_I32, n_tokens, n_tokens); ggml_set_input(cur); + cb(cur, "inp_pos_bucket_enc", -1); res->add_input(std::move(inp)); @@ -2493,6 +2500,7 @@ ggml_tensor * llm_graph_context::build_inp_pos_bucket_dec() const { cur = ggml_new_tensor_2d(ctx0, GGML_TYPE_I32, n_kv, n_tokens); ggml_set_input(cur); + cb(cur, "inp_pos_bucket_dec", -1); res->add_input(std::move(inp)); @@ -2658,12 +2666,14 @@ llm_graph_input_attn_no_cache * llm_graph_context::build_attn_inp_no_cache() con // note: there is no KV cache, so the number of KV values is equal to the number of tokens in the batch inp->self_kq_mask = ggml_new_tensor_4d(ctx0, type_mask, n_tokens, n_tokens, 1, 1); ggml_set_input(inp->self_kq_mask); + cb(inp->self_kq_mask, "attn_inp_kq_mask", -1); inp->self_kq_mask_cnv = inp->self_kq_mask; if (hparams.swa_type != LLAMA_SWA_TYPE_NONE) { inp->self_kq_mask_swa = ggml_new_tensor_4d(ctx0, type_mask, n_tokens, n_tokens, 1, 1); ggml_set_input(inp->self_kq_mask_swa); + cb(inp->self_kq_mask_swa, "attn_inp_kq_mask_swa", -1); inp->self_kq_mask_swa_cnv = inp->self_kq_mask_swa; } else { @@ -3161,6 +3171,7 @@ llm_graph_input_attn_cross * llm_graph_context::build_attn_inp_cross() const { inp->cross_kq_mask = ggml_new_tensor_4d(ctx0, type_mask, n_enc, n_tokens, 1, 1); ggml_set_input(inp->cross_kq_mask); + cb(inp->cross_kq_mask, "attn_inp_cross_kq_mask", -1); inp->cross_kq_mask_cnv = inp->cross_kq_mask; @@ -3218,7 +3229,7 @@ llm_graph_input_attn_k_dsa * llm_graph_context::build_attn_inp_k_dsa() const { { inp->self_k_idxs_mla = mctx_cur->get_mla()->build_input_k_idxs(ctx0, ubatch); - inp->self_kq_mask_mla = build_attn_inp_kq_mask(ctx0, mctx_cur->get_mla(), ubatch, cparams); + inp->self_kq_mask_mla = build_attn_inp_kq_mask(ctx0, mctx_cur->get_mla(), ubatch, cparams, "attn_inp_kq_mask_mla"); inp->self_kq_mask_mla_cnv = inp->self_kq_mask_mla; } @@ -3229,7 +3240,7 @@ llm_graph_input_attn_k_dsa * llm_graph_context::build_attn_inp_k_dsa() const { auto cparams_copy = cparams; cparams_copy.flash_attn = cparams.fused_lid; - inp->self_kq_mask_lid = build_attn_inp_kq_mask(ctx0, mctx_cur->get_lid(), ubatch, cparams_copy); + inp->self_kq_mask_lid = build_attn_inp_kq_mask(ctx0, mctx_cur->get_lid(), ubatch, cparams_copy, "attn_inp_kq_mask_lid"); inp->self_kq_mask_lid_cnv = inp->self_kq_mask_lid; inp->self_k_rot_lid = mctx_cur->get_lid()->build_input_k_rot(ctx0); @@ -3288,7 +3299,7 @@ llm_graph_input_attn_kv_iswa * llm_graph_context::build_attn_inp_kv_iswa() const inp->self_k_idxs_swa = mctx_cur->get_swa()->build_input_k_idxs(ctx0, ubatch); inp->self_v_idxs_swa = mctx_cur->get_swa()->build_input_v_idxs(ctx0, ubatch); - inp->self_kq_mask_swa = build_attn_inp_kq_mask(ctx0, mctx_cur->get_swa(), ubatch, cparams); + inp->self_kq_mask_swa = build_attn_inp_kq_mask(ctx0, mctx_cur->get_swa(), ubatch, cparams, "attn_inp_kq_mask_swa"); inp->self_kq_mask_swa_cnv = inp->self_kq_mask_swa; } @@ -3318,7 +3329,7 @@ llm_graph_input_attn_k_iswa * llm_graph_context::build_attn_inp_k_iswa() const { inp->self_k_idxs_swa = mctx_cur->get_swa()->build_input_k_idxs(ctx0, ubatch); - inp->self_kq_mask_swa = build_attn_inp_kq_mask(ctx0, mctx_cur->get_swa(), ubatch, cparams); + inp->self_kq_mask_swa = build_attn_inp_kq_mask(ctx0, mctx_cur->get_swa(), ubatch, cparams, "attn_inp_kq_mask_swa"); inp->self_kq_mask_swa_cnv = inp->self_kq_mask_swa; } @@ -3404,6 +3415,7 @@ static std::unique_ptr build_rs_inp_impl( inp->s_copy = ggml_new_tensor_1d(ctx0, GGML_TYPE_I32, n_rs); ggml_set_input(inp->s_copy); + ggml_set_name(inp->s_copy, "inp_s_copy"); inp->s_copy_main = ggml_view_1d(ctx0, inp->s_copy, n_seqs, 0); inp->s_copy_extra = ggml_view_1d(ctx0, inp->s_copy, n_rs - n_seqs, n_seqs * inp->s_copy->nb[0]); @@ -3520,7 +3532,7 @@ llm_graph_input_mem_hybrid_iswa * llm_graph_context::build_inp_mem_hybrid_iswa() inp_attn->self_k_idxs_swa = attn_ctx->get_swa()->build_input_k_idxs(ctx0, ubatch); inp_attn->self_v_idxs_swa = attn_ctx->get_swa()->build_input_v_idxs(ctx0, ubatch); - inp_attn->self_kq_mask_swa = build_attn_inp_kq_mask(ctx0, attn_ctx->get_swa(), ubatch, cparams); + inp_attn->self_kq_mask_swa = build_attn_inp_kq_mask(ctx0, attn_ctx->get_swa(), ubatch, cparams, "attn_inp_kq_mask_swa"); inp_attn->self_kq_mask_swa_cnv = inp_attn->self_kq_mask_swa; } diff --git a/src/llama-kv-cache-dsv4.cpp b/src/llama-kv-cache-dsv4.cpp index 5caa05e8b07..e90fb34f1b9 100644 --- a/src/llama-kv-cache-dsv4.cpp +++ b/src/llama-kv-cache-dsv4.cpp @@ -1854,6 +1854,7 @@ ggml_tensor * llama_kv_cache_dsv4_raw_context::build_input_k_idxs(ggml_context * ggml_tensor * k_idxs = ggml_new_tensor_1d(ctx, GGML_TYPE_I64, n_tokens); ggml_set_input(k_idxs); + ggml_set_name(k_idxs, "dsv4_raw_attn_inp_k_idxs"); return k_idxs; } diff --git a/src/llama-kv-cache.cpp b/src/llama-kv-cache.cpp index d33a2a16109..fe3109960d3 100644 --- a/src/llama-kv-cache.cpp +++ b/src/llama-kv-cache.cpp @@ -1395,6 +1395,7 @@ ggml_tensor * llama_kv_cache::build_input_k_idxs(ggml_context * ctx, const llama ggml_tensor * k_idxs = ggml_new_tensor_1d(ctx, GGML_TYPE_I64, n_tokens); ggml_set_input(k_idxs); + ggml_set_name(k_idxs, "attn_inp_k_idxs"); return k_idxs; } @@ -1411,6 +1412,7 @@ ggml_tensor * llama_kv_cache::build_input_v_idxs(ggml_context * ctx, const llama } ggml_set_input(v_idxs); + ggml_set_name(v_idxs, "attn_inp_v_idxs"); return v_idxs; } @@ -1923,6 +1925,7 @@ ggml_cgraph * llama_kv_cache::build_graph_shift(llm_graph_result * res, llama_co inp->k_shift = ggml_new_tensor_1d(ctx, GGML_TYPE_I32, (int64_t) get_size()*n_stream); ggml_set_input(inp->k_shift); + ggml_set_name(inp->k_shift, "inp_k_shift"); inp->k_rot = build_input_k_rot(ctx); From 70192ac4bcbcb6d139563c588660a2daeac3ea88 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Mon, 17 Aug 2026 17:50:41 +0200 Subject: [PATCH 05/18] cuda: verify the cgraph uid fast path instead of trusting it ggml_cuda_graph_update_required() treats an unchanged non-zero cgraph->uid as proof that nothing the captured graph baked in has changed, and returns early without comparing node properties. That holds today only because the uid is re-stamped in ggml_backend_sched_split_graph(), and splitting always precedes re-allocation. The failure mode if it ever stops holding is bad: there is no fallback, the executable graph is simply replayed against stale addresses and the results are wrong with no diagnostic. A change that re-points tensors without going through a re-split - pipelining or ring-buffered graph inputs, say - would hit exactly this. Verify the promise rather than trusting it: on the fast path, recompute the node properties and abort if they differ. On by default in debug builds and available via GGML_CUDA_GRAPH_VERIFY_UID elsewhere, since release builds are where such a change is actually exercised. Document the contract at both ends under [TAG_CUDA_GRAPH_UID]. Checked by freezing the split uid to a constant: without the check a multi-ubatch prefill completes silently with stale addresses (exit 0), with it enabled the run aborts naming the offending node. No false positives - the fast path is taken 38 times in a 40-token decode and stays quiet, with decode speed unchanged. Assisted-By: Claude Opus 5 --- ggml/src/ggml-backend.cpp | 4 +++ ggml/src/ggml-cuda/ggml-cuda.cu | 59 +++++++++++++++++++++++++++------ 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index e68b0da5fff..34f829a65c2 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -1583,6 +1583,10 @@ void ggml_backend_sched_split_graph(ggml_backend_sched_t sched, struct ggml_cgra } // set ids for all splits + // [TAG_CUDA_GRAPH_UID] a backend may take an unchanged uid as a promise that the split's + // tensor addresses have not moved, and skip re-checking them. this is only true because + // splitting always precedes (re-)allocation, so anything that re-points tensors without + // re-splitting has to re-stamp these itself. for (int i = 0; i < sched->n_splits; ++i) { sched->splits[i].graph.uid = ggml_graph_next_uid(); } diff --git a/ggml/src/ggml-cuda/ggml-cuda.cu b/ggml/src/ggml-cuda/ggml-cuda.cu index a8a1c09ca3b..b2b83a44625 100644 --- a/ggml/src/ggml-cuda/ggml-cuda.cu +++ b/ggml/src/ggml-cuda/ggml-cuda.cu @@ -2578,6 +2578,44 @@ static const void * ggml_cuda_graph_get_key(ggml_cgraph * cgraph) { return cgraph->nodes[0]; } +static ggml_cuda_graph::node_properties ggml_cuda_graph_node_props(const ggml_tensor * node) { + ggml_cuda_graph::node_properties prop = {}; + memcpy(&prop.node, node, sizeof(ggml_tensor)); + + for (int j = 0; j < GGML_MAX_SRC; ++j) { + if (node->src[j]) { + prop.node_src_data_ptrs[j] = node->src[j]->data; + memcpy(prop.node_src_ne[j], node->src[j]->ne, sizeof(prop.node_src_ne[j])); + memcpy(prop.node_src_nb[j], node->src[j]->nb, sizeof(prop.node_src_nb[j])); + } + } + + return prop; +} + +// on in debug builds, opt-in via GGML_CUDA_GRAPH_VERIFY_UID elsewhere - release builds are +// where a ring/pipelining change is actually exercised, so the check has to be reachable there +static bool ggml_cuda_graph_verify_uid() { + static const bool verify = []() { +#ifndef NDEBUG + return true; +#else + return getenv("GGML_CUDA_GRAPH_VERIFY_UID") != nullptr; +#endif + }(); + return verify; +} + +// [TAG_CUDA_GRAPH_UID] +// A non-zero cgraph->uid that has not changed since the last call is a promise from the caller +// that nothing the captured graph baked in has changed either - in particular that no node's +// data pointer, and no src's data pointer, ne or nb, has moved. On that promise we skip the +// property comparison below and replay the existing executable graph as-is. +// +// Breaking the promise does not fall back to a slow path, it replays a graph that reads stale +// addresses, which corrupts results silently. Any caller that re-points tensors without going +// through a re-split (which re-stamps the uid, see ggml_backend_sched_split_graph) must +// invalidate the uid itself. Debug builds verify the promise instead of trusting it. static bool ggml_cuda_graph_update_required(ggml_backend_cuda_context * cuda_ctx, ggml_cgraph * cgraph) { bool res = false; @@ -2588,6 +2626,16 @@ static bool ggml_cuda_graph_update_required(ggml_backend_cuda_context * cuda_ctx cgraph->uid == graph->uid) { GGML_LOG_DEBUG("CUDA Graph id %zu reused\n", cgraph->uid); GGML_ASSERT((int)graph->node_props.size() == cgraph->n_nodes); + if (ggml_cuda_graph_verify_uid()) { + for (int i = 0; i < cgraph->n_nodes; i++) { + const ggml_cuda_graph::node_properties prop = ggml_cuda_graph_node_props(cgraph->nodes[i]); + if (memcmp(&graph->node_props[i], &prop, sizeof(prop)) != 0) { + GGML_LOG_ERROR("%s: node %d (%s) changed while cgraph->uid stayed %llu\n", + __func__, i, cgraph->nodes[i]->name, (unsigned long long) cgraph->uid); + GGML_ABORT("CUDA graph uid reused after node properties changed - see [TAG_CUDA_GRAPH_UID]"); + } + } + } return false; } @@ -2600,16 +2648,7 @@ static bool ggml_cuda_graph_update_required(ggml_backend_cuda_context * cuda_ctx } for (int i = 0; i < cgraph->n_nodes; i++) { - ggml_cuda_graph::node_properties prop = {}; - memcpy(&prop.node, cgraph->nodes[i], sizeof(ggml_tensor)); - - for (int j = 0; j < GGML_MAX_SRC; ++j) { - if (cgraph->nodes[i]->src[j]) { - prop.node_src_data_ptrs[j] = cgraph->nodes[i]->src[j]->data; - memcpy(prop.node_src_ne[j], cgraph->nodes[i]->src[j]->ne, sizeof(prop.node_src_ne[j])); - memcpy(prop.node_src_nb[j], cgraph->nodes[i]->src[j]->nb, sizeof(prop.node_src_nb[j])); - } - } + const ggml_cuda_graph::node_properties prop = ggml_cuda_graph_node_props(cgraph->nodes[i]); if (res || memcmp(&graph->node_props[i], &prop, sizeof(prop)) != 0) { graph->node_props[i] = prop; From f141436510ff4c4b810d7b924a75792436447dbd Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Mon, 17 Aug 2026 18:03:54 +0200 Subject: [PATCH 06/18] ggml-backend: ring buffer graph inputs when a backend computes on host memory Graph inputs are pinned to the last (CPU) backend, and llama.cpp hands the scheduler a device host buffer type for that slot. On an integrated GPU the device also accepts that buffer type, so ggml_backend_sched_buffer_supported() is true, no split input copy is made, and the device reads the very memory the host thread writes. Nothing then stops the host from writing the next ubatch's inputs while the device is still reading the previous one. Detect that case and give the graph inputs a ring of 2 instead of a single buffer. The predicate tests the exact condition that elides the copy - a host buffer type in the CPU slot that some compute backend accepts - rather than guessing at "is this an APU", so backends that deliberately refuse to compute on pinned host memory are unaffected. Verified on a Strix Halo APU: ROCm gets a ring, Vulkan on the same device does not, and neither does CPU-only. Note the buffer type has to be resolved directly here, since sched->bufts[] is not populated until further down. This is detection and allocation only - rotating the ring on the graph reuse path is a separate change, and until it lands the race is reduced but not fixed (87 reports to 61 on a multi-ubatch prefill, and a deeper ring only widens the window: 49 at N=3, 42 at N=4, never 0). Depth 2 is the default because the extra input memory is not free: measured +8 MiB at n_ctx 8192 / n_ubatch 512, and +128 MiB at 32768 / 2048, all of it in the host buffer. GGML_SCHED_UMA_RING overrides the detection - 0 or 1 disables, larger sets the depth - as an escape hatch and to A/B the cost. Assisted-By: Claude Opus 5 --- ggml/src/ggml-backend.cpp | 40 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index 34f829a65c2..c09c6a58ca7 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -1879,7 +1879,45 @@ ggml_backend_sched_t ggml_backend_sched_new( sched->debug_realloc = GGML_SCHED_DEBUG_REALLOC ? atoi(GGML_SCHED_DEBUG_REALLOC) : sched->debug_realloc; sched->n_backends = n_backends; - sched->n_copies = parallel ? GGML_SCHED_MAX_COPIES : 1; + + // graph inputs are pinned to the last (CPU) backend, and the caller may hand us a device + // host buffer type for that slot. if a compute backend also accepts that buffer type, then + // ggml_backend_sched_buffer_supported() is true for graph inputs, no split input copy is + // made, and the device reads the very memory the host thread writes - so the host cannot + // write the next set of inputs until the device is done reading the previous one. + // + // detect that here so the inputs can be ring buffered instead of synchronized. this tests + // the exact condition that elides the copy rather than guessing at "is this an APU". + // note: sched->bufts[] is not populated until below, so resolve the buffer type directly. + bool is_uma = false; + if (!parallel && n_backends >= 2) { + ggml_backend_buffer_type_t cpu_buft = bufts ? bufts[n_backends - 1] + : ggml_backend_get_default_buffer_type(backends[n_backends - 1]); + + if (cpu_buft && ggml_backend_buft_is_host(cpu_buft)) { + for (int b = 0; b < n_backends - 1; b++) { + if (ggml_backend_supports_buft(backends[b], cpu_buft)) { + is_uma = true; + GGML_LOG_DEBUG("%s: %s computes directly on %s, ring buffering graph inputs\n", + __func__, ggml_backend_name(backends[b]), ggml_backend_buft_name(cpu_buft)); + break; + } + } + } + } + + // a ring of 2 removes the steady state synchronization just as well as a longer one, at half + // the extra input memory - which on a unified memory device comes out of system RAM + int n_copies_uma = is_uma ? 2 : 1; + + // GGML_SCHED_UMA_RING overrides the detection above: 0 or 1 forces it off, a larger value + // sets the ring depth. the extra input buffers are not free, so keep an escape hatch. + const char * GGML_SCHED_UMA_RING = getenv("GGML_SCHED_UMA_RING"); + if (GGML_SCHED_UMA_RING) { + n_copies_uma = std::min(std::max(atoi(GGML_SCHED_UMA_RING), 1), GGML_SCHED_MAX_COPIES); + } + + sched->n_copies = parallel ? GGML_SCHED_MAX_COPIES : n_copies_uma; // initialize hash table // FIXME: needs to be size*2 to account for leafs (do it in graph_split instead) From 9bdc6ea22b8d1014ab5258058d1dd1f7eea8332f Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Mon, 17 Aug 2026 18:41:17 +0200 Subject: [PATCH 07/18] ggml-backend,llama: rotate the graph input ring on the reuse path Enabling the ring was not enough on its own. The addresses only move as a side effect of re-splitting, so on the graph reuse path - which re-runs neither the split nor the allocator - bumping cur_copy moved nothing, and the host still overwrote inputs the device was reading. Add ggml_backend_sched_prepare_inputs(), which llama.cpp calls before writing the inputs of a reused graph. It steps onto the next ring slot, waits for that slot's previous reader, and re-points each graph input by hand from addresses captured when the allocator placed them. Nodes hold ggml_tensor pointers and split_graph does not rewrite node->src[] for graph inputs, so re-pointing the tensor moves every reader with it. It replaces an unconditional ggml_backend_sched_synchronize() that was already at that call site for pipeline parallelism, so multi GPU setups now rotate instead of stalling on every reused graph. Three things this needs to get right: - An input reached only through a view of it - the recurrent state copy - was never registered, so it never rotated and kept racing. Resolve through view chains when registering, and re-derive the views after re-pointing, since a view's address is baked at allocation time. - The wait belongs on the allocate path too. Rotating onto a slot is only safe once its previous reader is finished, however the addresses got there, so both paths share it. - [TAG_CUDA_GRAPH_UID] re-stamp the split uids when re-pointing. Moving addresses without a re-split is exactly the case the uid fast path assumes cannot happen, and a captured graph would otherwise replay against the old ones. Rotation is confined to where it is needed: anything that synchronizes clears the flag, so single token decode inside a sampling loop holds its slot, keeps its addresses stable and keeps its captured graphs. Measured on a Strix Halo APU, multi ubatch prefill, sanitizer race reports to zero: n_ubatch 512 (rebuild every ubatch) 87 -> 0 n_ubatch 32 (11 graphs reused) 21 -> 0 Decode stays clean, and so do the CPU only and Vulkan controls. Over a 200 token generation the reuse path rotates zero times, the captured graph is reused 198 times, and warmup completes once and never resets - so decode addresses are as stable as before. GGML_CUDA_GRAPH_VERIFY_UID is quiet across both, including the 9 reuse path rotations. Assisted-By: Claude Opus 5 --- ggml/include/ggml-backend.h | 6 + ggml/src/ggml-backend.cpp | 213 ++++++++++++++++++++++++++++++++++-- src/llama-context.cpp | 11 +- 3 files changed, 214 insertions(+), 16 deletions(-) diff --git a/ggml/include/ggml-backend.h b/ggml/include/ggml-backend.h index 0e4d1d9b17c..77d0565d83a 100644 --- a/ggml/include/ggml-backend.h +++ b/ggml/include/ggml-backend.h @@ -347,6 +347,12 @@ extern "C" { // Allocate and compute graph on the backend scheduler GGML_API bool ggml_backend_sched_alloc_graph(ggml_backend_sched_t sched, struct ggml_cgraph * graph); // returns success + // make the graph inputs safe for the caller to write again. must be called before writing + // inputs for an iteration that reuses an already allocated graph, since a previously issued + // compute may still be reading them. rotates onto a spare set of inputs where one is + // available, and only falls back to waiting when it is not. calling it after + // ggml_backend_sched_alloc_graph() is a no-op - allocating already did this. + GGML_API void ggml_backend_sched_prepare_inputs(ggml_backend_sched_t sched); GGML_API enum ggml_status ggml_backend_sched_graph_compute(ggml_backend_sched_t sched, struct ggml_cgraph * graph); GGML_API enum ggml_status ggml_backend_sched_graph_compute_async(ggml_backend_sched_t sched, struct ggml_cgraph * graph); GGML_API void ggml_backend_sched_synchronize(ggml_backend_sched_t sched); diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index c09c6a58ca7..0d3a061fee6 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -859,6 +859,19 @@ struct ggml_backend_sched { int n_graph_inputs; int graph_inputs_capacity; + // where each graph input's N ring slots live, captured once the allocator has placed them. + // rotating on the graph reuse path means re-pointing the tensor by hand, because that path + // re-runs neither the split nor the allocator - see ggml_backend_sched_rotate_inputs() + struct ggml_backend_sched_input_slots { + ggml_backend_buffer_t buffer[GGML_SCHED_MAX_COPIES]; + void * data [GGML_SCHED_MAX_COPIES]; + } * graph_input_slots; // [graph_inputs_capacity] + + // a compute has been issued and the graph inputs have not been rotated since. set when the + // splits are computed, cleared by anything that makes it safe to write the inputs again - + // either a rotation or a full synchronization + bool needs_rotate; + struct ggml_context * ctx; ggml_backend_sched_eval_callback callback_eval; @@ -910,9 +923,44 @@ static void ggml_backend_sched_graph_inputs_grow(ggml_backend_sched_t sched) { GGML_ABORT("failed to grow graph inputs container"); } sched->graph_inputs = pnew; + + auto * snew = (struct ggml_backend_sched::ggml_backend_sched_input_slots *) realloc( + (void *) sched->graph_input_slots, new_cap * sizeof(sched->graph_input_slots[0])); + if (snew == NULL) { + GGML_LOG_ERROR("%s: failed to allocate %zu bytes\n", __func__, new_cap * sizeof(sched->graph_input_slots[0])); + GGML_ABORT("failed to grow graph input slots container"); + } + sched->graph_input_slots = snew; + sched->graph_inputs_capacity = new_cap; } +// the graph input a tensor ultimately refers to, following view chains, or NULL if it is not +// (a view of) a graph input +static struct ggml_tensor * ggml_backend_sched_graph_input(struct ggml_tensor * t) { + while (t != NULL) { + if (t->flags & GGML_TENSOR_FLAG_INPUT) { + return t; + } + t = t->view_src; + } + return NULL; +} + +// re-derive a view's address from its source, the way ggml_gallocr_init_tensor would have. only +// needed after an input has been re-pointed by hand: a view's address is baked at allocation +// time, so moving the tensor it views does not move the view with it +static void ggml_backend_sched_reinit_view(struct ggml_tensor * t) { + if (t->view_src == NULL) { + return; + } + + ggml_backend_sched_reinit_view(t->view_src); + + t->buffer = t->view_src->buffer; + t->data = (char *) t->view_src->data + t->view_offs; +} + // returns the priority of the backend, lower id is higher priority static int ggml_backend_sched_backend_id(ggml_backend_sched_t sched, ggml_backend_t backend) { for (int i = 0; i < sched->n_backends; i++) { @@ -1421,27 +1469,35 @@ void ggml_backend_sched_split_graph(ggml_backend_sched_t sched, struct ggml_cgra const int src_backend_id = sched->hv_tensor_backend_ids[src_id]; GGML_ASSERT(src_backend_id != -1); // all inputs should be assigned by now - if (src->flags & GGML_TENSOR_FLAG_INPUT && sched->n_copies > 1) { - if (tensor_id_copy(src_id, src_backend_id, 0) == NULL) { - ggml_backend_t backend = sched->backends[src_backend_id]; + // an input is not always reached directly - the recurrent state copy, for one, is + // only ever read through views of it. the ring has to rotate the input itself, so + // resolve through any views here, otherwise it is never registered and never moves + struct ggml_tensor * inp = ggml_backend_sched_graph_input(src); + + if (inp != NULL && sched->n_copies > 1) { + const size_t inp_id = hash_id(inp); + const int inp_backend_id = sched->hv_tensor_backend_ids[inp_id]; + + if (inp_backend_id != -1 && tensor_id_copy(inp_id, inp_backend_id, 0) == NULL) { + ggml_backend_t backend = sched->backends[inp_backend_id]; for (int c = 0; c < sched->n_copies; c++) { struct ggml_tensor * tensor_copy; if (c == sched->cur_copy) { - tensor_copy = src; // use the original tensor as the current copy + tensor_copy = inp; // use the original tensor as the current copy } else { - tensor_copy = ggml_dup_tensor_layout(sched->ctx, src); - ggml_format_name(tensor_copy, "%s#%s#%d", ggml_backend_name(backend), src->name, c); + tensor_copy = ggml_dup_tensor_layout(sched->ctx, inp); + ggml_format_name(tensor_copy, "%s#%s#%d", ggml_backend_name(backend), inp->name, c); } ggml_set_input(tensor_copy); ggml_set_output(tensor_copy); // prevent ggml-alloc from overwriting the tensor - tensor_id_copy(src_id, src_backend_id, c) = tensor_copy; + tensor_id_copy(inp_id, inp_backend_id, c) = tensor_copy; SET_CAUSE(tensor_copy, "4.cpy"); } int n_graph_inputs = sched->n_graph_inputs++; if (n_graph_inputs >= sched->graph_inputs_capacity) { ggml_backend_sched_graph_inputs_grow(sched); } - sched->graph_inputs[n_graph_inputs] = src; + sched->graph_inputs[n_graph_inputs] = inp; } } @@ -1852,6 +1908,10 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s ggml_san_split(-1, NULL, 0); + // the device may still be reading the graph inputs, so they must not be written again until + // either the inputs are rotated onto another slot or everything has been waited for + sched->needs_rotate = true; + return GGML_STATUS_SUCCESS; } @@ -1944,6 +2004,8 @@ ggml_backend_sched_t ggml_backend_sched_new( sched->graph_inputs_capacity = GGML_SCHED_MAX_SPLIT_INPUTS; sched->graph_inputs = (struct ggml_tensor **) calloc(sched->graph_inputs_capacity, sizeof(struct ggml_tensor *)); + sched->graph_input_slots = (struct ggml_backend_sched::ggml_backend_sched_input_slots *) calloc( + sched->graph_inputs_capacity, sizeof(sched->graph_input_slots[0])); for (int b = 0; b < n_backends; b++) { sched->backends[b] = backends[b]; @@ -1982,6 +2044,7 @@ void ggml_backend_sched_free(ggml_backend_sched_t sched) { } free(sched->splits); free(sched->graph_inputs); + free(sched->graph_input_slots); free(sched->hv_tensor_backend_ids); free(sched->hv_tensor_copies); free(sched->node_backend_ids); @@ -2004,6 +2067,8 @@ void ggml_backend_sched_reset(ggml_backend_sched_t sched) { sched->is_reset = true; } sched->is_alloc = false; + // deliberately not clearing needs_rotate: resetting only drops bookkeeping, it does not wait + // for anything, so a compute issued before this can still be reading the graph inputs } void ggml_backend_sched_reserve_size(ggml_backend_sched_t sched, struct ggml_cgraph * measure_graph, size_t * sizes) { @@ -2037,13 +2102,132 @@ bool ggml_backend_sched_reserve(ggml_backend_sched_t sched, struct ggml_cgraph * return true; } +// record where the allocator put each graph input's ring slots. the tensor sitting in slot +// cur_copy is the user's own tensor (split_graph aliases it there), the rest are placeholders +// that exist only to reserve distinct, non-overlapping memory for the other slots. +static void ggml_backend_sched_capture_input_slots(ggml_backend_sched_t sched) { + if (sched->n_copies <= 1) { + return; + } + + for (int i = 0; i < sched->n_graph_inputs; i++) { + struct ggml_tensor * input = sched->graph_inputs[i]; + + const size_t id = hash_id(input); + const int backend_id = tensor_backend_id(input); + + for (int c = 0; c < sched->n_copies; c++) { + struct ggml_tensor * slot = tensor_id_copy(id, backend_id, c); + + sched->graph_input_slots[i].buffer[c] = slot ? slot->buffer : NULL; + sched->graph_input_slots[i].data [c] = slot ? slot->data : NULL; + } + + if (sched->debug) { + GGML_LOG_DEBUG("%s: graph input %d: %s\n", __func__, i, input->name); + } + } +} + +// step onto the next ring slot, waiting until the device is finished reading it if anything is +// still in flight. the slot being stepped onto was last read n_copies-1 iterations ago, so this +// is normally already signalled - that is the entire point of the ring. the writer we are +// protecting is the host thread, so it has to be a host side wait, not ggml_backend_event_wait(). +static void ggml_backend_sched_advance_copy(ggml_backend_sched_t sched) { + sched->cur_copy = sched->next_copy; + sched->next_copy = (sched->next_copy + 1) % sched->n_copies; + + if (sched->n_copies <= 1 || !sched->needs_rotate) { + return; + } + + for (int b = 0; b < sched->n_backends; b++) { + if (sched->events[b][sched->cur_copy] != NULL) { + ggml_backend_event_synchronize(sched->events[b][sched->cur_copy]); + } else { + ggml_backend_synchronize(sched->backends[b]); + } + } + + if (sched->debug) { + GGML_LOG_DEBUG("%s: advanced to copy %d/%d\n", __func__, sched->cur_copy, sched->n_copies); + } +} + +// move the graph inputs onto the next ring slot. this is what the graph reuse path needs: it +// re-runs neither the split nor the allocator, so bumping cur_copy on its own would move nothing. +static void ggml_backend_sched_rotate_inputs(ggml_backend_sched_t sched) { + GGML_ASSERT(sched->n_copies > 1); + + ggml_backend_sched_advance_copy(sched); + + for (int i = 0; i < sched->n_graph_inputs; i++) { + struct ggml_tensor * input = sched->graph_inputs[i]; + + if (sched->graph_input_slots[i].data[sched->cur_copy] == NULL) { + continue; + } + + // nodes reference inputs by ggml_tensor *, and split_graph does not rewrite node->src[] + // for graph inputs, so re-pointing the tensor is enough to move every reader with it + input->buffer = sched->graph_input_slots[i].buffer[sched->cur_copy]; + input->data = sched->graph_input_slots[i].data [sched->cur_copy]; + } + + // views of the inputs still point into the slot they were allocated against, so bring them + // along. only views rooted at a graph input are touched; for anything else this would be + // recomputing the address it already has + for (int i = 0; i < sched->graph.n_nodes; i++) { + struct ggml_tensor * node = sched->graph.nodes[i]; + + for (int j = 0; j < GGML_MAX_SRC; j++) { + struct ggml_tensor * src = node->src[j]; + if (src != NULL && src->view_src != NULL && ggml_backend_sched_graph_input(src) != NULL) { + ggml_backend_sched_reinit_view(src); + } + } + } + + // [TAG_CUDA_GRAPH_UID] tensor addresses just moved without a re-split, so the uids no longer + // stand for "nothing has changed" - a backend holding a captured graph has to recheck + for (int i = 0; i < sched->n_splits; i++) { + sched->splits[i].graph.uid = ggml_graph_next_uid(); + } + + if (sched->debug) { + GGML_LOG_DEBUG("%s: rotated %d graph inputs onto copy %d\n", + __func__, sched->n_graph_inputs, sched->cur_copy); + } +} + +void ggml_backend_sched_prepare_inputs(ggml_backend_sched_t sched) { + GGML_ASSERT(sched); + + if (!sched->needs_rotate) { + // nothing is in flight, the inputs can be written where they are + return; + } + + if (sched->n_copies > 1) { + ggml_backend_sched_rotate_inputs(sched); + } else { + // no ring to rotate, so the only way to make the inputs safe to write is to wait + for (int b = 0; b < sched->n_backends; b++) { + ggml_backend_synchronize(sched->backends[b]); + } + } + + sched->needs_rotate = false; +} + bool ggml_backend_sched_alloc_graph(ggml_backend_sched_t sched, struct ggml_cgraph * graph) { GGML_ASSERT(sched); GGML_ASSERT((int)sched->hash_set.size >= graph->n_nodes + graph->n_leafs); GGML_ASSERT(!sched->is_alloc); - sched->cur_copy = sched->next_copy; - sched->next_copy = (sched->next_copy + 1) % sched->n_copies; + // same wait as the reuse path: rotating onto a slot is only safe once its previous reader + // is done, whether the addresses come from re-splitting or from re-pointing by hand + ggml_backend_sched_advance_copy(sched); ggml_backend_sched_split_graph(sched, graph); @@ -2051,6 +2235,12 @@ bool ggml_backend_sched_alloc_graph(ggml_backend_sched_t sched, struct ggml_cgra return false; } + ggml_backend_sched_capture_input_slots(sched); + + // splitting placed the inputs afresh and re-stamped the split uids, so whatever was in + // flight before is no longer a reason to rotate + sched->needs_rotate = false; + sched->is_alloc = true; return true; @@ -2082,6 +2272,9 @@ void ggml_backend_sched_synchronize(ggml_backend_sched_t sched) { for (int i = 0; i < sched->n_backends; i++) { ggml_backend_synchronize(sched->backends[i]); } + + // nothing is reading the graph inputs any more + sched->needs_rotate = false; if (!sched->is_alloc) { // if the graph is not already allocated, always use copy 0 after a synchronization // this ensures that during generation the same copy is used every time, diff --git a/src/llama-context.cpp b/src/llama-context.cpp index 52f8d53672a..391d800bfd0 100644 --- a/src/llama-context.cpp +++ b/src/llama-context.cpp @@ -1339,12 +1339,11 @@ llm_graph_result * llama_context::process_ubatch(const llama_ubatch & ubatch, ll if (!graph_reuse_disable && res->can_reuse(gparams)) { //LLAMA_LOG_DEBUG("%s: reusing previous graph\n", __func__); - // with pipeline parallelism, the previous graph_compute_async may still be running - // on the GPU. we must synchronize before set_inputs to avoid overwriting input tensors - // that the previous compute is still reading. - if (cparams.pipeline_parallel) { - ggml_backend_sched_synchronize(sched.get()); - } + // a previously issued graph_compute_async may still be reading the graph inputs, so they + // cannot simply be overwritten here. this holds whenever a compute can still be in + // flight - under pipeline parallelism, and on a device that computes directly on the + // buffer the inputs live in, where no input copy is made to decouple the two. + ggml_backend_sched_prepare_inputs(sched.get()); n_reused++; } else { From 2dd7ff1ab35b14780017600a3c1bb4566cc2af7f Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Mon, 17 Aug 2026 23:17:29 +0200 Subject: [PATCH 08/18] ggml-backend: document the graph input ring buffer in the scheduler docs The ring buffer arrived as a running commentary spread over the scheduler, the CUDA backend and llama.cpp, which is the wrong place for it: the parts that matter to a caller or to a backend author were only discoverable by reading the implementation, and the parts that only matter to the implementation were repeated at every site that touched it. Move the description into the "Backend scheduler" block in ggml-backend.h as its own section, covering when the ring engages and why, what it costs, when it rotates, and the two obligations it creates - callers must call ggml_backend_sched_prepare_inputs() before writing the inputs of a reused graph, and backends that cache work against a graph must key it on ggml_cgraph::uid. Drop the commentary from the code. Two one-line pointers are kept rather than removed outright, both at places where the invariant is not local and violating it fails silently: the cgraph->uid check in the CUDA backend, and the note that sched->bufts[] is not populated yet where the ring is detected. No functional change. Sanitizer race reports are unchanged on all three models - Qwen3.5 87 -> 0 at n_ubatch 512 and 21 -> 0 at 32, Muse-Glimmer 135 -> 0 - and the reuse path still rotates 9 times on the small ubatch workload. Assisted-By: Claude Opus 5 --- ggml/include/ggml-backend.h | 51 +++++++++++++++++++++++----- ggml/src/ggml-backend.cpp | 59 +-------------------------------- ggml/src/ggml-cuda/ggml-cuda.cu | 14 ++------ src/llama-context.cpp | 4 --- 4 files changed, 45 insertions(+), 83 deletions(-) diff --git a/ggml/include/ggml-backend.h b/ggml/include/ggml-backend.h index 77d0565d83a..e0ae7058723 100644 --- a/ggml/include/ggml-backend.h +++ b/ggml/include/ggml-backend.h @@ -95,10 +95,7 @@ extern "C" { GGML_API void ggml_backend_tensor_get_2d(const struct ggml_tensor * tensor, void * data, size_t offset, size_t size, size_t n_copies, size_t stride_tensor, size_t stride_data); GGML_API void ggml_backend_tensor_memset( struct ggml_tensor * tensor, uint8_t value, size_t offset, size_t size); - // declare an imminent direct write to tensor->data, bypassing ggml_backend_tensor_set. - // callers that write host-visible tensors in place must announce it here, otherwise the - // scheduler sanitizer cannot see the write and will not detect races against it. - // no-op unless GGML_SCHED_SANITIZE is set. + // announce a direct write to tensor->data that bypasses ggml_backend_tensor_set GGML_API void ggml_backend_tensor_set_direct(struct ggml_tensor * tensor, size_t offset, size_t size); GGML_API void ggml_backend_synchronize(ggml_backend_t backend); @@ -310,6 +307,46 @@ extern "C" { } */ + // + // Graph input ring buffer + // + // Graph inputs are assigned to the last backend, which is assumed to be the CPU. The caller may + // pass a device host buffer type for that slot, and some devices - integrated GPUs in + // particular - accept that buffer type for compute. When that happens no input copy is created + // and the device reads exactly the memory the host thread writes. The host must then not write + // the next iteration's inputs while the device is still reading the previous ones. + // + // The scheduler detects this in ggml_backend_sched_new() by testing the condition that elides + // the copy - a host buffer type in the last slot that some other backend accepts - rather than + // by identifying particular devices. Backends that deliberately refuse to compute on pinned + // host memory are therefore unaffected. When it holds, each graph input is given a ring of + // buffers instead of one, and the scheduler moves the inputs onto the next slot rather than + // waiting for the device. GGML_SCHED_UMA_RING overrides the detection: 0 or 1 disables it, a + // larger value sets the ring depth. + // + // The extra buffers are not free. The cost is one additional copy of every graph input per + // extra slot, which for attention masks scales with the context and batch size, so the default + // depth is 2 - enough to remove the wait, at the smallest cost that does. + // + // Inputs are only rotated when a compute may still be in flight. Anything that waits for the + // backends clears that state, so a caller that synchronizes between iterations - reading logits + // in a sampling loop, say - keeps its inputs at fixed addresses and does not pay for the ring. + // Callers that issue several computes without synchronizing rotate between them. + // + // Two obligations come with this: + // + // - Callers must call ggml_backend_sched_prepare_inputs() before writing the inputs of a graph + // that is being reused, since that path allocates nothing and so cannot rotate on its own. + // After ggml_backend_sched_alloc_graph() it is a no-op; allocating already rotated. + // + // - Rotating moves tensor addresses without re-splitting the graph. A backend that caches work + // against a graph, such as the CUDA graph cache, may treat an unchanged ggml_cgraph::uid as a + // promise that nothing it captured has moved, and skip re-reading the addresses. The + // scheduler re-stamps the split uids whenever it re-points inputs; a backend keeping such a + // cache must key it on the uid or re-check the addresses itself. Violating this does not + // degrade gracefully - the cached work replays against stale addresses. + // + typedef struct ggml_backend_sched * ggml_backend_sched_t; // Evaluation callback for each node in the graph (set with ggml_backend_sched_set_eval_callback) @@ -347,11 +384,7 @@ extern "C" { // Allocate and compute graph on the backend scheduler GGML_API bool ggml_backend_sched_alloc_graph(ggml_backend_sched_t sched, struct ggml_cgraph * graph); // returns success - // make the graph inputs safe for the caller to write again. must be called before writing - // inputs for an iteration that reuses an already allocated graph, since a previously issued - // compute may still be reading them. rotates onto a spare set of inputs where one is - // available, and only falls back to waiting when it is not. calling it after - // ggml_backend_sched_alloc_graph() is a no-op - allocating already did this. + // make the graph inputs safe to write again, see "Graph input ring buffer" above GGML_API void ggml_backend_sched_prepare_inputs(ggml_backend_sched_t sched); GGML_API enum ggml_status ggml_backend_sched_graph_compute(ggml_backend_sched_t sched, struct ggml_cgraph * graph); GGML_API enum ggml_status ggml_backend_sched_graph_compute_async(ggml_backend_sched_t sched, struct ggml_cgraph * graph); diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index 0d3a061fee6..edc4bb75290 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -859,17 +859,11 @@ struct ggml_backend_sched { int n_graph_inputs; int graph_inputs_capacity; - // where each graph input's N ring slots live, captured once the allocator has placed them. - // rotating on the graph reuse path means re-pointing the tensor by hand, because that path - // re-runs neither the split nor the allocator - see ggml_backend_sched_rotate_inputs() struct ggml_backend_sched_input_slots { ggml_backend_buffer_t buffer[GGML_SCHED_MAX_COPIES]; void * data [GGML_SCHED_MAX_COPIES]; } * graph_input_slots; // [graph_inputs_capacity] - // a compute has been issued and the graph inputs have not been rotated since. set when the - // splits are computed, cleared by anything that makes it safe to write the inputs again - - // either a rotation or a full synchronization bool needs_rotate; struct ggml_context * ctx; @@ -935,8 +929,6 @@ static void ggml_backend_sched_graph_inputs_grow(ggml_backend_sched_t sched) { sched->graph_inputs_capacity = new_cap; } -// the graph input a tensor ultimately refers to, following view chains, or NULL if it is not -// (a view of) a graph input static struct ggml_tensor * ggml_backend_sched_graph_input(struct ggml_tensor * t) { while (t != NULL) { if (t->flags & GGML_TENSOR_FLAG_INPUT) { @@ -947,9 +939,6 @@ static struct ggml_tensor * ggml_backend_sched_graph_input(struct ggml_tensor * return NULL; } -// re-derive a view's address from its source, the way ggml_gallocr_init_tensor would have. only -// needed after an input has been re-pointed by hand: a view's address is baked at allocation -// time, so moving the tensor it views does not move the view with it static void ggml_backend_sched_reinit_view(struct ggml_tensor * t) { if (t->view_src == NULL) { return; @@ -1469,9 +1458,6 @@ void ggml_backend_sched_split_graph(ggml_backend_sched_t sched, struct ggml_cgra const int src_backend_id = sched->hv_tensor_backend_ids[src_id]; GGML_ASSERT(src_backend_id != -1); // all inputs should be assigned by now - // an input is not always reached directly - the recurrent state copy, for one, is - // only ever read through views of it. the ring has to rotate the input itself, so - // resolve through any views here, otherwise it is never registered and never moves struct ggml_tensor * inp = ggml_backend_sched_graph_input(src); if (inp != NULL && sched->n_copies > 1) { @@ -1639,10 +1625,6 @@ void ggml_backend_sched_split_graph(ggml_backend_sched_t sched, struct ggml_cgra } // set ids for all splits - // [TAG_CUDA_GRAPH_UID] a backend may take an unchanged uid as a promise that the split's - // tensor addresses have not moved, and skip re-checking them. this is only true because - // splitting always precedes (re-)allocation, so anything that re-points tensors without - // re-splitting has to re-stamp these itself. for (int i = 0; i < sched->n_splits; ++i) { sched->splits[i].graph.uid = ggml_graph_next_uid(); } @@ -1908,8 +1890,6 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s ggml_san_split(-1, NULL, 0); - // the device may still be reading the graph inputs, so they must not be written again until - // either the inputs are rotated onto another slot or everything has been waited for sched->needs_rotate = true; return GGML_STATUS_SUCCESS; @@ -1940,15 +1920,7 @@ ggml_backend_sched_t ggml_backend_sched_new( sched->n_backends = n_backends; - // graph inputs are pinned to the last (CPU) backend, and the caller may hand us a device - // host buffer type for that slot. if a compute backend also accepts that buffer type, then - // ggml_backend_sched_buffer_supported() is true for graph inputs, no split input copy is - // made, and the device reads the very memory the host thread writes - so the host cannot - // write the next set of inputs until the device is done reading the previous one. - // - // detect that here so the inputs can be ring buffered instead of synchronized. this tests - // the exact condition that elides the copy rather than guessing at "is this an APU". - // note: sched->bufts[] is not populated until below, so resolve the buffer type directly. + // sched->bufts[] is not populated yet, resolve the buffer type directly bool is_uma = false; if (!parallel && n_backends >= 2) { ggml_backend_buffer_type_t cpu_buft = bufts ? bufts[n_backends - 1] @@ -1966,12 +1938,8 @@ ggml_backend_sched_t ggml_backend_sched_new( } } - // a ring of 2 removes the steady state synchronization just as well as a longer one, at half - // the extra input memory - which on a unified memory device comes out of system RAM int n_copies_uma = is_uma ? 2 : 1; - // GGML_SCHED_UMA_RING overrides the detection above: 0 or 1 forces it off, a larger value - // sets the ring depth. the extra input buffers are not free, so keep an escape hatch. const char * GGML_SCHED_UMA_RING = getenv("GGML_SCHED_UMA_RING"); if (GGML_SCHED_UMA_RING) { n_copies_uma = std::min(std::max(atoi(GGML_SCHED_UMA_RING), 1), GGML_SCHED_MAX_COPIES); @@ -2067,8 +2035,6 @@ void ggml_backend_sched_reset(ggml_backend_sched_t sched) { sched->is_reset = true; } sched->is_alloc = false; - // deliberately not clearing needs_rotate: resetting only drops bookkeeping, it does not wait - // for anything, so a compute issued before this can still be reading the graph inputs } void ggml_backend_sched_reserve_size(ggml_backend_sched_t sched, struct ggml_cgraph * measure_graph, size_t * sizes) { @@ -2102,9 +2068,6 @@ bool ggml_backend_sched_reserve(ggml_backend_sched_t sched, struct ggml_cgraph * return true; } -// record where the allocator put each graph input's ring slots. the tensor sitting in slot -// cur_copy is the user's own tensor (split_graph aliases it there), the rest are placeholders -// that exist only to reserve distinct, non-overlapping memory for the other slots. static void ggml_backend_sched_capture_input_slots(ggml_backend_sched_t sched) { if (sched->n_copies <= 1) { return; @@ -2129,10 +2092,6 @@ static void ggml_backend_sched_capture_input_slots(ggml_backend_sched_t sched) { } } -// step onto the next ring slot, waiting until the device is finished reading it if anything is -// still in flight. the slot being stepped onto was last read n_copies-1 iterations ago, so this -// is normally already signalled - that is the entire point of the ring. the writer we are -// protecting is the host thread, so it has to be a host side wait, not ggml_backend_event_wait(). static void ggml_backend_sched_advance_copy(ggml_backend_sched_t sched) { sched->cur_copy = sched->next_copy; sched->next_copy = (sched->next_copy + 1) % sched->n_copies; @@ -2154,8 +2113,6 @@ static void ggml_backend_sched_advance_copy(ggml_backend_sched_t sched) { } } -// move the graph inputs onto the next ring slot. this is what the graph reuse path needs: it -// re-runs neither the split nor the allocator, so bumping cur_copy on its own would move nothing. static void ggml_backend_sched_rotate_inputs(ggml_backend_sched_t sched) { GGML_ASSERT(sched->n_copies > 1); @@ -2168,15 +2125,10 @@ static void ggml_backend_sched_rotate_inputs(ggml_backend_sched_t sched) { continue; } - // nodes reference inputs by ggml_tensor *, and split_graph does not rewrite node->src[] - // for graph inputs, so re-pointing the tensor is enough to move every reader with it input->buffer = sched->graph_input_slots[i].buffer[sched->cur_copy]; input->data = sched->graph_input_slots[i].data [sched->cur_copy]; } - // views of the inputs still point into the slot they were allocated against, so bring them - // along. only views rooted at a graph input are touched; for anything else this would be - // recomputing the address it already has for (int i = 0; i < sched->graph.n_nodes; i++) { struct ggml_tensor * node = sched->graph.nodes[i]; @@ -2188,8 +2140,6 @@ static void ggml_backend_sched_rotate_inputs(ggml_backend_sched_t sched) { } } - // [TAG_CUDA_GRAPH_UID] tensor addresses just moved without a re-split, so the uids no longer - // stand for "nothing has changed" - a backend holding a captured graph has to recheck for (int i = 0; i < sched->n_splits; i++) { sched->splits[i].graph.uid = ggml_graph_next_uid(); } @@ -2204,14 +2154,12 @@ void ggml_backend_sched_prepare_inputs(ggml_backend_sched_t sched) { GGML_ASSERT(sched); if (!sched->needs_rotate) { - // nothing is in flight, the inputs can be written where they are return; } if (sched->n_copies > 1) { ggml_backend_sched_rotate_inputs(sched); } else { - // no ring to rotate, so the only way to make the inputs safe to write is to wait for (int b = 0; b < sched->n_backends; b++) { ggml_backend_synchronize(sched->backends[b]); } @@ -2225,8 +2173,6 @@ bool ggml_backend_sched_alloc_graph(ggml_backend_sched_t sched, struct ggml_cgra GGML_ASSERT((int)sched->hash_set.size >= graph->n_nodes + graph->n_leafs); GGML_ASSERT(!sched->is_alloc); - // same wait as the reuse path: rotating onto a slot is only safe once its previous reader - // is done, whether the addresses come from re-splitting or from re-pointing by hand ggml_backend_sched_advance_copy(sched); ggml_backend_sched_split_graph(sched, graph); @@ -2237,8 +2183,6 @@ bool ggml_backend_sched_alloc_graph(ggml_backend_sched_t sched, struct ggml_cgra ggml_backend_sched_capture_input_slots(sched); - // splitting placed the inputs afresh and re-stamped the split uids, so whatever was in - // flight before is no longer a reason to rotate sched->needs_rotate = false; sched->is_alloc = true; @@ -2273,7 +2217,6 @@ void ggml_backend_sched_synchronize(ggml_backend_sched_t sched) { ggml_backend_synchronize(sched->backends[i]); } - // nothing is reading the graph inputs any more sched->needs_rotate = false; if (!sched->is_alloc) { // if the graph is not already allocated, always use copy 0 after a synchronization diff --git a/ggml/src/ggml-cuda/ggml-cuda.cu b/ggml/src/ggml-cuda/ggml-cuda.cu index b2b83a44625..1bd65e453bd 100644 --- a/ggml/src/ggml-cuda/ggml-cuda.cu +++ b/ggml/src/ggml-cuda/ggml-cuda.cu @@ -2593,8 +2593,6 @@ static ggml_cuda_graph::node_properties ggml_cuda_graph_node_props(const ggml_te return prop; } -// on in debug builds, opt-in via GGML_CUDA_GRAPH_VERIFY_UID elsewhere - release builds are -// where a ring/pipelining change is actually exercised, so the check has to be reachable there static bool ggml_cuda_graph_verify_uid() { static const bool verify = []() { #ifndef NDEBUG @@ -2606,16 +2604,8 @@ static bool ggml_cuda_graph_verify_uid() { return verify; } -// [TAG_CUDA_GRAPH_UID] -// A non-zero cgraph->uid that has not changed since the last call is a promise from the caller -// that nothing the captured graph baked in has changed either - in particular that no node's -// data pointer, and no src's data pointer, ne or nb, has moved. On that promise we skip the -// property comparison below and replay the existing executable graph as-is. -// -// Breaking the promise does not fall back to a slow path, it replays a graph that reads stale -// addresses, which corrupts results silently. Any caller that re-points tensors without going -// through a re-split (which re-stamps the uid, see ggml_backend_sched_split_graph) must -// invalidate the uid itself. Debug builds verify the promise instead of trusting it. +// an unchanged cgraph->uid promises the caller has not moved anything this graph baked in. +// see "Graph input ring buffer" in ggml-backend.h - breaking it replays stale addresses silently static bool ggml_cuda_graph_update_required(ggml_backend_cuda_context * cuda_ctx, ggml_cgraph * cgraph) { bool res = false; diff --git a/src/llama-context.cpp b/src/llama-context.cpp index 391d800bfd0..766da83630f 100644 --- a/src/llama-context.cpp +++ b/src/llama-context.cpp @@ -1339,10 +1339,6 @@ llm_graph_result * llama_context::process_ubatch(const llama_ubatch & ubatch, ll if (!graph_reuse_disable && res->can_reuse(gparams)) { //LLAMA_LOG_DEBUG("%s: reusing previous graph\n", __func__); - // a previously issued graph_compute_async may still be reading the graph inputs, so they - // cannot simply be overwritten here. this holds whenever a compute can still be in - // flight - under pipeline parallelism, and on a device that computes directly on the - // buffer the inputs live in, where no input copy is made to decouple the two. ggml_backend_sched_prepare_inputs(sched.get()); n_reused++; From c7f83ded6ae873377ada03134b47a324c38ebef1 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Tue, 18 Aug 2026 00:02:53 +0200 Subject: [PATCH 09/18] ggml: report the allocation root in sanitizer race reports A race is reported against the tensor that was accessed, but when that tensor is a view the memory belongs to its root, and the two names can be completely unrelated. Chasing one such report cost a wrong diagnosis and two rebuilds: the report named a recurrent state tensor, while the memory being recycled belonged to an unnamed intermediate several links up the view chain. Name both, as "accessed <- root", so a report points at the allocation that is actually in contention. Assisted-By: Claude Opus 5 --- ggml/src/ggml-backend-sanitize.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/ggml/src/ggml-backend-sanitize.cpp b/ggml/src/ggml-backend-sanitize.cpp index 624ef6c6c87..11fb2c45c59 100644 --- a/ggml/src/ggml-backend-sanitize.cpp +++ b/ggml/src/ggml-backend-sanitize.cpp @@ -44,7 +44,7 @@ struct san_access { uint64_t clock = 0; int split = -1; const char * what = ""; // always a string literal - char tensor[GGML_MAX_NAME] = { 0 }; // copied: source tensor is recycled per graph + char tensor[2*GGML_MAX_NAME + 8] = { 0 }; // copied: source tensor is recycled per graph }; struct san_entry { @@ -302,7 +302,17 @@ bool touch(san_state & s, int a, uint64_t c, const ggml_tensor * t, info.clock = c; info.what = what; info.split = tl_split; - snprintf(info.tensor, sizeof(info.tensor), "%s", t->name); + // report the tensor that owns the memory, not just the one that was accessed - a view's + // name says nothing about which allocation is involved, and the two are easily confused + const ggml_tensor * root = t; + while (root->view_src != NULL) { + root = root->view_src; + } + if (root != t) { + snprintf(info.tensor, sizeof(info.tensor), "%s <- %s", t->name, root->name); + } else { + snprintf(info.tensor, sizeof(info.tensor), "%s", t->name); + } if (ggml_san_level() >= 2) { GGML_LOG_DEBUG("san [split %3d %-10s] %-10s %-5s %s[%zu+%zu] %s (%s)\n", From f543f2305669a1c39d33d22f36bcb5b63483c332 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Tue, 18 Aug 2026 00:02:53 +0200 Subject: [PATCH 10/18] ggml-backend: keep memory read in place by another backend out of the reuse pool The graph allocator frees a tensor's memory once its last consumer in graph order has run, and hands it to a later tensor. That is only sound if the consumers have actually finished. When a backend computes directly on a buffer another backend owns - as an integrated GPU does on host memory - the reading split is asynchronous and is still reading well after graph order says the memory is dead, so a later split on the owning backend writes over it. Give the allocator an explicit pin and apply it to exactly those tensors. The pin is keyed on the view root, since that is the tensor that owns the memory and the one ggml_gallocr_free_node() acts on - keying it on the accessed tensor misses every access that goes through a view, which is most of them here. A dedicated pin rather than the existing GGML_TENSOR_FLAG_OUTPUT: the flag also suppresses fusion in the CUDA and SYCL backends, and the tensors that need pinning here are precisely gated delta net state, so reusing it would have disabled ggml_cuda_try_gdn_cache_fusion() on the models this fixes. It also propagates along view chains, which pins more than intended. Measured on a Strix Halo APU with a recurrent model, sanitizer race reports over partially offloaded layers: -ngl 0 10 20 30 99 before 160 120 60 20 0 after 0 0 0 0 0 Cost is +4.3 MiB of compute buffer, flat in context size, and nothing at all when no split reads across backends in place. GGML_SCHED_PIN_ASYNC_READS=0 disables it. Assisted-By: Claude Opus 5 --- ggml/include/ggml-alloc.h | 7 +++++++ ggml/src/ggml-alloc.c | 30 ++++++++++++++++++++++++++++++ ggml/src/ggml-backend.cpp | 30 ++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+) diff --git a/ggml/include/ggml-alloc.h b/ggml/include/ggml-alloc.h index a7926a21a9a..b8f5d87cb5e 100644 --- a/ggml/include/ggml-alloc.h +++ b/ggml/include/ggml-alloc.h @@ -73,6 +73,13 @@ GGML_API bool ggml_gallocr_alloc_graph(ggml_gallocr_t galloc, struct ggml_cgraph GGML_API size_t ggml_gallocr_get_buffer_size(ggml_gallocr_t galloc, int buffer_id); +// keep a tensor's memory reserved for the whole graph instead of reusing it once its last +// consumer in graph order has run. required when a backend may still be reading the tensor +// asynchronously at that point, which graph order does not express. pins must be re-applied +// after ggml_gallocr_clear_pins() and before reserving. +GGML_API void ggml_gallocr_pin_tensor(ggml_gallocr_t galloc, struct ggml_tensor * t); +GGML_API void ggml_gallocr_clear_pins(ggml_gallocr_t galloc); + // Utils // Create a buffer and allocate all the tensors in a ggml_context // ggml_backend_alloc_ctx_tensors_from_buft_size returns the size of the buffer that would be allocated by ggml_backend_alloc_ctx_tensors_from_buft diff --git a/ggml/src/ggml-alloc.c b/ggml/src/ggml-alloc.c index 3bda9abbe03..d0a8d246e4f 100644 --- a/ggml/src/ggml-alloc.c +++ b/ggml/src/ggml-alloc.c @@ -492,6 +492,9 @@ struct ggml_gallocr { struct leaf_alloc * leaf_allocs; // [n_leafs] int n_leafs; + + struct ggml_hash_set pinned; + bool has_pinned; }; ggml_gallocr_t ggml_gallocr_new_n(ggml_backend_buffer_type_t * bufts, int n_bufs) { @@ -569,6 +572,9 @@ void ggml_gallocr_free(ggml_gallocr_t galloc) { } ggml_hash_set_free(&galloc->hash_set); + if (galloc->pinned.size > 0) { + ggml_hash_set_free(&galloc->pinned); + } free(galloc->hash_values); free(galloc->bufts); free(galloc->buffers); @@ -687,6 +693,25 @@ static void ggml_gallocr_allocate_node(ggml_gallocr_t galloc, struct ggml_tensor } } +void ggml_gallocr_pin_tensor(ggml_gallocr_t galloc, struct ggml_tensor * t) { + GGML_ASSERT(galloc); + if (galloc->pinned.size == 0) { + galloc->pinned = ggml_hash_set_new(GGML_DEFAULT_GRAPH_SIZE); + } + if (ggml_hash_insert(&galloc->pinned, t) == GGML_HASHSET_FULL) { + GGML_ABORT("%s: pinned tensor set is full\n", __func__); + } + galloc->has_pinned = true; +} + +void ggml_gallocr_clear_pins(ggml_gallocr_t galloc) { + GGML_ASSERT(galloc); + if (galloc->pinned.size > 0) { + ggml_hash_set_reset(&galloc->pinned); + } + galloc->has_pinned = false; +} + static void ggml_gallocr_free_node(ggml_gallocr_t galloc, struct ggml_tensor * node) { // graph outputs are never freed if (node->flags & GGML_TENSOR_FLAG_OUTPUT) { @@ -694,6 +719,11 @@ static void ggml_gallocr_free_node(ggml_gallocr_t galloc, struct ggml_tensor * n return; } + if (galloc->has_pinned && ggml_hash_contains(&galloc->pinned, node)) { + AT_PRINTF("not freeing pinned %s\n", node->name); + return; + } + struct hash_node * hn = ggml_gallocr_hash_get(galloc, node); int buffer_id = hn->buffer_id; struct ggml_dyn_tallocr * alloc = galloc->buf_tallocs[buffer_id]; diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index edc4bb75290..6192e50de16 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -1142,6 +1142,7 @@ void ggml_backend_sched_split_graph(ggml_backend_sched_t sched, struct ggml_cgra // reset splits sched->n_splits = 0; sched->n_graph_inputs = 0; + ggml_gallocr_clear_pins(sched->galloc); sched->is_reset = false; struct ggml_init_params params = { @@ -1487,6 +1488,35 @@ void ggml_backend_sched_split_graph(ggml_backend_sched_t sched, struct ggml_cgra } } + // this split may read src in place out of a buffer another backend owns. graph + // order says the memory dies at its last consumer, but an asynchronous reader is + // still reading past that point, so it must not be handed to a later split. the + // memory belongs to the view root, and the root is what the allocator frees + { + struct ggml_tensor * root = src; + while (root->view_src != NULL) { + root = root->view_src; + } + + const int root_backend_id = sched->hv_tensor_backend_ids[hash_id(root)]; + + static const bool pin_async_reads = []() { + const char * env = getenv("GGML_SCHED_PIN_ASYNC_READS"); + return env ? atoi(env) != 0 : true; + }(); + + if (pin_async_reads && + root_backend_id != -1 && root_backend_id != cur_backend_id && + sched->backends[cur_backend_id]->iface.synchronize != NULL && + ggml_backend_sched_buffer_supported(sched, root, cur_backend_id)) { + ggml_gallocr_pin_tensor(sched->galloc, root); + if (sched->debug) { + GGML_LOG_DEBUG("%s: pinned %s, read in place by %s\n", __func__, + root->name, ggml_backend_name(sched->backends[cur_backend_id])); + } + } + } + if (src_backend_id != cur_backend_id && !ggml_backend_sched_buffer_supported(sched, src, cur_backend_id)) { // create a copy of the input in the split's backend if (tensor_id_copy(src_id, cur_backend_id, 0) == NULL) { From 9dea828371472368b4403d0ac0a47ef766157496 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Tue, 18 Aug 2026 00:39:18 +0200 Subject: [PATCH 11/18] ggml-backend: run ops that alias their source on the source's backend An op whose result aliases one of its sources - ggml_set() and friends, where the destination is a view of src0 - writes through to that source. Pass 4 already says "views are always on the same backend as the source", but only applies it to nodes that are still unassigned; passes 2 and 3 can have moved an aliasing node to another backend before that. Pass 5 then finds a source on a different backend than the split, substitutes a copy for it, and the write lands in the copy. The copy is never written back, so the update is discarded. Nothing detects this. The graph computes, every op runs, and the result is quietly wrong. It is only reachable when the placement puts such a node across a backend boundary, which on a recurrent model means the state carried between tokens stops being updated and generation degenerates into a repeated token. Enforce the invariant for nodes that alias their source and are not pure view ops. Found on a Strix Halo APU with a hybrid recurrent model at partial offload, where every one of the 13 split input copies fed a SET: -ngl 0 8 16 24 30 99 before garbage ... ... ... ... ok after ok ok ok ok ok ok The trigger is the backend accepting another's buffers, which changes placement: forcing devices[].integrated = false, as the CUDA path already does, also avoids it. That makes it reachable today on HIP integrated GPUs and on any backend that computes on buffers it does not own. The fix moves nothing at full offload, and at partial offload it moves only the SET nodes - 13 at -ngl 16 - removing the copies that fed them. Assisted-By: Claude Opus 5 --- ggml/src/ggml-backend.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index 6192e50de16..30b83cbfe4d 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -1346,6 +1346,23 @@ void ggml_backend_sched_split_graph(ggml_backend_sched_t sched, struct ggml_cgra *cur_backend_id = tensor_backend_id(node->view_src); SET_CAUSE(node, "4.vsrc"); } + + // an op that is not a pure view but still aliases its source writes through to it, so it + // has to run where that source lives. earlier passes may have moved it elsewhere, and + // pass 5 would then substitute a copy for the source - leaving the write in the copy, + // which is discarded. the write is silently lost, the graph still computes + if (node->view_src != NULL && !ggml_is_view_op(node->op)) { + const int view_src_backend_id = tensor_backend_id(node->view_src); + if (view_src_backend_id != -1 && *cur_backend_id != view_src_backend_id) { + if (sched->debug) { + GGML_LOG_DEBUG("%s: %s (%s) moved to %s to keep it with the tensor it aliases (%s)\n", + __func__, node->name, ggml_op_name(node->op), + ggml_backend_name(sched->backends[view_src_backend_id]), node->view_src->name); + } + *cur_backend_id = view_src_backend_id; + SET_CAUSE(node, "4.alias"); + } + } for (int j = 0; j < GGML_MAX_SRC; j++) { struct ggml_tensor * src = node->src[j]; if (src == NULL) { From 20c6c5be8c8d3cbcfb2114fb51cb7106c8187078 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Tue, 18 Aug 2026 10:59:08 +0200 Subject: [PATCH 12/18] docs: document the backend scheduler The scheduler's documentation was a prose block at the top of the sched section in ggml-backend.h, and the mechanisms added since were commented at the sites that implement them. Neither is somewhere a reader looks to find out how the scheduler works, and the header block had grown past what belongs in a header. Add docs/development/backend-scheduler.md covering backend assignment, splits, allocation and graph reuse, and then the parts that exist because the reuse path runs neither the split nor the allocator: the graph input ring buffer and its two contracts, pinning memory that another backend reads in place, and the placement rule for ops that alias their source. Document the sanitizer with it, since every one of those is an ordering rule and the sanitizer is how they are checked. Move the header block and its usage example there and drop the commentary from the code, leaving a pointer at the three places where the invariant is not local: the cgraph uid check in the CUDA backend, llama_host_write(), and the sched section of the header. No functional change - race reports are unchanged at 87/0 with the ring off and on, 0 at partial offload, and partial offload output is still correct. Assisted-By: Claude Opus 5 --- docs/development/backend-scheduler.md | 216 ++++++++++++++++++++++++++ ggml/include/ggml-alloc.h | 4 - ggml/include/ggml-backend.h | 83 +--------- ggml/src/ggml-backend-sanitize.cpp | 5 - ggml/src/ggml-backend.cpp | 9 -- ggml/src/ggml-cuda/ggml-cuda.cu | 3 +- src/llama-impl.h | 4 +- 7 files changed, 219 insertions(+), 105 deletions(-) create mode 100644 docs/development/backend-scheduler.md diff --git a/docs/development/backend-scheduler.md b/docs/development/backend-scheduler.md new file mode 100644 index 00000000000..b902e4a6412 --- /dev/null +++ b/docs/development/backend-scheduler.md @@ -0,0 +1,216 @@ +# The backend scheduler + +`ggml_backend_sched` lets one graph run across several backend devices. It assigns every node to +a backend, allocates the compute buffers, inserts the copies needed where a node's operands live +on another device, and executes the result. + +Callers interact with it through `ggml-backend.h`; this document describes what it does and the +contracts it places on callers and on backends. + +## Contents + +- [Assigning nodes to backends](#assigning-nodes-to-backends) +- [Splits](#splits) +- [Allocation and graph reuse](#allocation-and-graph-reuse) +- [Graph input ring buffer](#graph-input-ring-buffer) +- [Memory read in place by another backend](#memory-read-in-place-by-another-backend) +- [Ops that alias their source](#ops-that-alias-their-source) +- [The scheduler sanitizer](#the-scheduler-sanitizer) +- [Environment variables](#environment-variables) + +## Assigning nodes to backends + +Backends are passed in priority order, highest first, and the last one must be the CPU. A node is +assigned by, in order: + +1. **Where its data already is.** A node whose tensor is already allocated, or which is a view of + an allocated tensor, must run on the backend owning that buffer - it cannot be moved. +2. **Op support and operand location.** Otherwise the highest priority backend that supports the + op is used, preferring the backend holding the operands. Ops reading tensors in a buffer marked + `GGML_BACKEND_BUFFER_USAGE_WEIGHTS` prefer that buffer's backend, so that weights are not + copied. +3. **Expansion.** Assignments are then expanded along the graph, so that runs of adjacent nodes + end up on the same backend rather than alternating and forcing a copy at every step. +4. **Graph inputs** (`GGML_TENSOR_FLAG_INPUT`) are assigned to the last backend, which is the CPU. + The caller writes them, so they have to live somewhere the host can write directly. + +`ggml_backend_sched_set_tensor_backend()` overrides the choice for a specific tensor. + +## Splits + +Once every node has a backend, the graph is cut into **splits**: maximal runs of consecutive nodes +sharing one backend. Each split is submitted to its backend as a single graph, in order. + +Where a split reads a tensor produced on another backend, the scheduler checks whether the +consuming backend can read that buffer directly (`ggml_backend_supports_buft`). If it can, the +tensor is read where it is. If it cannot, a copy is created in the consuming backend's buffer and +the node's operand is repointed at the copy - these are the tensors named `##`. + +Copies are issued asynchronously where the backends support it, with events ordering the consumer +behind the producer. + +## Allocation and graph reuse + +Compute buffers are sized by `ggml_backend_sched_reserve()` with a worst case graph, so that later +graphs of the same shape need no reallocation. Within a graph the allocator reuses memory: a +tensor's memory is handed to a later tensor once its last consumer *in graph order* has run. + +A caller that rebuilds an identical graph can skip the split and the allocation entirely and reuse +the previous one - llama.cpp does this whenever the graph topology is unchanged. This is fast, but +it means neither the split nor the allocator runs, so anything that normally happens there does not +happen on that path. The mechanisms below exist because of that. + +## Graph input ring buffer + +Graph inputs are assigned to the CPU backend, but the caller may pass a **device host buffer type** +for that slot - pinned memory owned by a GPU. Some devices, integrated GPUs in particular, accept +that buffer type for compute. When that happens no input copy is created and the device reads +exactly the memory the host thread writes. + +Nothing then stops the host writing the next iteration's inputs while the device is still reading +the previous ones. The writes are a plain `memcpy` on the calling thread; they are not ordered +against an in-flight `ggml_backend_graph_compute_async`. + +The scheduler detects this in `ggml_backend_sched_new()` by testing the condition that elides the +copy - a host buffer type in the last slot that some other backend accepts - rather than by +identifying particular devices. Backends that deliberately refuse to compute on pinned host memory +are unaffected. + +When it holds, each graph input gets a **ring of buffers** instead of one, and the scheduler moves +the inputs onto the next slot rather than waiting for the device. The slot being stepped onto was +last read `n_copies - 1` iterations ago, so the wait for it is normally already satisfied. + +The extra buffers are not free: one additional copy of every graph input per extra slot, which for +attention masks scales with context and batch size. The default depth is 2 - enough to remove the +wait at the smallest cost that does. + +**Rotation only happens when a compute may still be in flight.** Anything that waits for the +backends clears that state, so a caller that synchronizes between iterations - reading logits in a +sampling loop, say - keeps its inputs at fixed addresses and never pays for the ring. Callers that +issue several computes without synchronizing rotate between them. + +Two obligations come with this: + +- **Callers** must call `ggml_backend_sched_prepare_inputs()` before writing the inputs of a graph + that is being reused. That path allocates nothing and so cannot rotate on its own. After + `ggml_backend_sched_alloc_graph()` it is a no-op - allocating already rotated. + +- **Backends** that cache work against a graph, such as the CUDA graph cache, may treat an + unchanged `ggml_cgraph::uid` as a promise that nothing the cached work captured has moved, and + skip re-reading the addresses. Rotating moves tensor addresses without re-splitting, so the + scheduler re-stamps the split uids whenever it re-points inputs. A backend keeping such a cache + must key it on the uid or re-check the addresses itself. Violating this does not degrade + gracefully: the cached work replays against stale addresses and the results are quietly wrong. + +## Memory read in place by another backend + +The allocator frees a tensor's memory once its last consumer in graph order has run. That is only +sound if the consumers have finished. When a split reads a tensor in place out of a buffer another +backend owns, the reading split is asynchronous and may still be reading well after graph order +says the memory is dead - so a later split on the owning backend can be given the same memory and +overwrite it. + +Such tensors are pinned for the lifetime of the graph with `ggml_gallocr_pin_tensor()`, keeping +them out of the reuse pool. The pin is keyed on the **view root**, since that is the tensor that +owns the memory and the one the allocator frees; keying it on the accessed tensor would miss every +access that goes through a view. + +## Ops that alias their source + +Some ops write through to one of their sources rather than to fresh memory: `ggml_set()` and +friends, where the result is a view of `src0`. Such a node must run on the backend where that +source lives. + +If it does not, the split logic sees an operand on another backend, substitutes a copy for it, and +the op writes into the copy. The copy is never written back, so the update is silently discarded - +the graph still computes, every op still runs, and the result is wrong. On a recurrent model this +shows up as state that stops being carried between tokens, and generation degenerates into a +repeated token. + +The scheduler therefore assigns any node that aliases its source, and is not a pure view op, to +the backend of the tensor it aliases. + +## The scheduler sanitizer + +The above are ordering rules, and ordering bugs do not announce themselves - they produce wrong +numbers on some runs and not others. The sanitizer is a happens-before checker for the backend +API, built into `ggml-base` and enabled at runtime. + +It maintains a vector clock per actor - the host thread and each backend - and a shadow map of +which actor last read or wrote every byte of every buffer. Synchronization points (backend +synchronize, event record, event wait, event synchronize, async copies) advance those clocks. When +an access conflicts with a previous one and no happens-before edge connects them, it reports: + +``` +ggml-sched-sanitize: RACE (write-after-read) on ROCm_Host[0, 2048) +ggml-sched-sanitize: read ROCm0#2 @3 split 0 inp_tokens (compute) +ggml-sched-sanitize: write HOST @15 split -1 inp_tokens (tensor_set) +ggml-sched-sanitize: no happens-before edge: HOST knows ROCm0#2@2, needs >=3 +``` + +Where the access is through a view, the report names both the accessed tensor and the allocation +that owns the memory, as `accessed <- root`. The two are frequently unrelated, and the root is the +one that matters. + +**It only sees what goes through the backend API.** A caller that writes a host-visible tensor by +taking `tensor->data` and memcpy'ing into it is invisible to it, and races against such writes are +missed entirely. Callers doing that must announce it with `ggml_backend_tensor_set_direct()`, +which is a no-op unless the sanitizer is enabled. In llama.cpp every such site goes through +`llama_host_write()`. + +By default the first race aborts. `GGML_SCHED_SANITIZE_NONFATAL=1` reports each one and continues, +so a single run enumerates a whole workload; a count is printed at exit. + +## Environment variables + +| variable | effect | +|---|---| +| `GGML_SCHED_DEBUG` | `1` prints assignments and scheduler decisions, `2` adds per-node detail | +| `GGML_SCHED_DEBUG_REALLOC` | report, or abort on, unexpected graph reallocations | +| `GGML_SCHED_UMA_RING` | override ring detection: `0`/`1` disables, larger sets the depth | +| `GGML_SCHED_PIN_ASYNC_READS` | `0` disables pinning of memory read in place by another backend | +| `GGML_SCHED_SANITIZE` | `1` enables the sanitizer, `2` also traces synchronization edges | +| `GGML_SCHED_SANITIZE_NONFATAL` | `1` reports every race instead of aborting on the first | + +## Example usage + +```c +// operations that use tensors allocated in a buffer with USAGE_WEIGHTS will be assigned +// preferably to run on the same backend as the buffer +ggml_backend_buffer_set_usage(buf_weights, GGML_BACKEND_BUFFER_USAGE_WEIGHTS); + +sched = ggml_backend_sched_new({backend_gpu, backend_gpu2, backend_cpu}, NULL, num_backends, + GGML_DEFAULT_GRAPH_SIZE, false, true); + +// initialize buffers from a max size graph (optional) +reserve_graph = build_graph(sched, max_batch_size); + +// manually assign nodes to a backend (optional, should not be needed in most cases) +struct ggml_tensor * node = ggml_mul_mat(ctx, ...); +ggml_backend_sched_set_tensor_backend(sched, node, backend_gpu); + +ggml_backend_sched_reserve(sched, reserve_graph); + +// compute +// the graph and its tensors are single-use in terms of allocation, multi-use in terms of computation +graph = build_graph(sched); +for (int i = 0; i < 10; ++i) { + ggml_backend_sched_graph_compute(sched, graph); // on the first iteration the graph is allocated automatically +} + +// if there are graph inputs: +graph = build_graph(sched); // a new graph that is not allocated +ggml_backend_sched_reset(sched); // clear the allocation of the previous graph +ggml_backend_sched_alloc_graph(sched, graph); +ggml_backend_tensor_set(input_tensor, ...); +ggml_backend_sched_graph_compute(sched, graph); + +// when reusing an already allocated graph rather than allocating a new one, the inputs must be +// made safe to write first - see "Graph input ring buffer" +ggml_backend_sched_prepare_inputs(sched); +ggml_backend_tensor_set(input_tensor, ...); +ggml_backend_sched_graph_compute(sched, graph); +``` + +An alternative to the above is to assign the inputs to a dedicated context and allocate them +statically with `ggml_backend_alloc_ctx_tensors()`. diff --git a/ggml/include/ggml-alloc.h b/ggml/include/ggml-alloc.h index b8f5d87cb5e..2b5084cb40a 100644 --- a/ggml/include/ggml-alloc.h +++ b/ggml/include/ggml-alloc.h @@ -73,10 +73,6 @@ GGML_API bool ggml_gallocr_alloc_graph(ggml_gallocr_t galloc, struct ggml_cgraph GGML_API size_t ggml_gallocr_get_buffer_size(ggml_gallocr_t galloc, int buffer_id); -// keep a tensor's memory reserved for the whole graph instead of reusing it once its last -// consumer in graph order has run. required when a backend may still be reading the tensor -// asynchronously at that point, which graph order does not express. pins must be re-applied -// after ggml_gallocr_clear_pins() and before reserving. GGML_API void ggml_gallocr_pin_tensor(ggml_gallocr_t galloc, struct ggml_tensor * t); GGML_API void ggml_gallocr_clear_pins(ggml_gallocr_t galloc); diff --git a/ggml/include/ggml-backend.h b/ggml/include/ggml-backend.h index e0ae7058723..68ec8901124 100644 --- a/ggml/include/ggml-backend.h +++ b/ggml/include/ggml-backend.h @@ -95,7 +95,6 @@ extern "C" { GGML_API void ggml_backend_tensor_get_2d(const struct ggml_tensor * tensor, void * data, size_t offset, size_t size, size_t n_copies, size_t stride_tensor, size_t stride_data); GGML_API void ggml_backend_tensor_memset( struct ggml_tensor * tensor, uint8_t value, size_t offset, size_t size); - // announce a direct write to tensor->data that bypasses ggml_backend_tensor_set GGML_API void ggml_backend_tensor_set_direct(struct ggml_tensor * tensor, size_t offset, size_t size); GGML_API void ggml_backend_synchronize(ggml_backend_t backend); @@ -266,86 +265,7 @@ extern "C" { // Backend scheduler // - // The backend scheduler allows for multiple backend devices to be used together - // Handles compute buffer allocation, assignment of tensors to backends, and copying of tensors between backends - // The backends are selected based on: - // - the backend that supports the operation - // - the location of the pre-allocated tensors (e.g. the weights) - /* - Example usage: - - // operations that use tensors allocated in a buffer with USAGE_WEIGHTS will be assigned - // preferably to run on the same backend as the buffer - ggml_backend_buffer_set_usage(buf_weights, GGML_BACKEND_BUFFER_USAGE_WEIGHTS); - - sched = ggml_backend_sched_new({backend_gpu, backend_gpu2, backend_cpu}, NULL, num_backends, GGML_DEFAULT_GRAPH_SIZE, false, true); - - // initialize buffers from a max size graph (optional) - reserve_graph = build_graph(sched, max_batch_size); - - // manually assign nodes to a backend (optional, should not be needed in most cases) - struct ggml_tensor * node = ggml_mul_mat(ctx, ...); - ggml_backend_sched_set_tensor_backend(sched, node, backend_gpu); - - ggml_backend_sched_reserve(sched, reserve_graph); - - // compute - graph = build_graph(sched); // the graph and its tensors are single-use in terms of allocation, multi-use in terms of computation - for (int i = 0; i < 10; ++i) { - ggml_backend_sched_graph_compute(sched, graph); // on the first iteration the graph is allocated automatically - } - - // if there are graph inputs: - graph = build_graph(sched); // get a new graph that is not allocated (the metadata for the old graph is freed once ggml_free is called) - ggml_backend_sched_reset(sched); // clear the allocation of the previous graph - ggml_backend_sched_alloc_graph(sched, graph); // explicitly allocate the new graph but do not execute it - ggml_backend_tensor_set(input_tensor, ...); // copy data to the newly allocated graph tensors - ggml_backend_sched_graph_compute(sched, graph); // execute the graph - - // as an alternative to the above it is also possible to assign the inputs to a dedicated context and - // allocate them statically via ggml_backend_alloc_ctx_tensors - } - */ - - // - // Graph input ring buffer - // - // Graph inputs are assigned to the last backend, which is assumed to be the CPU. The caller may - // pass a device host buffer type for that slot, and some devices - integrated GPUs in - // particular - accept that buffer type for compute. When that happens no input copy is created - // and the device reads exactly the memory the host thread writes. The host must then not write - // the next iteration's inputs while the device is still reading the previous ones. - // - // The scheduler detects this in ggml_backend_sched_new() by testing the condition that elides - // the copy - a host buffer type in the last slot that some other backend accepts - rather than - // by identifying particular devices. Backends that deliberately refuse to compute on pinned - // host memory are therefore unaffected. When it holds, each graph input is given a ring of - // buffers instead of one, and the scheduler moves the inputs onto the next slot rather than - // waiting for the device. GGML_SCHED_UMA_RING overrides the detection: 0 or 1 disables it, a - // larger value sets the ring depth. - // - // The extra buffers are not free. The cost is one additional copy of every graph input per - // extra slot, which for attention masks scales with the context and batch size, so the default - // depth is 2 - enough to remove the wait, at the smallest cost that does. - // - // Inputs are only rotated when a compute may still be in flight. Anything that waits for the - // backends clears that state, so a caller that synchronizes between iterations - reading logits - // in a sampling loop, say - keeps its inputs at fixed addresses and does not pay for the ring. - // Callers that issue several computes without synchronizing rotate between them. - // - // Two obligations come with this: - // - // - Callers must call ggml_backend_sched_prepare_inputs() before writing the inputs of a graph - // that is being reused, since that path allocates nothing and so cannot rotate on its own. - // After ggml_backend_sched_alloc_graph() it is a no-op; allocating already rotated. - // - // - Rotating moves tensor addresses without re-splitting the graph. A backend that caches work - // against a graph, such as the CUDA graph cache, may treat an unchanged ggml_cgraph::uid as a - // promise that nothing it captured has moved, and skip re-reading the addresses. The - // scheduler re-stamps the split uids whenever it re-points inputs; a backend keeping such a - // cache must key it on the uid or re-check the addresses itself. Violating this does not - // degrade gracefully - the cached work replays against stale addresses. - // + // see docs/development/backend-scheduler.md typedef struct ggml_backend_sched * ggml_backend_sched_t; @@ -384,7 +304,6 @@ extern "C" { // Allocate and compute graph on the backend scheduler GGML_API bool ggml_backend_sched_alloc_graph(ggml_backend_sched_t sched, struct ggml_cgraph * graph); // returns success - // make the graph inputs safe to write again, see "Graph input ring buffer" above GGML_API void ggml_backend_sched_prepare_inputs(ggml_backend_sched_t sched); GGML_API enum ggml_status ggml_backend_sched_graph_compute(ggml_backend_sched_t sched, struct ggml_cgraph * graph); GGML_API enum ggml_status ggml_backend_sched_graph_compute_async(ggml_backend_sched_t sched, struct ggml_cgraph * graph); diff --git a/ggml/src/ggml-backend-sanitize.cpp b/ggml/src/ggml-backend-sanitize.cpp index 11fb2c45c59..f9b37742e17 100644 --- a/ggml/src/ggml-backend-sanitize.cpp +++ b/ggml/src/ggml-backend-sanitize.cpp @@ -21,9 +21,6 @@ int ggml_san_level(void) { return level; } -// GGML_SCHED_SANITIZE_NONFATAL=1 reports every distinct race and keeps going, instead of -// aborting on the first one. the shadow state is updated after a report, so a reported -// range does not keep firing - this lets a single run enumerate all races in a workload. static int ggml_san_nonfatal(void) { static const int nonfatal = []() { const char * env = getenv("GGML_SCHED_SANITIZE_NONFATAL"); @@ -302,8 +299,6 @@ bool touch(san_state & s, int a, uint64_t c, const ggml_tensor * t, info.clock = c; info.what = what; info.split = tl_split; - // report the tensor that owns the memory, not just the one that was accessed - a view's - // name says nothing about which allocation is involved, and the two are easily confused const ggml_tensor * root = t; while (root->view_src != NULL) { root = root->view_src; diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index 30b83cbfe4d..cfc95a8c68c 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -1347,10 +1347,6 @@ void ggml_backend_sched_split_graph(ggml_backend_sched_t sched, struct ggml_cgra SET_CAUSE(node, "4.vsrc"); } - // an op that is not a pure view but still aliases its source writes through to it, so it - // has to run where that source lives. earlier passes may have moved it elsewhere, and - // pass 5 would then substitute a copy for the source - leaving the write in the copy, - // which is discarded. the write is silently lost, the graph still computes if (node->view_src != NULL && !ggml_is_view_op(node->op)) { const int view_src_backend_id = tensor_backend_id(node->view_src); if (view_src_backend_id != -1 && *cur_backend_id != view_src_backend_id) { @@ -1505,10 +1501,6 @@ void ggml_backend_sched_split_graph(ggml_backend_sched_t sched, struct ggml_cgra } } - // this split may read src in place out of a buffer another backend owns. graph - // order says the memory dies at its last consumer, but an asynchronous reader is - // still reading past that point, so it must not be handed to a later split. the - // memory belongs to the view root, and the root is what the allocator frees { struct ggml_tensor * root = src; while (root->view_src != NULL) { @@ -1967,7 +1959,6 @@ ggml_backend_sched_t ggml_backend_sched_new( sched->n_backends = n_backends; - // sched->bufts[] is not populated yet, resolve the buffer type directly bool is_uma = false; if (!parallel && n_backends >= 2) { ggml_backend_buffer_type_t cpu_buft = bufts ? bufts[n_backends - 1] diff --git a/ggml/src/ggml-cuda/ggml-cuda.cu b/ggml/src/ggml-cuda/ggml-cuda.cu index 1bd65e453bd..91f0bf1ff3e 100644 --- a/ggml/src/ggml-cuda/ggml-cuda.cu +++ b/ggml/src/ggml-cuda/ggml-cuda.cu @@ -2604,8 +2604,7 @@ static bool ggml_cuda_graph_verify_uid() { return verify; } -// an unchanged cgraph->uid promises the caller has not moved anything this graph baked in. -// see "Graph input ring buffer" in ggml-backend.h - breaking it replays stale addresses silently +// see docs/development/backend-scheduler.md static bool ggml_cuda_graph_update_required(ggml_backend_cuda_context * cuda_ctx, ggml_cgraph * cgraph) { bool res = false; diff --git a/src/llama-impl.h b/src/llama-impl.h index 7202d8d9360..b938f801c17 100644 --- a/src/llama-impl.h +++ b/src/llama-impl.h @@ -94,9 +94,7 @@ struct buffer_view { } }; -// announce that the whole of `t` is about to be written in place via t->data, bypassing -// ggml_backend_tensor_set. every direct write to a graph input must go through this, or the -// scheduler sanitizer cannot see the write and will silently miss races against it. +// see docs/development/backend-scheduler.md static inline void llama_host_write(struct ggml_tensor * t) { GGML_ASSERT(ggml_backend_buffer_is_host(t->buffer)); ggml_backend_tensor_set_direct(t, 0, ggml_nbytes(t)); From 5db5750cadbbcc2f99f934a417a34c9a4fc1fe12 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Tue, 18 Aug 2026 14:14:13 +0200 Subject: [PATCH 13/18] ggml-backend: do not ring buffer graph inputs for a CPU-only scheduler Two problems, both reached by test-opt, which builds a scheduler with the backend under test in front of the full backend list - so for the CPU device that is a CPU backend in front of the CPU backend. The detection asked whether any backend other than the last accepts the last backend's buffer type. A CPU backend trivially accepts CPU memory, so a CPU-only scheduler was treated as a device computing on host memory and given a ring it has no use for. Skip CPU devices: the ring exists for a non-CPU device reading memory the host writes. That alone was a segfault rather than wasted memory. The tensor aliased by the ring sat at slot cur_copy, so which tensor occupied a given leaf index changed from one allocation to the next. ggml_gallocr_needs_realloc() compares node and leaf counts and sizes but never identity, so it saw no reason to reallocate, and a leaf_alloc recorded for a pre-allocated tensor (buffer_id -1) was applied to a placeholder that still needed allocating, indexing galloc->buffers out of bounds. Keep the aliased tensor at slot 0 so leaf identity is stable, and rotate purely by re-pointing the inputs, which both paths now do explicitly. This is what the graph reuse path already relied on; the allocate path was getting rotation as a side effect of where split_graph placed the alias. Also stop GGML_SCHED_UMA_RING from switching the ring on where it was not detected. It is an escape hatch for a scheduler that has one, not a way to impose one on a caller that does not meet its contract - ggml_opt keeps its inputs across separate allocations and does not. Sanitizer race reports are unchanged: 94/0 with the ring off and on, 0 at partial offload, reuse path still rotating, output correct at full and partial offload. Assisted-By: Claude Opus 5 --- docs/development/backend-scheduler.md | 2 +- ggml/src/ggml-backend.cpp | 24 +++++++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/docs/development/backend-scheduler.md b/docs/development/backend-scheduler.md index b902e4a6412..ae11d63a1d2 100644 --- a/docs/development/backend-scheduler.md +++ b/docs/development/backend-scheduler.md @@ -167,7 +167,7 @@ so a single run enumerates a whole workload; a count is printed at exit. |---|---| | `GGML_SCHED_DEBUG` | `1` prints assignments and scheduler decisions, `2` adds per-node detail | | `GGML_SCHED_DEBUG_REALLOC` | report, or abort on, unexpected graph reallocations | -| `GGML_SCHED_UMA_RING` | override ring detection: `0`/`1` disables, larger sets the depth | +| `GGML_SCHED_UMA_RING` | when the ring is in use: `0`/`1` disables it, larger sets the depth. cannot enable it where it was not detected | | `GGML_SCHED_PIN_ASYNC_READS` | `0` disables pinning of memory read in place by another backend | | `GGML_SCHED_SANITIZE` | `1` enables the sanitizer, `2` also traces synchronization edges | | `GGML_SCHED_SANITIZE_NONFATAL` | `1` reports every race instead of aborting on the first | diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index cfc95a8c68c..cf8dd17fe20 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -1482,7 +1482,7 @@ void ggml_backend_sched_split_graph(ggml_backend_sched_t sched, struct ggml_cgra ggml_backend_t backend = sched->backends[inp_backend_id]; for (int c = 0; c < sched->n_copies; c++) { struct ggml_tensor * tensor_copy; - if (c == sched->cur_copy) { + if (c == 0) { tensor_copy = inp; // use the original tensor as the current copy } else { tensor_copy = ggml_dup_tensor_layout(sched->ctx, inp); @@ -1966,6 +1966,9 @@ ggml_backend_sched_t ggml_backend_sched_new( if (cpu_buft && ggml_backend_buft_is_host(cpu_buft)) { for (int b = 0; b < n_backends - 1; b++) { + if (ggml_backend_dev_type(ggml_backend_get_device(backends[b])) == GGML_BACKEND_DEVICE_TYPE_CPU) { + continue; + } if (ggml_backend_supports_buft(backends[b], cpu_buft)) { is_uma = true; GGML_LOG_DEBUG("%s: %s computes directly on %s, ring buffering graph inputs\n", @@ -1979,7 +1982,7 @@ ggml_backend_sched_t ggml_backend_sched_new( int n_copies_uma = is_uma ? 2 : 1; const char * GGML_SCHED_UMA_RING = getenv("GGML_SCHED_UMA_RING"); - if (GGML_SCHED_UMA_RING) { + if (GGML_SCHED_UMA_RING && is_uma) { n_copies_uma = std::min(std::max(atoi(GGML_SCHED_UMA_RING), 1), GGML_SCHED_MAX_COPIES); } @@ -2151,11 +2154,7 @@ static void ggml_backend_sched_advance_copy(ggml_backend_sched_t sched) { } } -static void ggml_backend_sched_rotate_inputs(ggml_backend_sched_t sched) { - GGML_ASSERT(sched->n_copies > 1); - - ggml_backend_sched_advance_copy(sched); - +static void ggml_backend_sched_point_inputs(ggml_backend_sched_t sched) { for (int i = 0; i < sched->n_graph_inputs; i++) { struct ggml_tensor * input = sched->graph_inputs[i]; @@ -2188,6 +2187,13 @@ static void ggml_backend_sched_rotate_inputs(ggml_backend_sched_t sched) { } } +static void ggml_backend_sched_rotate_inputs(ggml_backend_sched_t sched) { + GGML_ASSERT(sched->n_copies > 1); + + ggml_backend_sched_advance_copy(sched); + ggml_backend_sched_point_inputs(sched); +} + void ggml_backend_sched_prepare_inputs(ggml_backend_sched_t sched) { GGML_ASSERT(sched); @@ -2221,6 +2227,10 @@ bool ggml_backend_sched_alloc_graph(ggml_backend_sched_t sched, struct ggml_cgra ggml_backend_sched_capture_input_slots(sched); + if (sched->n_copies > 1) { + ggml_backend_sched_point_inputs(sched); + } + sched->needs_rotate = false; sched->is_alloc = true; From c8d05db2e0c0ec190fa909240cab3ca7c6ecddfc Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Tue, 18 Aug 2026 17:00:00 +0200 Subject: [PATCH 14/18] ggml-backend: only ring buffer graph inputs for an asynchronous backend The detection excluded CPU devices, but BLAS is an accelerator device that computes on host memory, so a BLAS plus CPU scheduler was still given a ring. ggml_opt keeps its inputs across separate allocations and does not meet the ring's contract, so test-opt went to 73/118 with it enabled. Excluding device types one at a time is the wrong shape. The hazard is a reader that is still reading after graph_compute returns, so require the backend to be asynchronous. That covers CPU and BLAS together, and anything else synchronous: their work is finished before control comes back, and nothing of theirs can be reading the inputs the host is about to overwrite. Verified against a BLAS build of test-opt, which is the configuration that failed: 118/118, ring never enabled. The ring is still enabled for ROCm and its race reports are unchanged at 94/0 with it off and on, 0 at partial offload. Assisted-By: Claude Opus 5 --- docs/development/backend-scheduler.md | 6 ++++-- ggml/src/ggml-backend.cpp | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/development/backend-scheduler.md b/docs/development/backend-scheduler.md index ae11d63a1d2..a091fa73813 100644 --- a/docs/development/backend-scheduler.md +++ b/docs/development/backend-scheduler.md @@ -73,8 +73,10 @@ against an in-flight `ggml_backend_graph_compute_async`. The scheduler detects this in `ggml_backend_sched_new()` by testing the condition that elides the copy - a host buffer type in the last slot that some other backend accepts - rather than by -identifying particular devices. Backends that deliberately refuse to compute on pinned host memory -are unaffected. +identifying particular devices, and requiring that backend to be asynchronous. Backends that +deliberately refuse to compute on pinned host memory are unaffected, and so are synchronous ones +such as BLAS - their work is finished before `graph_compute` returns, so nothing of theirs can +still be reading. When it holds, each graph input gets a **ring of buffers** instead of one, and the scheduler moves the inputs onto the next slot rather than waiting for the device. The slot being stepped onto was diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index cf8dd17fe20..c2b33218f9b 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -1966,7 +1966,9 @@ ggml_backend_sched_t ggml_backend_sched_new( if (cpu_buft && ggml_backend_buft_is_host(cpu_buft)) { for (int b = 0; b < n_backends - 1; b++) { - if (ggml_backend_dev_type(ggml_backend_get_device(backends[b])) == GGML_BACKEND_DEVICE_TYPE_CPU) { + ggml_backend_dev_props props; + ggml_backend_dev_get_props(ggml_backend_get_device(backends[b]), &props); + if (!props.caps.async) { continue; } if (ggml_backend_supports_buft(backends[b], cpu_buft)) { From 508d9a2c44f0e930b3c897e7e7ad17ccc0540b62 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Tue, 18 Aug 2026 17:25:42 +0200 Subject: [PATCH 15/18] ggml-backend: only move an aliasing op to a backend that supports it Pass 4 assigns a node that aliases its source to the backend owning that source, so the write reaches the aliased memory instead of a discarded copy. It did so unconditionally. Support for these ops can be conditional on the tensor types - CUDA runs GGML_OP_SET only for F32 and I32, and OpenCL has no GGML_OP_ACC at all - so the backend that owns the aliased memory is not guaranteed to be able to run the op writing into it. Forcing the node there hands the backend an op it rejects, which aborts in ggml_backend_graph_compute. There is no placement that computes such a graph correctly: operands are copied into a split, results are never copied out, so the write cannot reach the aliased memory from any other backend. Check support before moving, and leave the node where the earlier passes put it otherwise - the behaviour before this rule existed. The declined move is logged under GGML_SCHED_DEBUG. Measured on Qwen3.5-4B at -ngl 16, the case the rule was added for: 13 moves before and after, 0 declined, output unchanged. Races still 0 with the ring on and 1 with it off, and test-opt passes 46/46. Assisted-By: Claude Opus 5 --- docs/development/backend-scheduler.md | 8 ++++++++ ggml/src/ggml-backend.cpp | 18 ++++++++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/docs/development/backend-scheduler.md b/docs/development/backend-scheduler.md index a091fa73813..7b58ebbcada 100644 --- a/docs/development/backend-scheduler.md +++ b/docs/development/backend-scheduler.md @@ -132,6 +132,14 @@ repeated token. The scheduler therefore assigns any node that aliases its source, and is not a pure view op, to the backend of the tensor it aliases. +That move is only made when the target backend supports the op. Support can be conditional on +the tensor types - CUDA runs `GGML_OP_SET` only for F32 and I32 - so the backend owning the +aliased memory is not guaranteed to be able to run the op writing into it. There is no correct +placement in that case: the scheduler copies operands into a split, never results out of one, so +whichever backend runs the op, the write cannot reach the aliased memory. The node is left where +the earlier passes put it, which is what the scheduler did before this rule existed, and the +reason is logged under `GGML_SCHED_DEBUG`. + ## The scheduler sanitizer The above are ordering rules, and ordering bugs do not announce themselves - they produce wrong diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index c2b33218f9b..76cad56cd55 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -1350,13 +1350,19 @@ void ggml_backend_sched_split_graph(ggml_backend_sched_t sched, struct ggml_cgra if (node->view_src != NULL && !ggml_is_view_op(node->op)) { const int view_src_backend_id = tensor_backend_id(node->view_src); if (view_src_backend_id != -1 && *cur_backend_id != view_src_backend_id) { - if (sched->debug) { - GGML_LOG_DEBUG("%s: %s (%s) moved to %s to keep it with the tensor it aliases (%s)\n", - __func__, node->name, ggml_op_name(node->op), - ggml_backend_name(sched->backends[view_src_backend_id]), node->view_src->name); + if (ggml_backend_supports_op(sched->backends[view_src_backend_id], node)) { + if (sched->debug) { + GGML_LOG_DEBUG("%s: %s (%s) moved to %s to keep it with the tensor it aliases (%s)\n", + __func__, node->name, ggml_op_name(node->op), + ggml_backend_name(sched->backends[view_src_backend_id]), node->view_src->name); + } + *cur_backend_id = view_src_backend_id; + SET_CAUSE(node, "4.alias"); + } else if (sched->debug) { + GGML_LOG_DEBUG("%s: %s (%s) aliases %s on %s, which cannot run it - the write will be lost\n", + __func__, node->name, ggml_op_name(node->op), node->view_src->name, + ggml_backend_name(sched->backends[view_src_backend_id])); } - *cur_backend_id = view_src_backend_id; - SET_CAUSE(node, "4.alias"); } } for (int j = 0; j < GGML_MAX_SRC; j++) { From 1486aa733daa5d6f48d84040b7f5f1b0340c43d6 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Thu, 20 Aug 2026 11:16:16 +0200 Subject: [PATCH 16/18] ggml: harden scheduler ring allocation Assisted-by: Codex --- docs/development/backend-scheduler.md | 36 +++++------- ggml/src/ggml-alloc.c | 17 +++++- ggml/src/ggml-backend.cpp | 13 +---- tests/CMakeLists.txt | 1 + tests/test-alloc.cpp | 37 +++++++++++++ tests/test-backend-sched-ring.cpp | 80 +++++++++++++++++++++++++++ 6 files changed, 151 insertions(+), 33 deletions(-) create mode 100644 tests/test-backend-sched-ring.cpp diff --git a/docs/development/backend-scheduler.md b/docs/development/backend-scheduler.md index 7b58ebbcada..7da85902d63 100644 --- a/docs/development/backend-scheduler.md +++ b/docs/development/backend-scheduler.md @@ -20,21 +20,16 @@ contracts it places on callers and on backends. ## Assigning nodes to backends -Backends are passed in priority order, highest first, and the last one must be the CPU. A node is -assigned by, in order: - -1. **Where its data already is.** A node whose tensor is already allocated, or which is a view of - an allocated tensor, must run on the backend owning that buffer - it cannot be moved. -2. **Op support and operand location.** Otherwise the highest priority backend that supports the - op is used, preferring the backend holding the operands. Ops reading tensors in a buffer marked - `GGML_BACKEND_BUFFER_USAGE_WEIGHTS` prefer that buffer's backend, so that weights are not - copied. -3. **Expansion.** Assignments are then expanded along the graph, so that runs of adjacent nodes - end up on the same backend rather than alternating and forcing a copy at every step. -4. **Graph inputs** (`GGML_TENSOR_FLAG_INPUT`) are assigned to the last backend, which is the CPU. - The caller writes them, so they have to live somewhere the host can write directly. - -`ggml_backend_sched_set_tensor_backend()` overrides the choice for a specific tensor. +The caller defines backend priority by the order of the array passed to `ggml_backend_sched_new()`: lower indices have higher priority, and the last backend must be the CPU. In llama.cpp this array contains the selected model devices in their configured order, then accelerator backends such as BLAS, then the CPU. + +Placement combines that order with hard constraints and locality heuristics: + +1. An assignment made with `ggml_backend_sched_set_tensor_backend()` is retained. +2. A preallocated tensor or view must use a backend that supports both its existing buffer type and its operation. The first compatible backend in caller order is selected; the allocation itself cannot move. +3. An unallocated graph input (`GGML_TENSOR_FLAG_INPUT`) is assigned to the last backend so that the caller can write it through the CPU slot. +4. Operations using weights prefer the first backend that can use the weight buffer and run the operation, avoiding a weight copy. Selected operations may instead be offloaded to an earlier backend when `op_offload` requests it. +5. Existing non-CPU assignments are expanded forward and backward through adjacent supported operations. Remaining assignments are expanded after that so contiguous runs stay together instead of alternating backends. +6. An unassigned operation is placed on the backend supporting the largest number of its assigned inputs. Caller order breaks ties. An assigned operation may be upgraded to an earlier backend when both use the same buffer type and that backend supports the operation and every source buffer. ## Splits @@ -93,9 +88,7 @@ issue several computes without synchronizing rotate between them. Two obligations come with this: -- **Callers** must call `ggml_backend_sched_prepare_inputs()` before writing the inputs of a graph - that is being reused. That path allocates nothing and so cannot rotate on its own. After - `ggml_backend_sched_alloc_graph()` it is a no-op - allocating already rotated. +- **Callers** must call `ggml_backend_sched_prepare_inputs()` before writing the inputs of a graph that is being reused. That path allocates nothing and so cannot rotate on its own. The function has an effect only when the scheduler has multiple copies and a compute may still be in flight; it is a no-op for single-copy schedulers, after `ggml_backend_sched_alloc_graph()`, and after synchronization. - **Backends** that cache work against a graph, such as the CUDA graph cache, may treat an unchanged `ggml_cgraph::uid` as a promise that nothing the cached work captured has moved, and @@ -112,10 +105,9 @@ backend owns, the reading split is asynchronous and may still be reading well af says the memory is dead - so a later split on the owning backend can be given the same memory and overwrite it. -Such tensors are pinned for the lifetime of the graph with `ggml_gallocr_pin_tensor()`, keeping -them out of the reuse pool. The pin is keyed on the **view root**, since that is the tensor that -owns the memory and the one the allocator frees; keying it on the accessed tensor would miss every -access that goes through a view. +Such tensors are pinned for the lifetime of the graph with `ggml_gallocr_pin_tensor()`, preventing both ordinary reuse after their last graph-order consumer and in-place reuse by a child operation. The pin is keyed on the **view root**, since that is the tensor that owns the memory; the allocator resolves candidate in-place parents to the same root. Keying the pin on the accessed tensor would miss every access through a view. + +Pins are recomputed when the scheduler splits a graph and are encoded in the resulting allocation, so graph reuse preserves the protected addresses without rerunning the allocator. They are separate from `GGML_TENSOR_FLAG_OUTPUT`: marking a tensor as an output would also extend its allocator lifetime, but it changes tensor semantics and can disable backend optimizations unrelated to the cross-backend read. ## Ops that alias their source diff --git a/ggml/src/ggml-alloc.c b/ggml/src/ggml-alloc.c index d0a8d246e4f..d4bd10dbdfa 100644 --- a/ggml/src/ggml-alloc.c +++ b/ggml/src/ggml-alloc.c @@ -595,6 +595,16 @@ static bool ggml_gallocr_is_own(ggml_gallocr_t galloc, struct ggml_tensor * t) { return ggml_gallocr_hash_get(galloc, t)->allocated; } +static bool ggml_gallocr_is_pinned(ggml_gallocr_t galloc, struct ggml_tensor * t) { + if (!galloc->has_pinned) { + return false; + } + while (t->view_src != NULL) { + t = t->view_src; + } + return ggml_hash_contains(&galloc->pinned, t); +} + static bool ggml_gallocr_is_allocated(ggml_gallocr_t galloc, struct ggml_tensor * t) { return t->data != NULL // tensor data already set externally || t->buffer // tensor on external buffer (but not yet allocated) @@ -647,6 +657,11 @@ static void ggml_gallocr_allocate_node(ggml_gallocr_t galloc, struct ggml_tensor continue; } + if (ggml_gallocr_is_pinned(galloc, parent)) { + AT_PRINTF("not reusing parent %s for %s as it is pinned\n", parent->name, node->name); + continue; + } + // outputs cannot be reused if (parent->flags & GGML_TENSOR_FLAG_OUTPUT || (parent->view_src != NULL && parent->view_src->flags & GGML_TENSOR_FLAG_OUTPUT)) { AT_PRINTF("not reusing parent %s for %s as it is an output\n", parent->name, node->name); @@ -719,7 +734,7 @@ static void ggml_gallocr_free_node(ggml_gallocr_t galloc, struct ggml_tensor * n return; } - if (galloc->has_pinned && ggml_hash_contains(&galloc->pinned, node)) { + if (ggml_gallocr_is_pinned(galloc, node)) { AT_PRINTF("not freeing pinned %s\n", node->name); return; } diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index 76cad56cd55..677ab259eab 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -1935,7 +1935,7 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s ggml_san_split(-1, NULL, 0); - sched->needs_rotate = true; + sched->needs_rotate = sched->n_copies > 1; return GGML_STATUS_SUCCESS; } @@ -2205,18 +2205,11 @@ static void ggml_backend_sched_rotate_inputs(ggml_backend_sched_t sched) { void ggml_backend_sched_prepare_inputs(ggml_backend_sched_t sched) { GGML_ASSERT(sched); - if (!sched->needs_rotate) { + if (sched->n_copies <= 1 || !sched->needs_rotate) { return; } - if (sched->n_copies > 1) { - ggml_backend_sched_rotate_inputs(sched); - } else { - for (int b = 0; b < sched->n_backends; b++) { - ggml_backend_synchronize(sched->backends[b]); - } - } - + ggml_backend_sched_rotate_inputs(sched); sched->needs_rotate = false; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e517d2c6359..91cc3180229 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -346,6 +346,7 @@ if (NOT LLAMA_USE_SYSTEM_GGML) # Needs non-public ggml{,-backend}-impl.h llama_build_and_test(test-alloc.cpp) + llama_build_and_test(test-backend-sched-ring.cpp) endif() llama_build(test-export-graph-ops.cpp) diff --git a/tests/test-alloc.cpp b/tests/test-alloc.cpp index 6d5428493e7..192cc2f0b7f 100644 --- a/tests/test-alloc.cpp +++ b/tests/test-alloc.cpp @@ -583,6 +583,41 @@ static void test_reallocation() { } } +static void test_pinned_no_inplace() { + dummy_backend backend = dummy_backend_init(SIZE_MAX); + auto [ctx, graph, ctx_ptr] = make_context(); + + ggml_tensor * input = make_input_1d(ctx, 4); + ggml_tensor * parent = ggml_scale(ctx, input, 2.0f); + ggml_tensor * child = ggml_scale(ctx, parent, 3.0f); + + ggml_set_output(child); + ggml_build_forward_expand(graph, child); + + ggml_gallocr_ptr galloc(ggml_gallocr_new(&backend.buffer_type)); + ggml_gallocr_pin_tensor(galloc.get(), parent); + GGML_ASSERT(ggml_gallocr_alloc_graph(galloc.get(), graph)); + GGML_ASSERT(!memory_overlap(parent, child)); +} + +static void test_pinned_view_root_no_inplace() { + dummy_backend backend = dummy_backend_init(SIZE_MAX); + auto [ctx, graph, ctx_ptr] = make_context(); + + ggml_tensor * input = make_input_1d(ctx, 4); + ggml_tensor * root = ggml_scale(ctx, input, 2.0f); + ggml_tensor * view = ggml_view_1d(ctx, root, 4, 0); + ggml_tensor * child = ggml_scale(ctx, view, 3.0f); + + ggml_set_output(child); + ggml_build_forward_expand(graph, child); + + ggml_gallocr_ptr galloc(ggml_gallocr_new(&backend.buffer_type)); + ggml_gallocr_pin_tensor(galloc.get(), root); + GGML_ASSERT(ggml_gallocr_alloc_graph(galloc.get(), graph)); + GGML_ASSERT(!memory_overlap(root, child)); +} + static void run(const char * name, void (*f)()) { printf("%s ", name); fflush(stdout); @@ -604,5 +639,7 @@ int main() { run("test_multiple_buffer_types", test_multiple_buffer_types); run("test_buffer_size_zero", test_buffer_size_zero); run("test_reallocation", test_reallocation); + run("test_pinned_no_inplace", test_pinned_no_inplace); + run("test_pinned_view_root_no_inplace", test_pinned_view_root_no_inplace); return 0; } diff --git a/tests/test-backend-sched-ring.cpp b/tests/test-backend-sched-ring.cpp new file mode 100644 index 00000000000..7047233206b --- /dev/null +++ b/tests/test-backend-sched-ring.cpp @@ -0,0 +1,80 @@ +#include "ggml-backend.h" +#include "../ggml/src/ggml-backend-impl.h" +#include "ggml-cpp.h" +#include "ggml.h" + +struct test_backend_context { + int synchronize_count = 0; +}; + +static const char * test_backend_name(ggml_backend_t) { + return "test"; +} + +static void test_backend_synchronize(ggml_backend_t backend) { + auto * context = static_cast(backend->context); + context->synchronize_count++; +} + +static ggml_status test_backend_graph_compute(ggml_backend_t, ggml_cgraph *) { + return GGML_STATUS_SUCCESS; +} + +static const char * test_device_name(ggml_backend_dev_t) { + return "test"; +} + +static enum ggml_backend_dev_type test_device_type(ggml_backend_dev_t) { + return GGML_BACKEND_DEVICE_TYPE_CPU; +} + +static bool test_device_supports_op(ggml_backend_dev_t, const ggml_tensor *) { + return true; +} + +static bool test_device_supports_buft(ggml_backend_dev_t, ggml_backend_buffer_type_t buft) { + return buft == ggml_backend_cpu_buffer_type(); +} + +int main() { + test_backend_context context; + + ggml_backend_device device = {}; + device.iface.get_name = test_device_name; + device.iface.get_type = test_device_type; + device.iface.supports_op = test_device_supports_op; + device.iface.supports_buft = test_device_supports_buft; + + ggml_backend backend = {}; + backend.iface.get_name = test_backend_name; + backend.iface.synchronize = test_backend_synchronize; + backend.iface.graph_compute = test_backend_graph_compute; + backend.device = &device; + backend.context = &context; + + ggml_backend_t backends[] = { &backend }; + ggml_backend_buffer_type_t bufts[] = { ggml_backend_cpu_buffer_type() }; + ggml_backend_sched_ptr sched(ggml_backend_sched_new(backends, bufts, 1, 16, false, false)); + + ggml_init_params params = {}; + params.mem_size = 4*ggml_tensor_overhead() + ggml_graph_overhead(); + params.no_alloc = true; + ggml_context_ptr ctx(ggml_init(params)); + + ggml_tensor * input = ggml_new_tensor_1d(ctx.get(), GGML_TYPE_F32, 4); + ggml_set_input(input); + ggml_tensor * output = ggml_scale(ctx.get(), input, 2.0f); + ggml_set_output(output); + + ggml_cgraph * graph = ggml_new_graph(ctx.get()); + ggml_build_forward_expand(graph, output); + + GGML_ASSERT(ggml_backend_sched_alloc_graph(sched.get(), graph)); + GGML_ASSERT(ggml_backend_sched_graph_compute_async(sched.get(), graph) == GGML_STATUS_SUCCESS); + GGML_ASSERT(context.synchronize_count == 0); + + ggml_backend_sched_prepare_inputs(sched.get()); + GGML_ASSERT(context.synchronize_count == 0); + + return 0; +} From b3823d816748eeeb6bd9859f8650b3f08ef739c5 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Thu, 20 Aug 2026 15:46:08 +0200 Subject: [PATCH 17/18] cuda: simplify graph uid verification setting Assisted-by: Codex --- ggml/src/ggml-cuda/ggml-cuda.cu | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ggml/src/ggml-cuda/ggml-cuda.cu b/ggml/src/ggml-cuda/ggml-cuda.cu index 91f0bf1ff3e..bf06b215b7c 100644 --- a/ggml/src/ggml-cuda/ggml-cuda.cu +++ b/ggml/src/ggml-cuda/ggml-cuda.cu @@ -2594,14 +2594,12 @@ static ggml_cuda_graph::node_properties ggml_cuda_graph_node_props(const ggml_te } static bool ggml_cuda_graph_verify_uid() { - static const bool verify = []() { #ifndef NDEBUG - return true; + return true; #else - return getenv("GGML_CUDA_GRAPH_VERIFY_UID") != nullptr; -#endif - }(); + static const bool verify = getenv("GGML_CUDA_GRAPH_VERIFY_UID") != nullptr; return verify; +#endif } // see docs/development/backend-scheduler.md From c530ea79c753573c98796ebfaccb1aed8f5897b5 Mon Sep 17 00:00:00 2001 From: Piotr Wilkin Date: Wed, 26 Aug 2026 19:19:46 +0200 Subject: [PATCH 18/18] docs: clarify asynchronous output lifetime Assisted-by: Codex --- docs/development/backend-scheduler.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/development/backend-scheduler.md b/docs/development/backend-scheduler.md index 7da85902d63..b56816fe4f7 100644 --- a/docs/development/backend-scheduler.md +++ b/docs/development/backend-scheduler.md @@ -13,6 +13,7 @@ contracts it places on callers and on backends. - [Splits](#splits) - [Allocation and graph reuse](#allocation-and-graph-reuse) - [Graph input ring buffer](#graph-input-ring-buffer) +- [Output lifetime under asynchronous execution](#output-lifetime-under-asynchronous-execution) - [Memory read in place by another backend](#memory-read-in-place-by-another-backend) - [Ops that alias their source](#ops-that-alias-their-source) - [The scheduler sanitizer](#the-scheduler-sanitizer) @@ -97,6 +98,24 @@ Two obligations come with this: must key it on the uid or re-check the addresses itself. Violating this does not degrade gracefully: the cached work replays against stale addresses and the results are quietly wrong. +## Output lifetime under asynchronous execution + +The scheduler ring versions graph inputs, not graph outputs. `GGML_TENSOR_FLAG_OUTPUT` keeps a tensor's allocation alive until the end of its graph; it does not create one result allocation per asynchronous submission. Reusing a graph can therefore write its output tensors again. + +This does not require an output ring when each output is consumed in submission order. Operations submitted to one backend are ordered, so a copy or downstream consumer queued after one graph runs before a later graph on that backend can overwrite the same output allocation. Internal backend streams must join or otherwise preserve that order before subsequent operations use the result. + +llama.cpp queues every required output readback immediately after each micro-batch and before it submits the next one. Each readback writes to that micro-batch's rows in the persistent host output buffer. The terminal backend therefore sees: + +``` +compute A -> readback A -> compute B -> readback B +``` + +Pipeline stages can overlap on different backends, but micro-batch B cannot overtake micro-batch A on the terminal backend. Different execution times do not change the queue order. If asynchronous readback is unsupported, the backend API synchronizes before copying instead. The public llama.cpp result accessors also synchronize before exposing host result pointers. + +This differs from graph input preparation: the host writes the next inputs directly, outside the backend queue, so no ordering edge protects an input allocation without synchronization or rotation. + +A scheduler caller that needs every asynchronous result must enqueue an ordered copy or consumer before another submission can overwrite the output, or synchronize before reusing the graph. If several results must remain resident on a device at the same time, the caller must provide distinct output storage. `n_copies` does not provide output snapshots. + ## Memory read in place by another backend The allocator frees a tensor's memory once its last consumer in graph order has run. That is only