[Bug]: TensorRT-LLM 1.3.0rc18 Llama-3-70B inference failure on 8x NVIDIA RTX 6000D - #17034
[Bug]: TensorRT-LLM 1.3.0rc18 Llama-3-70B inference failure on 8x NVIDIA RTX 6000D#17034pjdurden wants to merge 2 commits into
Conversation
…ce failure on 8x NVIDIA RTX 6000D
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughCUDA IPC initialization now coordinates failures across tensor-parallel ranks, cleans up partial resources, and reports unavailable IPC. Allreduce initialization detects unusable IPC workspaces and falls back to NCCL. Unit tests cover success and failure paths. ChangesCUDA IPC fallback
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant AllReduce
participant IpcMemory
participant TPCollective
participant NCCL
AllReduce->>IpcMemory: allocate IPC workspace
IpcMemory->>TPCollective: exchange IPC availability
TPCollective-->>IpcMemory: return collective status
IpcMemory-->>AllReduce: return workspace or unavailable
AllReduce->>NCCL: select NCCL when IPC is unavailable
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/unittest/_torch/distributed/test_ipc_memory_fallback.py (1)
74-157: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExpand coverage for the remaining fallback contracts.
Coverage summary — insufficient. The three added tests cover successful IPC setup, local import failure, and peer-reported failure. CI/QA test-list membership cannot be verified because no
tests/integration/test_lists/test-db/ortests/integration/test_lists/qa/file was provided.
- Make
FakeCudart.cudaIpcGetMemHandle()configurable and test its new dummy-handle/cleanup path.- In
test_local_ipc_open_failure_falls_back_instead_of_raising, useFakeDist(TP_SIZE)so a local failure alone drives agreement;peer_ipc_ok=Falsecurrently masks failures to propagate the local error.- Add mocked coverage that unavailable IPC downgrades
AllReduceto NCCL withworkspace is None, and thatlamport_initialize()is skipped.As per path instructions, test changes require coverage and test-list status review.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unittest/_torch/distributed/test_ipc_memory_fallback.py` around lines 74 - 157, Expand the IPC fallback tests around FakeCudart.cudaIpcGetMemHandle, making its result configurable and covering the dummy-handle and cleanup behavior. Update test_local_ipc_open_failure_falls_back_instead_of_raising to use FakeDist(TP_SIZE), then add mocked coverage verifying unavailable IPC downgrades AllReduce to NCCL with workspace=None and skips lamport_initialize(); also review coverage and test-list status for these test changes.Source: Path instructions
tensorrt_llm/_ipc_utils.py (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse built-in generics in
open_ipc_memoryReplace
Optional[Tuple[List[int], int]]withtuple[list[int], int] | None.Listis still needed by the other annotations in this file, so thetypingimport can’t be dropped yet.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tensorrt_llm/_ipc_utils.py` at line 18, Update the return annotation of open_ipc_memory to use the built-in generic form tuple[list[int], int] | None instead of Optional[Tuple[List[int], int]]. Retain the existing typing imports because List remains required by other annotations in the file.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tensorrt_llm/_ipc_utils.py`:
- Around line 161-163: Update the CUDA IPC warning in the logger.warning call to
use the f-string conversion flag for ipc_error instead of explicitly calling
repr(), resolving Ruff RUF010 while preserving the existing message.
---
Nitpick comments:
In `@tensorrt_llm/_ipc_utils.py`:
- Line 18: Update the return annotation of open_ipc_memory to use the built-in
generic form tuple[list[int], int] | None instead of Optional[Tuple[List[int],
int]]. Retain the existing typing imports because List remains required by other
annotations in the file.
In `@tests/unittest/_torch/distributed/test_ipc_memory_fallback.py`:
- Around line 74-157: Expand the IPC fallback tests around
FakeCudart.cudaIpcGetMemHandle, making its result configurable and covering the
dummy-handle and cleanup behavior. Update
test_local_ipc_open_failure_falls_back_instead_of_raising to use
FakeDist(TP_SIZE), then add mocked coverage verifying unavailable IPC downgrades
AllReduce to NCCL with workspace=None and skips lamport_initialize(); also
review coverage and test-list status for these test changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ab9b4213-0365-4477-a4b7-557a34646fe2
📒 Files selected for processing (4)
tensorrt_llm/_ipc_utils.pytensorrt_llm/_torch/distributed/allreduce_helper.pytensorrt_llm/_torch/distributed/ops.pytests/unittest/_torch/distributed/test_ipc_memory_fallback.py
There was a problem hiding this comment.
Pull request overview
This PR addresses a Tensor Parallel initialization failure where CUDA IPC handle import/export can fail (e.g., cudaIpcOpenMemHandle returning cudaErrorInvalidDevice) even when cudaDeviceCanAccessPeer reports P2P capability, by degrading to the already-supported NCCL path instead of aborting engine initialization.
Changes:
- Make
IpcMemory.open_ipc_memory()returnNoneon CUDA IPC handle exchange failures (with TP-wide agreement + cleanup), and haveIpcMemory.__init__disable IPC (null pointers) instead of raising. - Gate Lamport buffer initialization on
lamport_buffers.open_ipcrather than the optimistic P2P check. - In
AllReduce.__init__, detect workspaces that have no usable IPC buffers and downgrade the strategy to NCCL; add a focused unit test module with stubbed CUDA runtime / distributed collectives.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tensorrt_llm/_ipc_utils.py |
Make CUDA IPC failures non-fatal by returning None and cleaning up; disables IPC state in IpcMemory on failure. |
tensorrt_llm/_torch/distributed/allreduce_helper.py |
Prevent lamport_initialize() from running when IPC buffers are not actually available. |
tensorrt_llm/_torch/distributed/ops.py |
Add IPC-workspace detection and force NCCL fallback when IPC buffers are missing; reorder LOWPRECISION allocation to avoid null-workspace usage. |
tests/unittest/_torch/distributed/test_ipc_memory_fallback.py |
Add stubbed unit tests covering IPC-success, local IPC-open failure, and peer IPC failure agreement behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.workspace = get_allreduce_workspace(self.mapping) | ||
| # Every custom all-reduce kernel reads the peers' IPC buffers. When | ||
| # CUDA IPC is unavailable those pointers are null, so NCCL is the | ||
| # only strategy that can run. See allreduce_workspace_has_ipc. | ||
| if not allreduce_workspace_has_ipc(self.mapping): |
| if not all(dist.tp_allgather(ipc_error is None)): | ||
| if ipc_error is not None: | ||
| logger.warning( | ||
| f"CUDA IPC is not usable on this system: {repr(ipc_error)}. " | ||
| "Custom all-reduce kernels are disabled, falling back to NCCL." |
Ruff RUF010. The warning was reported on the IPC fallback log line added by this PR. The repo's ruff hook runs --fix over the whole file rather than changed lines only, so the two pre-existing occurrences in _raise_if_error are converted in the same pass to keep ruff --fix --exit-non-zero-on-fix clean. Signed-off-by: pjdurden <prajjwalchittori1@gmail.com>
|
@BowenFu @schetlur-nv @JunyiXu-nv @allisonlim-nv could one of you take a look when you get a chance? The reporter of #16899 applied the patch on 8x RTX 5090 (PCIe, no NVLink), a different platform from the RTX 6000D in the original report: Before: crash at engine init, cudaErrorInvalidDevice: 101 from cudaIpcOpenMemHandle, all 8 ranks. So the failure is not specific to the RTX 6000D, it affects PCIe-only parts generally. Also pushed one more commit for Ruff nit. |
Issue #16899 — Llama-3-70B BF16 TP>1 fails on 8x RTX 6000D
Upstream: #16899
1. Root cause
The reporter's follow-up (v1.3.0rc22) pins the failure precisely:
and notes that
can_access_peer()returnedTrueon the same machine.can_access_peer()(tensorrt_llm/_ipc_utils.py:39) only callscudaDeviceCanAccessPeer. That answers "can GPU A address GPU B's memory",which is necessary but not sufficient for CUDA IPC: exporting/importing an
IPC memory handle is a separate capability that also fails on GPUs without CUDA
IPC support and when the ranks do not share an IPC namespace. On this system
cudaDeviceCanAccessPeersucceeds whilecudaIpcOpenMemHandledoes not.Because
can_access_peer()saidTrue,IpcMemorywas constructed withopen_ipc=True, andopen_ipc_memory()passed the raw CUDA error straight to_raise_if_error(). That unhandledRuntimeErroraborts engine initialization.This is a hard crash for a situation the runtime already fully supports. The
non-IPC path exists and works:
IpcMemory(open_ipc=False)leaves all pointersnull, and on the C++ side
AllReduceOp::ifFallbackToNCCL(
cpp/tensorrt_llm/thop/allreduceOp.cpp:1436) already routes every all-reduce toNCCL when P2P/NVLink is absent — which is exactly this hardware (RTX 6000D is a
PCIe part with no NVLink, so
mIsNVLINKSupportedis false and the custom kernelswould never have been selected anyway). Only the eager Python-side workspace
allocation stood in the way.
Two secondary observations that match the report:
reporter's FP8 runs used a configuration that did not hit this init path.
TRTLLM_DISABLE_CUSTOM_ALLREDUCE=1had no effect because no such env varexists anywhere in the tree (
grepreturns nothing). The supported knob isallreduce_strategy: NCCLin the LLM args, which skips the workspaceallocation entirely — a valid workaround, but users should not need it.
2. The fix
Degrade gracefully to the already-supported NCCL path instead of crashing.
tensorrt_llm/_ipc_utils.py—IpcMemory.open_ipc_memoryno longer raiseswhen IPC handles cannot be exchanged. It now returns
None, andIpcMemory.__init__reacts by settingopen_ipc = False, leaving the pointersnull. Two details matter:
either every rank gets IPC buffers or none does. Without this, a rank that
succeeded would keep using buffers its peers never mapped, and the subsequent
tp_allgathercalls could desync. A dummy handle is exchanged whencudaIpcGetMemHandleitself fails, to keep the collectives symmetric.is freed, so the fallback does not leak device memory.
cudaMalloc/cudaMemsetfailures still raise — those are genuine errors, not acapability gap.
tensorrt_llm/_torch/distributed/allreduce_helper.py— gatelamport_initialize()onlamport_buffers.open_ipcrather than onis_p2p_supported. Those two can now disagree, and callinglamport_initializewith a null
local_ptrwould be an illegal memory access.tensorrt_llm/_torch/distributed/ops.py— defense in depth. Addedallreduce_workspace_has_ipc()and, inAllReduce.__init__, downgrade thestrategy to
NCCLwhen the workspace has no IPC buffers. Without this, a machinethat does have NVLink but cannot do IPC would pass
ifFallbackToNCCLon the C++ side and dereference null workspace pointers insidea custom kernel — trading a clear init-time error for an obscure crash. The
LOWPRECISIONworkspace allocation moved below this check for the same reason(
initialize_static_lowprecision_bufferson a null workspace).SYMM_MEMis unaffected:AllReduce.forwardtriesself.symm_mem_allreducebefore consulting
self.strategy, so the downgrade only changes the fallbackpath.
3. Files changed
tensorrt_llm/_ipc_utils.pyopen_ipc_memoryreturnsOptional[...]; collective IPC-failure agreement + cleanup;IpcMemory.__init__clearsopen_ipc; copyright yeartensorrt_llm/_torch/distributed/allreduce_helper.pylamport_initializeon the actual IPC statetensorrt_llm/_torch/distributed/ops.pyallreduce_workspace_has_ipc(); strategy downgrade to NCCL; reorder LOWPRECISION allocationtests/unittest/_torch/distributed/test_ipc_memory_fallback.py4. Risk and uncertainty
sandbox has neither GPUs nor
torch. The root cause is derived from thereporter's traceback plus reading the code; the specific reason
cudaIpcOpenMemHandlereturns 101 on that GPU is not established. The fix isdeliberately agnostic to that reason — it handles "IPC handles cannot be
exchanged" however it arises.
slower-but-correct NCCL path. That is the right trade-off here (the runtime
already treats non-P2P topologies this way), but it does mean a genuinely
broken IPC setup now degrades silently-ish instead of failing loudly. The
logger.warningis there to make it visible.tp_allgatherof a bool perIpcMemoryconstruction (3 per TP group at init). Negligible, but it is a new
synchronization point — all ranks must construct
IpcMemorycollectively,which they already did for the handle exchange.
can_access_peer()still reportsTrueonsuch systems, so
self.is_p2p_supportedinmodeling_deepseekv3.py,modeling_glm.py,modeling_qwen3_moe.pyetc. remains optimistic. Those areMoE-specific fused paths that this Llama issue does not exercise. A complete
fix would also plumb the IPC verdict into
AllReduceOp::setGroupTopology(
cpp/tensorrt_llm/thop/allreduceOp.cpp) so C++ and Python agree from thestart; the strategy downgrade above covers the same ground from the Python
side without touching C++.
concurrency 1) is a different failure than the rc22 init crash addressed
here. Their rc22 report supersedes it, and that is what this fix targets.
5. How I verified it
Constraints: no GPU and no
torchin this environment, sopytestcannot importtensorrt_llm. The pytest file was not executed. Instead:(
/tmp/verify_ipc_fallback.py, not committed) loads the actualtensorrt_llm/_ipc_utils.pyviaimportlibwith the CUDA runtime,Distributed,logger, andMappingstubbed, and runs the same threescenarios as the pytest file. All 14 assertions pass:
open_ipc=True, non-null pointers, one handle opened.cudaIpcOpenMemHandle→cudaErrorInvalidDevice(101)(the reportedfailure) → no exception,
open_ipc=False, all pointers null, localbuffer freed, warning logged.
handles closed, buffer freed.
ruff check+ruff format --checkclean on_ipc_utils.py,allreduce_helper.py, and the new test (non-legacy files).yapf --diffandisort --diffclean onops.py(listed inlegacy-files.txt).ruff check --config ruff-legacy.toml ops.pyreportsthe same 2 pre-existing findings before and after my change (verified via
git stash).python -m py_compileon all four files.Not verified: end-to-end
trtllm-bench --tp 8on affected hardware, and thatNCCL throughput is acceptable for this workload. Both need the reporter's
machine.
Dev Engineer Review
None.QA Engineer Review
Added:
test_ipc_memory_is_opened_when_ipc_workstest_local_ipc_open_failure_falls_back_instead_of_raisingtest_peer_ipc_failure_disables_ipc_on_this_rankThese tests are not referenced in
test-db/orqa/integration test lists. Pytest and end-to-end validation were not performed because GPU and Torch support were unavailable. Static checks, compilation, and a standalone harness passed.Verdict: needs follow-up for GPU-backed end-to-end validation on RTX 6000D hardware.