From 1611f58daa7a2dc01c01c8c7a67575f115191c82 Mon Sep 17 00:00:00 2001 From: Michael Roy Date: Wed, 9 Sep 2026 14:17:06 -0700 Subject: [PATCH] fix(dash): detect amd-smi through WSL DXG Signed-off-by: Michael Roy --- Cargo.lock | 1 + apps/rocm/src/dash.rs | 4 +- crates/rocm-dash-collectors/Cargo.toml | 1 + crates/rocm-dash-collectors/src/amd_smi.rs | 55 +++++++++++++-------- crates/rocm-dash-daemon/src/runner.rs | 18 +++---- crates/rocm-dash-daemon/tests/end_to_end.rs | 6 +-- tests/e2e-cucumber/features/dash.feature | 7 +++ tests/e2e-cucumber/tests/e2e/dash_steps.rs | 28 +++++++++++ 8 files changed, 86 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f9a338fba..e12640a50 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3625,6 +3625,7 @@ dependencies = [ "serde", "serde_json", "sysinfo", + "tempfile", "thiserror 2.0.18", "tokio", "tracing", diff --git a/apps/rocm/src/dash.rs b/apps/rocm/src/dash.rs index 0d2dd8e34..47dfb16b1 100644 --- a/apps/rocm/src/dash.rs +++ b/apps/rocm/src/dash.rs @@ -66,9 +66,9 @@ pub fn runner_options( // amd-smi ships inside the managed runtime wheel's bin dir, not on PATH; // resolve the path so the GPU collector can find it. amd_smi_binary: Some(rocm_core::resolve_amd_smi_binary()), - // Production always runs the real `/dev/kfd` pre-flight; only daemon + // Production always runs the real GPU-device pre-flight; only daemon // integration tests with a fake binary skip it. - amd_smi_skip_kfd_preflight: false, + amd_smi_skip_device_preflight: false, } } diff --git a/crates/rocm-dash-collectors/Cargo.toml b/crates/rocm-dash-collectors/Cargo.toml index 4caf9e91a..da6c5c631 100644 --- a/crates/rocm-dash-collectors/Cargo.toml +++ b/crates/rocm-dash-collectors/Cargo.toml @@ -28,4 +28,5 @@ reqwest = { version = "0.12", default-features = false } [dev-dependencies] tokio = { version = "1", features = ["full"] } +tempfile = "3" wiremock = "0.6" diff --git a/crates/rocm-dash-collectors/src/amd_smi.rs b/crates/rocm-dash-collectors/src/amd_smi.rs index f19e20b82..cef29e17d 100644 --- a/crates/rocm-dash-collectors/src/amd_smi.rs +++ b/crates/rocm-dash-collectors/src/amd_smi.rs @@ -4,11 +4,12 @@ //! amd-smi subprocess + JSON parse. //! -//! Field paths and the KFD pre-flight check are vendored from the TypeScript +//! Field paths and the GPU-device pre-flight are vendored from the TypeScript //! `AmdSmiProvider` in instinct-dash. See `../wiki/entities/amd-smi.md`. use std::ffi::OsString; use std::io; +use std::path::Path; use std::time::Duration; use rocm_dash_core::metrics::{GpuMetrics, GpuSystemInfo}; @@ -20,6 +21,7 @@ use tokio::time::timeout; use tracing::warn; const KFD_DEVICE: &str = "/dev/kfd"; +const DXG_DEVICE: &str = "/dev/dxg"; const DETECT_TIMEOUT: Duration = Duration::from_secs(5); const RUN_TIMEOUT: Duration = Duration::from_secs(10); @@ -29,9 +31,11 @@ pub struct AmdSmiCollector { } impl AmdSmiCollector { - /// Returns `Some` only if `/dev/kfd` is readable AND `amd-smi version` succeeds. + /// Returns `Some` only if a supported GPU device is readable and + /// `amd-smi version` succeeds. /// - /// The KFD pre-flight is mandatory: without it, `amd-smi` blocks in + /// The device pre-flight is mandatory: without an accessible `/dev/kfd` on + /// bare-metal Linux or `/dev/dxg` on WSL, `amd-smi` can block in /// uninterruptible kernel sleep (D-state) that no signal can escape. pub async fn detect() -> Option { Self::detect_with_binary("amd-smi").await @@ -47,18 +51,18 @@ impl AmdSmiCollector { } /// Like [`detect_with_binary`](Self::detect_with_binary) but skips the - /// mandatory `/dev/kfd` pre-flight. + /// mandatory GPU-device pre-flight. /// - /// **Test-only.** The KFD pre-flight is a safety guard: against a *real* - /// `amd-smi` on a host without a usable `/dev/kfd`, the process can block in - /// uninterruptible kernel sleep (D-state) that no signal can escape. This - /// entry point exists solely so daemon integration tests can point - /// [`detect_with_binary`](Self::detect_with_binary) at a *fake* script (for - /// which the hang cannot happen) and have it actually run on a GPU-less CI - /// host, instead of short-circuiting to `None` and turning the test into a - /// no-op. Never call it against a real binary in production. + /// **Test-only.** The device pre-flight is a safety guard: against a *real* + /// `amd-smi` on a host without an accessible `/dev/kfd` or `/dev/dxg`, the + /// process can block in uninterruptible kernel sleep (D-state) that no + /// signal can escape. This entry point exists solely so daemon integration + /// tests can point [`detect_with_binary`](Self::detect_with_binary) at a + /// *fake* script (for which the hang cannot happen) and have it actually run + /// on a GPU-less CI host, instead of short-circuiting to `None` and turning + /// the test into a no-op. Never call it against a real binary in production. #[doc(hidden)] - pub async fn detect_with_binary_skipping_kfd_preflight( + pub async fn detect_with_binary_skipping_device_preflight( binary: impl Into, ) -> Option { Self::detect_with_binary_inner(binary, true).await @@ -66,9 +70,11 @@ impl AmdSmiCollector { async fn detect_with_binary_inner( binary: impl Into, - skip_kfd_preflight: bool, + skip_device_preflight: bool, ) -> Option { - if !skip_kfd_preflight && !kfd_accessible() { + if !skip_device_preflight + && !gpu_device_accessible(Path::new(KFD_DEVICE), Path::new(DXG_DEVICE)) + { return None; } let me = Self { @@ -134,11 +140,12 @@ impl AmdSmiCollector { } } -fn kfd_accessible() -> bool { - std::fs::OpenOptions::new() - .read(true) - .open(KFD_DEVICE) - .is_ok() +fn gpu_device_accessible(kfd_device: &Path, dxg_device: &Path) -> bool { + device_accessible(kfd_device) || device_accessible(dxg_device) +} + +fn device_accessible(path: &Path) -> bool { + std::fs::OpenOptions::new().read(true).open(path).is_ok() } fn val_u64(v: Option<&Value>) -> Option { @@ -364,6 +371,14 @@ fn parse_memory(v: Option<&Value>) -> MemoryPartitionMode { #[cfg(test)] mod tests { use super::*; + #[test] + fn dxg_access_satisfies_device_preflight_without_kfd() { + let dir = tempfile::tempdir().unwrap(); + let dxg = dir.path().join("dxg"); + std::fs::write(&dxg, []).unwrap(); + + assert!(gpu_device_accessible(&dir.path().join("missing-kfd"), &dxg)); + } const SAMPLE_METRIC: &str = r#"{ "gpu_data": [ diff --git a/crates/rocm-dash-daemon/src/runner.rs b/crates/rocm-dash-daemon/src/runner.rs index b4c95979a..365d71c78 100644 --- a/crates/rocm-dash-daemon/src/runner.rs +++ b/crates/rocm-dash-daemon/src/runner.rs @@ -74,14 +74,14 @@ pub struct RunnerOptions { /// so the caller resolves it (via `rocm_core::resolve_amd_smi_binary`) and /// passes it here. `None` falls back to looking up `amd-smi` on `PATH`. pub amd_smi_binary: Option, - /// **Test-only.** Skip the mandatory `/dev/kfd` pre-flight in amd-smi + /// **Test-only.** Skip the mandatory GPU-device pre-flight in amd-smi /// detection so a *fake* `amd_smi_binary` is actually invoked on a GPU-less /// CI host instead of short-circuiting to "no GPU". Never set in - /// production: the KFD guard prevents a *real* `amd-smi` from hanging in + /// production: the device guard prevents a *real* `amd-smi` from hanging in /// uninterruptible D-state. Only the daemon integration test that points /// `amd_smi_binary` at a deliberately-slow fake script flips this, so the /// off-critical-path detection behaviour is genuinely exercised. - pub amd_smi_skip_kfd_preflight: bool, + pub amd_smi_skip_device_preflight: bool, } impl Default for RunnerOptions { @@ -101,7 +101,7 @@ impl Default for RunnerOptions { persist_dir: None, services_dir: None, amd_smi_binary: None, - amd_smi_skip_kfd_preflight: false, + amd_smi_skip_device_preflight: false, } } } @@ -219,13 +219,13 @@ pub async fn run_loop( // the loop starts ticking immediately (surfacing serving instances within // one discovery tick) and GPU metrics fill in the moment detection lands. let amd_smi_binary = opts.amd_smi_binary.clone(); - let amd_smi_skip_kfd_preflight = opts.amd_smi_skip_kfd_preflight; + let amd_smi_skip_device_preflight = opts.amd_smi_skip_device_preflight; let (gpu_init_tx, mut gpu_init_rx) = tokio::sync::oneshot::channel::<(Option, Option)>(); tokio::spawn(async move { let gpu = match amd_smi_binary { - Some(binary) if amd_smi_skip_kfd_preflight => { - AmdSmiCollector::detect_with_binary_skipping_kfd_preflight(binary).await + Some(binary) if amd_smi_skip_device_preflight => { + AmdSmiCollector::detect_with_binary_skipping_device_preflight(binary).await } Some(binary) => AmdSmiCollector::detect_with_binary(binary).await, None => AmdSmiCollector::detect().await, @@ -273,7 +273,7 @@ pub async fn run_loop( ); } else { warn!( - "amd-smi not available (no /dev/kfd or `amd-smi version` failed); GPU disabled" + "amd-smi not available (no accessible GPU device or `amd-smi version` failed); GPU disabled" ); } gpu = detected; @@ -305,7 +305,7 @@ pub async fn run_loop( } } } else if gpu_init_done { - warnings.push("amd-smi unavailable (no /dev/kfd or binary missing)".into()); + warnings.push("amd-smi unavailable (GPU device inaccessible or probe failed)".into()); Vec::new() } else { // Detection is still in flight (spawned off the critical path), so diff --git a/crates/rocm-dash-daemon/tests/end_to_end.rs b/crates/rocm-dash-daemon/tests/end_to_end.rs index 49194f653..47632909e 100644 --- a/crates/rocm-dash-daemon/tests/end_to_end.rs +++ b/crates/rocm-dash-daemon/tests/end_to_end.rs @@ -339,7 +339,7 @@ async fn scrape_warning_persists_between_scrape_ticks() { /// first snapshot carrying the instance already has `gpu_system_info == Some`. /// /// The fake `amd-smi` is forced through the real detection path via -/// `amd_smi_skip_kfd_preflight` (see `RunnerOptions`): otherwise the `/dev/kfd` +/// `amd_smi_skip_device_preflight` (see `RunnerOptions`): otherwise the GPU-device /// pre-flight short-circuits to "no GPU" on GPU-less CI, the fake never runs, /// and the assertion passes vacuously even with the fix reverted. The surfaced /// snapshot must also carry NO "amd-smi unavailable" warning — detection is @@ -378,9 +378,9 @@ async fn slow_gpu_detection_does_not_delay_service_discovery() { services_dir: Some(services_dir), amd_smi_binary: Some(fake_bin), // Drive the fake through the real detection path on GPU-less CI: - // without this the `/dev/kfd` pre-flight would short-circuit before + // without this the GPU-device pre-flight would short-circuit before // the fake runs, making the ordering assertion vacuous. - amd_smi_skip_kfd_preflight: true, + amd_smi_skip_device_preflight: true, disable_vllm_metrics: true, ..Default::default() }; diff --git a/tests/e2e-cucumber/features/dash.feature b/tests/e2e-cucumber/features/dash.feature index 16ad1b267..0401a79ed 100644 --- a/tests/e2e-cucumber/features/dash.feature +++ b/tests/e2e-cucumber/features/dash.feature @@ -123,3 +123,10 @@ Feature: Interactive dashboard Then the launcher shows the model serving When the user quits the launcher Then the launcher exits successfully + + @id:dash-rocm-29846-wsl-gpu-telemetry @requires-wsl @requires-gpu + Scenario: dash-11 - Dashboard displays GPU telemetry through the WSL amd-smi backend + When the user opens the dashboard + Then the dashboard reports live WSL GPU telemetry + When the user quits the dashboard + Then the dashboard exits successfully diff --git a/tests/e2e-cucumber/tests/e2e/dash_steps.rs b/tests/e2e-cucumber/tests/e2e/dash_steps.rs index d49b9476b..e6e6ab1e9 100644 --- a/tests/e2e-cucumber/tests/e2e/dash_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/dash_steps.rs @@ -257,6 +257,34 @@ async fn home_view_displayed(world: &mut E2eWorld) { ); } +#[then("the dashboard reports live WSL GPU telemetry")] +async fn wsl_gpu_telemetry_displayed(world: &mut E2eWorld) { + let tui = session(world); + tui.use_detail_size() + .unwrap_or_else(|e| panic!("failed to enlarge the dashboard: {e}")); + + let budget = default_timeout(); + let deadline = Instant::now() + budget; + loop { + let screen = tui.screen_text(); + let nonzero_gpu_count = screen + .lines() + .any(|line| line.contains("GPUs · ") && !line.contains("GPUs · 0")); + if nonzero_gpu_count + && screen.contains("Node throughput ·") + && !screen.contains("Unknown GPU") + && !screen.contains("amd-smi unavailable") + { + break; + } + assert!( + Instant::now() < deadline, + "ROCM-29846: WSL GPU telemetry did not appear within {budget:?}:\n{screen}" + ); + tokio::time::sleep(Duration::from_millis(20)).await; + } +} + #[then("ROCm setup actions are displayed")] async fn rocm_actions_displayed(world: &mut E2eWorld) { session(world)