[i-kosarev]

Motivation

On a 2-node MI300X + ConnectX-7 setup, the RMA MPI tests (RmaMPITest) read back a wrong payload. These failures were hard to diagnose, and some were invisible:

1. VerifyBuf returned a bare false, so a failure read only "block N mismatched", with nothing to show whether the data never landed, landed late, or was wrong.
2. main_mpi.cpp detaches gtest's printer and XML writer on non-zero ranks. The IPut* tests (and parts of MixedIPutIGetIPutSignal) verify on rank 1, so their failures showed up only in the job's exit code, while rank 0's report said they passed.

This PR fixes both. It doesn't change what the tests check or any product code.

Technical Details

Both changes are in test/transport/RmaMPI/RmaMPITestBase.hpp:

• VerifyBuf now returns ::testing::AssertionResult. Existing EXPECT_TRUE(VerifyBuf(...)) << ... call sites are unchanged. On a mismatch it reports:

  • the mismatch count and the first mismatching offset
  • the seed and a 16-byte expected/actual hex window there
  • whether every mismatching byte is 0x00 (AllocBuf zero-fills, so this means the payload never landed)
  • rank, nContexts, and RegMr or DmaBuf

  65280 of 65536 bytes mismatched (seed 0xa0), first at offset 0; all mismatching bytes are 0x00
    expected @0: a0 a1 a2 a3 a4 a5 a6 a7 a8 a9 aa ab ac ad ae af
    actual   @0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    rank 1, nContexts 1, RegMr
  

• Rank-1 failure forwarding. RmaMPITestBase::TearDown first sends each non-zero rank's failed test parts (file, line, summary) to rank 0. Rank 0 re-raises each one with ADD_FAILURE_AT(file, line) << "[rank N] " << summary, so its console and XML show the original RmaMPITests.cpp:NN location.

  • It uses the same MPI_Send/MPI_Recv scheme as CastTests, with its own tags (9640–9642).
  • Rank 0 waits for each peer with the same deadline as MPIEnvironment::TearDown: RCCL_TEST_TEARDOWN_TIMEOUT_SEC, default 60 s.
  • If a peer never reaches TearDown, for example after a rank-local ASSERT skips a Barrier(), rank 0 reports "rank N did not reach TearDown … (ranks out of sync)" and calls MPI_Abort instead of hanging.

Why there's no iget flush. An earlier revision of this PR added an iflush after every IGet, on the theory that the failure was a visibility race. Hardware experiments ruled that out:

• Every mismatching buffer was entirely 0x00.
• A re-read 10 ms later was still wrong every time.
• Failures were unchanged with the flush on or off, with PCIe relaxed ordering on or off, and on both ib and ib-cast.

The corruption needs one GPU buffer object to go through this sequence:

1. registered through DMA-BUF;
2. touched by the GPU while registered;
3. deregistered;
4. registered again through peer memory (ibv_reg_mr).

After that, RDMA misses the memory the GPU reads. The ROCr fragment allocator amplifies this, because a small hipFree/hipMalloc pair reuses the same block at the same virtual address, so a DmaBuf test case poisons the next RegMr case. HSA_DISABLE_FRAGMENT_ALLOCATOR=1 avoids that pattern. The root cause is below RCCL, in amdgpu/amdkfd. A flush would only have changed the test's contract without fixing anything, so it was dropped. The diagnostics in this PR are what showed the real shape of the failure.

Issue Tracking

JIRA ID: AICOMRCCL-2327, AICOMRCCL-3005, AICOMRCCL-2652

Test Plan

• Configure rccl with MPI tests (-DENABLE_MPI_TESTS=ON -DBUILD_TESTS=ON, ROCm 7.2.4, gfx942). Compile RmaMPITests.cpp.o with the generated rccl-UnitTestsMPI compile command (MPI_TESTS_ENABLED, RCCL_HAS_RMA_IB_PROXY) at each commit, with -Wall -Wextra -Wshadow -Wformat=2.
• Run a two-rank Open MPI harness on the CPU with the extracted ForwardPeerFailures and VerifyBuf (hipMemcpy stubbed out), removing rank 1's gtest listeners exactly as main_mpi.cpp does. Cases:
  • a rank-1 zero-payload failure
  • a rank-1 test with two failures, one with a multi-line message
  • a rank-0 corrupted-byte failure
  • a passing case
  • a desync case where rank 1 stays in the test body's barrier, with RCCL_TEST_TEARDOWN_TIMEOUT_SEC=2
