Skip to content

fix(rdc): report the socket power of MI300 series and later GPUs - #13637

Merged
adam360x merged 2 commits into
developfrom
users/adam360x/ROCM-33008
Oct 16, 2026
Merged

adam360x merged 2 commits into
developfrom
users/adam360x/ROCM-33008

Conversation

@adam360x

@adam360x adam360x commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

rdci dmon -e 300 (RDC_FI_POWER_USAGE) reports 4294967295000000 µW on AMD Instinct MI300 and newer GPUs, while amd-smi metric --power shows the real socket power (140 W on the MI325X in the ticket).

RDC treats a socket power of 65535 as "not available", but AMD SMI has reported an unreadable power as UINT32_MAX since ROCm 7.11 (ROCm 7.2 and earlier: 65535). These GPUs have no average socket power, so RDC scaled UINT32_MAX to microwatts, and power policies triggered on every check.

Technical Details

  • Per-field fetch: UINT32_MAX (current AMD SMI) and UINT16_MAX (older AMD SMI) both mean "not available" for the average and current socket power. GPUs without an average socket power report the current one; partitions without any socket power report N/A (was the bogus value).
  • Bulk fetch (RDC_BULK_FETCH_ENABLED, off by default): a GPU whose 16-bit GPU metrics report neither an average nor a current socket power now falls through to the per-field fetch instead of reporting 65535 W.
  • Tests: new GPU-less rdc_power_usage_tests (9 tests) builds the metric fetcher against fake AMD SMI replies; tests/ci/test_gtest_runner.py runs it in the no-GPU CI job.
  • The tests share one metric fetcher and never destroy it (it keeps no state for the field they fetch): ~RdcMetricFetcherImpl() can miss waking its worker thread and then wait for it forever. That race predates this PR and can also hang rdcd when it stops; it is not fixed here.
  • CHANGELOG entry under Resolved Issues.

Not changed here

Issue Tracking

JIRA ID: ROCM-33008

Test Plan

Test Result

  • Tests: 3 of 9 fail when built against develop's fetcher; all 9 pass with the fix.
  • Hang check: with 16 copies of the test binary running 5000 repeats each at once, all 16 hung with a fetcher per test and none with the shared fetcher.
  • MI455X: 4294967295000000 -> 1125000000-1129000000 µW (amd-smi: 1126-1127 W), as root and non-root. With the bulk fetch on, develop already reported the current power.
  • MI300A: 4294967295000000 -> 59-133 W per GPU; TPX/CPX partitions without socket power -> N/A.
  • Radeon (has an average socket power): unchanged.
  • Power policy: stock RDC reported "A Power Limit exceeds the threshold 2000 with the value 4294967295" every 5 s at ~1126 W; the fix reported nothing in 12 s.
  • Trial merges clean in every order and with current develop; the combined tree builds and all GPU-less tests pass.

Submission Checklist

RDC_FI_POWER_USAGE treated an average or current socket power of 65535
as not available, the value amd-smi used until it switched to
UINT32_MAX. GPUs without an average socket power, such as the MI300
series and later, then reported UINT32_MAX * 10^6 uW instead of their
current socket power, and partitions without any socket power reported
the same value instead of N/A. Power policies compared that value with
their limit, so they triggered on every check.

Treat both UINT32_MAX and UINT16_MAX as not available. The bulk fetch
(RDC_BULK_FETCH_ENABLED) reads the 16-bit gpu_metrics fields and
already fell back to the current power, but reported 65535 W for a GPU
with neither; it now leaves that GPU to the per-field fetch.

Add rdc_power_usage_tests, which builds the metric fetcher against fake
amd-smi replies so it runs without a GPU, and run it from the CI gtest
runner.

Signed-off-by: Adam360x <Adam.pryor@amd.com>
@adam360x
adam360x requested review from a team as code owners October 16, 2026 15:13
Copilot AI balanced review requested due to automatic review settings October 16, 2026 15:13
@therock-pr-bot

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🚫 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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The narrowly scoped fix covers both fetching paths with regression tests, and no blocking issues were identified.

0 open findings

What changed in this PR

Fixes RDC socket-power reporting on AMD Instinct MI300 and newer GPUs, preventing unavailable readings from triggering power policies.

Changes:

  • Handles both legacy and current AMD SMI unavailable-power values.
  • Falls back to current power or per-field fetching as appropriate.
  • Adds nine GPU-less regression tests, CI integration, and release notes.
