Skip to content

CUDA: fix shared-memory WAR hazard in flash_attn_ext_vec - #30573

Merged
am17an merged 2 commits into
ggml-org:masterfrom
invaliddayta:cuda-fattn-vec-race
Oct 19, 2026
Merged

am17an merged 2 commits into
ggml-org:masterfrom
invaliddayta:cuda-fattn-vec-race

Conversation

@invaliddayta

@invaliddayta invaliddayta commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Overview

The KV loop reuses the shared KQ buffer between iterations. There is
a warp barrier between writing the scores and reading them for V
accumulation, but none before the next iteration overwrites them.

Add ggml_cuda_syncwarp() at the end of the loop so all lanes finish
reading before any lane overwrites its slot. A warp barrier is
sufficient because these reads stay within the writing warp.

I have not observed incorrect output from this race. Compute-sanitizer
reports the write-after-read hazards.

Additional information

Tested against master e467e5b on an RTX 3090 (sm_86),
NVIDIA driver 610.43.03:

  • Racecheck, FLASH_ATTN_EXT with hsk=256 and nb=1:
    8007424 WAR warnings before, zero after.
  • All 38 selected correctness tests passed before and after.
  • Full FLASH_ATTN_EXT correctness suite after the fix: 3993/3993 passed.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES - AI assisted with the patch and the testing

@invaliddayta
invaliddayta requested a review from a team as a code owner October 15, 2026 07:36
@github-actions github-actions Bot added ggml changes relating to the ggml tensor library for machine learning CUDA Related to the CUDA backend labels Oct 15, 2026
Comment thread ggml/src/ggml-cuda/fattn-vec.cuh Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ggml_cuda_syncwarp(); // Needed for KQ synchronization.

Comment thread ggml/src/ggml-cuda/fattn-vec.cuh Outdated
Comment on lines +378 to +379
// KQ is overwritten in the next iteration, wait for all lanes to finish reading it
ggml_cuda_syncwarp();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// KQ is overwritten in the next iteration, wait for all lanes to finish reading it
ggml_cuda_syncwarp();
ggml_cuda_syncwarp(); // Needed for KQ synchronization.

Co-authored-by: Johannes Gäßler <johannesg@5d6.de>
@am17an
am17an merged commit 99db719 into ggml-org:master Oct 19, 2026
1 check passed
edwardyoon pushed a commit to edwardyoon/focus-llama that referenced this pull request Oct 19, 2026
)

Add the missing syncwarp after the KQ read loop so the next outer
iteration cannot write KQ shared memory while VKQ still reads it.
Applied by hand (3 lines) to coexist with the da_sparse patch.

(cherry picked from commit 99db719)
nicheng251 added a commit to nicheng251/bonsai-turboquant-llama.cpp that referenced this pull request Oct 23, 2026
Port five small upstream fixes that apply to code this fork ships:

- fattn-vec: sync the warp at the end of the VKQ loop (ggml-org#30573). The next
  iteration's KQ write reused the shared buffer while other lanes were
  still reading it.
- cpy: the transposed fast path requires a contiguous destination (ggml-org#27663).
  It wrote to a non-contiguous dst as if it were dense.
- llama-kv-cache: only upload k_rot when the tensor has a buffer (ggml-org#27967).
  An unquantized K cache has no k_rot buffer and context shift crashed.
- ggml_permute: keep dims and strides 64-bit (ggml-org#29227). The int temporaries
  truncated large tensors.
- convert: run contiguous conversions four elements per thread (ggml-org#29155).

Verified: FLASH_ATTN_EXT turbo 25/25, 27B Bonsai PTQ1_0 model smoke with
q8_0 K + turbo3 V (prefill 547 tok/s, decode 31.2 tok/s at 2K; 30.0 at 17.4K).

The stock q8_0/q8_0 long-sweep crash is not caused by the WAR hazard: it
still aborts after 361 cases with this fix.
@invaliddayta
invaliddayta deleted the cuda-fattn-vec-race branch October 23, 2026 18:23
MuFengMuXue pushed a commit to MuFengMuXue/llama.cpp-gfx906 that referenced this pull request Oct 26, 2026
)

* CUDA: fix KQ write-after-read race in flash_attn_ext_vec

Assisted-by: ChatGPT

* CUDA: apply review suggestions for KQ sync comments

Co-authored-by: Johannes Gäßler <johannesg@5d6.de>

---------

Co-authored-by: Johannes Gäßler <johannesg@5d6.de>
auRose94 pushed a commit to auRose94/llama.cpp that referenced this pull request Oct 28, 2026
)

* CUDA: fix KQ write-after-read race in flash_attn_ext_vec

Assisted-by: ChatGPT

* CUDA: apply review suggestions for KQ sync comments

Co-authored-by: Johannes Gäßler <johannesg@5d6.de>

---------

Co-authored-by: Johannes Gäßler <johannesg@5d6.de>
@BrewTestBot BrewTestBot mentioned this pull request Oct 28, 2026
1 task done
Simione932 added a commit to Simione932/llama.cpp that referenced this pull request Oct 28, 2026
73ab759 (ggml-org#26705) — Branchless Q4_K/Q5_K unpack in MMVQ:

vecdotq.cuh: Replaces branched scale unpacking in vec_dot_q4_K_q8_1 and vec_dot_q5_K_q8_1 with branchless masks so nvcc can hoist scale unpacking out of the ncols_dst loop.

mmvq.cu & common.cuh: Updates ggml_cuda_should_use_mmvq crossover thresholds and adds bs traits in ggml_cuda_type_traits.

36d7b08 (ggml-org#26289) — Tune FP16 tile FlashAttention configs for head sizes 40–112:

fattn-tile.cuh: Updates ggml_cuda_fattn_tile_get_config_nvidia_fp16 thread/occupancy/nbatch tile parameters for DKQ 40, 64, 72, 80, 96, and 112.

fc9ce6b (ggml-org#29953) — Fix MMQ out-of-bounds reads:

mmq.cu & mmq.cuh: Computes J_best and nthreads_best upfront in ggml_cuda_mul_mat_q, rounds src1_q8_1 padding up to a full parallel load chunk (nthreads_best * sizeof(int)), and pads ids_dst by J_best - 1 elements for unconditional tile loads.

1bb2b9f (ggml-org#30103) — Fix shared-memory race in flash_attn_ext_f16_process_tile:

fattn-mma-f16.cuh: Synchronizes threads in flash_attn_ext_f16_process_tile when np > 1 || nbatch_combine != DKQ/2.

4c1004c (ggml-org#30553) — Cap MMQ Stream-K blocks at available work:

mmq.cuh: Caps block_nums_stream_k.x in launch_mul_mat_q at std::min(nsm, nwork) so Stream-K does not launch idle blocks on small matrices.

99db719 (ggml-org#30573) — Fix shared-memory WAR hazard in flash_attn_ext_vec:

fattn-vec.cuh: Adds warp synchronization (__syncwarp()) at the end of each KQ tile iteration in flash_attn_ext_vec before the next iteration overwrites shared memory KQ.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CUDA Related to the CUDA backend ggml changes relating to the ggml tensor library for machine learning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants