Proper fix for host buffer sync - #26225
Conversation
…uffers On backends that advertise support for their own host buffer type (HIP integrated GPUs expose ROCm_Host, and the same applies to CUDA_Host and Vulkan_Host), the scheduler may place graph inputs such as inp_tokens in that buffer and let the device read them directly. Such a buffer is created through the CPU buffer interface, so ggml_backend_tensor_set on it is a bare memcpy on the calling thread: it never reaches the owning backend and cannot be ordered against an in-flight graph_compute_async whose kernels are still reading the tensor. The scheduler inserts no copy or sync either, precisely because the backend claims it can consume the buffer directly. set_inputs then overwrites the previous ubatch's inputs while the GPU is still reading them, which corrupts the prompt. The existing guard above only synchronizes for pipeline parallelism, which requires n_devices() > 1 and is therefore never active on a single integrated GPU. Synchronize before set_inputs when the graph actually has such inputs. The condition is computed once per graph build and cached, so configs without device-owned host inputs are unaffected. Reproducer, on gfx1151 with any prompt longer than one ubatch: GGML_SCHED_SANITIZE=1 llama-completion -ngl 99 -f long.txt aborts with a write-after-read on ROCm_Host inp_tokens (detected by the scheduler sanitizer, ggml-org#26167) and without the sanitizer silently produces different output from a race-free build. Measured on Ornith-1.0-35B IQ4_XS, 33k-token prompt, ABBA-interleaved, gfx1151: prefill 720.8 -> 720.3 t/s, decode unchanged at ~51 t/s. Assisted-By: Claude <noreply@anthropic.com>
github-actions
Bot
added
the
ggml
|
I built this PR myself and ran an A/B against the commit right below it, on Strix Halo. Short version, My setup is a Framework Desktop with Strix Halo, gfx1151, 96 GB unified memory as a BIOS carve out, The nice thing about this PR is that it consists of exactly two commits, so no patching was needed at For measuring i use a needle test. A marker KANARIE-<8 hex> is put at a defined position inside a Here is the result. Same machine, same model, same flags, only the commit differs.
All 13 failures in the control are honest "not found", the model never invented a marker. This is the
With the fix applied every single one of these cells comes back ok. If you sort the cells not by prompt length but by how far the marker sits away from the end of the
One thing worth mentioning because i can not explain it. In the 4000 token row the marker at 5 percent I also ran the control against the official image ghcr.io/ggml-org/llama.cpp:server-rocm, digest Now the speed side, because that is what makes this PR important for this hardware and not just nice to
At 104777 tokens thats 196,0 with the fix against 196,9 without, so 0,5 percent apart while my The reason this matters so much here, the build from before #24233 reads a 100k token prompt in 49 Which leads to a question i can not answer myself. Does #25863, so disabling direct ROCm_Host compute on Happy to run anything else you want tested on gfx1151, and i can share the test scripts, they are plain |
|
@DeMaulwurfn yeah, we want to keep the direct host compute. Disabling it is masking the real problem. I'll have the ring buffer PR as per that discussion soon. |
3 participants
Overview
On top of #26167 , proper fix to synchronization issues with host buffers.
Additional information
Supersedes #25863
Requirements