File Description
projects/​rdc/​tests/​unit/​test_power_usage.cc Tests power selection and unavailable readings.
projects/​rdc/​tests/​unit/​CMakeLists.txt Builds and installs the new test binary.
projects/​rdc/​tests/​ci/​test_gtest_runner.py Runs power tests in GPU-less CI.
projects/​rdc/​rdc_libs/​rdc/​src/​RdcMetricFetcherImpl.cc Corrects unavailable-power handling and fallbacks.
projects/​rdc/​CHANGELOG.md Documents corrected power reporting.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

~RdcMetricFetcherImpl() stores false in task_started_ and calls
cv_.notify_all() without holding task_mutex_. If the worker thread has
checked its wait predicate but not yet blocked, it misses the wakeup, and
destroying updater_ after the destructor body then waits for the thread
forever. Each test built and destroyed its own fetcher, so the binary
could hang: with 16 copies of it running 5000 repeats each at once, all
16 hung.

The race is in the library: rdcd destroys its fetcher the same way when
it stops. This change only keeps the tests from hitting it. Build one
fetcher for all the tests and never destroy it; the fetcher keeps no
state for the field the tests fetch. With this change, the same 16
copies all pass.

Signed-off-by: Adam360x <Adam.pryor@amd.com>
adam360x added a commit that referenced this pull request Oct 16, 2026
## Motivation

`rdci dmon -e 9` (`RDC_FI_GPU_PARTITION_COUNT`) reports 65535 on an AMD
Instinct MI325X in SPX mode, where it should be 1.

RDC copied the partition count from the GPU metrics, which have that
field only in versions 1.6 to 1.8. Drivers that publish GPU metrics v1.9
(current MI300 and MI350 drivers, MI455X) never fill it, and v1.3
(MI200, Radeon) has no such field. AMD SMI then reports 65535, its "not
available" value. An earlier partition fix did not change this field.

## Technical Details

- Field 9 now comes from `get_num_partition()`, the call `rdci
discovery` already uses. It reads AMD SMI's current partition profile
(SPX 1, DPX 2, TPX 3, QPX 4, CPX = number of XCDs) and works without
root.
- **Behavior change:** a GPU that cannot be partitioned reports 1 (was
65535). A failed profile query makes the field N/A.
- The profile query takes ~0.28 ms per sample, against ~0.14 ms for the
old GPU metrics read (MI455X).
- `SmiUtils`: removed `get_metrics_info()`, which had no other caller.
- Tests: new GPU-less `rdc_partition_count_tests` (6 tests) builds the
metric fetcher against fake AMD SMI replies;
`tests/ci/test_gtest_runner.py` runs it in the no-GPU CI job.
- The tests share one metric fetcher and never destroy it (it keeps no
state for the field they fetch): `~RdcMetricFetcherImpl()` can miss
waking its worker thread and then wait for it forever, which hung this
PR's first CI run. That race predates this PR and can also hang rdcd
when it stops; it is not fixed here.
- CHANGELOG entry under Resolved Issues.

**Not changed here**
- Partition instances (`gN.P`) still report N/A for this field, as
before; each partition's own GPU index reports the count.
- Sibling fix for `RDC_FI_POWER_USAGE`: #13637 (it also updates the CI
runner's docstring).

## Issue Tracking

JIRA ID: ROCM-33007

## Test Plan

- `rdc_partition_count_tests` against develop's fetcher and the fix; CI
pytest wrapper against a private install.
- `rdcd -u` + `rdci dmon -u -e 9` in a private network/mount namespace:
1x MI455X (ROCm 10.2.0 nightly; stock, develop, fix; non-root), 4x
MI300A (gfx942) in SPX/TPX/CPX, 2x Radeon (RX 6800 XT, RX 7900 XT).
- Trial merges with #13386, #13570, #13637 and current develop.

## Test Result

- Tests: all 6 fail when built against develop's fetcher and `SmiUtils`;
all 6 pass with the fix.
- Hang check: with 16 copies of the test binary running 5000 repeats
each at once, all 16 hung with a fetcher per test and none with the
shared fetcher.
- MI455X in SPX: 65535 -> 1, as root and non-root.
- MI300A: SPX 65535 -> 1 on all 4 GPUs; TPX -> 3 on all 12 logical GPUs
(develop: 65535 on the 4 primaries, N/A on the other 8); CPX -> 6 on all
24.
- Radeon (cannot be partitioned): 65535 -> 1.
- Trial merges clean in every order and with current develop; the
combined tree builds and all GPU-less tests pass.

## Submission Checklist

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

---------

Signed-off-by: Adam360x <Adam.pryor@amd.com>
@adam360x
adam360x merged commit d0357d2 into develop Oct 16, 2026
75 of 78 checks passed
@adam360x
adam360x deleted the users/adam360x/ROCM-33008 branch October 16, 2026 17:49
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants