Skip to content

fix(serve): stop warning about VRAM capacity on unified-memory APUs - #196

Merged
volen-silo merged 2 commits into
mainfrom
fix/uma-vram-warning
Aug 12, 2026
Merged

volen-silo merged 2 commits into
mainfrom
fix/uma-vram-warning

Conversation

@volen-silo

Copy link
Copy Markdown
Collaborator
  • If this PR fixes a bug, searched tests/e2e-cucumber/expectations.toml for the fixed ticket ID and removed/narrowed any now-stale xfail rows. (No EAI-7369 rows exist; this warning has never had e2e coverage.)

Problem

On Strix Halo and other AMD APUs, rocm serve --gpu auto warns:

warning: GPU 0 has only 1.6 GiB of 4.0 GiB free; serving may fail on VRAM.
Free it or pick another with `--gpu <index|auto>`.

…on a machine with 122 GiB of shared RAM, where serving then proceeds without trouble. Reported from a gfx1151 laptop (Ryzen AI Max+ 395, 128 GB, Ubuntu 24.04).

Root cause

An APU has no private VRAM. amd-smi reports the fixed BIOS carveout — often ~4 GiB — as total_vram, while the allocator actually serves the model out of GTT-backed system RAM. gpu_low_memory_warning thresholds a free fraction of that carveout, so on a unified-memory part it is measuring a pool the engine does not use.

That makes the warning uninformative in both directions: it fires on healthy machines, and it would stay quiet on one that is genuinely out of memory. Withholding it removes false positives without introducing false negatives, because the number it thresholds never predicted serve failure on these parts.

Approach

Reuse the existing gfx_is_apu_family predicate rather than adding a gfx1150..1153 list beside it. It already covers every gfx115x part plus gfx1103+, correctly excludes the discrete gfx1100/1101/1102 neighbours that share the gfx110x prefix, and tolerates a trailing gcnArchName feature suffix. Promoting it to pub also keeps one definition of "this part is integrated" shared between the doctor and serve, instead of a second list that would drift.

  • gpu_low_memory_warning is unchanged and still pure. A new serve_gpu_low_memory_warning wraps it for the two serve call sites, and vram_capacity_is_meaningful holds the decision as a separate pure function.
  • Detection is lazy. The wrapper builds the warning first and returns early when there is nothing to say; only then does it resolve the gfx target, reusing the HostGpuSummary that serve may already hold. Host GPU detection can probe sysfs/WSL, and serve deliberately gates its own call on whether engine selection needs it — an eager lookup here would regress that.
  • Suppression requires a single-GPU host. The predicate classifies a part, but host detection yields one target for the whole machine. Unguarded, an APU verdict on a laptop pairing an APU with a discrete card would silence a legitimate warning about the discrete card — trading a false positive for a worse false negative. The amd-smi row count is already in hand, so the guard costs nothing.

Scope

AUTO_FREE_VRAM_FRACTION is shared between this warning and --gpu auto selection. Only the warning path changes here; touching the constant or the shared path would silently alter which GPU gets picked.

Suppressing rather than re-basing the warning on system RAM is deliberate. A capacity check against shared RAM needs a real free-memory signal and its own threshold, and on a unified-memory box the carveout is over 10% used essentially always — so porting the 90% rule across would nag on every serve. That is separable work.

On a hybrid APU + discrete host the current behaviour is preserved, including the false positive when the APU is the selected device. Fixing that properly means resolving a gfx target per amd-smi ordinal, which is more machinery than this warrants.

Testing

cargo fmt --all -- --check and cargo clippy --workspace --all-targets -- -D warnings clean. cargo test --workspace --all-targets --no-fail-fast passes apart from three pre-existing failures unrelated to this change and on untouched files: two rocm-core proc_lifecycle::tests::tree_* tests that fail on WSL2 for process-tree signalling reasons, and therock::tests::extracting_the_sdk_archive_removes_it, which flakes on main itself.

New unit tests cover suppression on gfx1151, retention on a busy discrete gfx1100, the idle-discrete baseline, and the hybrid two-GPU case. gfx1153 was missing from the APU predicate's contract test and is now asserted.

Not run locally: verification on real gfx1151 hardware — this host has no AMD GPU, and the behaviour depends on what amd-smi reports on actual unified-memory silicon. The unit tests pin the decision logic against synthetic telemetry; confirmation on hardware is still outstanding.

No e2e scenario asserts this warning today, and the serve scenarios on the Strix Halo Ubuntu lane are currently xfail'd under EAI-7423, so a new e2e there would land in a known-failing lane rather than protect this fix.

Closes EAI-7369.

@juhovainio

Copy link
Copy Markdown
Collaborator

I reviewed this diff (apps/rocm/src/main.rs, crates/rocm-core/src/examine.rs, crates/rocm-core/src/lib.rs) and didn't find anything blocking.

A few things I checked specifically since this touches serve-path logic:

  • vram_capacity_is_meaningful's gpu_count comes from the full amd-smi telemetry (vram.len()), not just the selected GPU indices, so it's a reliable "is this actually a single-GPU host" signal regardless of which GPU was picked for serving.
  • The lazy host_gpu_summary fallback (rocm_core::detect_host_gfx_target() when host_gpu_summary is None, which is the common case whenever --engine/default_engine is set) resolves through the same underlying detection call as the eager path, so there's no mismatch between the two branches.
  • The hybrid APU + discrete-GPU case (warning still fires if a second GPU is present, even when only the APU is selected) is a known, intentional tradeoff and is covered by apu_verdict_does_not_silence_a_second_gpu.

I also looked at the two CI failures on this run:

  • E2E tests (Strix Halo, Ubuntu) failed with cannot find value gfx_is_apu_family in crate rocm_core, but the export is present in the source (lib.rs:46, examine.rs:567). The build log shows no Compiling rocm-core line before the error, meaning cargo reused a stale cached rocm-core rlib from the self-hosted runner's persistent CARGO_TARGET_DIR instead of rebuilding against this branch. Looks like runner cache staleness, not a code problem — worth a re-run.
  • windows-build-and-test failed on providers::tests::local_provider_reports_ready_managed_models with a Windows non-blocking-socket error. That's in apps/rocm/src/providers.rs, the exact flake test(providers): stop the loopback stub inheriting non-blocking mode #199 ("stop the loopback stub inheriting non-blocking mode") is already fixing — unrelated to this PR's changes.

This looks safe to approve pending a green re-run of those jobs. Note this is a diff-only read — it doesn't replace CI or a maintainer's own pass.

@volen-silo
volen-silo added this pull request to the merge queue Aug 12, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Aug 12, 2026
@volen-silo
volen-silo force-pushed the fix/uma-vram-warning branch from b5ec05f to a9b7c33 Compare August 12, 2026 08:01
@volen-silo
volen-silo enabled auto-merge August 12, 2026 08:01
@volen-silo
volen-silo added this pull request to the merge queue Aug 12, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 12, 2026
@volen-silo
volen-silo added this pull request to the merge queue Aug 12, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Aug 12, 2026
On Strix Halo and other AMD APUs, `rocm serve --gpu auto` warned "GPU 0 has
only 1.6 GiB of 4.0 GiB free; serving may fail on VRAM" on a machine with
122 GiB of shared RAM, where serving then succeeded.

An APU has no private VRAM. amd-smi reports the fixed BIOS carveout as
total_vram, while the allocator serves the model out of GTT-backed system
RAM, so the free-fraction check was thresholding a pool the engine does not
use: it fires on healthy machines and would stay quiet on a genuinely
starved one. Withholding it removes false positives without adding false
negatives, because that number never predicted serve failure on these parts.

Reuse the existing `gfx_is_apu_family` predicate rather than adding a
gfx1150..1153 list that would drift from it; promoting it to `pub` also gives
the classification a single definition across the doctor and serve.

Suppression is gated on the host reporting exactly one GPU. The predicate
classifies a part while host detection yields one target for the whole
machine, so on an APU+dGPU laptop an unguarded verdict could silence a
legitimate warning about the discrete card.

Closes EAI-7369.

Signed-off-by: Eugene Volen <Eugene.Volen@amd.com>
@volen-silo
volen-silo force-pushed the fix/uma-vram-warning branch from a9b7c33 to 64f620f Compare August 12, 2026 13:09
@volen-silo
volen-silo enabled auto-merge August 12, 2026 13:10
Signed-off-by: Eugene Volen <Eugene.Volen@amd.com>
@volen-silo
volen-silo added this pull request to the merge queue Aug 12, 2026
Merged via the queue into main with commit 8d564dd Aug 12, 2026
22 checks passed
@volen-silo
volen-silo deleted the fix/uma-vram-warning branch August 12, 2026 14:46

@rominf rominf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at 6c3e293. No blocking issues. Flagging two things and a few smaller notes — this is currently the only review covering the current head.

The post-approval delta is benign

The raw b5ec05f..6c3e293 diff looks alarming (46 files) but it spans a rebase: b5ec05f sat on 04f568b8, head sits on ce923d42. Comparing the PR's own patch at each base isolates the real delta, and it is entirely mechanical — a parameter reorder forced by collect_serve_notes gaining an engine_flag_note argument from main, two pre-existing test call sites gaining a trailing None (both pass gpu_vram = None, so the warning path short-circuits either way), and a cargo fmt reflow. Zero new logic; the predicate, the guard and the three new tests are byte-identical to what was approved.

The detection predicate holds up

Since a wrong predicate here silently suppresses a genuine out-of-VRAM warning on a discrete card — much worse than the warning it removes — I checked it against the target space rather than trusting the description. gfx1100/1101/1102 → false, gfx1103 and all gfx115x → true, feature suffixes such as :sramecc+:xnack- handled, and the gfx115/gfx110 prefixes diverge at their sixth character so no string can match both — branch ordering is provably irrelevant rather than accidentally safe. The fallback is safe in the right direction: !gfx_target.is_some_and(gfx_is_apu_family) leaves the warning in place for an unknown or absent target, and nothing in the chain collapses "unknown" into "UMA".

One thing the existing approval genuinely does not cover, and it isn't code

The rebase pulled in a project rule that postdates the approval. AGENTS.md §3 — "User-observable behavior needs a scenario, not only a unit test… a unit test asserting the internal helper does NOT discharge this" — landed on main as f551677f, which is not an ancestor of b5ec05f but is an ancestor of 6c3e293.

This PR removes a warning line from rocm serve stdout, which is squarely user-observable, and ships unit tests only. It takes a fourth path beyond the rule's three sanctioned outs, arguing the only lane that could run such a scenario is already xfail'd. I checked the factual claims behind that and they're all true rather than overstated: no feature file asserts this warning, the xfails genuinely cover the serve scenarios on the relevant lane, and the natural extension point would panic inside wait_for_model before reaching any new assertion. It reads as sound judgement — but it is a deviation from a brand-new rule and deserves an explicit maintainer nod rather than passing silently.

Worth adding, if anyone does ask for a scenario: on a host whose unified-memory carve-out is large, the free-VRAM fraction is healthy and the warning wouldn't fire anyway, so a scenario asserting "no warning appears" would pass even without this fix. Getting a non-vacuous scenario needs a host configured with a small carve-out, which is a harder ask than it sounds.

Non-blocking

  • Top residual risk: the whole fix hinges on amd-smi reporting exactly one GPU row on Strix Halo. If it reports two, gpu_count != 1 short-circuits and the bug simply isn't fixed. The PR is honest that gfx1151 hardware verification wasn't run, and unit tests structurally cannot validate this — worth confirming on the reporting user's machine.
  • The dashboard still shows the same misleading signal: crates/rocm-dash-tui/src/ui/dock.rs:118 and ui/tabs/hardware.rs:417 compute a VRAM percentage from the same carve-out, so a Strix Halo will still display a near-full gauge in the TUI after this lands, and the two surfaces will disagree. Pre-existing and out of scope, but it is the same bug on another surface.
  • MI300A isn't covered — gfx942 is reported by both MI300X (discrete) and MI300A (unified), so MI300A keeps the spurious warning. Not fixable from the gfx string alone and it fails in the safe direction, but worth a line in the gfx_is_apu_family doc comment, which now enumerates its consumers without noting this exception.
  • The deferred UMA-appropriate capacity check has no tracking issue, so the gap exists only in this PR's prose. The user-visible consequence is worth stating: on these machines there is now no memory signal at all before serve, and actual RAM exhaustion surfaces only as an engine OOM.
  • apus_verdict_does_not_silence_a_second_gpu's comment frames the test as verifying per-ordinal attribution, but vram_capacity_is_meaningful never inspects gpu_indices — the guard is row-count-only, so &[1] vs &[0] changes nothing in the path under test.

Reusing gfx_is_apu_family rather than adding a parallel gfx list, and keeping gpu_low_memory_warning pure with the policy decision isolated in vram_capacity_is_meaningful, both made this change easy to audit — and the gpu_count != 1 guard is the detail that makes it safe rather than reckless.

@juhovainio juhovainio linked an issue Sep 8, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Issue]: Memory checking for APU ignores GTT

3 participants