• Hardware: 2 nodes × 1 rank, MI300X + ConnectX-7 (RoCE). Filter *RmaMPITest.* (108 cases), 3 iterations per configuration, NCCL_NET=ib and ib-cast, once with defaults and once with HSA_DISABLE_FRAGMENT_ALLOCATOR=1.

Test Result

• Compile: clean at both commits; the only warning in the RMA files is the existing unused kBlockSize.
• Harness:
  • Each rank-1 failure is raised on rank 0 at its original file and line, tagged [rank 1], in both console and XML. Multi-line messages arrive intact, and the passing case prints nothing.
  • In the desync case, rank 0 reports the out-of-sync failure and the job aborts after 2 s instead of hanging.
• Hardware, default run: the previously hidden rank-1 failures (54–56 per run) now appear on rank 0, so console, XML and exit code agree. About 96% of VerifyBuf messages carry the all-0x00 note.
• Hardware, HSA_DISABLE_FRAGMENT_ALLOCATOR=1: all 108 RmaMPITest cases pass on both transports, with no spurious forwarding.
• No hangs, and run time is unchanged.
• The hardware runs used an earlier revision of the same two changes. Since then: failures are attributed by file and line, the seed prints in hex, comments are trimmed, and the rank-0 wait is bounded. The bounded wait changes only the desync path, which didn't occur in those runs.

Submission Checklist

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

  

[therock-pr-bot]

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	⚠️ Warning	Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*). Current: code file(s) changed: projects/rccl/test/transport/RmaMPI/RmaMPITestBase.hpp; no test file found
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

[Copilot]

🟡 Changes recommended

The fatal flush assertion can skip rank 0's barrier and hang the MPI suite on flush failure.

1 open issue

• Avoid MPI hang when FlushAfterGet fails before barrier · New

What changed in this PR

This test-only PR adds visibility flushes before verifying RMA IGet payloads and improves diagnostics for nightly data mismatches.

Changes:

• Flush each relevant context before payload verification.
• Add a test-only switch to compare behavior without flushing.
• Report mismatch patterns, byte ranges, and delayed re-read results.

File	Description
projects/​rccl/​test/​transport/​RmaMPI/​RmaMPITests.cpp	Adds flush calls before IGet payload checks.
projects/​rccl/​test/​transport/​RmaMPI/​RmaMPITestBase.hpp	Adds the flush helper, skip switch, and detailed mismatch diagnostics.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Blocking failure forwarding can prevent existing timeout recovery and leave desynchronized MPI jobs hanging indefinitely.

1 open issue

• Add timeout to failure-report handshake to prevent indefinite hangs · New

1 resolved since last review

• Avoid MPI hang when FlushAfterGet fails before barrier

_{Review effort: Balanced}

[amd-argus]

ARGUS

Findings

1 more finding(s), not posted inline

• projects/rccl/test/transport/RmaMPI/RmaMPITestBase.hpp:139 (Low) ForwardPeerFailures is a third copy of the CastTests failure-forwarding handshake; could live in test/common as a shared helper

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

[amd-argus]

ARGUS

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

[Copilot]

🟢 Approval recommended

The focused test-only changes have no identified unresolved blocking issues.

No open issues 🎉

1 resolved since last review

• Add timeout to failure-report handshake to prevent indefinite hangs

_{Review effort: Balanced}

[amd-argus]

ARGUS

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

[VadimKutovoi]

ForwardPeerFailures() only runs once rank 0 reaches fixture teardown. A fatal failure on rank 1 before a subsequent test-body barrier can still strand rank 0 in that barrier, so the original rank-1 failure is not forwarded or represented in rank 0’s XML. The global teardown timeout only aborts later. I think this is fine for the non-fatal VerifyBuf failures targeted here.

[systems-assistant]

TheRock Submodule Bump Activity

Newest first

• 2026-11-05 — Included in TheRock via ROCm/TheRock#9739.