Skip to content

[ROCm][CI] Keep rocprofiler-sdk out of DeepEP HT MoE test workers - #51173

Merged
AndreasKaratzas merged 2 commits into
vllm-project:mainfrom
stefankoncarevic:rocm-deepep-ht-moe-rocprofiler-segv
Aug 5, 2026
Merged

[ROCm][CI] Keep rocprofiler-sdk out of DeepEP HT MoE test workers#51173
AndreasKaratzas merged 2 commits into
vllm-project:mainfrom
stefankoncarevic:rocm-deepep-ht-moe-rocprofiler-segv

Conversation

@stefankoncarevic

@stefankoncarevic stefankoncarevic commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Purpose

On ROCm, every high-throughput variant of test_deep_ep_moe fails with SIGSEGV
(28 of 28), while every low-latency variant passes. The crash is not in the test
and not in DeepEP: the test body completes, and the process then dies during
teardown inside the ROCm HSA runtime, where AqlQueue's destructor writes to
signals whose backing memory has already been unmapped.

That destructor is only reached when rocprofiler-sdk is attached to the process,
which recent torch builds do implicitly, so the failure appeared without any
change on the vLLM side. It reproduces with HIP program run under
rocprofv3, with no torch, DeepEP or vLLM involved, so the defect belongs to
ROCm and it will be report to the AMD team. The issue link will be added here
once it is filed.

Until the ROCm fix lands, this keeps the profiler out of the spawned workers for
the affected test only: guarded by current_platform.is_rocm(), applied only to
the high-throughput test, and set through monkeypatch so it is restored
afterwards.

Test Plan

pytest -v -s tests/kernels/moe/test_deepep_moe.py

on 2x MI355X (gfx950), ROCm 7.2.3.

Test Result

Before:

28 failed, 28 passed

Every test_deep_ep_moe variant fails with SIGSEGV at process exit, every
test_low_latency_deep_ep_moe variant passes.

After:

56 passed

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

All high-throughput test_deep_ep_moe variants fail with SIGSEGV on ROCm. The crash happens after the test body, during process teardown inside the HSA runtime, where AqlQueue's destructor writes to signals whose backing memory is already unmapped. That path is only reached when rocprofiler-sdk is attached, which torch attaches implicitly, and it reproduces with a plain HIP program under rocprofv3, so the fix belongs to ROCm. Until that fix lands, disable rocprofiler registration for the workers spawned by that test, on ROCm only.

Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added the rocm Related to AMD ROCm label Aug 5, 2026
@github-project-automation github-project-automation Bot moved this to Todo in AMD Aug 5, 2026
@AndreasKaratzas

Copy link
Copy Markdown
Member

/ci run

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #82525 for commit 876fa309b608.

@AndreasKaratzas

Copy link
Copy Markdown
Member

/ci run

@AndreasKaratzas AndreasKaratzas added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

@stefankoncarevic, CI is now available for this PR.

  • /ci run starts a CI build.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.

@AndreasKaratzas

Copy link
Copy Markdown
Member

/ci run

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #82543 for commit b321919cdfd6.

@AndreasKaratzas AndreasKaratzas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AndreasKaratzas
AndreasKaratzas merged commit 65addac into vllm-project:main Aug 5, 2026
35 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in AMD Aug 5, 2026
@stefankoncarevic

Copy link
Copy Markdown
Contributor Author

The root cause here is already fixed upstream in ROCm/rocm-systems#6942. Reminder, drop this workaround, together with the os._exit one from #51174 once the base image ships a ROCm build that contains #6942.

@AndreasKaratzas

Copy link
Copy Markdown
Member

@stefankoncarevic maybe create an issue then and share it with me to assign it to you so that we do not forget

@stefankoncarevic

Copy link
Copy Markdown
Contributor Author

@stefankoncarevic maybe create an issue then and share it with me to assign it to you so that we do not forget

Opened it #51644. It covers both this workaround and
the os._exit one from #51174, with the condition for dropping them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed rocm Related to AMD ROCm

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants