Skip to content

wip/tensor-split-fixes: page faults and graph churn under -sm tensor + MTP (issue #105) - #106

Open
briansp2020 wants to merge 1 commit into
stew675:mainfrom
briansp2020:wip-tensor-split-fixes
Open

briansp2020 wants to merge 1 commit into
stew675:mainfrom
briansp2020:wip-tensor-split-fixes

Conversation

@briansp2020

@briansp2020 briansp2020 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

This packages the -sm tensor fixes from #105 as a wip/ item in the same form as #104, so you can review, re-validate and fold them in however suits you. Updated for r17: rebased onto c862072 (no conflicts), now nine patches; tested on r16.

Contents (wip/tensor-split-fixes/, nothing else touched): nine format-patches on top of v16-a55e952b8-r17 (applied tree 04764deb) that apply with git am after patches/00*.patch, plus a README with cause, fix and validation for each.

patch what switch
0001 keep a grown-out Q8_1 arena alive while captured graphs still point into it (fixes the quantize_q8_1 page fault) GGML_CUDA_Q8_1_ARENA_FREE_OLD=1
0002 hash node/source data pointers into the graph key, so each MTP layout keeps its own graph GGML_CUDA_GRAPH_KEY_NO_DATA=1
0003 recapture a graph when memory it captured (pool temporaries, FA/H2D staging) was freed since; fixes a page fault in a freed FA staging buffer GGML_CUDA_GRAPH_MEM_GEN=0
0004 keep 8 meta split-state cache versions instead of clearing on every mismatch (~21 % of main-thread time under MTP) GGML_META_SS_VERIFY=1 checks hits
0005 LLAMA_KV_N_PAD_MIN raises the n_kv padding floor (opt-in, default unchanged) unset
0006 stop pinning every layer's block_out as a prefill graph output (~1.9 GiB of the compute buffer at -ub 2048); the fused combine+norm reads a pool copy if needed LLAMA_HC_PIN_BLOCK_OUT=1
0007 compact split-state cache entries + hash maps for the meta tensor caches (lookups were ~11 % of the main thread) GGML_META_SS_VERIFY=1 checks hits
0008 copy the conv-state tail straight into each rollback slot (no cont) LLAMA_CONV_TAIL_CONT=1
0009 ggml_hc_mix_set_planar(): planar HC_MIX output, no ggml_cont of the mixed head in the verify band (BF16 CUDA only) LLAMA_HC_MIX_PLANAR=0

Results on r16 (2 × R9700, qwen4exp GSQ-RCO IQ3_XXS + MTP n-max 3 p-min 0, all experts in VRAM, 256K, -ub 2048, stock ROCm 10.0, one session):

  • -sm tensor with the nine patches vs our production -sm layer: greedy 113.9-114.5 vs 94.6-96.6 t/s, chat 98.7 vs 90.1, 2 concurrent 142-147 vs 122-124, prefill 37k / 155k 2428-2447 / 2022 vs 2032-2054 / 1709, 259.6k prompt prefill / decode 1754 / 47.2 vs 1473 / 30.6. Same as r15 within noise; not re-run on r17.
  • Every patch leaves greedy output unchanged; long-context suite 4/4, warm-up + fresh-slot 2/2, no GPU faults.
  • KL divergence vs the layer config (r15, same greedy output on r16): 0.0106, the same as layer split at a different -ub.
  • --spec-draft-p-min 0 (configuration, no patch) is part of the tensor result: a constant verify width lets the graph be reused; details in the README.
  • With p-min 0, tensor no longer needs GGML_HIP_GRAPH_FORCE_UPDATE=1 or the TheRock nightly that avoids the ROCm 10.0 hipGraphExecUpdate leak (hipGraph: kernarg staging never reclaims slots within an exec's lifetime — request slot reuse on exec update ROCm/rocm-systems#10713); with them it is ~6-7 % faster on brand-new prompts, details in the README.

The README lists what I didn't measure: 1-GPU and non-RDNA4 layouts, more than 2 GPUs, and host-resident experts with these patches.

Happy to re-run anything on this box or rework the patches if you'd rather they land differently. No rush on our side.

(Claude-assisted, as before.)

@briansp2020

Copy link
Copy Markdown
Contributor Author

A heads-up before you spend time on this: I found a crash caused by 0002 (the data-pointer graph key), in plain -sm layer use. Sequence: our server warm-up script (a long cached prefix, several follow-up turns with cache_prompt, then a 200-token generation), then a request on a fresh slot. The server evicts a host prompt-cache entry, server_slot::prompt_save → state_seq_get_data reports an illegal memory access on device 1, and the log shows Memory access fault by GPU node-2 ... Page not present. Stock r12 + 0002 crashes with or without MTP experts on the GPU and with or without our pool cap; GGML_CUDA_GRAPH_KEY_NO_DATA=1 (the old key) survives the same sequence, as does r11. I haven't pinned down the mechanism yet. Please treat 0002 as withdrawn for now. 0001 (the arena fix) is unaffected: its kill switch made no difference. I'll follow up once I understand it.

(Claude-assisted, as before.)

@briansp2020

Copy link
Copy Markdown
Contributor Author

Thanks for bearing with me on this one, and apologies: my earlier comment was wrong. The crash I attributed to 0002 turned out to come from our own environment, not from either patch.

We run the server with GLIBC_TUNABLES=glibc.malloc.hugetlb=1, which makes glibc back the heap with transparent huge pages. With that set, the warm-up + fresh-slot sequence I described faults in prompt_save → state_seq_get_data right after the server evicts a ~2 GB host prompt-cache entry (GPU page faults land on host heap addresses). It isn't related to 0002. My earlier "0002 crashes, the old key survives" came from single runs of a crash that only happens about half the time. Repeated runs on this box (2 × R9700, ROCm 10.0, -sm layer, production flags):

build tunable on tunable off
r12 + 0001 + 0002 (dev build) 2/5 crashed —
same, GGML_CUDA_GRAPH_KEY_NO_DATA=1 1/5 crashed —
r12 + 0001, no 0002 4/13 crashed 0/16 crashed
our previous r11 build 3/5 crashed —

One more correction: the build I called "stock r12 + 0002" was actually r12 + 0001 + 0002 plus two inactive env switches of ours.

So 0002 can come back off the withdrawn list if it's still useful to you; the numbers in the PR description stand (-sm tensor + MTP decode 71.1 → 75.9 t/s, -sm layer unchanged, greedy output identical). Separately, I've found a related host-side cost under -sm tensor + MTP: the meta buffer's split-state cache is cleared whenever verify and draft graphs alternate. Keeping a few cache versions instead gives a further sizeable gain on this box. I can send that as its own PR with data if you're interested. I'm happy to re-run anything on our side, and nothing here is urgent.

(Claude-assisted, as before.)

@briansp2020
briansp2020 force-pushed the wip-tensor-split-fixes branch from 9d50c52 to 00446c5 Compare October 6, 2026 02:42
@briansp2020 briansp2020 changed the title wip/tensor-split-fixes: Q8_1-arena page fault + MTP graph-cache churn under -sm tensor (issue #105) wip/tensor-split-fixes: page faults and graph churn under -sm tensor + MTP (issue #105) Oct 6, 2026
@briansp2020

Copy link
Copy Markdown
Contributor Author

Thanks for r15 — the block-06 Meta host-buft fallback is what made -sm tensor usable with this model at all, and the TENSOR-CORRUPTION.md write-up was a great read.

I've updated this PR for r15 (rebased onto 493808b), so it's now five patches instead of two:

  • 0001 and 0002 are unchanged, just rebased.
  • 0003 fixes a second page fault of the same family as 0001: graphs replaying addresses of buffers that were freed since (here the FA prefill staging arena). It showed up once graphs lived longer.
  • 0004 keeps several meta split-state cache versions. I'd offered this as a separate PR in my last comment, but it seemed simpler to keep everything -sm tensor in one place; happy to split it out if you prefer.
  • 0005 is an opt-in KV padding floor.

With all five, -sm tensor -ub 1024 now matches -sm layer on greedy decode here and is ahead on long prompts. KL divergence against the layer config is at the noise floor of layer split itself. The README has the numbers and a stock-r15 column.

On TODO #41, flagging in case it's useful: these runs keep all experts in VRAM, so the staging ring never activates and we don't hit it. If a second box would help, I'm happy to run your repro here (on our GSQ IQ3_XXS model) with -ncmoe, with and without 0003 (it touches when the H2D ring is freed, though not what it copies), and send you the results.

No rush on any of this.

(Claude-assisted, as before.)

…sm tensor + MTP (issue stew675#105)

Nine format-patches on top of v16-a55e952b8-r17 (tree 04764deb) and a README
with cause, fix, switches and validation: Q8_1 arena retention, data-pointer
graph key, recapture after captured memory is freed, split-state cache
versions, LLAMA_KV_N_PAD_MIN, unpinned block_out in prefill, compact
split-state cache entries, direct conv-state tail copy, planar HC_MIX output.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@briansp2020
briansp2020 force-pushed the wip-tensor-split-fixes branch from 00446c5 to e7de090 Compare October 6, 2026 15:09
@briansp2020

Copy link
Copy Markdown
Contributor Author

Thanks again for r16, and for the TODO #43 write-up - the per-device guard was a neat find. One more round on this PR, flagging it in case it's useful: rebased onto r17 (no conflicts; tested on r16), plus four more small patches (0006-0009), all on the same -sm tensor + MTP path, each with a switch that restores the old behaviour.

  • 0006 stops build_hc_combine from pinning every layer's block_out as a graph output in prefill. An allocator peak dump showed 96 of them (20 MiB each at -ub 2048) live at the end of the graph, ~1.9 GiB of a 3.5 GiB compute buffer; it is what kept -ub 2048 from fitting at 256K under -sm tensor. The repeat-anchored combine+norm matcher now reads block_out from a pool copy when its outputs reuse the freed buffer, so the fusion still fires. KL divergence against the pinned build is 0.000000 for both split modes.
  • 0007 makes the meta split-state and simple-tensor caches cheap: a split state is ~2.2 KiB, so the lookups alone were ~11 % of the main thread.
  • 0008 and 0009 remove ~240 copy kernels per verify graph (the conv-state tail per rollback slot, and a ggml_cont of the HC_MIX head - 0009 adds a small ggml_hc_mix_set_planar() for that, BF16 CUDA path only).

Separately, a configuration note rather than a patch: under -sm tensor, --spec-draft-p-min 0 was worth ~+20 % here, because a constant verify width lets llama.cpp reuse its graph (~76 % -> ~98 % of verify calls).

With all nine patches and p-min 0, -sm tensor on our 2 x R9700 is now ahead of -sm layer on every row we measure (on r16, stock ROCm 10.0: greedy +19 %, chat +10 %, 37k-155k prefill +18-19 %, decode at a 259.6k prompt +54 %), with greedy output unchanged by every patch and the long-context suite 4/4. With p-min 0 it no longer needs the ROCm nightly or GGML_HIP_GRAPH_FORCE_UPDATE either. r16 measures the same as r15 here, as expected, since blocks 16 and 17 are on the host-expert path; I haven't re-run on r17, whose changes are on that path too. The README has the table, the causes and the switches; the PR branch also builds and runs on its own without our two local knobs.

No rush on any of it - happy to split these out, rework them, or re-run anything here.

(Claude-assisted, as before.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant