N:m QP sharing: replace comm grouping with pool-based architecture#20
Open
bharatb226 wants to merge 8 commits intoROCm:rel-pvt-M-1from
Open
N:m QP sharing: replace comm grouping with pool-based architecture#20bharatb226 wants to merge 8 commits intoROCm:rel-pvt-M-1from
bharatb226 wants to merge 8 commits intoROCm:rel-pvt-M-1from
Conversation
This reverts commit 4b71c4f.
Rewrite N:m QP sharing to use a pool-based architecture with composite keys, refcounted QP/CQ lifecycle, and commId-based completion routing via wr_id and imm_data encoding. Remove anpPrintSharingSummary (debug-only, can be re-added when needed). Use rcclParamAnpCommNGroups() > 0 as the consistent sharing gate. Enable CTS receiver offload. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
bharatb226
changed the title
These were dropped by the revert commits but are still used in the completion polling path. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
karthikarum
reviewed
May 8, 2026
| RCCL_PARAM(IbAbortOnError, "IB_ABORT_ON_ERROR", 0); | ||
| RCCL_PARAM(AnpCommNGroups, "ANP_COMM_NGROUPS", 0); | ||
| RCCL_PARAM(AnpQpDepthMultiplier, "ANP_QP_DEPTH_MULTIPLIER", 1); | ||
| RCCL_PARAM(AnpQpDepthMultiplier, "ANP_QP_DEPTH_MULTIPLIER", 4); |
Collaborator
There was a problem hiding this comment.
Default depth multiplier to be 1 right ?
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
When QP sharing is enabled (groupIdx >= 0), assign UDMA masks based on group index modulo 2 instead of channel-based tracking. This gives deterministic, balanced UDMA distribution across sharing groups. Non-sharing path (groupIdx < 0) retains existing channel-based logic. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
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
2 participants
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.
Summary
Replaces the comm grouping QP sharing implementation with a pool-based architecture that supports multi-NIC topologies and per-QP refcounted lifecycle.
What changed
Data structures — Old
anpCommGroup/anpCommGroupKey(single QP+CQ per group, PID-scoped keys) replaced withanpSharedQp/anpSharedQpKey(per-QP pool entries keyed by local NIC, peer address, remote NIC index, direction, group index, and QP slot index). Separate refcounts for QPs (refcount) and CQs (cqRefcount).Connect/accept handshake — Primary/secondary role determined by pool lookup on
(peerAddr, groupIdx, qpIdx=0). Primary creates depth-scaled CQs and all QPs for the group. Secondaries map their QP count to the primary's via modulo, reuse primary's QPs and CQs (destroying their own), and skip RTR/RTS transitions.Completion routing —
wr_idupper 16 bits encode the sender'scommId.imm_dataencodes(req_idx << 16) | receiverCommId, allowing direct recv completion routing to the correct communicator and request slot without FIFO-order dependency. Send completions route viawr_id; recv completions route viaimm_data.Teardown — Full-key pool lookup (not QPN) for deregistration. CQ destruction deferred via
cqRefcountand deduplicated across pool entries.anpFreeCommId()releases comm table slots.CTS receiver offload — Enabled (
CTS_RCVR_OFFLOAD_ENABLED). Sender bypasses FIFO-based CTS handshake; NIC hardware handles CTS routing.Parameter defaults —
AnpQpDepthMultiplierdefault changed from 1 to 4. Removed unused device fields (maxQpWr,maxCqe) and parameters (IbDataDirect,IbQpsPerConn,IbQpsPerP2p,IbAbortOnError).Removed
groupRecvDone— Old recv completion used a pending-request FIFO withgroupRecvDoneflag. New design routes directly viaimm_datareq_idx to the target request slot.Removed
anpPrintSharingSummary— Debug-only function that scanned the shared QP pool under mutex. Can be re-added when needed.Test plan
🤖 Generated with Claude Code