v4: fix multi-GPU SET_ROWS crash via cpy-into-view substitution#1
Merged
Merged
Conversation
Multi-GPU scheduler was rejecting the 5 V4 custom ops (rope_tail, hc_split_sinkhorn, hc_weighted_sum, hc_expand, fp8_kv_quantize) at ggml_backend_cuda_device_supports_op because their weight tensors land in cuda_split buffers when V4 is split across multiple devices. The op then fell back to CPU, corrupting data via mismatched host<->device transfers and crashing in cudaMemcpyPeerAsync during prompt processing. Add the 5 DSV4 ops to the same exemption list that GGML_OP_MUL_MAT already uses. Single-GPU is unaffected (split-buffer check only fires when ggml_backend_buft_is_cuda_split() returns true, which requires multi-device tensor split). Reported, root-caused, and fixed independently by @DenisVASI9 on an 8x A100 40GB rig (his parallel feat/v4-port branch, commit 7a6a7a29d). This commit ports the same fix to feat/v4-port-cuda. Validates on his rig; needs cross-validation: - single-GPU: should be no-op (no behavior change at -ngl 999 on 1 GPU) - multi-GPU: expected to fix the cudaMemcpyPeerAsync crash on V4 inference Co-Authored-By: Denis Vasilyev <DenisVASI9@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds two log sites, both gated by GGML_DSV4_DEBUG=1 environment variable
(no overhead and zero log noise when unset):
1. At entry of ggml_cuda_compute_forward: when the op is one of the 5
DSV4 custom ops, prints the op type, target device, dst tensor name +
shape, and for every source: its buffer type, whether it's a
cuda_split buffer, the raw data pointer, and the extra pointer.
2. Just before every cudaMemcpyPeerAsync call: prints src/dst devices,
bytes, both tensor names + types + originating ops, and both raw data
pointers. The line printed immediately before the crash identifies
the exact tensor and src->dst device pair that failed.
Usage on the multi-GPU host where the cudaMemcpyPeerAsync crash repros:
GGML_DSV4_DEBUG=1 ./build/bin/llama-server <flags> 2>&1 | tee crash.log
Then trigger one request, wait for the crash, share the tail of crash.log.
The diagnostic answers two questions directly: (a) are any DSV4 op sources
landing in cuda_split buffers? (b) which peer copy was in flight when the
illegal-memory-access fired?
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two improvements per codex review of the prior diagnostic commit: 1. After cudaMemcpyPeerAsync issuance, when GGML_DSV4_DEBUG=1, force a cudaStreamSynchronize on the src stream. CUDA async errors otherwise surface on a later, unrelated API call — meaning the last log line before the abort might NOT be the actual offending copy. The synchronize forces the error to surface at the offending peer copy so the diagnostic is accurate. Heavy perturbation (every peer copy becomes synchronous), only with debug enabled. 2. Before issuance, when debug enabled, check cudaGetLastError() and log any pre-existing stale error. Same goal: pin the failure to the copy that actually caused it. 3. Peer-copy log line now also includes src->buffer->buft and dst->buffer->buft names, so the cuda_split vs regular distinction shows up directly in the log line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The existing supports_op check validated that all SOURCE tensors live on ctx.device, but did NOT check the destination buffer. Most ops have a sched-allocated dst (always on ctx.device), so the gap is invisible — except for ops like GGML_OP_SET_ROWS that write into a pre-allocated buffer (e.g. a KV cache). For those, dst lives wherever the cache was allocated, which may differ from ctx.device. The kernel then writes through dst->data (a foreign-device pointer) → cudaErrorIllegalAddress. Diagnosed via CUDA_LAUNCH_BLOCKING=1 + GGML_DSV4_DEBUG=1 on @DenisVASI9's 8x A100 40GB rig: V4 Flash's dsv4_store_cache_rows emits SET_ROWS into the K-cache at layer 7 (cache buffer on CUDA1, V4 ops dispatched on CUDA1). Sched then dispatched SET_ROWS on CUDA0 because its newly-peer- copied src tensors were there. The kernel wrote to a CUDA1 pointer from a CUDA0 stream → illegal address. Without LAUNCH_BLOCKING the error surfaced later at the next sync API call (peer-copy or flash-attn), and we chased the wrong site for three rounds of diagnostics. Adding the dst-device check forces sched to only consider devices where dst lives. Sched will then schedule the necessary peer-copies for srcs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ggml_set_rows returns a view tensor (ggml_view_tensor(ctx, a)), so op->buffer is nullptr at supports_op query time. The dst-device guard added in ca8734a was guarded by `if (op->buffer)` and silently skipped, leaving the original multi-GPU SET_ROWS crash in place. Walk view_src to the root tensor (matching the pattern used in ggml_backend_cuda_cpy_tensor_async) and check the real buffer's device. The walk is a loop rather than a one-hop because V4's dsv4_store_cache_rows builds a multi-level view chain (cache -> set_rows view of reshape of cont of source). Verified in https://github.com/cchuter/llama.cpp on fix/v4-cuda-multigpu-supports-op with V4 Q2_K-XL Flash on 2x RTX 6000 Ada. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
On multi-GPU CUDA, ggml_set_rows is routed by the device affinity of its SOURCES (kv_cur, row indices) after peer-copy, while the cache destination is anchored to a different device — the kernel then writes through a foreign-device pointer (dst->data), surfacing as cudaErrorIllegalAddress in ggml_cuda_compute_forward / SET_ROWS. ggml_cpy into a contiguous view of the destination routes by dst-buffer affinity and works in production multi-GPU today (cf. dsv4_store_state_segment at lines 375-389 of this file, which uses this exact pattern and has never been implicated in any multi-GPU crash log). Substitute the working pattern at the two implicated V4 sites: - dsv4_store_cache_rows: contiguous n-row write via ggml_view_2d + ggml_cpy. - dsv4_build_compressor_decode_projected: single-row write via the same. The cpy result tensor has op=GGML_OP_CPY and src[0]=src, so downstream consumers of the returned kv_state/score_state get a proper data dependency on the cpy through normal ggml graph traversal — no explicit ggml_build_forward_expand needed at site 2 (gf isn't plumbed there anyway). No ggml/src/ changes; this is a graph-construction fix only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex code-review caught that returning the cpy result (which is a [width, 1] row-shape view) as kv_state breaks downstream readers: dsv4_view_cols at lines 869-894 slices the full [width, rows] state by columns AND rows. With the row-shape cpy result, offsets become relative to a single row rather than the full state origin. Mimic ggml_set_rows's internal construction: create a view_tensor of prev_kv_state (full shape, view_src = prev_kv_state) and manually set src[0] to the cpy node. Sched orders cpy before any consumer of this view, and the view reports prev_kv_state's full shape so dsv4_view_cols slicing produces the expected coordinate system. This is the same pattern ggml_set_rows uses internally (view-of-dst + op-tagged + src[] populated), just split across two ops we control at the graph-builder level so we get cpy's dst-affinity routing instead of set_rows's src-affinity routing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
documentation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the
ggml_cuda_compute_forward: SET_ROWS failed / cudaErrorIllegalAddresscrash that hit V4 Flash inference on multi-GPU CUDA hosts during prompt processing at the first cross-device layer boundary.Root cause
The CUDA scheduler routes
GGML_OP_SET_ROWSto the device of its source tensors (after any peer-copies have moved them), but the destination cache buffer is anchored to a different device. The kernel then writes throughdst->data— a pointer on a foreign device — causing an illegal-address abort. WithCUDA_LAUNCH_BLOCKING=1the failure was localized to twoggml_set_rowscall sites insrc/models/deepseek4.cpp:dsv4_store_cache_rows(line 391-407): contiguous row writes into the K-cache during prompt processingdsv4_build_compressor_decode_projected(line 853-897): single-row writes into the recurrent compressor state during decodeFix
V4-graph-only substitution; zero changes to
ggml/src/. Replaceggml_set_rows(cache, src, contiguous_rows)withggml_cpy(src, ggml_view_2d(cache at row_start..row_start+n_rows)).ggml_cpyis routed by destination-buffer affinity (the same pattern the existingdsv4_store_state_segmentuses successfully).At site 2, downstream code at lines 869-894 takes views of the returned full-state tensor (
dsv4_view_cols), so the substitution returns aggml_view_tensor(prev_kv_state)(full shape) withsrc[0]manually set to the cpy node — mirroringggml_set_rows's own internal construction. This preserves the original return-value coordinate system and threads a proper data dependency on the write.Other commits in this branch (kept as harmless defense for unrelated failure modes)
13df7dfe3Adds the 5 DSV4 ops to the supports_op cuda-split exemption (covers a different scenario: tensor-split-mode multi-GPU)5db52f6d9+1aed9b5e9Env-gatedGGML_DSV4_DEBUG=1diagnostic logging for op-entry and peer-copy pathsca8734ab6+19c3f8fd9Supports_op dst-buffer-device checks (no-ops for this bug because intermediate buffers are nullptr at query time, but defensive against related failure modes)The actual SET_ROWS fix is
a4bb644d6+72e6de0ac.Validation
Tested on 2x RTX 6000 Ada (sm_89, CUDA 12.8) with V4 Flash Q2_K-XL and
-ngl 999 -cmoe -ub 128:test-backend-ops19/19 V4 ops pass on both GPUs (no regression)llama-serverchat completion returns coherent text ("What is 2+2?" → "2+2 equals 4" with valid reasoning_content channel)SET_ROWS failed/illegal memory access/Abortedin any test runTest plan
test-backend-ops19/19 DSV4 ops-sm row/-sm tensortensor-split mode🤖 Generated with Claude Code