From 999c58eeef98d778a69899b1eb55a73ade7fcbad Mon Sep 17 00:00:00 2001 From: Georgi Gerganov Date: Mon, 21 Sep 2026 09:12:01 +0300 Subject: [PATCH] ggml : address review comments on alloc_buffer_n - restore GGML_LOG_ERROR on buffer alloc / tensor init failure in the default impl (name the failing tensor) - check the malloc result and drop the _impl indirection in ggml_backend_alloc_ctx_tensors_from_buft - remove comments that restate the code - fix the TAG_ALLOC_SHARED_BUFFER_SPLIT typo Assisted-by: pi:llama.cpp/Qwen3.8-27B --- ggml/src/ggml-alloc.c | 13 +++++-------- ggml/src/ggml-backend.cpp | 15 ++++++++------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/ggml/src/ggml-alloc.c b/ggml/src/ggml-alloc.c index ca8b9f935d..f1415cb872 100644 --- a/ggml/src/ggml-alloc.c +++ b/ggml/src/ggml-alloc.c @@ -1117,11 +1117,9 @@ size_t ggml_gallocr_get_buffer_size(ggml_gallocr_t galloc, int buffer_id) { // utils -static ggml_backend_buffer_t ggml_backend_alloc_ctx_tensors_from_buft_impl( - struct ggml_context * ctx, ggml_backend_buffer_type_t buft) { +ggml_backend_buffer_t ggml_backend_alloc_ctx_tensors_from_buft(struct ggml_context * ctx, ggml_backend_buffer_type_t buft) { GGML_ASSERT(ggml_get_no_alloc(ctx) == true); - // collect tensors into a list int n_tensors = 0; for (struct ggml_tensor * t = ggml_get_first_tensor(ctx); t != NULL; t = ggml_get_next_tensor(ctx, t)) { n_tensors++; @@ -1131,6 +1129,9 @@ static ggml_backend_buffer_t ggml_backend_alloc_ctx_tensors_from_buft_impl( } struct ggml_tensor ** tensors = (struct ggml_tensor **) malloc(n_tensors * sizeof(struct ggml_tensor *)); + if (tensors == NULL) { + return NULL; + } int i = 0; for (struct ggml_tensor * t = ggml_get_first_tensor(ctx); t != NULL; t = ggml_get_next_tensor(ctx, t)) { tensors[i++] = t; @@ -1141,11 +1142,7 @@ static ggml_backend_buffer_t ggml_backend_alloc_ctx_tensors_from_buft_impl( return buffer; } -ggml_backend_buffer_t ggml_backend_alloc_ctx_tensors_from_buft(struct ggml_context * ctx, ggml_backend_buffer_type_t buft) { - return ggml_backend_alloc_ctx_tensors_from_buft_impl(ctx, buft); -} - -// TODO [TAG_ALLOC_SAHRED_BUFFER_SPLIT]: reuse shared buffer-splitting logic from ggml_backend_buft_alloc_buffer_n_default +// TODO [TAG_ALLOC_SHARED_BUFFER_SPLIT]: reuse shared buffer-splitting logic from ggml_backend_buft_alloc_buffer_n_default size_t ggml_backend_alloc_ctx_tensors_from_buft_size(struct ggml_context * ctx, ggml_backend_buffer_type_t buft) { GGML_ASSERT(ggml_get_no_alloc(ctx) == true); diff --git a/ggml/src/ggml-backend.cpp b/ggml/src/ggml-backend.cpp index 98005aaed9..950dee4baf 100644 --- a/ggml/src/ggml-backend.cpp +++ b/ggml/src/ggml-backend.cpp @@ -45,7 +45,7 @@ ggml_backend_buffer_t ggml_backend_buft_alloc_buffer(ggml_backend_buffer_type_t return buft->iface.alloc_buffer(buft, size); } -// TODO [TAG_ALLOC_SAHRED_BUFFER_SPLIT]: extract shared buffer-splitting logic with ggml_backend_alloc_ctx_tensors_from_buft_size +// TODO [TAG_ALLOC_SHARED_BUFFER_SPLIT]: extract shared buffer-splitting logic with ggml_backend_alloc_ctx_tensors_from_buft_size // default implementation of alloc_buffer_n // allocates tensors from a list into one or more buffers of the given type static ggml_backend_buffer_t ggml_backend_buft_alloc_buffer_n_default(ggml_backend_buffer_type_t buft, struct ggml_tensor ** tensors, int n_tensors) { @@ -69,9 +69,9 @@ static ggml_backend_buffer_t ggml_backend_buft_alloc_buffer_n_default(ggml_backe // flush the current buffer if adding this tensor would exceed max_size, or if we are at the end bool should_flush = (i == n_tensors) || (cur_buf_size > 0 && (cur_buf_size + this_size) > max_size); if (should_flush && cur_buf_size > 0) { - // allocate the buffer with the computed size for this range ggml_backend_buffer_t buffer = ggml_backend_buft_alloc_buffer(buft, cur_buf_size); if (buffer == NULL) { + GGML_LOG_ERROR("%s: failed to allocate %s buffer of size %zu\n", __func__, ggml_backend_buft_name(buft), cur_buf_size); for (size_t b = 0; b < n_buffers; b++) { ggml_backend_buffer_free(buffers[b]); } @@ -81,18 +81,18 @@ static ggml_backend_buffer_t ggml_backend_buft_alloc_buffer_n_default(ggml_backe struct ggml_tallocr tallocr = ggml_tallocr_new(buffer); // allocate tensors in the current buffer - bool ok = true; + struct ggml_tensor * t_failed = NULL; for (int j = first; j < i; j++) { struct ggml_tensor * t = tensors[j]; if (t->data == NULL) { if (t->view_src == NULL) { if (ggml_tallocr_alloc(&tallocr, t) != GGML_STATUS_SUCCESS) { - ok = false; + t_failed = t; break; } } else if (t->buffer == NULL) { if (ggml_backend_view_init(t) != GGML_STATUS_SUCCESS) { - ok = false; + t_failed = t; break; } } @@ -100,13 +100,14 @@ static ggml_backend_buffer_t ggml_backend_buft_alloc_buffer_n_default(ggml_backe if (t->view_src != NULL && t->buffer == NULL) { // view of a pre-allocated tensor if (ggml_backend_view_init(t) != GGML_STATUS_SUCCESS) { - ok = false; + t_failed = t; break; } } } } - if (!ok) { + if (t_failed != NULL) { + GGML_LOG_ERROR("%s: failed to initialize tensor %s\n", __func__, t_failed->name); for (size_t b = 0; b < n_buffers; b++) { ggml_backend_buffer_free(buffers[b]); }