[bokrosbalint]

Overview

Clear CUDA graphs after the memory pool returns buffers to the driver to ensure they're recaptured, thus avoid writing to a stale pointer during replay from a captured graph leading to a page fault. Fixes #26738.

As far as I can tell, this has been a latent bug that the changes introduced in #22155 made possible to happen, but it hasn't surfaced as the combination of graphs being enabled, no VMM, no FA, MoE with partial CPU offload and enough fragmentation to trigger it is pretty obscure. On my setup, after #22254 in addition to no VMM, graphs were also enabled, thus it only needed to meet the last three conditions to fail.

Additional information

On the following hardware:

Intel i7-8700K
AMD RX Vega 56, gfx900, 8 GB HBM2 VRAM
32 GB DDR4 RAM

running on top of ROCm 7.2.4

and the following build options:

HIPCXX="$(hipconfig -l)/clang" HIP_PATH="$(hipconfig -p)" \
cmake -S . -B build -G Ninja \
  -DCMAKE_BUILD_TYPE=Release \
  -DGGML_HIP=ON \
  -DGPU_TARGETS=gfx900 \
  -DLLAMA_BUILD_TESTS=OFF \
  -DLLAMA_BUILD_EXAMPLES=OFF

Without the fix applied, the following run:

./build/bin/llama-bench -v \
  -m "$MODEL" \
  --device ROCm0 -ngl 99 -ncmoe 20 -lm none \
  -fa 0 -d 16384 -r 3 -o csv

where MODEL is ggml-org/gemma-4-26B-A4B-it-GGUF:Q4_0, fails with the following:

ggml_backend_cuda_graph_compute: CUDA graph warmup complete
ggml_backend_cuda_graph_compute: CUDA graph warmup complete
ROCm pool[0]: alloc of 109.20 MiB failed, flushing 619.77 MiB of cached buffers and retrying
ROCm pool[0]: retry succeeded
Memory access fault by GPU node-1 (Agent handle: 0x5557320b7210) on address 0x7f5b91802000. Reason: Page not present or supervisor privilege.

With the fix applied, the same run completes, with the recapture visibly doing its job whenever the pool flushes (first flush below):

ggml_backend_cuda_graph_compute: CUDA graph warmup complete
ROCm pool[0]: alloc of 109.20 MiB failed, flushing 619.77 MiB of cached buffers and retrying
ROCm pool[0]: memory freed, 74 captured CUDA graphs must be captured again
ROCm pool[0]: retry succeeded
ggml_backend_cuda_graph_compute: CUDA graph warmup complete

All 5 flushes in the run:

alloc failed	cached flushed	captured graphs dropped
109.20 MiB	619.77 MiB	74
142.80 MiB	512.67 MiB	37
168.00 MiB	479.07 MiB	37
218.40 MiB	579.87 MiB	64
252.00 MiB	479.07 MiB	37

If I push it towards -d 32768 with the same configuration otherwise, it simply runs out of memory:

ggml_backend_cuda_buffer_type_alloc_buffer: allocating 1293.14 MiB on device 0: cudaMalloc failed: out of memory
ggml_gallocr_reserve_n_impl: failed to allocate ROCm0 buffer of size 1355952384
graph_reserve: failed to allocate compute buffers
llama_init_from_model: failed to initialize the context: failed to allocate compute pp buffers

Performance with the fix applied vs the previous GGML_CUDA_DISABLE_GRAPHS=1 workaround (same binary, env toggle only, -d 16384 -r 3):

test	with graphs (fix)	GGML_CUDA_DISABLE_GRAPHS=1
pp512	216.72 ± 1.42	217.28 ± 1.97
tg128	22.08 ± 0.03	22.14 ± 0.02

so no regression from keeping graphs enabled.

ggml_cuda_pool_leg stores ggml_backend_cuda_context & ctx. The pool is owned by the context through unique_ptr, so the reference cannot outlive the context.

Invalidation is done in two places:

1. Alloc-failure path, after clear_pool(). This is the flush that triggers Page fault at depth with -fa 0 and MoE expert offload on HIP #26738.
2. "Buffer pool full" path in free(), after cudaFree. Frees a buffer that a captured graph can still use, which is the same bug class. It needs all 256 slots to be in use, and I couldn't find a way to trigger this path, so this has not been tested in practice, though I believe it to be correct.

The graphs are invalidated per context, so the global over-invalidation across devices from the initial counter-based implementation is gone.

This is reproducible on similar Nvidia hardware with GGML_CUDA_NO_VMM=1 set to ensure the legacy pool implementation is used, although I can't verify it myself.

The contributing guidelines say a regression test should be added, but I'm not exactly sure if one can be implemented for this.

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: YES, I used Claude Opus 5 and 5.5 to help tracking down the root cause and draft the initial version of the submitted patch.

[bokrosbalint]

Rebased on latest master, re-ran the reproducer against master and the fix branch, updated description and added some context on why recaptures jump to 118 from 5.

[JohannesGaessler]

Sorry for the long radio silence, I had taken a break from llama.cpp and am currently working through my backlog. If you are still interested in working on this I would suggest you change the implementation like this: extend ggml_backend_cuda_context::pool in such a way that a reference to ggml_backend_cuda_context is passed. Then in the legacy pool when flushing the old buffers, clear ggml_backend_cuda_context::cuda_graphs to invalidate all pre-existing graphs.

[bokrosbalint]

No worries, thanks for getting back! I pushed the new implementation based on your suggestion, let me know what you think. I'm not entirely sure if the Buffer pool full path is needed, but it seems correct to me.

You might notice that the pp512 numbers took a hit compared to the previous iteration, this is also true on master. Most likely caused by #27841 which matches gfx900 in addition to gfx906. I have a local patch to introduce a separate ggml_cuda_mmq_config for gfx900, similarly to how it's done for Pascal, happy to clean it up and open a separate PR for that after this one.

[JohannesGaessler]

While reviewing this PR I realized that the way the legacy pool is being handled is causing issues with dangling pointers. So while this PR is fixing one issue there are likely to be more. To be clear: we should just merge this PR as-is and not make further changes.

[JohannesGaessler]

I have a local patch to introduce a separate ggml_cuda_mmq_config for gfx900, similarly to how it's done for Pascal, happy to clean it up and open a separate PR for that after this one.

Put this in a separate PR, please.