DIAGNOSTIC: locate the MI355X hipErrorIllegalAddress with serialized kernel launches - #2641
DIAGNOSTIC: locate the MI355X hipErrorIllegalAddress with serialized kernel launches#2641sammshen wants to merge 52 commits into
Conversation
Add an lmcache kv-offload-backend point at TP8 conc 10 on top of the existing DSpark MTP serving stack, mirroring the vllm-simple offload arm for a direct backend comparison. The benchmark script gains an lmcache case arm that installs the LMCache 0.5.4rc1 ROCm wheel (torch/ROCm stack untouched), starts one MP server per the Kimi-K3 recipe (chunk size 768 = K3 unified block size at 8 GPUs, --separate-object-groups for the hybrid KDA/MLA two-group KV layout, --enable-extra-logging, --max-cpu-workers 8 --max-gpu-workers 1), and wires vLLM to it via LMCacheMPConnector.
A separate kimik3-fp4-mi355x-vllm-agentic-mtp-lmcache key lets the changelog select only the LMCache points instead of re-running the resident and vllm-simple arms of the base key. The base key returns to its upstream shape.
The LMCache MP server's L1 lives in /dev/shm and the script rejects budgets above 90% of free shm. mi355x-amds nodes mount ~1.5 TB of shm (cap ~1360 GB), so 0.50's 1499 GB budget failed the check in run 31644286169. 0.40 generates ~1199 GB, which fits with margin.
vLLM sizes the K3 unified attention block to 1536 tokens on the MI355X fp8-KV TRITON_MLA path (attention page >= mamba page), and the MP connector asserts chunk %% block == 0, so the recipe's CUDA-path 768 fails engine init (run 31644990546).
The connector requires the chunk to be a multiple of every engine KV group's tokens_per_block. On this stack the hybrid layout registers attention groups at 1536 and a KDA state group at 3072 (run 31645828378), so 1536 fails registration; 3072 is the minimum valid chunk.
Auto mode loads both transfer paths; pin server-driven STORE/RETRIEVE (as the MiniMax-M3 arm does) so the benchmark measures one deterministic path. The L1 stays shm-backed either way, so the /dev/shm capacity check still applies.
The default 300s read-lock TTL expires under a single GPU worker serializing huge K3 transfers: run 31648224111 logged 57k finish-read-on-non-read-locked-key warnings starting exactly at warmup+300s, followed by a GPU illegal-access crash mid-profile. Match the MiniMax-M3 arm's 7200s read TTL.
This reverts commit 20b4fda.
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: ApostaC <yihua98@uchicago.edu>
Signed-off-by: ApostaC <yihua98@uchicago.edu>
Signed-off-by: ApostaC <yihua98@uchicago.edu>
Signed-off-by: ApostaC <yihua98@uchicago.edu>
# Conflicts: # perf-changelog.yaml
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
…SemiAnalysisAI/InferenceX into amd/kimi-k3-agentic-perf-tuning
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
Signed-off-by: Sirra <asirra@amd.com>
# Conflicts: # perf-changelog.yaml
The previous merge left conflict markers in the file and my scripted fallback asserted before rewriting it, so the broken version was committed and pushed. Rebuild deterministically as 'upstream/main verbatim + this PR's entry': zero deletions vs main, valid YAML, 733 entries, #2602's entry absent so its sweep no longer runs here.
Signed-off-by: ApostaC <yihua98@uchicago.edu>
Signed-off-by: ApostaC <yihua98@uchicago.edu>
Not for merge. Adds kimik3-fp4-mi355x-vllm-agentic-mtp-nokv: identical to the -lmcache key (same image nightly-ac7509e2b1, same script, TP8/EP1, spec-mtp, conc 4/8/10/12/16) with kv-offloading: none, so the run starts no LMCache server and passes no --kv-transfer-config. Purpose: isolate the hipErrorIllegalAddress crashes. They have appeared on mia1-p01-g11 across the LMCache arm, #2602's vllm-simple arm and two LMCache versions -- 0-for-8 on that node -- which points at the node rather than the connector. This runs the same points with the connector absent to confirm that directly. Expected: if the crashes persist with no KV transfer config, they are not LMCache's. If they vanish, the connector is implicated after all.
# Conflicts: # perf-changelog.yaml
Not for merge. Adds kimik3-fp4-mi355x-vllm-agentic-mtp-serialize and a
DIAGNOSTIC env block in kimik3_fp4_mi355x_mtp.sh that sets
AMD_SERIALIZE_KERNEL=3 plus TORCH_USE_HIP_DSA/TORCH_USE_CUDA_DSA.
Every traceback collected for the hipErrorIllegalAddress so far is unusable
for locating it. HIP reports kernel faults asynchronously, so the Python
stack unwinds at whatever CUDA call next checks the error flag. In our logs
that is triton's _init_handles() -> get_current_device() while *loading*
_fwd_kernel_stage2 -- a kernel that had not been launched yet. vLLM prints
the caveat itself ("the stacktrace below might be incorrect"), and the
mid-run variant of the same crash unwinds at SetDevice with no Python frames
at all. Serializing launches makes the fault surface at the kernel that
actually causes it.
Uses vllm-simple rather than LMCache deliberately: SimpleCPUOffloadConnector
is vLLM's own CPU offload and reproduces the identical crash with no LMCache
in the picture, which keeps the repro minimal. Both c8 and c10 failed on that
backend in run 31837518310. The crash reproduces during warmup, so the run
only needs to reach startup; serialized launches make every timing from this
key meaningless.
Also repairs the #2635 changelog entry, whose pr-link had been overwritten by
a stray duplicate fragment of the #2609 entry (duplicate YAML key, last one
winning), leaving #2635 credited to #2609 and #2609 listed twice.
…lize # Conflicts: # perf-changelog.yaml
The branch was cut from kimik3-mi355x-nokv, so it carried that PR's changelog entry as well and the sweep selected both keys -- re-running the five nokv points that already have clean results, under AMD_SERIALIZE_KERNEL=3, on the same MI355X nodes the serialize jobs need. Those runs would be enormously slow and their numbers meaningless. Drop the nokv entry here so only kimik3-fp4-mi355x-vllm-agentic-mtp-serialize sweeps. The entry stays on #2635, which owns it; this removes nothing that exists on main.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=32093834284 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| # ---- DIAGNOSTIC: synchronous kernel fault reporting ------------------------- | ||
| # NOT FOR MERGE. Remove this block once the hipErrorIllegalAddress is located. | ||
| # | ||
| # Every traceback we have collected for that crash is useless for locating it. | ||
| # HIP reports kernel faults asynchronously, so the Python stack unwinds at | ||
| # whatever CUDA call happens to check the error flag next -- in our logs that | ||
| # is triton's _init_handles() -> get_current_device() while *loading* | ||
| # _fwd_kernel_stage2, a kernel that had not been launched yet. vLLM prints the | ||
| # caveat itself: "the stacktrace below might be incorrect." | ||
| # | ||
| # AMD_SERIALIZE_KERNEL=3 makes every kernel launch wait for completion, so the | ||
| # fault surfaces at the kernel that actually caused it. TORCH_USE_HIP_DSA turns | ||
| # on the device-side assertion tracking the abort message explicitly asks for | ||
| # ("Device-side assertion tracking was not enabled by user"). | ||
| # | ||
| # This is very slow by design. The crash reproduces during warmup | ||
| # (compile_or_warm_up_model -> _dummy_run), so the run only needs to reach | ||
| # startup; any throughput numbers it produces are meaningless. | ||
| DIAG_SERIALIZE_KERNEL="${DIAG_SERIALIZE_KERNEL:-1}" | ||
| if [[ "$DIAG_SERIALIZE_KERNEL" == "1" ]]; then | ||
| export AMD_SERIALIZE_KERNEL=3 | ||
| export TORCH_USE_HIP_DSA=1 | ||
| export TORCH_USE_CUDA_DSA=1 | ||
| # Serialized launches make warmup far slower than the normal budget allows; | ||
| # the point is to reach the fault, not to finish quickly. | ||
| export VLLM_ENGINE_READY_TIMEOUT_S="${VLLM_ENGINE_READY_TIMEOUT_S:-14400}" | ||
| export VLLM_EXECUTE_MODEL_TIMEOUT_SECONDS="${VLLM_EXECUTE_MODEL_TIMEOUT_SECONDS:-3600}" |
There was a problem hiding this comment.
🔴 kimik3_fp4_mi355x_mtp.sh:124 sets DIAG_SERIALIZE_KERNEL=${DIAG_SERIALIZE_KERNEL:-1}, defaulting AMD_SERIALIZE_KERNEL=3 + TORCH_USE_HIP_DSA/CUDA_DSA + 4h/1h timeouts ON whenever the var is unset (i.e. always in CI). This one script backs all four kimik3-fp4-mi355x-vllm-agentic-mtp* config keys, so the base perf key and the -lmcache key (neither of which set DIAG_SERIALIZE_KERNEL=0) also run fully serialized, producing invalid throughput and much longer runs — contradicting the PR description that only -serialize should pay this cost. Default should be 0, with only the -serialize scenario opting in.
Extended reasoning...
kimik3_fp4_mi355x_mtp.sh is the single launch script shared by all four kimik3-fp4-mi355x-vllm-agentic-mtp* config keys in configs/amd-master.yaml — base, -lmcache, -nokv, and the new -serialize diagnostic key. Line 124 sets DIAG_SERIALIZE_KERNEL="${DIAG_SERIALIZE_KERNEL:-1}", so whenever the environment variable is unset — which it always is in CI, since none of the four config keys set it via additional-settings or otherwise — the block on lines 125-132 fires unconditionally: AMD_SERIALIZE_KERNEL=3, TORCH_USE_HIP_DSA=1, TORCH_USE_CUDA_DSA=1, plus a 14400s engine-ready timeout and a 3600s execute timeout.
The PR description is explicit that this diagnostic cost should be isolated to the new -serialize key: 'The crash reproduces during warmup... All timings from this key are meaningless.' It frames -serialize as the one arm that trades speed for a real backtrace, implying the other three keys are unaffected. But because the default lives in the shared script and nothing overrides it back to 0 for the other keys, every one of them inherits the same serialization and DSA overhead.
This matters concretely because the PR itself modifies the base key (kimik3-fp4-mi355x-vllm-agentic-mtp) — a real, non-diagnostic perf sweep whose conc-list this PR expands to [1, 4, 8, 10, 12, 14, 16, 20] — and the -lmcache key, another production-perf arm. Both will now run with kernels fully serialized and device-side assertions on, which invalidates their throughput numbers (serialized launches are dramatically slower than normal, by design) and burns far more GPU time than intended. It also perturbs the -nokv key's timing, undermining its purpose as a clean isolation control for the crash investigation.
Step-by-step proof: (1) In CI, no shell sets DIAG_SERIALIZE_KERNEL before the script runs, for any of the four config keys — confirmed by grep, the variable appears nowhere else in the repo. (2) Line 124 evaluates ${DIAG_SERIALIZE_KERNEL:-1}, which resolves the unset variable to 1. (3) The if [[ "$DIAG_SERIALIZE_KERNEL" == "1" ]] check on line 125 is therefore true for every invocation, regardless of which of the four config keys triggered it. (4) The base key's search-space entry (configs/amd-master.yaml, the kimik3-fp4-mi355x-vllm-agentic-mtp block) has no additional-settings field at all, so it has no way to set DIAG_SERIALIZE_KERNEL=0 and opt out. (5) Result: the base key's full conc-list sweep runs serialized, producing throughput numbers that do not reflect real performance and take far longer in CI than the sweep is budgeted for.
The fix is to flip the default: DIAG_SERIALIZE_KERNEL="${DIAG_SERIALIZE_KERNEL:-0}", and have only the new -serialize config key opt in, e.g. via additional-settings: ["DIAG_SERIALIZE_KERNEL=1"]. That keeps the diagnostic behavior scoped to the one key designed to pay for it, matching what the PR description says should happen.
| # _patch <file> <already-patched-marker> <<'PYEOF' ... old/new python ... PYEOF | ||
| # The heredoc body must define OLD and NEW strings. | ||
| _patch() { | ||
| local target="$1" marker="$2" label="$3" | ||
| if [ -z "$target" ] || [ ! -f "$target" ]; then | ||
| echo "[$label] target not found; skipping." | ||
| return 0 | ||
| fi | ||
| if grep -q "$marker" "$target"; then | ||
| echo "[$label] already patched." | ||
| return 0 | ||
| fi | ||
| cp -n "$target" "$target.orig" 2>/dev/null || true | ||
| if $PY - "$target" "$label"; then | ||
| return 0 | ||
| else | ||
| echo "[$label] patch failed; left unchanged." >&2 | ||
| return 0 |
There was a problem hiding this comment.
🟡 sweep:^_patch\(\)
The generic _patch() helper (lines 41-56) documents a heredoc-based OLD/NEW replacement contract but is never called; e.g. patch_aiter_pybind11 reimplements the same find-target/marker-check/backup/apply/report sequence inline instead of calling it. Delete the dead helper or route the three patch functions through it.
Extended reasoning...
This PR rewrites apply_k3_container_patches.sh from scratch and, as part of that rewrite, introduces a generic _patch() helper (lines 39-56) with a documented contract: pass a target path, an already-patched marker string, and a heredoc body that must define OLD/NEW strings for a Python-based string replacement. A grep -n '_patch' over the file turns up exactly two hits — the doc comment on line 39 and the function definition on line 41 — and no call site anywhere else in the ~264-line script.
Instead, all three real patches (patch_aiter_pybind11, patch_triton_mla_cudagraph, patch_kv_blockpool) hand-roll the identical control flow inline: resolve the target file via _modfile, bail if it's missing, grep for an already-patched marker and skip if present, cp -n a .orig backup, invoke a self-contained Python heredoc ($PY - "$target" <<'EOF' ... EOF) to do the actual source edit, and echo a failure message if the Python step exits non-zero. That's the exact sequence _patch() was written to capture, duplicated three times.
Worse, _patch()'s own body doesn't even match its documented contract: it calls $PY - "$target" "$label" with no heredoc piped into it at all, so even if something called it today, there's no OLD/NEW heredoc mechanism actually wired up — the helper rotted out of sync with its own docstring before ever having a caller. This is a strong signal it was written aspirationally during the rewrite (or copied from KV-block-pool-style patch scripts elsewhere) and never actually integrated.
Impact is purely maintenance/quality — the script behaves correctly today because the three patch functions are self-contained and don't rely on _patch() at all. But it leaves ~15 lines of dead, internally-inconsistent code sitting in a file that future edits (per the file's own stated idempotent/marker-gated design) are likely to reference or copy from, risking someone wiring a fourth patch through the broken _patch() contract instead of the working inline pattern.
Fix is mechanical: either delete _patch() (lines 39-56) entirely since it has zero references, or refactor the three functions to route through it once its OLD/NEW heredoc contract is actually implemented to match how the real patches build their Python edit scripts.
Proof: running grep -n '_patch(' benchmarks/single_node/agentic/apply_k3_container_patches.sh against the PR's version of the file returns only the definition line (_patch() {) — no other line in the file contains _patch( as a call. Each of the three real functions independently defines its own $PY heredoc rather than passing one to _patch, confirming the helper is unused rather than merely underused.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=32094050099 |
Two points is too few to say anything about an intermittent crash. All four of these failed on vllm-simple in run 31837518310, so they are the points with a known unserialized failure to compare against.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=32167623715 |
|
Diagnostic complete, result negative. AMD_SERIALIZE_KERNEL=3 induces its own HSA_STATUS_ERROR_OUT_OF_RESOURCES failures (surfacing as NCCL all_reduce errors), so this arm cannot produce a clean signal regardless of how many times it runs. Zero illegal-access hits across all jobs, but that is uninterpretable given the instrumentation-induced failures. Also noted: TORCH_USE_HIP_DSA does not work as a runtime env var -- logs still print 'Device-side assertion tracking was not enabled by user'. It requires a torch built with it. Closing as a dead end. #2598 reproduces the crash at startup without instrumentation, which is a better repro. |
4 participants
Diagnostic only — not for merge.
Why
Every traceback we have for the MI355X
hipErrorIllegalAddressis unusable for locating the fault. HIP reports kernel errors asynchronously, so the Python stack unwinds at whatever CUDA call next checks the error flag:_init_handles()is Triton loading the compiled kernel —_fwd_kernel_stage2had not launched yet. vLLM prints the caveat itself: "the stacktrace below might be incorrect." The mid-run variant of the same crash unwinds atSetDevicewith no Python frames at all.What this does
Sets
AMD_SERIALIZE_KERNEL=3(every launch waits for completion, so the fault surfaces at the causing kernel) andTORCH_USE_HIP_DSA/TORCH_USE_CUDA_DSA(the abort message explicitly asks for device-side assertion tracking). Engine timeouts are raised since serialized launches are slow.Why vllm-simple, not LMCache
SimpleCPUOffloadConnectoris vLLM's own CPU offload and reproduces the identical crash with no LMCache in the picture, which keeps the repro minimal. Both c8 and c10 failed on that backend in run 31837518310.Context
Established so far: crashes occur only with DRAM KV offload and are backend-agnostic. The no-KV control (#2635) passed 5/5 with DSpark fully active, so speculative decoding alone is not the trigger. Full writeup: Confluence.
Expected outcome
The crash reproduces during warmup, so the run only needs to reach startup. All timings from this key are meaningless — serialized launches make the run far slower than normal. The deliverable is a traceback naming the real faulting kernel.
Also in this PR
Repairs #2635's changelog entry: its
pr-linkhad been overwritten by a stray duplicate fragment of the #2609 entry (duplicate YAML key, last one winning), leaving #2635 credited to #2609 and #2609 listed twice.