comm: synchronize the rail barrier with putValue flags instead of GIN indexed signals - #13
Draft
KeitaW wants to merge 5 commits into
Draft
comm: synchronize the rail barrier with putValue flags instead of GIN indexed signals#13KeitaW wants to merge 5 commits into
KeitaW wants to merge 5 commits into
Conversation
…exed signals The rail instantiation of `gin_barrier_wo_local_sync` now publishes arrival as a 4-byte `putValue` of the round number into a per-peer workspace slot, and waits by polling the peers' slots. It consumes no GIN indexed signals. The world instantiation is unchanged. Round numbers come from device-local counters that mirror the signal shadow the world branch uses, so nothing is plumbed from the host and no call site changes. Slots alternate between two phases. A `putValue` overwrites where the previous `SignalInc` accumulated, so rounds k and k+2 sharing one slot could land out of order under SRD, leaving a stale lower value that strands every waiter. With the phase split each slot carries one in-flight write at a time: a rank issues its round k+2 write only after seeing every peer at round k+1, which means every peer had already read its round k value. This removes the assert in nccl.cu that required one indexed signal per rail peer, which is what capped scale-out at 22 NVLink domains. Data-path signal ids are unchanged. The ordered hybrid kernels share the same function and inherit the new rail barrier. Not verified: reproducing the failure past 22 domains needs more than the four nodes available here.
The rail barrier indexes flag slots by tag, so a tag at or above `kNumBarrierTags` would run off the table with no diagnostic. Assert the highest tag constant fits.
…ntext The signal protocol builds its own QP 0 context with CTA-scoped sharing. The flag path was reusing the caller's handle, which belongs to a channel and is shared grid-wide once the SM count exceeds the QP count, so the barrier's 4-byte write queued behind the data path on that context. Candidate explanation for the 371 us seen on combine at 4096 tokens and absent at decode shape, where the same context is idle.
…the requirements the phase argument rests on Review of the rail barrier found three things the code left implicit. The tag guard checked only whichever tag constant happened to be last, so a new tag past the flag table would have passed it. The check now sits inside the rail branch as `static_assert(kTag < kNumBarrierTags)`, where every instantiation is covered; a stub with kTag=16 fails to compile. The wait compared with `(int32_t)(flag - round) >= 0`. Under the lockstep the slot holds exactly `round` when the arrival lands, so the compare is now `flag == round`: identical when the requirements hold, and a timeout at the offending round with the stale value printed when they do not. The timeout print names the peer rank instead of the slot index. The requirements themselves are written down at the rail branch (same number of rail barriers per tag on every rank, a rank-wide join between two rounds of one tag, a workspace that is never re-zeroed), at `kNumBarrierTags` in the layout, and as a carve-out on the keep-workspace-zero note in `buffer.hpp`. The world branch asserts it is the only other team.
…onstruction Both hybrid kernel variants carry `EP_STATIC_ASSERT(kNumScaleoutRanks <= 32)`, and once the per-peer signal assert was removed nothing rejected a larger topology before JIT compilation. The check moves the ceiling back to a host assert with a message that names the limit.
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.
Problem
Above 22 NVLink domains, buffer construction fails on
EP_HOST_ASSERT(gin_indexed_signals_cnt >= num_rdma_ranks - 1)innccl.cu. The rail barrier ingin_barrier_wo_local_syncgives every rail peer a dedicated indexed signal, and with the default 11 GIN contexts the per-context budget is(256 - 2 * 11) / 11 = 21signals, so the assert holds up to 22 domains and fails at 23.#5 lifts that ceiling by turning the rail barrier into a counting barrier on one reserved signal. Review of #5 objected that reserving the signal offsets every data-path signal id by one and restructures the unordered arm's resource request around the reservation. This PR makes the other change: the rail barrier stops using indexed signals at all, so nothing in the data path has to move.
Change
Arrival is a 4-byte
putValueof a round number into a slot the sender owns on each peer, and the wait polls the peers' slots withld_acquire_sysuntil each holds the current round. The slot table sits at the end ofWorkspaceLayout, indexed by (barrier tag, phase, peer). Round numbers come from device-local send and receive counters in the same workspace, one pair per (tag, index), the same arrangement the World branch already uses through the signal shadow, so no value is plumbed from the host and nogpu_barriercall site changes.Slots alternate between two phases so that each slot has at most one write in flight. A
putValueoverwrites where aSignalIncaccumulated, and overwrites do not commute, so rounds k and k+2 must not share a slot under an unordered transport. A rank issues its round k+2 write only after every peer has reported round k+1, which each peer does only after reading the rank's round k value, so the round k write to that phase slot has already been delivered. The wait compares withflag == round: under that lockstep the slot holds exactlyroundonce the arrival is delivered, and an exact compare turns a broken requirement into a timeout at the offending round, with the stale value in the print.The flag is posted through a
NCCLGinhandle on QP 0 with CTA-scoped sharing, the same context the previous signal protocol used, rather than through the caller's channel handle. The World instantiation is unchanged.The phase argument rests on three requirements that hold at every call site today and are written at the rail branch: every rank runs the same number of rail barriers per tag, two rail barriers on one tag are separated by a rank-wide join (a grid sync or a kernel boundary), and the workspace is never re-zeroed after construction. Nothing enforces them at compile time or at runtime beyond a
static_assertthat each instantiation's tag fits the slot table.Commits: 828697d (the barrier), 730fcc6 and 5de6dea (tag check, exact compare, the written requirements), 33e8de0 (QP 0), 5315cc2 (host assert for the next limit, see below).
What stays as it is on main
deep_ep/include/deep_ep/impls/,csrc/kernels/elastic/,qp_mapping.cuh,gin_resource_alloc.cuhandhandle.cuhare byte-identical tomain; the diff iscomm.cuh,layout.cuh,nccl.cuand a comment inbuffer.hpp. The GIN resource request is unchanged, and both arms printgin_context_cnt=11, gin_indexed_signals_cnt=21, num_qp=11withEP_BUFFER_DEBUG=1; the difference is that the barrier no longer consumes any of those signals.WorkspaceLayout::get_num_bytes()grows by 256 KiB (16 tags x 2 phases x 1024 ranks x 4 bytes of flags, and the same again for the counters). The symmetric workspace is rounded up to 2 MiB, and the GPU-side allocation is 14 MiB before and after; only the pinned host mirror grows.Validation
Everything below ran on four nodes of eight H200 GPUs over EFA (
Libfabric_GDAKIv14 GIN plugin), EP32,--num-sms=24 --num-allocated-qps=11, hidden 7168, top-8 of 256 experts, against a baseline built frommainat 54fffef over the same libnccl.test_epon the unordered hybrid kernels passes correctness in every cell, 13 cells at an earlier commit and one at 5315cc2, withDeepEP rail barrier timeoutabsent from every pod's log. These runs were also the first execution of the SM90 sender path.test_barrier(1000Buffer::barrier()rounds per timed call) passes at 5315cc2 with 22.63 to 23.04 us per barrier across the four nodes. An earlier pair of single runs measured 22.9 us for the flags against 24.9 us for the indexed signals; one run each, so this is not offered as an effect size.EP_HYBRID_KERNEL=orderedfails at buffer construction onmainand on this branch alike, withGIN strong signals are required, but the GIN plugin does not support them, so the ordered hybrid kernels never reach any rail barrier on this plugin.