Skip to content

[fix] VM resource options #3343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
martindurant wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[fix] VM resource options #3343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 7 additions & 6 deletions cuda_core/cuda/core/_memory/_virtual_memory_resource.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,19 @@ class VirtualMemoryResourceOptions:
Attributes
----------
allocation_type: :obj:`~_memory.VirtualMemoryAllocationType` | str
Controls the type of allocation.
Controls the type of allocation. Only ``"pinned"`` is supported;
use :class:`ManagedMemoryResource` for managed memory.
location_type: :obj:`~_memory.VirtualMemoryLocationType` | str
Controls the location of the allocation. ``"host_numa"`` requires
``numa_id``. ``"host_numa_current"`` is rejected because the driver
does not accept it for physical allocations.
handle_type: :obj:`~_memory.VirtualMemoryHandleType` | str
handle_type: :obj:`~_memory.VirtualMemoryHandleType` | str | None
Export handle type for the physical allocation, and the switch for
sharing buffers with other processes (see
:attr:`VirtualMemoryResource.is_ipc_enabled`). Use ``"posix_fd"`` on
Linux if you plan to import/export the allocation. Use `None` if you
don't need an exportable handle. ``location_type="host"`` requires
`None`.
if you plan to import/export the allocation. Use `None` if you
don't need an exportable handle. The default on Windows is `None`,
"posix_fd" otherwise. ``location_type="host"`` requires `None`.
gpu_direct_rdma: bool
Hint that the allocation should be GDR-capable (if supported).
granularity: :obj:`~_memory.VirtualMemoryGranularityType` | str
Expand All @@ -64,7 +65,7 @@ class VirtualMemoryResourceOptions:
"""
allocation_type: VirtualMemoryAllocationType = VirtualMemoryAllocationType.PINNED
location_type: VirtualMemoryLocationType = VirtualMemoryLocationType.DEVICE
handle_type: VirtualMemoryHandleType = VirtualMemoryHandleType.POSIX_FD
handle_type: VirtualMemoryHandleType | None = field(default_factory=lambda: None if platform.system() == 'Windows' else VirtualMemoryHandleType.POSIX_FD)
granularity: VirtualMemoryGranularityType = VirtualMemoryGranularityType.RECOMMENDED
gpu_direct_rdma: bool = False
addr_hint: int | None = 0
Expand Down
22 changes: 12 additions & 10 deletions cuda_core/cuda/core/_memory/_virtual_memory_resource.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ from typing import TYPE_CHECKING, Iterable

from cuda.core._device import Device, _device_from_uuid
from cuda.core._utils.cuda_utils import driver
from cuda.core._utils.version import BUILD_CUDA_MAJOR
from cuda.core.typing import (
DevicePointerType,
VirtualMemoryAccessType,
Expand Down Expand Up @@ -97,18 +96,19 @@ class VirtualMemoryResourceOptions:
Attributes
----------
allocation_type: :obj:`~_memory.VirtualMemoryAllocationType` | str
Controls the type of allocation.
Controls the type of allocation. Only ``"pinned"`` is supported;
use :class:`ManagedMemoryResource` for managed memory.
location_type: :obj:`~_memory.VirtualMemoryLocationType` | str
Controls the location of the allocation. ``"host_numa"`` requires
``numa_id``. ``"host_numa_current"`` is rejected because the driver
does not accept it for physical allocations.
handle_type: :obj:`~_memory.VirtualMemoryHandleType` | str
handle_type: :obj:`~_memory.VirtualMemoryHandleType` | str | None
Export handle type for the physical allocation, and the switch for
sharing buffers with other processes (see
:attr:`VirtualMemoryResource.is_ipc_enabled`). Use ``"posix_fd"`` on
Linux if you plan to import/export the allocation. Use `None` if you
don't need an exportable handle. ``location_type="host"`` requires
`None`.
if you plan to import/export the allocation. Use `None` if you
don't need an exportable handle. The default on Windows is `None`,
"posix_fd" otherwise. ``location_type="host"`` requires `None`.
gpu_direct_rdma: bool
Hint that the allocation should be GDR-capable (if supported).
granularity: :obj:`~_memory.VirtualMemoryGranularityType` | str
Expand All @@ -134,7 +134,9 @@ class VirtualMemoryResourceOptions:

allocation_type: VirtualMemoryAllocationType = VirtualMemoryAllocationType.PINNED
location_type: VirtualMemoryLocationType = VirtualMemoryLocationType.DEVICE
handle_type: VirtualMemoryHandleType = VirtualMemoryHandleType.POSIX_FD
handle_type: VirtualMemoryHandleType | None = field(

@coderabbitai coderabbitai Bot Nov 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor · Data Integrity & Integration · Quick win

important: Include strings in the handle type annotation. The documented and tested value "win32_kmt" is accepted at runtime, but the annotation and generated stub reject it in type-checked code. Add str to the union and regenerate the stub.

As per coding guidelines, “The argument and return types as defined in the docstrings should match the type annotations.”

Source: Coding guidelines

@mdboom mdboom Nov 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to add win32_kmt to the VirtualMemoryHandleType enum?

@coderabbitai coderabbitai Bot Nov 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Analysis chain

Script executed:

#!/bin/bash
ast-grep outline cuda_core/cuda/core/typing.py --match VirtualMemoryHandleType --view expanded
rg -n -A35 -B3 '^class VirtualMemoryHandleType\b' cuda_core/cuda/core/typing.py

Length of output: 1840

@mdboom suggestion: VirtualMemoryHandleType already defines WIN32_KMT = "win32_kmt" in cuda_core/cuda/core/typing.py, Line 156. Passing VirtualMemoryHandleType.WIN32_KMT satisfies the current annotation.

My concern applies only to passing the raw string "win32_kmt", which the API documents and accepts at runtime. Adding an enum member does not make that string assignable to the enum type. If raw strings remain supported, VirtualMemoryHandleType | str | None would make the annotation match that contract.

Learnings used
Learnt from: CR
Repo: NVIDIA/cuda-python PR: 0
File: cuda_core/CLAUDE.md:0-0
Timestamp: 2026-10-31T05:09:21.347Z
Learning: Applies to cuda_core/** : Source excerpt:
## API design guidelines

### Type annotations

Python or Cython type annotations should be included for all public APIs.  Avoid
the use of `Any` unless absolutely necessary.  The argument and return types as
defined in the docstrings should match the type annotations.

Learnt from: CR
Repo: NVIDIA/cuda-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-10-31T04:59:43.467Z
Learning: Source excerpt:
# Code Implementation

- Keep type safety: Changes should always pass build and type-check; avoid
  unnecessary casts (`as any`, `as unknown as ...`); prefer proper types and
  guards, and reuse existing helpers (e.g., normalizing identifiers) instead of
  type-asserting.

Learnt from: CR
Repo: NVIDIA/cuda-python PR: 0
File: cuda_core/CLAUDE.md:0-0
Timestamp: 2026-10-31T05:09:21.347Z
Learning: Applies to cuda_core/** : Source excerpt:
## API design guidelines

### Enumerations

For `__post_init__` validation in frozen dataclasses, use the
`not isinstance(value, EnumType) → try EnumType(value) except (ValueError,
TypeError)` pattern (modelled on `_normalize_enum` in
`cuda/core/texture/_texture.pyx`). This accepts the enum itself or a valid
string, and raises `ValueError` eagerly for any other type rather than
silently storing it.

You are interacting with an AI system.

default_factory=lambda: None if platform.system() == "Windows" else VirtualMemoryHandleType.POSIX_FD

@coderabbitai coderabbitai Bot Nov 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major · Functional Correctness · Quick win

important: Update tests that rely on the old Windows default. On Windows, test_vmm_host_location_types_report_host_accessible expects the default host configuration to raise, but it now succeeds. test_vmm_host_modify_allocation_rejects_exportable_handle_type has the same assumption when host VMM is supported. Pass an explicit exportable handle in those rejection tests and test the new default separately. (raw.githubusercontent.com)

)
granularity: VirtualMemoryGranularityType = VirtualMemoryGranularityType.RECOMMENDED
gpu_direct_rdma: bool = False
addr_hint: int | None = 0
Expand Down Expand Up @@ -170,12 +172,11 @@ class VirtualMemoryResourceOptions:
VirtualMemoryLocationType.HOST_NUMA_CURRENT: _l.CU_MEM_LOCATION_TYPE_HOST_NUMA_CURRENT,
}
_t = driver.CUmemAllocationType
# CUDA 13 added MANAGED to CUmemAllocationType. The CUDA 12 build has no such member.
# cuMemCreate only supports PINNED, even on CUDA 13 builds with a MANAGED enum member.
_allocation_type = {VirtualMemoryAllocationType.PINNED: _t.CU_MEM_ALLOCATION_TYPE_PINNED} # noqa: RUF012
if BUILD_CUDA_MAJOR >= 13:
_allocation_type[VirtualMemoryAllocationType.MANAGED] = _t.CU_MEM_ALLOCATION_TYPE_MANAGED

def __post_init__(self) -> None:
self._allocation_type_to_driver(self.allocation_type)
# Allocation and pickling each read the peers, so a one-shot iterable
# such as a generator must not reach them.
self.peers = tuple(int(peer) for peer in self.peers)
Expand Down Expand Up @@ -490,6 +491,7 @@ cdef class VirtualMemoryResource(MemoryResource):
Shared by ``__init__`` and ``modify_allocation``, so a per-call
configuration is held to the same rules as the resource's own.
"""
VirtualMemoryResourceOptions._allocation_type_to_driver(cfg.allocation_type)
if cfg.location_type != self.config.location_type:
raise ValueError(
f"config.location_type {str(cfg.location_type)!r} does not match the resource's "
Expand Down
2 changes: 1 addition & 1 deletion cuda_core/cuda/core/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ class VirtualMemoryAllocationType(StrEnum):
"""Physical memory type for a virtual memory backing allocation.

* ``PINNED`` — pinned/non-migratable physical allocation (placement via :class:`VirtualMemoryLocationType`).
* ``MANAGED`` — CUDA managed (unified) memory (CUDA 13+ only).
* ``MANAGED`` — reserved; not supported by :class:`~cuda.core.VirtualMemoryResource`.
"""

PINNED = "pinned"
Expand Down
9 changes: 9 additions & 0 deletions cuda_core/docs/source/release/1.3.0-notes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,15 @@ Fixes and enhancements
``repr`` now shows ``U`` for unified operands.
(`#2649 <https://github.com/NVIDIA/cuda-python/issues/2649>`__)

- :class:`VirtualMemoryResourceOptions` rejects ``allocation_type="managed"``
with ``ValueError`` as ``cuMemCreate`` only supports
pinned allocations. Use :class:`ManagedMemoryResource` for managed memory.
(`#2910 <https://github.com/NVIDIA/cuda-python/issues/2910>`__)

- The default :class:`VirtualMemoryResourceOptions` ``handle_type`` on Windows
is now ``None``, avoiding the unsupported POSIX descriptor; Linux keeps its ``"posix_fd"`` default.
(`#2910 <https://github.com/NVIDIA/cuda-python/issues/2910>`__)

- :class:`VirtualMemoryResource` can now allocate with
``location_type="host_numa"``. Every such allocation failed with
``CUDA_ERROR_INVALID_VALUE``, because the driver received ``-1`` as the NUMA
Expand Down
13 changes: 13 additions & 0 deletions cuda_core/tests/test_memory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3348,6 +3348,19 @@ def test_vmm_options_spec_validators_raise(method, spec, match):
fn(spec)


@pytest.mark.parametrize("system", ["Linux", "Windows"])
@pytest.mark.thread_unsafe(reason="patches platform.system")
def test_vmm_options_default_handle_type(monkeypatch, system):
monkeypatch.setattr(platform, "system", lambda: system)
options = VirtualMemoryResourceOptions()
expected = None if system == "Windows" else VirtualMemoryHandleType.POSIX_FD
assert options.handle_type == expected
assert VirtualMemoryResourceOptions(handle_type="win32_kmt").handle_type == "win32_kmt"
assert VirtualMemoryResourceOptions(handle_type=None).handle_type is None
with pytest.raises(ValueError):
VirtualMemoryResourceOptions(allocation_type=VirtualMemoryAllocationType.MANAGED)


def test_vmm_options_handle_type_win32_raises():
"""_handle_type_to_driver raises NotImplementedError for 'win32'."""
with pytest.raises(NotImplementedError, match="win32 is currently not supported"):
Expand Down
Loading