Skip to content

rpc: avoid serializing buffers from other servers - #26500

Merged
ggerganov merged 2 commits into
ggml-org:masterfrom
hmirin:fix/rpc-multi-server-buffer-ownership
Aug 30, 2026
Merged

rpc: avoid serializing buffers from other servers#26500
ggerganov merged 2 commits into
ggml-org:masterfrom
hmirin:fix/rpc-multi-server-buffer-ownership

Conversation

@hmirin

@hmirin hmirin commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Overview

  • Goal: Support multiple llama.cpp RPC servers from a single client.
  • Bug: RPC buffer pointers were serialized even when they referred to buffers owned by a different RPC server, resulting in invalid pointers on the receiving server.
  • Fix: Include a remote buffer pointer only when the buffer belongs to the RPC socket receiving the graph.
  • Test: Add a two-server regression test for cross-server tensor serialization.

The bug caused the following error:

[create_node] invalid data ptr
[graph_compute] failed to create graph node 0

Additional Information

curl -fsSL https://gist.githubusercontent.com/hmirin/64cd976fe04e4a4b5405641c72647e10/raw/reproduce.sh -o reproduce.sh
bash reproduce.sh

Requirements

  • I have read and agree to the contributing guidelines.
  • AI usage disclosure: YES. The fix was created with Codex 5.6 Sol and Claude Code Fable. Fix is reviewed by the author and PR comment is written by the author.

@hmirin
hmirin requested review from a team and ggerganov as code owners August 3, 2026 07:01
@github-actions github-actions Bot added testing Everything test related examples ggml changes relating to the ggml tensor library for machine learning labels Aug 3, 2026
@hmirin
hmirin force-pushed the fix/rpc-multi-server-buffer-ownership branch from 6e24f65 to f0c483c Compare August 8, 2026 14:15
@himorishige

Copy link
Copy Markdown

Tested this on 2x DGX Spark (GB10, sm_121, arm64, CUDA 13.0, driver 580.159.03) — it fixes the failure here.

Before the patch, both master (74ce157) and the rpc_tensor branch (db5abb4) abort as soon as the first graph reaches the remote worker:

[create_node] invalid data ptr[graph_compute] failed to create graph node 3085

Only the remote worker dies; the loopback worker on the client machine logs nothing unusual, and a single-worker setup never reproduces it — which lines up with the ownership problem described here.

With f0c483c applied on both nodes, two workers (one remote over a 200Gb QSFP direct link with RoCEv2 RDMA, one loopback) complete:

model size pp512 tg128
deepseek4 IQ2_M 84.68 GiB 338.36 20.19
deepseek4 MXFP4 MoE 144.44 GiB 369.69 16.49

The 144 GiB quant does not fit in a single node's 121 GiB of unified memory, so more than one RPC worker is the only way to run it at all. That is the case this patch unblocks for me.

Beyond llama-bench: llama-server over the same two workers survives repeated requests (with --cache-ram 0 from #26529), a ctx ladder of 2048/8192/32768 runs clean, and a 12-slot concurrency sweep finished with zero failed requests.

Same signature is reported in #26820; I left the reproduction details there.

@dwhoop55

Copy link
Copy Markdown

I had the same issue ([create_node] invalid data ptr[graph_compute] failed to create graph node) when trying to run DeepSeek V4 Flash and Kimi K3 on multiple CPU-only RPC nodes (with RDMA transport; even thought it should be the same with TCP).

ggml-org/llama.cpp:master with this PR fixes the issue for DeekSeek V4 Flash and the unslothai/llama.cpp:kimi-k3-text-base with manually merging this PR does also fix running Kimi K3 on multiple CPU-only RPC nodes in my tests.

@ludwigjossieaux

Copy link
Copy Markdown

Confirming this also fixes GLM-5.2 multi-node RPC crashes (#26583), deterministic invalid data ptr at graph node 30 before, fully working after, on a mixed SM120 head + SM121 workers cluster with RDMA. MTP-over-RPC works too. Nice fix, thanks

@fenneclabs

fenneclabs commented Aug 18, 2026

Copy link
Copy Markdown

I have the same problem with DS4 REAP with RTX 4060Ti + 2x BC250 over RPC all on Vulkan.

@hmirin maybe resolving the branch conflict could help get this PR merged? Thanks for the fix :)

@hmirin
hmirin force-pushed the fix/rpc-multi-server-buffer-ownership branch from f0c483c to e144634 Compare August 21, 2026 16:32
@hmirin

hmirin commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Updated to keep up with the main branch.

Only include remote buffer pointers when the buffer belongs to the RPC dispatcher receiving the graph. Add a two-server regression test for cross-server tensor serialization.

Assisted-by: Codex
@hmirin
hmirin force-pushed the fix/rpc-multi-server-buffer-ownership branch from e144634 to 41d0d5f Compare August 29, 2026 13:37
Comment thread ggml/src/ggml-rpc/ggml-rpc.cpp
@ggerganov ggerganov added the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Aug 30, 2026
@ggerganov
ggerganov merged commit a7cc83b into ggml-org:master Aug 30, 2026
1 check passed
MarkShark2 added a commit to MarkShark2/llama.cpp that referenced this pull request Sep 2, 2026
68 upstream commits, 9 conflicted files. Brings qwen4exp fixes needed for
Qwen3.8-Flash-Next: recurrent state rollback (ggml-org#28123), seq_cp/block position
keying/mtmd input (ggml-org#27941), indexer head slicing (ggml-org#28023), and the central
n_layer_nextn load (ggml-org#28159).

Resolutions:

- ggml-rpc.cpp: resolved entirely to the fork side again. Upstream's
  rpc_dispatcher/proto-6.0 port stays deferred; the fork keeps 5.1.3. Note
  that upstream ggml-org#26500 now implements the same foreign-endpoint skip in
  serialize_graph that the fork carries, so that one is a candidate to drop
  when the dispatcher port finally happens.
- llama-graph.cpp: adopted upstream's fused ggml_swiglu_clamp for both the
  dense and MoE clamp paths and kept the fork's LLM_ARCH_GLM5NEXT in the
  condition. The fused kernel is min(gate,limit) / clamp(up,+-limit) /
  swiglu_split, i.e. bit-identical to the three-op sequence it replaces, and
  Vulkan implements it, so the boards get it too. Costs the
  ffn_*_clamped cb() trace names, same tradeoff upstream took for DSV4.
- speculative.cpp: upstream ggml-org#27310 folded the DFlash encoder into the
  injection decode and deleted features_buf. The fork's non-finite feature
  scrub moves onto batch_inject.embd in place; the last_tap_nextn tap
  automerged.
- dflash.cpp: kept the fork's Laguna kv_inp norm and deferred gated o_proj,
  layered on upstream's newly-populated wk_s/wv_s/wo_s scale arguments
  (previously NULL). The gated path latches wo_s alongside wo.
- nemotron-h.cpp: dropped the now-duplicate NEXTN_PREDICT_LAYERS get_key,
  kept the fork's stricter assert and n_layer_nextn_per_head derivation.
- llama-model-loader.cpp: kept the fork's rpc_preloaded skip ahead of
  upstream's use_mmap || lazy.has(cur) predicate.
- tests: kept both sets of flash-attn cases; ported the fork's GLM5NEXT
  test_dsa_kpool/test_mtp onto upstream's int-verbosity logging convention.

Windows CUDA build green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U3H5motr51eTWujztSXykc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples ggml changes relating to the ggml tensor library for machine learning merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Eval bug: PR20908 breaks rpc-server functionality when balancing split a model across multiple machines.

6 participants