Repository navigation
Conversation
6 tasks done
Draft
6 tasks done
|
Seems a rebase is needed after #30. And the rebase would send the scratch-buffer guard to legacy codepath, we also need to replicate it on the new |
vllm-project#19 allocated the split-KV accumulators only when the scheduler has more than one partition. The V4.1 sync (vllm-project#20) brought back the upstream interface, which allocates them whenever split-KV scheduling is enabled, and on SM90 that is always. With a single partition no request is split: the SM90 kernel writes `out` / `lse` directly and the combine has nothing to merge, so the (b + num_sm_parts) x s_q x h_q x d_v fp32 buffers are never read. In vLLM's mixed-batch layout (b=1, s_q=num_tokens) they grow to 8 GiB at 32k tokens and reintroduce the runtime OOM of vllm-project/vllm#53413 (vllm-project/vllm#59305). Skip the accumulators and the combine launch on SM90 when there is a single partition, and bring back the workspace test from vllm-project#19 for SM90. SM100 is left to vllm-project#42: its kernels in the new tree build the o_accum tensor map and offset its pointers unconditionally, so there split-KV has to be turned off before scheduling instead. Signed-off-by: Mikhail Kostryukov <mike@triptrack.net> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
drakosha
force-pushed
the
fix-sm90-nosplit-workspace
branch
from
November 3, 2026 16:26
f2c270c to
6ddc023
Compare
Author
|
Rebased onto AI assistance was used for this comment. |
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
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.
Fixes vllm-project/vllm#59305 on SM90 (regression of vllm-project/vllm#53413). The SM100 side is #42.
What happens
sparse_attn_decode_interfaceallocateso_accumandlse_accum,(b + num_sm_parts) x s_q x h_q x d_vin fp32, wheneverenable_split_kvis set, and on SM90 it always is (must_split_kv = arch.is_sm90a() || ...). That holds for the scheduler, not for the accumulators. Withnum_sm_parts == 1the scheduler puts every request into the single partition and marks nothing as split (get_decoding_sched_meta.cu), so the SM90 kernel takes itsis_no_splitbranch and writesout/lsedirectly (sm90/decode/sparse/splitkv_mla.cuh), and the combine kernel returns for every request (combine.cu). The accumulators are never read.#19 skipped them in exactly this case. The V4.1 sync (#20) replaced
sparse_decode.hwith upstream'ssparse_decode.cppand the guard did not survive. vLLM picked that up with the pin bump in vllm-project/vllm#56893: in its mixed-batch layout (b=1, s_q=num_tokens) the dead workspace is2 x s_q x h_q x d_v x 4bytes, 8 GiB at 32k tokens with 64 heads, and the runtime OOM from #53413 is back.Change
needs_split_workspace = enable_split_kv && !(arch.is_sm90a() && impl_meta.num_sm_parts == 1)gates the accumulator allocation and the combine launch. The scheduler metadata is still built and passed wheneverenable_split_kvis set, so kernel selection does not change.tests/test_flash_mla_sparse_decoding.py:test_no_split_workspace_allocationfrom [Bugfix] Avoid unused sparse decode split-KV workspace #19 is back, SM90 only, over(h_q, d_qk)in{(64, 576), (128, 576), (128, 512)}with top-k 2048.SM100 is not touched. Since #30 its kernels live in the new
csrc/cuda_kernels/sm100/tree, which builds theo_accumtensor map and offsets the accumulator pointers unconditionally, so skipping the allocation there would need the null guards from #19 ported into that tree. #42 takes the other route for SM100: it turnsenable_split_kvoff when it is not mandatory and there is one partition. That condition never fires on SM90 (must_split_kvis always true there) and this PR's gate never fires off SM90, so the two do not overlap and merge cleanly in either order.The first revision of this PR also restored #19's null guards on the legacy SM100 head128 path. After #30 that path is no longer the one that runs, so this revision drops them.
Validation
H200 NVL (sm_90a, 132 SMs), vLLM nightly
0bfc7a15d,_flashmla_Cbuilt from the first revision of this branch and swapped into the image. Baseline is the pinned0eee43b. Mixed-batch layout (b=1, s_q=T), fp8 V3.2 cache, top-k 2048, 20% of the indices masked out. On SM90 the first revision's gate (num_sm_parts > 1) and this one select the same calls.Transient allocation during one
flash_mla_with_kvcachecall on top of inputs and outputs:outandlseare bit-identical to the pinned build for h_q in {64, 128} and T in {1, 8, 128, 132, 1024, 8192}, for both split and no-split schedules. In the no-split case both builds write the output from the kernel; the old one also allocated the buffers and launched a combine kernel that returned immediately. Per-call time is unchanged within 0.8% over the same grid (64 heads / 8192 tokens: 7512 us before, 7474 us after; 128 heads / 8192 tokens: 14158 us before, 14078 us after).The restored test fails on the pinned build (
No-split decode (h_q=64, d_qk=576) allocated 42.35 MiB for 8.28 MiB of outputs) and passes with the branch. The FlashMLA test suite needskernelkit, which is not on PyPI, so it was run against vLLM's build through a small stand-in for the helpers the test touches; the full correctness suite was not run.Not yet rebuilt after the rebase onto
847ef30(#38, #39, #41 touch the fused RoPE kernels, the SM100 head64 decode and the SM90 dense decode, not this file's path). @cjackal reports in vllm-project/vllm#59305 that the first revision of this branch together with #42, both rebased onto847ef30, fixes the memory peak for Mistral Large 4.AI assistance (Claude) was used for the analysis and the draft of this change.