Repository navigation
#583: the fused layout shrinks the MoE buffers only when every layer can take the fused path - #958
Krypto-Whitehat wants to merge 1 commit into
Conversation
…y layer can take the fused path fused_layout() decided to shrink GU/H/Xq to the fused path's sizes on `mmq_plan().any` - ONE layer's experts go through MMQ - while the run loop picks the path per layer, on `mmq_plan().layer[l]`. A layer MMQ does not cover keeps the FP16 path, and that path writes the layer's routed rows of the full chunk into GU (`m.gemm.f16(..., m.GU + o0 * 1280, ne, ...)`) exactly as MMQ does (`mmq::quantize(..., m.Xq, ..., T * K, ...)`). The fused layout holds stream_all_min() - 1 = 1023 tokens of those rows, so at a 16384-token chunk the write runs 16x past the buffer: garbage, an illegal access, or a kernel that never returns. The first `ck()` to see the sticky error is `prefill mmq: iota` one prompt later, which is why it reads as a hang in the ring rather than as a buffer that was too small. This is the same hole `fused_ring()` closes for the native kernels (220e0e8) one level up: that one asks "do the FUSED kernels cover every layer", this asks "do MMQ's", and neither covers both. So the shrink is now all-or-nothing over both, and the SYCL mirror matches. `m.fused_bufs` records which layout was carved, and the run loop refuses - with the layer and the chunk, not a hang - a layer that would write full-chunk rows into the fused buffers. That is the backstop for anything that drifts between the two decisions later, and on a rig like Niko1221#954 it is also the one run that says which layer is the odd one out. The ring is not at fault: the byte-budget ring only moved the chunk (16384 on the reported rig) and therefore how far past the buffer that write runs, and STRATA_RING_BYTES=0 works because it lands on a smaller chunk. That is also why the same configuration ran 4,700 requests on 0.1.37: its chunk was below the fused layout's MMQ floor.
|
Thanks for the careful read. The overflow you describe was real on 0.1.39: fused_ring() was true for a native pack, but a layer that MMQ had no tile for (mmq_plan().layer[l] == 0) took neither MMQ nor the fused kernels at a streamed chunk, so it wrote the full chunk's rows into the shrunk GU/H/Xq. In 0.1.40 the run loop already sends such a layer to the fused kernels (73a1a4d: layers MMQ does not cover but the native kernels do are marked fused-only, and fused_ring() already requires the native kernels to cover every layer). Every layer of a pack that gets the shrunk layout therefore runs fused or MMQ at a streamed chunk; none reaches the FP16 path. We are not merging mmq_all(): in 0.1.40 it would switch the shrink off for those fused-only layers (for example UD-Q4_K_XL on gfx11) and give back the VRAM the fused layout saves, with no overflow left to prevent. Closing this one; the guard idea is a good one and we may add it separately. |
…ing the layer, not a hang (#954, #958) fused_layout() and fused_ring() already keep the shrunk GU/H/Xq buffers and the per-layer expert path in step. If they ever drift (a layer that takes MMQ or the FP16 path at a chunk of stream_all_min() tokens or more while the buffers are the fused ones), the prompt now stops with 'the fused layout's MoE buffers are too small for layer N' instead of writing past the buffer: an illegal access, or a kernel that never returns and shows up as 'prefill mmq: iota' one prompt later. Same check in the SYCL copy. Co-authored-by: Krypto-Whitehat <41930040+Krypto-Whitehat@users.noreply.github.com>
|
Thank you, Krypto-Whitehat. The root cause you found is fixed in 0.1.40 (fused_ring and the per-layer plan), and the backstop error from your PR is in 0.1.40.2 with you as co-author. |
…ing the layer, not a hang (Niko1221#954, Niko1221#958) fused_layout() and fused_ring() already keep the shrunk GU/H/Xq buffers and the per-layer expert path in step. If they ever drift (a layer that takes MMQ or the FP16 path at a chunk of stream_all_min() tokens or more while the buffers are the fused ones), the prompt now stops with 'the fused layout's MoE buffers are too small for layer N' instead of writing past the buffer: an illegal access, or a kernel that never returns and shows up as 'prefill mmq: iota' one prompt later. Same check in the SYCL copy. Co-authored-by: Krypto-Whitehat <41930040+Krypto-Whitehat@users.noreply.github.com>
Fix for #954.
The defect
fused_layout()shrinks a layout'sGU,HandXqto the fused path's sizes, with an MMQ-sized floor forstream_all_min() - 1= 1023 tokens, so that a last sub-1024-token chunk can still run MMQ in them. It decided to do that on:The run loop picks the path per layer (
prefill.cpp:2286):A layer MMQ does not cover keeps the FP16 path, and that path writes the layer's routed rows of the full chunk into exactly those buffers —
m.gemm.f16(..., m.GU + o0 * 1280, ne, 1280, N)overnerows (prefill.cpp:2804), and MMQ likewise atT * Krows intoXq(prefill.cpp:2428). The fused layout holds 1023 tokens of those rows, so at a 16384-token chunk the write runs ~16x past the end: garbage, an illegal access, or a kernel that never returns. The firstck()to reach the sticky error isprefill mmq: iotaa prompt later, which is why the report reads as a hang in the ring rather than as a buffer that was too small.This is the same hole
fused_ring()closes one level up for the native kernels (220e0e8): that one asks "do the fused kernels cover every layer", this asks "do MMQ's", and neither covered both. The shrink is now all-or-nothing over both, and the SYCL mirror matches.Why the ring shows up in the report
The byte-budget ring is not at fault. It moved the chunk (16384 on this rig), and the chunk is what sets how far past the buffer the write runs. That is also consistent with all three control runs on the same binary:
STRATA_PF_FUSED=1(default ring)prefill mmq: iotaSTRATA_PF_FUSED=0(default ring)fused_layout()is false → MMQ-sized buffers → worksSTRATA_PF_FUSED=1+STRATA_RING_BYTES=0It is also consistent with 0.1.37 running the same configuration for ~4,700 requests: its chunk was below the fused layout's 1023-token MMQ floor, or the layer that overflows was not on the prompt path.
What this PR changes
mmq_all()— a new predicate next tommq_plan();fused_layout()requires it instead ofmmq_plan().any. A pack with one layer MMQ does not cover now keeps MMQ's buffers, asfused_ring()already does for the native kernels.m.fused_bufsrecords which layoutcarve()made, and the run loop refuses a layer that would write full-chunk rows into the fused buffers — with the layer and the chunk named, instead of hanging. This is the backstop for anything that drifts between the two decisions later.On a pack where every layer is both MMQ-capable and natively covered (including a homogeneous IQ3_XXS one),
mmq_all() == mmq_plan().anyand the fix changes nothing: no chunk, no loan, no buffer size. On a pack with an odd layer out, the fused layout switches off there, exactly as it already does for the native kernel coverage.Verification — please read, this is not runtime-verified
I do not have a CUDA toolchain or a Blackwell card on the machine this was prepared on, so I could not build
strata_prefill, run the test suite, or reproduce #954. What I did verify:/W4and returns the right answer for every layer vector, exercised in a standalone harness withmmq_all()copied verbatim from the patch: all-MMQ → shrink; one FP16 layer (first, middle, last) → no shrink; no MMQ / no layers → no shrink. That harness confirms the hole directly — with 48 layers and serve: fix zero-token replies from engine-queue race between requests #7 on the FP16 path, the old predicate says shrink and the new one says no.prefill_fused_moe_test/prefill_fused_iq_testdo not cover this: they exercise the kernels with their own buffers and never build a layout, so the ring × fused × layout interaction has no test. I did not add one either —mmq_all()isinlinein an anonymous namespace over a loaded pack, so a test would have to duplicate it rather than call it, which would test the copy.So the overflow mechanism above is established by reading the code, and the guard will name the offending layer on the first run if some other condition is what actually triggers #954 on that rig. The reporter should confirm against their own 0.1.39 build — a 0.1.39 binary with only this patch, same rig. If it still hangs, the new message will say which layer and which chunk, and that is the next thread to pull.
STRATA_RING_BYTES=0remains a valid workaround meanwhile.Not changed on purpose
bytes_needed()prices the loan withfused_layout(T, true)whilecarve()uses the realm.src != nullptr. When a layout is created without an expert source the loan is under-priced, andAlloc::takebounds-checks, so the result is a clean"device buffers ... do not fit"atinitrather than a hang. Loud, not silent, and out of scope here — happy to fix it in a follow-up if you want the two call sites to share one predicate.