Skip to content

cuda: reserve the f16 K/V scratch for the short-query MMA route (#346) - #350

Open
professorpalmer wants to merge 1 commit into
PrismML-Eng:prismfrom
professorpalmer:fix-346-short-query-scratch
Open

professorpalmer wants to merge 1 commit into
PrismML-Eng:prismfrom
professorpalmer:fix-346-short-query-scratch

Conversation

@professorpalmer

Copy link
Copy Markdown

Overview

Fixes #346: an out-of-bounds write in launch_fattn with quantized K/V when prompt checkpoints are on.

Cause. With a quantized V and 3-4 queries, ggml_cuda_flash_attn_ext_mma_f16_switch_ncols1 takes the 64-column f16 tiles (#189). That route calls ggml_cuda_flash_attn_ext_mma_f16_case without type_KV, so launch_fattn converts K and V to f16 in the op's extra buffer, also when the in-place quantized K/V kernel exists for the head size (q4_0 / q8_0, D = 128 / 256, from #221). ggml_cuda_flash_attn_ext_get_alloc_size reserves nothing beyond dst on that in-place path, so the f16 K/V is written past the op's allocation. Server prompt checkpoints evaluate the last 4 prompt tokens as their own batch, which reaches this route; with --ctx-checkpoints 0 it is not reached. This matches the memcheck trace in #346 (dequantize_block_q4_0 from launch_fattn, ggml_cuda_flash_attn_ext_mma_f16_case<256, 256, 8, 8, F16>).

Change (ggml/src/ggml-cuda/fattn.cu only):

  • ggml_cuda_fattn_mma_short_query_f16(): the route's condition, now used by the dispatch and by get_alloc_size.
  • get_alloc_size reserves the f16 K/V for that route also on the in-place path.
  • The MMA dispatch asserts that the buffer reserved the f16 K/V the route will write. A mismatch now aborts with a message instead of corrupting memory.

The #189 routing itself is unchanged.

Additional information

RTX 4070 (sm_89), CUDA 13, Windows, on prism-b10770-6684606 (the build in #346) with the same change:

build FLASH_ATTN_EXT(hsk=256,hsv=256,nh=4,nr23=[6,1],kv=16384,nb=3,...,type_K=q4_0,type_V=q4_0) test-backend-ops -o FLASH_ATTN_EXT -b CUDA0
assert only (no get_alloc_size change) GGML_ASSERT ... f16 K/V scratch not reserved for the short-query route stops at that case
this PR OK 4007/4007

On Ada the overflow did not fault in my earlier repro attempts (the write lands in memory the process owns), so the assert is what makes it visible here. @Powerful-Plantain, a test of this branch on your 3070 setup (checkpoints on, same command) would confirm it on the card that crashed.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES - the analysis, the change and the tests above were done with Claude Code (Claude Opus 5.5).

…mML-Eng#346)

With a quantized V and 3-4 queries, ggml_cuda_flash_attn_ext_mma_f16_switch_ncols1
takes the 64-column f16 tiles (PrismML-Eng#189), and launch_fattn converts K and V to f16 in
the op's extra buffer. ggml_cuda_flash_attn_ext_get_alloc_size reserved nothing
beyond dst when the in-place quantized K/V kernel exists for the head size, so
that conversion wrote past the op's allocation. Server prompt checkpoints
evaluate the last 4 prompt tokens as their own batch, which reaches this route.

- ggml_cuda_fattn_mma_short_query_f16(): the route's condition, used by the
  dispatch and by get_alloc_size.
- get_alloc_size reserves the f16 K/V for that route also on the in-place path.
- The MMA dispatch asserts that the buffer reserved the f16 K/V the route
  writes, so a mismatch aborts with a message instead of corrupting memory.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CUDA illegal memory access after long prefill with q4_0 K/V + FA (prism-b10770, regression from b10709)

1 participant