Feat/env check gpu memory - #566
Merged
Merged
Conversation
amirakb89
force-pushed
the
feat/env-check-gpu-memory
branch
2 times, most recently
from
August 17, 2026 16:45
688c509 to
da20fcb
Compare
amirakb89
marked this pull request as ready for review
August 17, 2026 16:47
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da20fcbcde
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+508
to
+510
| **GPU memory.** Steps 5/6 run the host-memory mesh first (reachability across | ||
| all NIC pairs), then a second pass over the **rail-aligned pairs only** | ||
| (`local[i]` ↔ `remote[i]`) on **GPU memory** via `--use_rocm`, with each NIC |
There was a problem hiding this comment.
This deployment guidance says both steps 5 and 6 run a GPU-memory pass and that it covers only rail-aligned pairs, but the implementation runs GPU memory only from check_inter_node_bw; it first probes every host-reachable pair and then performs a serial diagonal pass, while check_inter_node_lat explicitly performs no GPU pass. Operators following this skill will expect a latency result that never appears and misunderstand the bandwidth matrix, so the description should match the implemented phases.
Useful? React with 👍 / 👎.
amirakb89
force-pushed
the
feat/env-check-gpu-memory
branch
7 times, most recently
from
September 3, 2026 18:13
1fdffd4 to
43f6ea1
Compare
…w severity Three related changes to `mori check` (tools/env_check.sh). 1. Kernel driver detection produced false FAILs report_driver_version() collapsed three states into two, so "active but exports no version" was reported as "not installed". On a host with an in-tree bnxt_re (live, no version node) and a built-in bnxt_en, both drivers failed the check while working perfectly. lsmod also cannot see a driver compiled into the kernel. Adds module_state() -> built-in | loaded | absent, keyed on /sys/module plus a builtin probe, and rewrites report_driver_version() around those three states. Note `modinfo -F version` returns the bogus string "name: <mod>" for a built-in, so it is only consulted for a real .ko. The bnxt path dropped its inline copy of the same buggy logic, and the mlx5_ib presence check moved off lsmod for the same reason. 2. Bandwidth/latency checks never ran, and measured the wrong memory perftest was resolved via $PATH only, at both detection and invocation. The mesh server leg runs over non-interactive ssh whose PATH is a bare /usr/local/bin:/usr/bin:..., so a bare tool name could never resolve there. Detection also probed only the local host while step 5 runs its server remotely, so a peer without perftest showed up as every NIC pair being unreachable rather than as a missing tool. Resolution now searches $PATH then $MORI_PERFTEST_PREFIX/bin and yields an absolute path, per host. --install-perftest builds upstream linux-rdma/perftest with --enable-rocm (opt-in; a check should not compile software as a side effect). The inter-node steps additionally run a GPU-memory pass over the rail-aligned pairs, since MORI transfers VRAM-to-VRAM: if those fail while the host-memory mesh passed, the fault is GPUDirect, not the fabric. Each NIC is paired with its PCIe-closest GPU, mirroring the ordering MORI itself uses in MatchGpuAndNic (NUMA locality, then PCIe proximity) but in dependency-free bash, since the same query must run on a peer where MORI may not be built. The intra-node mesh stays on host memory: it is a fabric reachability probe, and MORI moves data intra-node over XGMI, not RDMA. GPU runs pay a HIP init per process, so their timeouts scale; the host-memory mesh keeps its original budget. Degrades with a single warning rather than per-pair noise when perftest lacks ROCm support or no GPUs are present, and --no-gpu-mem forces host memory. Capability is probed from --help text, not exit status: perftest exits non-zero from --help and this script runs under `set -o pipefail`. 3. Wrong NIC firmware only warned Firmware that is not known-good now FAILs instead of warning, including firmware on a branch that has never been validated. The wrong firmware is a hard blocker for cross-node MORI, not an advisory. An undetectable version stays a warning, as that is a tooling gap rather than a verdict on the firmware. Verified on a 2-node cv350 allocation: all 8 rails pass on GPU memory at 386.8-387.2 Gbps and 21.5-28.8 us, with each NIC on its correctly paired GPU. The QoS, DCQCN and reporting functions are byte-identical to main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…heck
The RoCE userspace library check searched only /usr/local/lib, and only for
the Broadcom out-of-tree shape libbnxt_re-<fw-style version>.so. A host using
the rdma-core in-tree provider therefore got
[WARN] libbnxt_re-<ver>.so not found under /usr/local/lib
while a perfectly functional provider sat at
/usr/lib64/libibverbs/libbnxt_re-rdmav59.so. The directory was wrong and the
name pattern could not have matched it either: the in-tree provider is
versioned by rdma-core ABI (rdmav59), not by a firmware-style version.
Search the standard libibverbs provider directories alongside /usr/local/lib,
and recognise both shapes. The out-of-tree build still takes precedence and
still reports its version, since that is the one expected to track the kernel
driver; the in-tree provider reports its rdma-core package version instead.
$LD_LIBRARY_PATH entries are searched too, so containers that stage the host
provider on a non-standard path work without hardcoding their layout.
Only a genuinely absent provider warns now, and the message lists where it
looked.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nstall Bring the deploy-mori skill in line with the current `mori check`: - perftest: note that the distro package only covers the host-memory mesh, that the GPU pass needs a ROCm-enabled build, how the binary is resolved ($PATH then $MORI_PERFTEST_PREFIX/bin), and that steps 5/6 need it on the peer as well -- which shared /home satisfies via the default prefix. - Step 7: document the GPU-memory rail pass in steps 5/6, why step 4 stays on host memory (intra-node MORI uses XGMI, not RDMA), the automatic timeout scaling for HIP init, the degrade behaviour, --no-gpu-mem, and --install-perftest. Prefer upstream linux-rdma/perftest over ROCm/rdma-perftest, which is ~15 months stale. - Step 1: firmware that is not known-good now fails rather than warns. - Drop the claim that `mori check` only looks under /usr/local/lib for libbnxt_re; the in-tree provider and $LD_LIBRARY_PATH are accepted too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MORI reported almost nothing about the firmware it was running against. The NIC firmware was read only by the ionic provider, only as a side effect of the CCQE capability probe, and only logged at TRACE; bnxt_re and mlx5 read it nowhere. GPU firmware was not surfaced at all. Both are among the first things asked for in a cross-node bug report, and `mori check` -- which does report them -- is a separate tool users may never run. Log one line per rank at INFO, next to the existing "select device" line, for the NIC/GPU pair that rank actually uses: rank 0 firmware: nic bnxt_re0 fw_ver 232.0.219.0 | gpu 0 [0000:08:00.0] mec_fw 0x00000022 NIC firmware comes from /sys/class/infiniband/<dev>/fw_ver, which every vendor exposes identically, so all three providers are covered by one path with no per-vendor code. GPU MEC firmware comes from /sys/bus/pci/devices/<bdf>/fw_version/mec_fw_version, keyed on the BDF from hipDeviceGetPCIBusId: XCP-partitioned GPUs do not map onto /sys/class/drm/ card<N> in any straightforward order, and the BDF avoids that mapping entirely. It also avoids extending the rsmi symbol loader in topology/gpu.cpp for a value sysfs already has. Missing or unreadable values render as "unknown" rather than failing init. The ionic CCQE probe is left exactly as it was: it keeps its own TRACE line and its existing pass/fail behaviour, and this change does not touch it. Adds ReadSysfsLine to host_utils.hpp and moves ReadKernelBootId onto it. Note vpod_topology.cpp and cco_init.cpp still carry their own private copies of the same helper; consolidating those is left alone here to keep this change free of unrelated risk. Verified on cv350: helpers return 0x00000022 (rocm-smi reports MEC 34) and 232.0.219.0, matching `mori check` Step 1; unknown devices degrade to "unknown"; ReadKernelBootId still resolves after the refactor; context.cpp passes -fsyntax-only; and the log line above is real rendered output. A full library build and multi-rank run were not possible in this environment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…_check
The GPU-memory pass used --use_rocm unconditionally. That path does hipMalloc
plus an ordinary ibv_reg_mr, which requires a peer-memory client in the
kernel; on a host without one the whole pass fails, even though dma-buf
(ibv_reg_dmabuf_mr, --use_rocm_dmabuf) would have worked and needs no peer_mem
at all.
Pick between them by trying, not by inferring. Static detection is
untrustworthy here: /sys/kernel/mm/memory_peers is a Mellanox-OFED artefact
that the in-tree AMD peer-memory client never creates -- amdgpu registers via
ib_uverbs and leaves no sysfs trace, so a host with working peer_mem looks
bare -- and /proc/kallsyms is routinely restricted inside containers. One tiny
loopback transfer answers the question directly, so probe_gpu_mem_mode runs
--use_rocm first and retries with --use_rocm_dmabuf if that fails, then
records which path won. The mode is named in the pass header
("... via peer_mem" / "... via dma-buf") and falling back warns, since it
means the host is missing something the other one has.
The probe sets globals instead of echoing its result: a $(...) capture would
run it in a subshell and discard the mode.
--install-perftest now configures with --enable-rocm-dmabuf and retries
without it if configure rejects it, so the fallback exists on kernels that
support it without breaking the build on those that do not. Both ends must
agree on the registration path, so dma-buf is only offered when the peer's
binary supports it too.
Verified on a 2-node cv350 allocation. With peer_mem present the probe selects
it and reports "via peer_mem". With a shim that makes plain --use_rocm fail,
the probe falls back, warns, and all 8 rails pass "via dma-buf" at
392.5-393.1 Gbps -- matching the peer_mem numbers, and a manual A/B on one
rail gave 359.64 (dma-buf) vs 359.59 Gbps (peer_mem).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dict true Three defects found running `mori check` across two nodes of an mlx5 cluster. The first two made the GPU-memory pass unreachable; the third made its conclusion unfalsifiable. 1. --install-perftest refused to build on a host where the build works The prereq loop gated on `libtool`. perftest's autogen.sh calls `libtoolize`; the `libtool` wrapper script is *generated* by configure into the build tree and is not a build input. Ubuntu ships libtoolize (in libtool-bin's sibling package) without the wrapper, so the check reported "cannot build perftest, missing: libtool" on a host that builds it fine. 2. ...and when it did build, the result was ignored _PERFTEST_RESOLVE_SNIPPET searched $PATH before $MORI_PERFTEST_PREFIX/bin. Distros ship a perftest built without ROCm (--use_cuda only) and it sits on PATH, so it outranked the ROCm-capable binary --install-perftest had just installed. The GPU pass was then skipped advising the reader to run --install-perftest -- which they had. The prefix is only ever populated deliberately, so it is now the stronger signal. 3. "host memory worked" was asserted, never checked rail_check() was never passed the host-memory matrix, so every GPU-memory failure was reported as "GPUDirect RDMA problem (host memory worked)" regardless of whether that rail had a fabric path at all. On the test cluster the two ConnectX-6 management NICs (not on the RDMA fabric, `x` in the host mesh) were reported as GPUDirect faults, pointing at amdgpu and peer_mem for a NIC that never carried RDMA traffic. The differential the feature exists to provide needs both results, so the host CELL is now threaded in and failures are split: passed-on-host -> FAIL, GPUDirect; failed-on-host too -> WARN, no fabric path. Verified on 2 x MI300X + 8 x CX-7: all 8 rails 388-389 Gbps on GPU memory via peer_mem, and mlx5_6 correctly demoted from FAIL to WARN. Co-Authored-By: Claude <noreply@anthropic.com>
Follow-ups from a review of the branch, mostly defects in the GPU-memory work.
1. Probe NIC was chosen arbitrarily (highest impact)
mesh_prepare picked the mode-probe device by iterating ${!MESH_RGPU[@]},
i.e. in bash hash order. MESH_RGPU is built from every device under
/sys/class/infiniband, not the dominant-vendor set the mesh actually tests,
so on a mixed host the probe could land on a management NIC with no fabric
path. Both peer_mem and dma-buf would then fail and the entire GPU pass
would be skipped -- non-deterministically -- on a host where every rail
works. Probe with the first LOCAL_DEVS entry that has a paired GPU instead.
2. $MORI_PERFTEST_PREFIX was unquoted inside the remote ssh command
A prefix containing a space silently truncated on the peer, which then
reported perftest as missing; anything with a glob or $ would be worse.
3. dma-buf state leaked between steps
mesh_prepare reset the other MESH_* globals but not MESH_GPU_DMABUF_OK /
MESH_GPU_DMABUF / MESH_GPU_MODE, and line 617 only ever sets the first. A
step 5 binary with dma-buf followed by a step 6 binary without it left the
stale flag set, producing the wrong remediation hint.
4. Unparseable firmware hard-failed, contradicting its own contract
The docstring says an undetectable version stays a warning, but only the
empty string was guarded. niccli reports "N/A" for RoCE firmware on some
cards, which fell through to the unverified-branch arm and printed
"[FAIL] ... firmware N/A is on an unverified branch (N/A.x)". Both vendor
classifiers now shape-check before judging.
5. In-script docs still described the old perftest resolution order
57ae679 flipped resolution to prefix-then-PATH and updated SKILL.md but
not the function docstring, the --help text, or the two skip messages --
all of which still told the reader $PATH wins. That is exactly the
confusion that commit set out to remove. SKILL.md had one stale line too.
6. A failed GPU rail blamed the local GPUDirect stack
The mode is proven by a loopback on this host only, so a peer missing the
same registration path is indistinguishable from here. Say that in the
failure output rather than implying the fault is local.
Also note the kept build tree on a failed --install-perftest instead of
leaving it silently in $TMPDIR.
Verified: firmware classifier re-tested across empty/N/A/231.x/232.x/235.x/
237.x and the ionic equivalents; snippets confirmed still free of single
quotes (they are embedded in an ssh single-quoted string); full 2-node run
green with the probe now deterministic.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…thresholds The GPU-memory rail pass inherited MESH_PARALLEL from the mesh and ran all 8 rails at once. Eight rails at ~390 Gbps oversubscribes the host, so across four consecutive runs one or two rails landed far below line rate every time -- 73, 75, 127, then 183 and 124 Gbps -- and never the same rail twice. Nothing caught it, because BW_THRESHOLD and LAT_THRESHOLD had been defined and never read since they were introduced, so the pass printed a rail at a third of line rate and concluded "all 8 rail(s) passed". Concurrency is fine for the mesh above it: that step only answers "is this pair reachable", which survives contention. The rail pass exists to produce the per-rail numbers a user would actually quote, so it now runs serially. n rails at a few seconds each is a cheap price for numbers that mean something. With that fixed, both thresholds are usable as written and are now enforced: a rail that completes but misses its threshold fails the step instead of passing quietly. Serial measurement also corrects the record on GPUDirect latency. The pass previously reported 11-28 us and that was attributed to GPUDirect overhead; run serially it is 6.77-7.10 us, matching the host-memory diagonal almost exactly. The overhead was an artefact of the concurrent pass, not a property of GPU memory -- which is also why LAT_THRESHOLD=10 had looked unusable. Measured spread across 8 rails, two consecutive runs: 392.4-393.8 Gbps and 6.77-7.10 us, against 124-393 Gbps and 11-28 us before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Run `mori check` against $NODE2_HOST as a fabric preflight, so a bad fabric shows up as a fabric problem instead of a MORI failure. Runs on the runner host: the CI image has no ssh client or perftest. Non-gating for now (continue-on-error); timeout 45 -> 60 to cover the added mesh. env_check.sh had no exit code -- log_fail only printed, so it exited 0 however many [FAIL] lines it emitted. Count them and exit 1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- intra-node mesh: parallel 32 -> 8. 32 concurrent pairs time out on server bind; measured 7/56 "reachable", all 49 failures false (serially ~312 Gbps). - firmware: poll only niccli indices backing a bnxt_re device. Non-RoCE NICs report 0.0.0.0, which passes the N/A guard and FAILs as branch 0.x. - mesh: drop ports that are not ACTIVE/LinkUp or have a minority link_layer, reporting each exclusion. - GPU pass: gated mesh (only pairs host memory proved) for reachability, plus a serial rail pass for the numbers. Concurrent figures are no longer judged -- they flagged 6-10 of 64 false FAILs per run. GPU latency pass removed: every rail sat at 10.9-11.7 us against a 10 us bar on a fabric at line rate. - warn at the end when the fabric is rail-only, since MORI-EP does not support it; also partial and no-path cases. - label concurrent matrices as reachability probes, not benchmarks. Verified on 2x cv350 gfx950, 8x bnxt_re: intra-node 56/56, GPU rails 385.0-386.4 Gbps, 11 -> 10 failures, the remaining 10 all real. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- context.cpp: classify the NIC firmware it already logs and WARN when it is not known-good. Needs MORI_GLOBAL_LOG_LEVEL=warn or lower; the default is ERROR, so the INFO line above it was already hidden by default. - host_utils.hpp: CheckNicFirmware / CompareFwVersion / ReadNicVendorId, mirroring the table env_check.sh applies in Step 1. Duplicated by hand -- the shell tool and the library share no code, so the constants must be kept in sync. - 232.x is known not to work on Thor2, not merely unverified. Marked as such in env_check.sh, host_utils.hpp, SKILL.md and README; the verdict was already FAIL via the unverified-branch arm, so only the wording changes. - docs: point users at `mori check <peer_ip>` before a cross-node run. It was documented only in the deploy-mori skill and one README parenthetical, so every internode-facing guide said nothing about validating the fabric. Added to the IO and EP benchmark guides and the README. - The rail-only limitation is stated once, in the EP guide: MORI-EP v1 does not support rail-only clusters currently, and rail-only support for the internode-v1 and v1_ll kernels is in progress. env_check.sh's runtime warning says the same, since it cannot link to a doc; the other docs point at the guide rather than repeating it. Verified on cv350 gfx950 + 8x bnxt_re in a ROCm 7.2 container: full build clean, and a single-node shmem init logs [application] [info] rank 0 firmware: nic bnxt_re0 fw_ver 232.0.219.0 | gpu 0 [0000:08:00.0] mec_fw 0x00000022 [application] [warning] rank 0 nic bnxt_re0: Broadcom firmware 232.0.219.0 is on the 232.x branch, which is known not to work on Thor2 -- upgrade to >= 235.2.86.0 or >= 237.1.137.0 Classifier unit-tested across 16 cases (231/232/235/237 boundaries, 0.0.0.0, N/A, empty, AINIC 1.117.1 and the -a-45 boundary, mlx5). host_utils.hpp also gained <algorithm>/<vector>: it used std::vector and std::max while relying on context.cpp including them first, so any other TU would have failed to build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…findings Single-QP ib_write_bw tops out near half of line rate on the ionic GPUDirect path (~162 vs ~340 Gbps), so the rail-aligned GPU pass falsely missed the 300 Gbps threshold; fan the bandwidth test across BW_QP=4. Also apply three PR review findings: - fail (not warn) when GPU memory is unreachable but ROCm perftest and a paired GPU are present, so the check cannot go green with GPUDirect dead - clear HIP/ROCR/CUDA_VISIBLE_DEVICES for the GPU pass so KFD ordinals match perftest's process-visible HIP ordinal space - prefer the symmetric dma-buf path over locally-proven peer_mem when a peer is involved, since a local loopback does not prove remote peer_mem Co-Authored-By: Claude <noreply@anthropic.com>
Satisfies the pre-commit clang-format hook on the NIC/GPU firmware logging added earlier in this branch; whitespace and line-wrapping only. Co-Authored-By: Claude <noreply@anthropic.com>
…ail pairing, docs)
A diagnostic should read state, not fetch and build software. Remove --install-perftest and install_perftest() from env_check.sh: it cloned linux-rdma/perftest HEAD unpinned at check time, made mori check unreproducible and dependent on GitHub access plus a toolchain on the host, and only ever installed locally so the peer still lacked it and steps 5/6 skipped anyway. The GPU-memory pass still runs whenever a ROCm-capable perftest is present; when it is missing the check warns and prints the manual build command (for this host and the peer), then skips that pass. Update deploy-mori docs to match. Also remove the fabric preflight (mori check) step from the internode CI job and revert its timeout bump, since it invoked the removed --install-perftest flag. Also drop the fabric-check notes added to the MORI-EP guide and the MORI-EP / MORI-IO benchmark docs: rail-only guidance there was inaccurate and the topic is covered elsewhere. Addresses PR review feedback from QizhouZhang97.
Remove the firmware-provenance block from Context init and the firmware helpers (ReadNicFirmware, ReadNicVendorId, ReadGpuMecFirmware, CompareFwVersion, CheckNicFirmware) from host_utils.hpp, returning both files to upstream. The firmware check stays in `mori check` (tools/env_check.sh Step 1), which is where this PR keeps it; MORI's runtime no longer duplicates that table.
amirakb89
force-pushed
the
feat/env-check-gpu-memory
branch
from
September 3, 2026 18:46
43f6ea1 to
3a4ca02
Compare
The host-proxy backend (MORI_ENABLE_HOST_PROXY=1) does cross-node RDMA without IBGDA, so old firmware still runs -- only IBGDA needs the known-good minimum. Downgrade the AINIC and Broadcom firmware-version verdicts (old branch, below minimum, unverified branch) from log_fail to log_warn, and point the reader at the proxy backend. Missing-tool and driver-detection failures stay as failures; mlx5 is unaffected.
QizhouZhang97
approved these changes
Sep 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
##Title:
feat(tools): validate GPUDirect RDMA in mori check, and log/judge NIC+GPU firmware at runtime
Summary
Extends
mori check(tools/env_check.sh) to validate the GPUDirect(GPU-memory) RDMA path that MORI-EP and MORI-IO actually transfer over —
rather than assuming host-memory reachability implies it. The check now
exercises VRAM-to-VRAM RDMA over each rail, picks the working GPU-memory
registration mode automatically, and reports NIC firmware against a
known-good table.
Scope is intentionally narrow: 3 files, all tooling/docs — no library or
runtime code changes.
What changed
mori check: GPU-memory RDMA validation (tools/env_check.sh)GPU memory (
--use_rocm, each NIC on its PCIe-closest GPU) and run the meshover the NIC-local GPU per rail, so the check exercises the path MORI
transfers over.
it: try
peer_mem, fall back todma-buf(ibv_reg_dmabuf_mr) when thekernel peer-memory client is absent.
threshold; the concurrent full mesh stays a reachability probe.
and a paired GPU are both present.
mori checknever compiles perftest. If aROCm-capable perftest is missing it warns and prints the build command (for
this host and the peer), then skips the GPU-memory pass.
known-bad/unverified branch) warns rather than fails, since the
host-proxy backend (
MORI_ENABLE_HOST_PROXY=1) does cross-node RDMA withoutIBGDA. Missing-tool / driver-detection failures still fail.