Repository navigation
Conversation
…alMemoryFdBufferDescriptor The class carries POSIX file descriptors for VirtualMemoryResource buffers. A fabric-handle descriptor will sit beside it, so its name now says which transport it is. No alias: the class first appears in the unreleased 1.3.0. No behavior change. Written by Codex (gpt-6.1-sol); reviewed by Claude. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Add VirtualMemoryFabricBufferDescriptor, a plain-data descriptor that carries one opaque 64-byte CUmemFabricHandle per physical allocation, the allocation sizes, the buffer size, and the exporting device's UUID. VirtualMemoryResource exports it when config.handle_type is "fabric", and Buffer.from_ipc_descriptor imports it through the shared _VirtualMemoryDescriptor interface that the POSIX fd descriptor also implements, so both transports run one import loop. Fabric descriptors travel through plain pickle and multiprocessing without the file-descriptor machinery and pin nothing: the exporter keeps the buffer alive until every importer has imported it. A stale or forged handle fails at import with a driver error and a note. Resource construction checks Device.properties.handle_type_fabric_supported and raises RuntimeError when the device lacks an NVLink fabric or an accessible IMEX channel. from_handles() rebuilds a descriptor received through a transport of your own. A location mismatch at import names the owning device by UUID. Sharing is supported on one node; multi-node sharing is untested. The project's CI has no fabric-capable runner; the release note says so. Part of NVIDIA#2980. Written by Codex (gpt-6.1-sol); reviewed by Claude. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Andy-Jost
added
P0
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Shared and fabric descriptor model cuda_core/cuda/core/_memory/_ipc.pxd, cuda_core/cuda/core/_memory/_ipc.pyi, cuda_core/cuda/core/_memory/_ipc.pyx |
A shared descriptor base provides handle type, allocation sizes, and indexed handle access. VirtualMemoryFabricBufferDescriptor validates and serializes fabric handles, sizes, and an optional owner UUID. |
Resource export and import cuda_core/cuda/core/_memory/_virtual_memory_resource.pyx, cuda_core/cuda/core/_memory/_virtual_memory_resource.pyi, cuda_core/cuda/core/_cpp/rt/VMM_DESIGN.md, cuda_core/docs/source/api_private.rst, cuda_core/docs/source/release/1.3.0-notes.rst |
Linux resource handling recognizes both handle types. Fabric export returns a fabric descriptor, and import uses the shared descriptor interface. Fabric descriptors do not pin allocations; the exporter must keep the buffer alive until import. Resource construction raises RuntimeError when the device reports no fabric-handle support. |
Fabric sharing validation cuda_core/tests/conftest.py, cuda_core/tests/memory_ipc/test_vmm_fabric.py, cuda_core/tests/memory_ipc/test_vmm_ipc.py |
Tests cover descriptor construction, serialization, cross-process sharing, ownership, lifetime, import errors, and unsupported devices. |
Possibly related issues
- EPIC: VMM feature completeness #2980: The change adds fabric descriptors and export and import paths, matching the issue’s stated missing capability.
- cuda.core: VirtualMemoryResourceOptions accepts values the driver rejects, and modify_allocation mishandles edge inputs #2910: The change adds a device support check and fabric-specific allocation error details; it addresses those fabric configuration cases but does not establish broader validation behavior.
Suggested reviewers: rwgk, mdboom
Priority:
Change: Feature
Merge Risk: 
26698
-
Clarify the fabric test probe and export assertion, and correct the transport description before merging unless their current behavior is confirmed to be intentional.
Comment @coderabbitai help to get the list of available commands.
![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
-
-
- Source mode: Auto
- Repository: NVIDIA/cuda-python
- Branch: main
- File: .coderabbit.yaml
- Commit: 2e6aa4e
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
2567ea85-9d0d-47b7-8ab6-6d6eac163733
-
cuda_core/cuda/core/_cpp/rt/VMM_DESIGN.mdcuda_core/cuda/core/_memory/_ipc.pxdcuda_core/cuda/core/_memory/_ipc.pyicuda_core/cuda/core/_memory/_ipc.pyxcuda_core/cuda/core/_memory/_virtual_memory_resource.pyicuda_core/cuda/core/_memory/_virtual_memory_resource.pyxcuda_core/docs/source/api_private.rstcuda_core/docs/source/release/1.3.0-notes.rstcuda_core/tests/conftest.pycuda_core/tests/memory_ipc/test_vmm_fabric.pycuda_core/tests/memory_ipc/test_vmm_ipc.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
-
Contributor
|
Zero the 64-byte handle struct before a fabric export. The driver fills its fields and leaves the padding, so two exports of one buffer carried different stack bytes (seen on B300); the export now returns the same bytes for the same allocation, and no stack bytes travel in a descriptor. The fabric test fixtures allocate once per device after the attribute check. Inside a container that has the IMEX channel device node the attribute is true while the GPU has no fabric, and the allocation is refused; that is what the CI runners do. A refused physical allocation on a fabric resource now carries a note about the fabric and IMEX requirements, and the docs say that the attribute can be true where the allocation still fails. The import-error tests check cuda.core's note, not the driver's status, which varies with the IMEX setup. Garbage handles use fixed patterns instead of os.urandom. The release note names the POSIX fd descriptor as the one that pins. One note per import failure. _handles is readonly on both descriptors. Written by Claude after the de novo review, CodeRabbit, CI, and the Hopper and B300 runs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
-
-
- Source mode: Auto
- Repository: NVIDIA/cuda-python
- Branch: main
- File: .coderabbit.yaml
- Commit: 2e6aa4e
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
d22607f1-f983-48de-9326-f14eb1a4e683
-
cuda_core/cuda/core/_cpp/rt/VMM_DESIGN.mdcuda_core/cuda/core/_memory/_ipc.pxdcuda_core/cuda/core/_memory/_ipc.pyicuda_core/cuda/core/_memory/_virtual_memory_resource.pyicuda_core/cuda/core/_memory/_virtual_memory_resource.pyxcuda_core/docs/source/release/1.3.0-notes.rstcuda_core/tests/conftest.pycuda_core/tests/memory_ipc/test_vmm_fabric.py
-
- cuda_core/docs/source/release/1.3.0-notes.rst
- cuda_core/cuda/core/_cpp/rt/VMM_DESIGN.md
- cuda_core/cuda/core/_memory/_virtual_memory_resource.pyi
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
-
…r helper Cython reported the stray line as unreachable code, and the wheel builds treat Cython warnings as errors. The return goes back to the note helper it belonged to. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The fixture probe skipped every fabric test on any CUDAError from the probe allocation, which would hide a real failure (an out-of-memory condition, for one) as 'no fabric here'. It now skips on the statuses that mean the device has no usable fabric (NOT_PERMITTED on the CI runners, INVALID_VALUE on an H200 without an IMEX channel, NOT_SUPPORTED) and lets anything else propagate, per the test guide's rule on capability probes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
-
-
- Source mode: Auto
- Repository: NVIDIA/cuda-python
- Branch: main
- File: .coderabbit.yaml
- Commit: 590a8bf
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
2629eefa-b0a5-4280-a743-a6c75ccd5260
-
cuda_core/tests/conftest.pycuda_core/tests/memory_ipc/test_vmm_ipc.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
-
…bric Every conflict was the rename that this branch already carried as f128963; main's version of each conflicted file equals that commit's, so the branch side was kept.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
1.3.0-notes.rst:88-92
cuda_core/docs/source/release/1.3.0-notes.rst:88-92
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winsuggestion: Limit the file-descriptor description to
"posix_fd". The introduction now includes"fabric"resources, but these sentences still say that every export produces POSIX file descriptors and cannot use plainpickle. A fabric export produces a fabric handle that can travel as data. Separate the two transport descriptions so readers do not apply FD-handling instructions to fabric descriptors. (docs.nvidia.com) As per coding guidelines, “Keep user-facing behaviors coherent with docs and samples, especially around stream semantics, memory ownership, and compile/link flows.”Source: Coding guidelines
-
-
- Source mode: Auto
- Repository: NVIDIA/cuda-python
- Branch: main
- File: .coderabbit.yaml
- Commit: f9212dd
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
2e1bc7ca-b386-43d3-835b-9c6f4623ec84
-
cuda_core/cuda/core/_cpp/rt/VMM_DESIGN.mdcuda_core/cuda/core/_memory/_ipc.pxdcuda_core/cuda/core/_memory/_ipc.pyicuda_core/cuda/core/_memory/_ipc.pyxcuda_core/cuda/core/_memory/_virtual_memory_resource.pyicuda_core/cuda/core/_memory/_virtual_memory_resource.pyxcuda_core/docs/source/api_private.rstcuda_core/docs/source/release/1.3.0-notes.rstcuda_core/tests/memory_ipc/test_vmm_ipc.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
-
The fixture probe skipped the fabric tests on three driver statuses from the whole probe allocation. A status from the address reservation, the mapping, or the stream setup could therefore pass as 'no fabric here' and hide a real failure on a fabric-capable device. The probe now skips only when the exception carries the note that VirtualMemoryResource attaches to a refused physical allocation with a fabric handle (appended to the message on Python 3.10), and lets every other CUDAError propagate. The 1.3.0 release note described every export as a POSIX file descriptor that plain pickle cannot carry. It now describes the two transports separately: file descriptors for "posix_fd", plain-data fabric handles for "fabric". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ation fails The docstrings, the allocation note, the release note, and the fixture comment explained the over-reporting attribute with a container that has the IMEX channel device node. The CI evidence says otherwise: the runner VMs disable NVLink and have no channel node, and the attribute is still true on the r595 and 610 drivers there. The 13.4 driver source adds the channel subscription to the attribute; no driver checks for an NVLink fabric. The text now states that, without the container example. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 task done
This branch has not been deployed
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
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.
Summary
Buffers of a
VirtualMemoryResourceconfigured withhandle_type="fabric"can now be shared between processes on one node. This adds the fabric transport that #2980 tracks, beside the POSIX fd transport from #3028.Changes
VirtualMemoryFabricBufferDescriptor(listed inapi_private.rst): one opaque 64-byteCUmemFabricHandleper physical allocation, the allocation sizes, the buffer size, and the exporting device's UUID. It is plain data:pickleandmultiprocessingcarry it without the file-descriptor machinery, it compares and hashes by value, andfrom_handles()rebuilds one received through a transport of your own, with validation. The export zeroes the handle struct first, so two exports of one allocation carry the same bytes and no stack bytes travel in a descriptor._VirtualMemoryDescriptor, soBuffer.from_ipc_descriptorand the resource's import run one loop for both transports. The fd descriptor keeps its ownership and identity semantics and itsfrom_fds().VirtualMemoryResourceconstruction checksDevice.properties.handle_type_fabric_supportedforhandle_type="fabric"and raisesRuntimeErrorthat names the NVLink fabric and IMEX channel requirements, before an allocation can fail opaquely. A device can report support while the allocation is refused (the attribute does not check for an NVLink fabric, and older drivers do not check for an IMEX channel); a refused allocation then carries a note that says so.is_ipc_enabledisTruefor"fabric"on Linux with device-located memory.VMM_DESIGN.mdsay so wherever descriptor lifetime is described._ipc.pyiand_virtual_memory_resource.pyiare regenerated by the stub hook.tests/memory_ipc/test_vmm_fabric.pycovers export shape, descriptor validation and pickling, Queue, Pipe, Process-argument, and Pool round trips for descriptors and buffers, a second-GPU import with peer access, handle-type mismatch, garbage and stale handles, re-export of imports, a queued buffer that does not pin, and the unsupported-device error. The fabric fixtures skip on devices without fabric handle support, and on devices that report it but refuse the allocation (one probe allocation per device).Validation
Fabric handles need an NVLink fabric and an accessible IMEX channel. The project's CI has no such runner, so the fabric cases skip there; the release note says that this release's fabric support is not covered by CI. On the CI runners the attribute is true while every fabric allocation is refused with a driver error: the runner VMs disable NVLink and have no IMEX channel, and the attribute checks neither on those drivers. That is why the fixtures probe instead of trusting the attribute.
Validation on NVIDIA internal hardware: a two-GPU B300 system ran the
memory_ipcsuite andtest_memory.pytwice: on the first commit (478 passed, 21 skipped; the one failure found the uninitialized padding in the exported handle struct) and on the fix and on the PR head withmainmerged (479 passed, 21 skipped each), and a Hopper system without fabric support ran the fabric and fd suites (30 passed, 17 skipped, with the unsupported-device error test among the passes). The fabric suite also passed inside a container on the B300 system (a stock CUDA base image with the GPUs injected by the NVIDIA container CLI and the IMEX channel device node mounted; 18 passed, 1 skipped, three runs). Multi-node sharing is out of scope and untested.Related Work
Part of #2980. Follows #3028 and #3229; this PR includes the #3229 commit until that PR merges.
🤖 Generated with Claude Code