Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
503 changes: 498 additions & 5 deletions crates/rocm-core/src/diagnose.rs

Large diffs are not rendered by default.

1,482 changes: 1,463 additions & 19 deletions crates/rocm-core/src/examine.rs

Large diffs are not rendered by default.

46 changes: 43 additions & 3 deletions crates/rocm-core/src/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ use std::time::Duration;
const RUN_TIMEOUT: Duration = Duration::from_mins(1);
const QUERY_TIMEOUT: Duration = Duration::from_secs(8);

/// Shared verbatim by this recipe's own note and `diagnose.rs`'s
/// `check_18_comgr_conflict` evidence, which states the same claim in its own
/// words at the point the real paths are known. A second statement of one
/// claim is a second thing to keep correct, and the two had already drifted
/// apart in wording before they shared this constant.
pub(crate) const COMGR_CONFLICT_NEITHER_OPTION_RECOMMENDED: &str = "Neither option is recommended over the other: which is right depends on which stack you mean to keep, and removing the wrong one breaks a working environment.";

/// Print a failure explanation to stderr, ignoring write failures (closed
/// stderr, full disk) so an I/O error while explaining a failure can't itself
/// panic the process.
Expand Down Expand Up @@ -694,6 +701,39 @@ const RECIPES: &[FixRecipe] = &[
applies_on: PRINT_ON_WSL,
runner: None,
},
FixRecipe {
fix_id: "fix-18-comgr-conflict",
title: "Code object manager library does not belong to the active HIP runtime",
rationale: "HIP compiles device code at run time through libamd_comgr, and this machine holds more than one copy of it. The copy the loader picks belongs to a different installation than the HIP runtime that loads, so compilation fails with an error that names neither the library nor the second copy. A second copy is not itself a fault -- many correct installations hold one -- so what is reported here is specifically the mismatch.",
// No repair, and no recommendation between the two. Removing a stack or
// reordering the search path can each break a working Python
// environment, and which is right depends on which stack the user means
// to keep -- a question only they can answer. `rocm diagnose` fills in
// the real paths for this machine; these are the shapes.
commands: &[
"# Find every copy and which one loads:",
"rocm examine --json # read comgr_paths, comgr_selected, hip_selected",
"# Then pick ONE of the following. They are alternatives, not steps.",
"# (a) Keep the system installation: remove or uninstall the wheel that",
"# supplies the second copy.",
"# (b) Keep the wheel: order the search path so the wheel's own copy of",
"# both libraries is found first, making the wheel the active runtime.",
"export LD_LIBRARY_PATH=\"<directory of the wheel's own copy>:$LD_LIBRARY_PATH\"",
],
needs_sudo: false,
needs_reboot: false,
needs_relogin: false,
verify: "python -c \"import torch; torch.zeros(1, device='cuda')\"",
notes: &[
COMGR_CONFLICT_NEITHER_OPTION_RECOMMENDED,
"This describes the environment outside the CLI's managed runtimes. `rocm serve` puts a managed runtime's libraries first on purpose, so inside one the wheel copy wins by design and that is correct.",
],
// Not `PRINT_ON_LINUX`: which copy the loader picks has nothing to do
// with the amdgpu module, and the two copies collide on WSL2 just the
// same. Print-only on both: neither way out can be chosen for the user.
applies_on: PRINT_ON_LINUX_AND_WSL,
runner: None,
},
FixRecipe {
fix_id: "fix-19-shm-too-small",
title: "Raise the shared memory allowance",
Expand Down Expand Up @@ -2109,9 +2149,9 @@ mod tests {
let count = ids.len();
ids.dedup();
assert_eq!(ids.len(), count, "duplicate fix-id in RECIPES");
// 17 bare-metal/Windows entries (fix-17 and fix-19 among them) plus the
// 7 WSL ones.
assert_eq!(count, 24, "expected 24 catalog entries");
// 18 bare-metal/Windows entries (fix-17, fix-18 and fix-19 among them)
// plus the 7 WSL ones.
assert_eq!(count, 25, "expected 25 catalog entries");
}

/// Restored, not new. This PR made the `wsl` arm of the platform lookup
Expand Down
150 changes: 130 additions & 20 deletions crates/rocm-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2600,7 +2600,7 @@ pub(crate) fn is_wsl1_kernel(kernel_release: &str) -> bool {
///
/// Search the conventional locations, and report "could not ask" as `None`
/// rather than as an empty answer.
fn ldconfig_cache() -> Option<String> {
pub(crate) fn ldconfig_cache() -> Option<String> {
for program in ["ldconfig", "/sbin/ldconfig", "/usr/sbin/ldconfig"] {
if let Some(text) = capture_optional_command(program, &["-p"]) {
return Some(text);
Expand Down Expand Up @@ -4117,7 +4117,9 @@ fn push_existing_runtime_path(paths: &mut Vec<PathBuf>, path: PathBuf) {
paths.push(path);
}

fn managed_therock_sdk_probe_candidates(registry_dir: &Path) -> Vec<TheRockSdkProbeCandidate> {
pub(crate) fn managed_therock_sdk_probe_candidates(
registry_dir: &Path,
) -> Vec<TheRockSdkProbeCandidate> {
let Ok(entries) = fs::read_dir(registry_dir) else {
return Vec::new();
};
Expand Down Expand Up @@ -4153,6 +4155,7 @@ fn managed_therock_sdk_probe_candidates(registry_dir: &Path) -> Vec<TheRockSdkPr
site_packages: sdk.site_packages,
root_path,
bin_path,
library_paths: sdk.library_paths,
});
}
candidates.sort_by_key(|candidate| std::cmp::Reverse(candidate.installed_at_unix_ms));
Expand All @@ -4174,20 +4177,11 @@ fn managed_sdk_tool_path(bin_path: &Path, tool: &str) -> Option<PathBuf> {

fn managed_sdk_ld_library_path(candidate: &TheRockSdkProbeCandidate) -> Option<OsString> {
let mut paths = Vec::new();
collect_sdk_library_paths(&candidate.root_path, &mut paths);
if let Some(site_packages) = candidate.site_packages.as_deref()
&& let Ok(entries) = fs::read_dir(site_packages)
{
for entry in entries.flatten() {
let path = entry.path();
let Some(name) = path.file_name().and_then(|value| value.to_str()) else {
continue;
};
if name.starts_with("_rocm_sdk_") {
collect_sdk_library_paths(&path, &mut paths);
}
}
}
collect_managed_runtime_library_paths(
&candidate.root_path,
candidate.site_packages.as_deref(),
&mut paths,
);
let wsl_lib = PathBuf::from("/usr/lib/wsl/lib");
if wsl_lib.is_dir() {
paths.push(wsl_lib);
Expand All @@ -4204,7 +4198,64 @@ fn managed_sdk_ld_library_path(candidate: &TheRockSdkProbeCandidate) -> Option<O
}
}

fn collect_sdk_library_paths(root: &Path, paths: &mut Vec<PathBuf>) {
/// Every library directory a managed runtime keeps, given its root and the
/// `site-packages` its SDK recorded.
///
/// One description of the layout, deliberately. A wheel-format runtime does not
/// keep its ROCm libraries under the root: they sit in a sibling `_rocm_sdk_*`
/// package inside `site-packages`, and a caller that walks the root alone sees
/// an empty runtime rather than a populated one. That is not a difference a
/// caller should have to remember, so it lives here and every search shares it.
pub(crate) fn collect_managed_runtime_library_paths(
root: &Path,
site_packages: Option<&Path>,
paths: &mut Vec<PathBuf>,
) {
collect_sdk_library_paths(root, paths);
// Nothing to do when `site_packages` is `None`, and the absence of an
// `else` is deliberate rather than an oversight.
//
// `None` is not reachable from a real candidate today: `ROCM_SDK_PROBE_SCRIPT`
// has recorded `site_packages` unconditionally, outside its `try`, since the
// probe's initial version, so every manifest that parses at all carries
// `Some` here. `site_packages` stays `Option` because the field is
// `serde(default)` (a read-only probe cannot depend on a registry record
// being current), not because there is a real shape it needs to degrade
// gracefully for.
//
// There also is not a guess worth making if this were ever reached: `root`
// is one of the runtime's own `_rocm_sdk_*` package directories (the probe
// script's `_devel.get_devel_root()` result, or the first package root it
// found when there is no `devel` extra), never a venv root with a
// `root/lib/<python>/site-packages` layout underneath it to re-derive. A
// fallback that assumed that shape previously shipped here and could not
// have been exercised by any real record; removed along with its test
// rather than kept as a guess for a case that cannot arise.
if let Some(recorded) = site_packages {
collect_sdk_package_library_paths(recorded, paths);
}
}

/// Library directories of the `_rocm_sdk_*` packages inside `site_packages`.
///
/// They belong to the runtime that contains them, not to themselves.
fn collect_sdk_package_library_paths(site_packages: &Path, paths: &mut Vec<PathBuf>) {
let Ok(entries) = fs::read_dir(site_packages) else {
return;
};
for entry in entries.flatten() {
let path = entry.path();
if path
.file_name()
.and_then(|value| value.to_str())
.is_some_and(|name| name.starts_with("_rocm_sdk_"))
{
collect_sdk_library_paths(&path, paths);
}
}
}

pub(crate) fn collect_sdk_library_paths(root: &Path, paths: &mut Vec<PathBuf>) {
for path in [
root.join("bin"),
root.join("lib"),
Expand Down Expand Up @@ -4294,11 +4345,19 @@ struct TheRockSdkProbeManifest {
}

#[derive(Debug, Clone)]
struct TheRockSdkProbeCandidate {
pub(crate) struct TheRockSdkProbeCandidate {
installed_at_unix_ms: u128,
site_packages: Option<PathBuf>,
root_path: PathBuf,
pub(crate) site_packages: Option<PathBuf>,
pub(crate) root_path: PathBuf,
bin_path: PathBuf,
/// The SDK's own recorded library directories -- every package root the
/// probe script actually imported and asked Python for (see
/// `ROCM_SDK_PROBE_SCRIPT`'s `add_runtime_root`), not a layout guessed from
/// `root_path`/`site_packages` after the fact. This is what
/// `probe_runtime_devices` puts on `LD_LIBRARY_PATH` for a served process;
/// a caller that needs to find a managed runtime's actual libraries (comgr
/// included) should prefer this over re-deriving the layout.
pub(crate) library_paths: Vec<PathBuf>,
}

pub fn detect_host_gfx_target() -> Option<String> {
Expand Down Expand Up @@ -10525,6 +10584,57 @@ Class Name: Display
Ok(())
}

/// `managed_therock_sdk_probe_candidates` surfaces the SDK's own recorded
/// `library_paths` rather than dropping them.
///
/// Those are what `examine`'s comgr/HIP search now reads to find a managed
/// runtime's libraries (see `known_install_roots`/`install_library_dirs` in
/// `examine.rs`), in place of re-deriving the layout from `root_path` and
/// `site_packages` after the fact -- a guess that does not hold for every
/// real install shape, which is what left a managed runtime's own code
/// object manager library unseen on a real host
/// (`examine-finds-the-managed-runtimes-own-compilation-library`). A
/// candidate whose `library_paths` came back empty would defeat that fix
/// silently, so this pins the field surviving the read.
#[test]
fn managed_sdk_probe_candidate_carries_recorded_library_paths() -> Result<()> {
let (root, paths) = temp_app_paths("managed-sdk-library-paths");
let registry = paths.data_dir.join("runtimes").join("registry");
let site_packages = root.join("site-packages");
let sdk_root = site_packages.join("_rocm_sdk_devel");
let sdk_bin = sdk_root.join("bin");
let comgr_dir = site_packages.join("_rocm_sdk_core").join("lib");
fs::create_dir_all(&sdk_bin)?;
fs::create_dir_all(&comgr_dir)?;
fs::create_dir_all(&registry)?;
fs::write(
registry.join("runtime.json"),
serde_json::to_vec_pretty(&serde_json::json!({
"runtime_id": "therock-release:gfx120X-all",
"family": "gfx120X-all",
"installed_at_unix_ms": 10,
"rocm_sdk": {
"import_ok": true,
"site_packages": site_packages,
"root_path": sdk_root,
"bin_path": sdk_bin,
"library_paths": [comgr_dir]
}
}))?,
)?;

let candidates = managed_therock_sdk_probe_candidates(&registry);
assert_eq!(candidates.len(), 1, "expected exactly one candidate");
assert_eq!(
candidates[0].library_paths,
vec![comgr_dir],
"the recorded library_paths must survive into the candidate, or the \
comgr/HIP search has nowhere else reliable to find them"
);
fs::remove_dir_all(root).ok();
Ok(())
}

#[test]
fn managed_sdk_probe_skips_non_therock_manifests() -> Result<()> {
let (root, paths) = temp_app_paths("managed-sdk-skip-non-therock");
Expand Down
7 changes: 6 additions & 1 deletion docs/wsl.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,13 +200,18 @@ The WSL entries, in the order a broken stack usually reveals them:
| `fix-wsl-5-distro-too-old` | The distro release is below the floor in the prerequisites above |
| `fix-wsl-6-host-driver-too-old` | The distro-side plumbing is complete but the Windows host driver is missing or too old |

One entry outside this list also applies here. `fix-19-shm-too-small` is not a
Two entries outside this list also apply here. `fix-19-shm-too-small` is not a
WSL entry, but WSL2 ships the same 64 MiB `/dev/shm` a container does, and a
serving workload needs gigabytes of it. It reports below 1 GiB, so a WSL2 user
can meet it on an otherwise healthy stack. Its guidance names the container and
bare-metal cases; under WSL2 the remedy is the host one, remounting `/dev/shm`
larger and adding the matching `/etc/fstab` line inside the distro.

`fix-18-comgr-conflict` also applies here: a wheel copy and a system copy of
the code object manager library collide on WSL2 exactly as they do on bare
metal, through the same `LD_LIBRARY_PATH`/loader-cache search, so the finding
is not specific to either platform.

Every WSL remedy is print-only. `rocm fix <id>` shows the commands and does not
run them: they either install packages with `sudo`, edit loader configuration, or
belong to the Windows host, and none of that meets the bar an auto-applied fix
Expand Down
2 changes: 1 addition & 1 deletion skills/rocm-doctor/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ passes — the GPU is AMD. Linux, Windows and WSL2 all run the same workflow.
rocm fix <fix-id> --yes # required to apply in a non-interactive shell
```

Only the four auto-applicable fixes are ones the CLI runs itself. The other 20
Only the four auto-applicable fixes are ones the CLI runs itself. The other 21
are **print-only** (bootloader, kernel, reinstall, Windows driver, …): `rocm
fix <id>` just prints the plan for the user to run themselves — no prompt, and
the CLI never performs those.
Expand Down
5 changes: 3 additions & 2 deletions skills/rocm-doctor/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ non-interactive shell without `--yes`, and confirm first. **Two exceptions:**
pinned. Run `rocm fix fix-9-igpu-dgpu --device-index N` (not the bare id)
once you know N.

## Closed catalog (24 failure modes)
## Closed catalog (25 failure modes)

The OS column is the platform family the CLI scopes an entry to, and WSL2 is a
family of its own — not a flavour of `linux`. An entry reaches a WSL host only
Expand All @@ -119,6 +119,7 @@ reporting confident nonsense.
| `fix-14-adrenalin-too-old` | windows | Adrenalin / kernel-mode driver too old for the HIP SDK | `hipInfo` can't enumerate, "driver too old", HSA "no agents found" | print-only |
| `fix-15-msvc-redist` | windows | MSVC runtime missing (HIP DLLs can't load) | `vcruntime140.dll` / `vcruntime140_1.dll` missing | print-only |
| `fix-17-torch-dlpack` | linux | `torch-c-dlpack-ext` loads its CUDA prebuilt on a ROCm torch, aborting vLLM's engine start at import time | vLLM engine start fails on import; error names `torch_c_dlpack_ext` or tvm_ffi's `_optional_torch_c_dlpack` | print-only |
| `fix-18-comgr-conflict` | linux/wsl | The code object manager library (`libamd_comgr`) that would load belongs to a different installation than the HIP runtime that would load, so device code compilation fails with an error naming neither | compilation error naming neither library; `rocm examine --json`'s `comgr_selected`/`hip_selected` resolve to two different `install_root`s | print-only |
| `fix-19-shm-too-small` | linux/wsl | `/dev/shm` too small for a serving workload, which needs gigabytes where a container and WSL2 both default to 64 MB | reported under 1 GiB; a data-loader worker killed by a bus error, or a failed write to a temporary file, with nothing naming shared memory | print-only |
| `fix-wsl-1-gpu-not-exposed` | wsl | `/dev/dxg` absent, so the distro cannot reach the GPU at all | no `/dev/dxg`; in a container, the device was never passed through | print-only |
| `fix-wsl-2-dxcore-missing` | wsl | `/usr/lib/wsl/lib` DXCore shims missing, so the runtime cannot reach the host driver | `/usr/lib/wsl/lib/libdxcore.so` missing (or the directory absent entirely) | print-only |
Expand All @@ -135,7 +136,7 @@ they answer for a different platform family.

Linux-only: fix-3, -4, -5, -7, -10, -11, -12, -17. Windows-only: fix-13, -14, -15.
WSL-only: fix-wsl-1 through fix-wsl-7. Linux + Windows: fix-9.
Linux + WSL: fix-19. Linux + Windows + WSL: fix-1, -2, -6, -8.
Linux + WSL: fix-18, -19. Linux + Windows + WSL: fix-1, -2, -6, -8.

## Framework routing

Expand Down
27 changes: 27 additions & 0 deletions tests/e2e-cucumber/features/diagnose.feature
Original file line number Diff line number Diff line change
Expand Up @@ -476,3 +476,30 @@ Feature: Diagnosing failures and listing fixes
Then the CLI either shows the whole report or says why it will not prepare one
And the CLI states that nothing has been sent
And the CLI prints the address to mail and a link, and starts nothing

# HIP compiles device code at run time through a library a machine can hold
# more than one copy of. When the copy that loads belongs to a different
# installation than the runtime, compilation fails with an error naming
# neither. Both remedies — remove one stack, or reorder the search path — can
# break a working Python environment, and which is right depends on which
# stack the user means to keep. So the CLI states them and changes nothing.
#
# The conflict itself cannot be provoked here: the suite cannot install a
# second ROCm stack, and the detection rule is proven by unit tests that build
# the machine state directly. What this pins is the half that matters if the
# entry ever stops being advisory — that asking for it changes nothing and
# recommends neither option.
#
# `@requires-os:linux` because `fix-18-comgr-conflict` is registered for
# `["linux", "wsl"]` (comgr and LD_LIBRARY_PATH are POSIX-loader concepts, not
# Windows ones). Unlike diagnose-20's preview, this step applies the fix for
# real, so it goes through the fix's own platform gate and would be refused
# for the wrong reason -- "wrong OS", not "advisory" -- on a native Windows
# lane. `@requires-os:linux` matches WSL2 too, which is where this fix does
# apply.
@id:diagnose-fix-comgr-conflict-is-advisory-only @requires-os:linux
Scenario: diagnose-33 - The fix for a shadowed compilation library changes nothing and recommends nothing
Given a user who has chosen the fix for a shadowed compilation library
When the user asks the CLI to apply that fix
Then the CLI explains that it will not make the change itself
And the CLI offers both options without ranking them
Loading
Loading