Skip to content

fix(rccl): fixed udma balancing for p2p comms - #12390

Closed
alexander-sannikov wants to merge 2 commits into
rccl/drop_2026-08from
users/asanniko/ROCM-31118
Closed

alexander-sannikov wants to merge 2 commits into
rccl/drop_2026-08from
users/asanniko/ROCM-31118

Conversation

@alexander-sannikov

Copy link
Copy Markdown
Contributor

Motivation

Adds RCCL_IB_UDMA_POLICY control to manage UDMA QPs distribution.

  • 0 channelId-based affinity,
  • 1 balances P2P QPs using round-robin distribution and preserving collective affinity
  • 2 balance all QPs using round-robin on a system-level.

Technical Details

Adds user control and ncclIbCastSelectUDMA routine to manage QPs UDMA distribution.

Issue Tracking

JIRA ID: ROCM-31118

Test Plan

Test Result

Submission Checklist

… 2026 08) (#11172)

## Motivation
This PR enables BY_ORDER on the no-CTS-offload path and makes it the
default on AINIC when the matching scheme is unset, so users do not need
to set NCCL_IB_RECEIVER_SIDE_MATCHING_SCHEME=2 manually for the common
AINIC + RCCL_CTS_OFFLOAD_ENABLED=0 configuration.

## Technical Details

Changes are confined to net_ib_cast:
- IbCastResolveRecvMatchingScheme() (connect.cc) — centralizes scheme
selection:
  - CTS offload enabled → BY_ORDER (unchanged)
  - Explicit NCCL_IB_RECEIVER_SIDE_MATCHING_SCHEME=2 → BY_ORDER
  - Unset default (-2) on AINIC → BY_ORDER (new)
  - Explicit =0 → BY_INDEX (user override preserved)
- IbCastByOrderRequested() — now reflects the resolved scheme (CTS
offload, AINIC default, or explicit env), not only an explicit =2.
- Incompatible features disabled when BY_ORDER is active: QP scheduler,
split-data, WRR, resiliency/port-failover, and out-of-order RQ (via
scheduler.cc, init.cc, p2p_resiliency.cc).
- IbCastReportMatchingScheme() — init-time INFO logging of collective vs
P2P CTS offload state and resolved RecvMatchingScheme.

## Issue Tracking
JIRA ID: AICOMNET-396

## Test Plan

Defined in Jira

## Test Result

Provided in Jira

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
alex-breslow-amd pushed a commit that referenced this pull request Oct 16, 2026
## Motivation

Multi-node alltoall on MI355X with AI-NIC regresses at all message sizes
in the August RCCL code drop. The root cause is uneven use of the two
AINIC uDMA pipelines by P2P QPs.

[internal-env]

## Technical Details

- Moves uDMA pipeline selection out of `ncclIbCreateQpIonic` into
`ncclIbCastSelectUDMA` and adds `RCCL_IB_UDMA_POLICY`:
- `0` (default): current behavior. With QP sharing, QPs alternate by
sharing group (or by QP index when there is one group); otherwise every
QP of a channel is pinned to one pipeline, assigned alternately as
channels are first seen.
- `1`: P2P QPs round-robin across pipelines in creation order;
collectives keep the policy 0 behavior.
- `2`: the PD mask is left unchanged, so the driver picks the pipeline.
- `3`: each connection is split across both pipelines; the channel id
picks the starting pipeline and consecutive per-device QPs alternate.
- Adds `qpIdx` and `IbCastQpStripe` to `ncclIbQpCreateAttr`, set on the
sender, receiver and AINIC port-recovery QP creation paths.
- Holds the dedicated `nccl_udma_mutex` from pipeline selection through
`ibv_create_qp`, because the PD and its uDMA mask are shared by all
comms on the device.
- Warns at init when `RCCL_IB_UDMA_POLICY` is outside 0-3; such values
behave as 0.

The default stays 0 because the new policies regress other workloads, so
they are opt-in until they have been evaluated with the customer. This
is the develop counterpart of #12390, which targets `rccl/drop_2026-08`.

## Issue Tracking

JIRA ID: ROCM-31118

## Test Plan

- Compile check of the changed translation units in the TheRock ROCm
7.14 container.
- Multi-node `alltoall_perf` on MI355X + AI-NIC with each
`RCCL_IB_UDMA_POLICY` value.

## Test Result

The compile check passes.

BASE1, BASE2 (both develop 5909ad9), PR (ba423a0)

**2 nodes results (mi355+AINIC):**

Alltoall (out-of-place busbw, GB/s; deltas are against base1 matched
repeat by repeat, with wins out of 8)

build | rccl-tests Avg busbw | Δ vs base1 | 16M–256M | 1G–8G
-- | -- | -- | -- | --
base1 | 61.50 | reference | 70.89 | 76.32
base2 (noise floor) | 60.92 | −0.94% (2/8) | −0.66% | −1.20%
PR, policy unset | 61.60 | +0.17% (5/8) | −0.54% | +1.15%
PR, policy 1 | 67.23 | +9.32% (8/8) | +2.80% (8/8) | +19.13% (8/8)
PR, policy 2 | 67.10 | +9.11% (8/8) | +2.71% (8/8) | +18.98% (8/8)
PR, policy 3 | 67.18 | +9.24% (8/8) | +2.89% (8/8) | +19.15% (8/8)

All_reduce and all_gather (regression check runs)

build | all_reduce Avg busbw | Δ (wins/8) | all_gather Avg busbw | Δ
(wins/8)
-- | -- | -- | -- | --
base1 | 221.44 | reference | 195.62 | reference
base2 (noise floor) | 221.00 | −0.20% (2/8) | 195.73 | +0.05% (5/8)
PR, policy unset | 221.79 | +0.16% (5/8) | 195.10 | −0.27% (4/8)

**4 nodes results (mi355+AINIC):**

Alltoall (out-of-place busbw, GB/s; deltas are against base1 matched
repeat by repeat, with wins out of 8)

arm | Avg busbw | 16M–256M | 1G–8G
-- | -- | -- | --
base1 | 46.34 | 54.08 | 54.20
base2 (noise floor) | +0.41% (5/7) | −0.00% (2/7) | +1.20% (5/7)
PR, policy unset | −0.68% (2/7) | −0.62% (3/7) | −1.26% (3/7)
policy 1 | +6.11% (7/7) | +4.48% (7/7) | +9.76% (7/7)
policy 2 | +3.92% (7/7) | +2.76% (7/7) | +6.89% (7/7)
policy 3 | +3.32% (5/6) | +2.07% (5/6) | +7.03% (5/6)

Allreduce/Allgather is flat as for 2N.


## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

(AI Asist)

---------

Co-authored-by: Anatolii Rozanov <arozanov@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: VadimKutovoi <Vadim.Kutovoi@amd.com>
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

  • Add new commits
  • Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

@github-actions github-actions Bot added the Stale PR has no activity for 25+ days label Oct 26, 2026
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

This pull request has been automatically closed due to inactivity (30 days with no updates).

If you'd like to continue working on this, feel free to reopen the PR or create a new one.

@github-actions github-actions Bot closed this Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

organization: ROCm project: rccl Stale PR has no activity for 25+ days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant