Repository navigation
[DO NOT MERGE][ROCm] Narrow down the elastic EP IPC failure on MI355 - #61582
Closed
stefankoncarevic wants to merge 75 commits into
Closed
stefankoncarevic wants to merge 75 commits into
stefankoncarevic wants to merge 75 commits into
Conversation
stefankoncarevic
requested review from
AndreasKaratzas,
Harry-Chen,
dllehr-amd,
khluu,
shen-shanshan and
tjtanaa
as code owners
October 14, 2026 07:32
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
stefankoncarevic
added
the
ready
Collaborator
Author
|
/ci run |
|
✅ @stefankoncarevic, CI is now available for this PR.
|
|
✅ Triggered Buildkite CI #94711 for commit |
Contributor
|
Hi @stefankoncarevic, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
Collaborator
Author
|
/amd-ci run |
|
✅ Triggered Buildkite AMD CI #14600 for commit |
Contributor
|
Hi @stefankoncarevic, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
Carries the diagnostic work from vllm-project#59684 onto current main unchanged. None of these six files were touched on main since that branch was cut, so this is the same content, only rebased so CI runs against a current tree. The follow-up commit adds the instrumentation that narrows the failure down. Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>
Follow-up diagnostics for vllm-project#59684, where the MI355 Elastic EP Scaling step fails on every run and nothing reproduces on a bare metal host. The CI logs report ValueError('Process group <torch...'). That is not the real error: while handling the collective failure, torch's c10d logger calls dist.get_backend() on the already invalidated group, which raises and replaces "hipIpcGetMemHandle failed : invalid argument" in everything downstream. Walk the exception chain and print the traceback so the original error survives. The mesh probe already runs immediately before the all_reduce, with nothing between them, and it passes on the very rank that then fails. So the export capability dies inside RCCL's lazy communicator init, not when the group is created. Ask RCCL directly which transport it picks per pair and which buffer it exports, and add two runs that avoid the IPC transport to establish whether it is the trigger rather than only a symptom. After the failure every export fails, including a fresh plain hipMalloc and a buffer owned by another device, so this is not one bad pointer. The fixed 64 handle burst cannot see a ceiling, since the same probe holds 8192 handles on a bare metal host with amdgpu 6.16.6, so report the count actually reached. amd-smi on the CI node reports XGMI as N/A for four of the six pairs, which could mean no link or merely no visibility from inside the VM. Dump the KFD node properties and io_links, which do not carry that ambiguity. Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>
stefankoncarevic
force-pushed
the
elastic-ep-mi355-rccl-init-diag
branch
from
October 14, 2026 08:51
6976cb5 to
62e74c4
Compare
Collaborator
Author
|
/ci run |
|
✅ Triggered Buildkite CI #94738 for commit |
…refused The previous run found the exact failing call. RCCL exports a P2P temp buffer at p2p_tmp.cc:290, HIP reports "IPC memory creation failed for memory", and ROCr answers hsa_amd_ipc_memory_create with hsa_status 4097, which is HSA_STATUS_ERROR_INVALID_ARGUMENT. What is still missing is why that argument is considered invalid, and no AMD_LOG_LEVEL exposes it: level 3 prints the HIP call and its arguments but nothing from inside ROCr, and it costs 1.5 MB of log for a single two rank all_reduce, so it cannot be turned on for the step. This round asks the question a different way. On an export failure the probe now reports what HIP believes the address is: hipPointerGetAttributes for the memory type, the owning device, the managed flag and the allocation flags, and hipMemGetAddressRange for the base and size of the containing allocation. ROCr rejects this pointer as the invalid argument, so a type other than device, a device index that is not ours, or an offset from the allocation base would each be an answer instead of a guess. A second probe spawns a fresh child process on the same device and has it do one allocation and one export, without importing torch. By the time the export fails here, this process has been through engine startup, several scaling transitions and thousands of RCCL exports. A child that has done none of that separates a process that lost the ability to export from a card no process can export from, which is the first thing anyone reading a ticket will ask. The export ceiling probe was measuring the wrong pool. It went through plain hipMalloc, while RCCL allocates what it exports with hipExtMallocWithFlags and hipDeviceMallocUncached, visible as "flags 3" in the p2p_tmp.cc allocation lines. It now uses the same allocator and also walks 6 MiB, which is the size RCCL holds roughly a thousand of per rank. The KFD topology probe used to skip any node it could not read, which made an absent node indistinguishable from an unreadable one. Inside a VM that is the whole question, so it now lists every node it saw, says which were unreadable and why, and flags io_links pointing at nodes that never appeared. Both transport passes are dropped. NCCL_P2P_DISABLE and NCCL_P2P_LEVEL=SYS hit the same 4097 last time, and measured locally NCCL_P2P_DISABLE=1 already takes RCCL to zero hipIpcGetMemHandle calls, so that failure came from the probe and not from RCCL. No transport setting can avoid it and there is nothing further to learn from those passes, so the time goes to a second clean run instead. Finally this reverts the TheRock opt in and builds on ROCm 7.2.3 again, the same image regular CI uses. The failure reproduced on both 7.2.3 with RCCL 2.27.7 and on ROCm SDK 10.0.0 with RCCL 2.30.4, so the newer stack buys nothing here, and going back restores base image caching and a much faster build. Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>
Collaborator
Author
|
/ci run |
|
✅ Triggered Buildkite CI #94780 for commit |
…on MI355
Reading Runtime::IPCCreate in ROCR-Runtime shows the export we fail on has two
unrelated implementations behind one API. It branches on ipc_dmabuf_supported_,
which is nothing more than the inverse of HSA_ENABLE_IPC_MODE_LEGACY: one side
calls hsaKmtShareMemory, the other hsaKmtExportDMABufHandle. Those are
different kernel calls, not two spellings of the same one.
Our ROCm images pin the legacy side:
docker/Dockerfile.rocm:862 ENV HSA_ENABLE_IPC_MODE_LEGACY=1
docker/Dockerfile.rock:842 ENV HSA_ENABLE_IPC_MODE_LEGACY=1
so the dmabuf side has never run in this CI. That matters here because the same
hsa_status 4097 appears on ROCm 7.2.3 with RCCL 2.27.7 and on ROCm SDK 10.0.0
with RCCL 2.30.4, which rules out userspace as the variable, while the host
where this passes differs only in the kernel module, amdgpu 6.16.6 against
6.14.14. A second kernel path is the obvious thing left to try.
The legacy side is also the reason no log level has ever shown the real error.
It ends in
if (ShareMemory(...) != HSA_STATUS_SUCCESS)
return HSA_STATUS_ERROR_INVALID_ARGUMENT;
so whatever the kernel said is thrown away. The dmabuf side returns the error
it got. Setting the flag to 0 is therefore both a candidate workaround and the
only way to read the actual failure.
HSA_ and HSAKMT_ are not in DEFAULT_ENV_VAR_PREFIXES and the ROCm platform only
adds GPU_PINNED_MIN_XFER_SIZE, so the Ray engine actors would otherwise keep
the value baked into the image and the flag would quietly do nothing. Every
pass names its variable in VLLM_RAY_EXTRA_ENV_VARS_TO_COPY for that reason. I
checked this against get_env_vars_to_copy rather than assuming it: named, the
variable is in the copy set, unnamed it is not.
The second pass removes the fragment branch. IPCCreate has six returns of
INVALID_ARGUMENT and one of them is for buffers suballocated out of a 2 MB
block, which HSA_DISABLE_FRAGMENT_ALLOCATOR takes out of the picture.
The third pass raises HSAKMT_DEBUG_LEVEL, which is the last place the kernel's
reason still exists before the legacy path discards it. Levels 1 and 3 print
nothing in this build, 7 names every thunk call including hsaKmtShareMemory,
at a cost of roughly seven lines per export. The run therefore goes to a file
and only the band around the last share call and the tail come back, which
keeps the job log readable.
dump_static now reports which implementation actually ran rather than which
one the environment asked for. Reading the variable back would only echo the
image; decoding the handle shows the branch, because the dmabuf side writes the
pointer into words 0 and 1 and the exporting pid into word 2 while the legacy
side writes the thunk handle there. Verified locally in all three states: as
the image ships it reports legacy, with the flag set to 0 it reports dmabuf,
and with the variable removed it also reports dmabuf, so the image is making a
deliberate choice.
Two changes to the probes follow from the last two runs. Both of them showed
the process at full capacity four seconds before the collective failed, 1536
six MiB exports available in the prepare dump, a successful mesh export at the
door, and then nothing at all. The nearest measurement was too far back to say
whether the headroom was already gone when RCCL started or whether RCCL's own
channels consumed the last of it, so warm_each now measures it immediately
before the collective. The limit there stays at 256 on purpose: walking to 1536
would hold 9 GiB and the probe would become a suspect for the failure it is
supposed to describe.
The fresh child now walks the same ceiling as the parent instead of doing a
single export, so the two numbers are directly comparable. Last run the parent
was at zero while the child reported ok, which answered whether the breakage is
per process or per card but left the child's own headroom unknown.
Finally the two passes that may fail now capture dmesg afterwards. It is likely
to be refused inside the container, as it is on my machine, but it is one line
and it is the only remaining place a kernel level reason could surface.
The verbose NCCL_DEBUG pass goes away to make room. It has run twice now and
reported the same transport and the same buffer sizes both times.
Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>
…ous retirement Build 96482 settled the last open question for all three. The background cleanup main already has passed six of six, three of them on captured graphs, and costs about 12.8 s less per scale cycle than the synchronous retirement. The shrink made no measurable difference under graphs either, 0.202 s out of 29, which was the last venue that could have rescued it. So these are deleted rather than gated. The shrink reached further than the EEP code: shrink_from ran through parallel_state and stateless_coordinator into cuda_communicator, so the whole chain goes and those files return to main unchanged. The graph probe stays for one more run, now with a count of the graphs that existed before the release. Without it the probe only had a negative branch, so it printed the same line in eager passes where no graph is ever captured, and eighteen such passes said nothing. Pass 32 runs six identical passes, two per case, as a regression check. Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>
Collaborator
Author
|
/ci run |
|
✅ Triggered Buildkite CI #96598 for commit |
Pass 32 confirmed the branch passes without the shrink trio, the deferred parent groups and the synchronous retirement, so this drops the remaining instrumentation as well: rocm_debug.py in full, the live graph probe that measured nothing in 72 of 72 calls, the watchdog, the timers, every EEP-DEBUG logger and the stateless bootstrap probe that lost its only caller with them. What stays is the four fixes and one diagnostic. The EPLB async worker is stopped before its group is destroyed, expert resharding runs synchronously so the engine cannot serve against weights that are still being moved, both unbounded waits are bounded, and the arena registration fix remains. The ncclCommAbort warning stays because it is the only signal that detects the rocm-systems#13018 signature from Python. Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>
Collaborator
Author
|
/ci run |
|
✅ Triggered Buildkite CI #96615 for commit |
Low accuracy was never observed without a rank dying alongside, and those crashes are the two defects since fixed by reusing the arena agent and by stopping the EPLB worker before its group is destroyed. One of three asynchronous runs was clean at 0.652, and the upstream H100 nightly passes this group. The window is real: commit_scale_up schedules the reshuffle and nothing waits before traffic resumes. That deserves its own investigation, but forcing it synchronous costs every platform about ten seconds per scale up on a mechanism never shown to fire on its own. Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>
Both were built on the reading in finding 108, that a fault address inside a peer's envelope but outside every published descriptor meant src_row had walked past the end of the peer's tensor. Pass 27 refuted it. Its two controls ran with the arena fix switched off and the bounds guard switched on, both faulted in layer 0, and the guard stayed silent in each. The fault reason there was Write access to a read-only page, which is what deregistering pages under a live reader looks like, not what bad row arithmetic looks like. So the row index was never out of bounds, and neither guard has fired in any pass. Removing them returns add_recv and _exchange_remote_send_meta to upstream and narrows the gathered metadata tuple back to three fields. It also takes valid_bounds, which was filled and never read once the descriptor range dump was removed. Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>
The deadline stays: drain_async has none in main, and a rank that stops producing results leaves the others reducing a flag that can never reach the group size, so nobody leaves and nothing is printed. The same shape was caught for real in _wait_for_all_transfers, where a peer whose GPU faulted left its READ in PROC and the deadline there fired four times in one test, naming layer 16 with 4 of 13 and 7 of 16 still in PROC. Two corrections. worker_state was computed on every iteration of a loop that sleeps a millisecond and used only inside the raise, so is_alive() ran a thousand times a second for nothing; it moves inside the branch. And the docstring claimed a reconfiguration had been seen spinning here for two hours, which was an inference drawn from silence in a local run and was never localised to this loop by any stack, so it now cites the sibling deadline that did fire instead. Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>
It fires on the path destroy() was deliberately built to take. The comment right above says the abort can block until the graphs that captured NCCL ops are destroyed, which is why it runs in a daemon thread with a deadline, and that the main thread proceeds while the graphs drop and the abort returns. So an abort thread still alive at five seconds is the normal path, not an anomaly. It also only ever fired in passes with CUDA graphs, twice in one of them and not once across five eager passes, which is what that comment predicts. The one finding that tried to rule the graphs out rested on a counter later shown to report zero in all 72 of its calls, including passes where 51 graphs had just been captured, so the graph explanation was never actually refuted. And the chain it was written to detect is broken. It was measured once, an abort that did not return and then a refused IPC export 49 s later in the same process, but that was before the arena agent was reused. The last run had two hanging aborts and no refusal at all. Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>
Collaborator
Author
|
/ci run |
|
✅ Triggered Buildkite CI #96631 for commit |
Seen once, and the test survived it. The drain bound stays. Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>
Collaborator
Author
|
/ci run |
|
✅ Triggered Buildkite CI #96638 for commit |
Collaborator
Author
|
/amd-ci run |
|
✅ Triggered Buildkite AMD CI #15217 for commit |
Reading the shutdown flag off self gave the handler task a reference back to AsyncLLM, so the object was never collected and v1/shutdown/test_delete.py timed out. Hold the flag in a list, like logger_manager already is. Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>
Collaborator
Author
|
/ci run |
|
✅ Triggered Buildkite CI #96652 for commit |
Collaborator
Author
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.
Overview
Claims
Validation
Details
Pull Request Checklist
I used vLLM's
/pr-checklistskill. (Mandatory for agents, optional for humans).AI assistance was used during the creation of this PR.
Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.
Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.