deepseek-ai/DeepEP

Fork 0

mirror of https://github.com/deepseek-ai/DeepEP.git synced 2026-11-05 00:47:32 +00:00

[PR #702] fix(legacy): make IBGDA CQ polling concurrency-safe #717

New issue

Open

opened 2026-10-27 21:05:33 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-27 21:05:33 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/deepseek-ai/DeepEP/pull/702
Author: @youchengsong
Created: 7/27/2026
Status: 🔄 Open

Base: main ← Head: fix-legacy-cq-poll

📝 Commits (1)

a26db61 fix(legacy): make IBGDA CQ polling concurrency-safe

📊 Changes

1 file changed (+28 additions, -7 deletions)

View changed files

📝 csrc/kernels/legacy/ibgda_device.cuh (+28 -7)

📄 Description

Summary

Make the legacy IBGDA CQ poller safe when multiple callers observe the same
collapsed CQ, and make its 16-bit completion-counter handling wrap-safe.

The change mirrors the core protocol used by NVSHMEM:

wait until the full-width target index has been submitted before
interpreting the 16-bit CQE counter;
re-read the shared consumer while polling so another caller can satisfy the
wait;
reconstruct the full consumer index from the hardware counter;
advance the shared consumer monotonically with atomicMax;
enforce the queue-depth bound required by the modular comparison.

Why

The existing helper explicitly retained an exclusive-poller assumption.
Concurrent callers can otherwise continue spinning after another caller has
already advanced the shared consumer, or write an older target over a newer
consumer index. A target sufficiently far ahead can also make a stale
16-bit WQE counter ambiguous across wraparound.

This extends the initial early-return added by #371 with the producer,
concurrent-consumer, wrap reconstruction, and monotonic update checks from
the NVSHMEM implementation.

Scope

This PR only changes CQ polling in
csrc/kernels/legacy/ibgda_device.cuh. It intentionally does not change:

RC QP selection or NVSHMEM layout compatibility;
WQE submission or post-send ordering;
CQE error reporting;
public APIs or call sites.

Validation

full build of this commit for sm_103 with CUDA 13.0.88,
NVSHMEM 3.7.2, NCCL 2.30.7, and PyTorch 2.11;
bash ./format.sh;
git diff --check upstream/main...HEAD;
8,126,464 boundary cases in a 16-bit-to-64-bit wrap reconstruction model;
protocol comparison against NVSHMEM 3.4.5, 3.5.19, and 3.7.2;
the equivalent legacy backport completed 18,926 rounds on 4 B300 nodes /
32 GPUs, with the final correctness check at round 18,921 and without the
prior CQ/queue timeout.

The multi-node validation binary also carried the required NVSHMEM 3.7.2 RC
QP-layout compatibility backport, so that result is integration evidence
rather than a current-main single-variable comparison.

Integration note

This patch relies on cq->prod_idx remaining the monotonic posted frontier.
Any async post-send changes, including when stacking with #677, must preserve
that invariant; the producer guard is required to disambiguate 16-bit
counter wraparound.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/deepseek-ai/DeepEP/pull/702 **Author:** [@youchengsong](https://github.com/youchengsong) **Created:** 7/27/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix-legacy-cq-poll` --- ### 📝 Commits (1) - [`a26db61`](https://github.com/deepseek-ai/DeepEP/commit/a26db61351bd94622b6b87b405b2eece7b4cf075) fix(legacy): make IBGDA CQ polling concurrency-safe ### 📊 Changes **1 file changed** (+28 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `csrc/kernels/legacy/ibgda_device.cuh` (+28 -7) </details> ### 📄 Description ## Summary Make the legacy IBGDA CQ poller safe when multiple callers observe the same collapsed CQ, and make its 16-bit completion-counter handling wrap-safe. The change mirrors the core protocol used by NVSHMEM: - wait until the full-width target index has been submitted before interpreting the 16-bit CQE counter; - re-read the shared consumer while polling so another caller can satisfy the wait; - reconstruct the full consumer index from the hardware counter; - advance the shared consumer monotonically with `atomicMax`; - enforce the queue-depth bound required by the modular comparison. ## Why The existing helper explicitly retained an exclusive-poller assumption. Concurrent callers can otherwise continue spinning after another caller has already advanced the shared consumer, or write an older target over a newer consumer index. A target sufficiently far ahead can also make a stale 16-bit WQE counter ambiguous across wraparound. This extends the initial early-return added by #371 with the producer, concurrent-consumer, wrap reconstruction, and monotonic update checks from the NVSHMEM implementation. ## Scope This PR only changes CQ polling in `csrc/kernels/legacy/ibgda_device.cuh`. It intentionally does not change: - RC QP selection or NVSHMEM layout compatibility; - WQE submission or post-send ordering; - CQE error reporting; - public APIs or call sites. ## Validation - full build of this commit for `sm_103` with CUDA 13.0.88, NVSHMEM 3.7.2, NCCL 2.30.7, and PyTorch 2.11; - `bash ./format.sh`; - `git diff --check upstream/main...HEAD`; - 8,126,464 boundary cases in a 16-bit-to-64-bit wrap reconstruction model; - protocol comparison against NVSHMEM 3.4.5, 3.5.19, and 3.7.2; - the equivalent legacy backport completed 18,926 rounds on 4 B300 nodes / 32 GPUs, with the final correctness check at round 18,921 and without the prior CQ/queue timeout. The multi-node validation binary also carried the required NVSHMEM 3.7.2 RC QP-layout compatibility backport, so that result is integration evidence rather than a current-main single-variable comparison. ## Integration note This patch relies on `cq->prod_idx` remaining the monotonic posted frontier. Any async post-send changes, including when stacking with #677, must preserve that invariant; the producer guard is required to disambiguate 16-bit counter wraparound. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-27 21:05:33 +00:00

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

deepseek-ai/DeepEP#717

No description provided.

Rows
Columns