Skip to content

Commit 35ae30c

Browse files
Andy-Jostclaude
andcommitted
cuda.core: close the old buffer when a VMM grow moves the mapping
The slow path of VirtualMemoryResource.modify_allocation unmapped the old VA range, remapped the physical memory into the new range, freed the old reservation by hand and then reset the old buffer's handle with Buffer._clear(). That reset runs the handle's deleter, which calls mr.deallocate() on the range that was just freed. The failing cuMemRetainAllocationHandle is reported as a CUDAWarning since #2759 and was silently swallowed before. Map the old physical memory into the new range as a second mapping instead, which the VMM APIs allow (virtual aliasing), and then close the old buffer. Closing runs deallocate() on a range that is still mapped, so the old range is unmapped, its reservation freed and its handle reference released through the normal path. The remap-on-rollback undo step, which swallowed its own errors, is no longer needed because the old mapping is never removed before the transaction commits. Issue #2877 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 872908c commit 35ae30c

3 files changed

Lines changed: 65 additions & 29 deletions

File tree

‎cuda_core/cuda/core/_memory/_virtual_memory_resource.py‎

Lines changed: 18 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -371,10 +371,11 @@ def _grow_allocation_slow_path(
371371
Slow path for growing a virtual memory allocation when the new region cannot be
372372
reserved contiguously after the existing buffer.
373373
374-
This function reserves a new, larger virtual address (VA) range, remaps the old
375-
physical memory to the beginning of the new VA range, creates and maps new physical
376-
memory for the additional size, sets access permissions, and updates the buffer's
377-
pointer and size.
374+
This function reserves a new, larger virtual address (VA) range, maps the old
375+
physical memory to the beginning of the new VA range as a second mapping, creates
376+
and maps new physical memory for the additional size, sets access permissions, and
377+
then closes the old buffer, which releases the old VA range through
378+
:meth:`deallocate`.
378379
379380
Args:
380381
buf (Buffer): The buffer to grow.
@@ -385,8 +386,9 @@ def _grow_allocation_slow_path(
385386
addr_align (int): The required address alignment for the new VA range.
386387
387388
Returns:
388-
Buffer: The buffer object updated with the new pointer and size.
389+
Buffer: A new buffer for the new VA range. ``buf`` is closed.
389390
"""
391+
aligned_prev_size = total_aligned_size - aligned_additional_size
390392
with Transaction() as trans:
391393
# Reserve a completely new, larger VA range
392394
res, new_ptr = driver.cuMemAddressReserve(total_aligned_size, addr_align, 0, 0)
@@ -396,28 +398,17 @@ def _grow_allocation_slow_path(
396398
lambda np=new_ptr, s=total_aligned_size: raise_if_driver_error(driver.cuMemAddressFree(np, s)[0])
397399
)
398400

399-
# Get the old allocation handle for remapping
401+
# Retain the old allocation handle to map it a second time. The mappings
402+
# keep the memory alive, so the retained reference is dropped again on
403+
# either outcome.
400404
result, old_handle = driver.cuMemRetainAllocationHandle(buf.handle)
401405
raise_if_driver_error(result)
402406
trans.on_exit(lambda h=old_handle: raise_if_driver_error(driver.cuMemRelease(h)[0]))
403407

404-
# Unmap the old VA range (aligned previous size)
405-
aligned_prev_size = total_aligned_size - aligned_additional_size
406-
(result,) = driver.cuMemUnmap(int(buf.handle), aligned_prev_size)
407-
raise_if_driver_error(result)
408-
409-
def _remap_old() -> None:
410-
# Try to remap the old physical memory back to the original VA range
411-
try:
412-
(res,) = driver.cuMemMap(int(buf.handle), aligned_prev_size, 0, old_handle, 0)
413-
raise_if_driver_error(res)
414-
except Exception: # noqa: S110
415-
# TODO: consider logging this exception
416-
pass
417-
418-
trans.on_failure(_remap_old)
419-
420-
# Remap the old physical memory to the new VA range (aligned previous size)
408+
# Map the old physical memory to the new VA range (aligned previous size).
409+
# The old VA range stays mapped too (virtual aliasing), so the old buffer
410+
# is untouched if anything below fails and is released as a whole by
411+
# buf.close() once the new mapping is complete.
421412
(res,) = driver.cuMemMap(int(new_ptr), aligned_prev_size, 0, old_handle, 0)
422413
raise_if_driver_error(res)
423414

@@ -449,12 +440,10 @@ def _remap_old() -> None:
449440
# All succeeded, cancel undo actions
450441
trans.commit()
451442

452-
# Free the old VA range (aligned previous size)
453-
(res2,) = driver.cuMemAddressFree(int(buf.handle), aligned_prev_size)
454-
raise_if_driver_error(res2)
455-
456-
# Invalidate the old buffer so its destructor won't try to free again
457-
buf._clear()
443+
# Release the old VA range through the resource: closing the buffer runs
444+
# deallocate(), which unmaps the old range and frees its reservation. The
445+
# physical memory stays alive through the new mapping.
446+
buf.close()
458447

459448
# Return a new Buffer for the new mapping
460449
return Buffer.from_handle(ptr=new_ptr, size=new_size, mr=self)

‎cuda_core/docs/source/release/1.3.0-notes.rst‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ New features
3434
Fixes and enhancements
3535
----------------------
3636

37+
- Growing a :class:`~_memory.VirtualMemoryResource` allocation with
38+
:meth:`~_memory.VirtualMemoryResource.modify_allocation` when the address
39+
range cannot be extended in place no longer reports a spurious
40+
:class:`CUDAWarning`. The old buffer is now closed through the resource's
41+
:meth:`~_memory.VirtualMemoryResource.deallocate` instead of having its
42+
range freed by hand and then released a second time.
43+
(`#2877 <https://github.com/NVIDIA/cuda-python/issues/2877>`__)
44+
3745
- ``Graph.__getitem__`` now declares an overload for each node type that has
3846
an executable view, so type checkers and editors see the precise view type:
3947
indexing with a :class:`~graph.KernelNode` yields an

‎cuda_core/tests/test_memory.py‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,6 +1537,45 @@ def allocate_and_close():
15371537
assert baseline - free < aligned_size
15381538

15391539

1540+
@pytest.mark.agent_authored(model="claude-fable-5-1")
1541+
@pytest.mark.thread_unsafe(reason="warning capture is process-global")
1542+
def test_vmm_allocator_grow_allocation_slow_path_closes_old_buffer(init_cuda):
1543+
"""The slow grow path closes the old buffer without a spurious CUDAWarning (#2877).
1544+
1545+
It used to free the old VA range by hand and then reset the old buffer's
1546+
handle, whose deleter called deallocate() on the freed range a second time.
1547+
"""
1548+
device = Device()
1549+
if not device.properties.virtual_memory_management_supported:
1550+
pytest.skip("Virtual memory management is not supported on this device")
1551+
1552+
vmm_mr = VirtualMemoryResource(
1553+
device,
1554+
config=VirtualMemoryResourceOptions(handle_type="win32_kmt" if IS_WINDOWS else "posix_fd"),
1555+
)
1556+
buf = vmm_mr.allocate(2 * 1024 * 1024)
1557+
old_ptr, old_size = int(buf.handle), buf.size
1558+
handle_return(driver.cuMemsetD8(old_ptr, 7, old_size))
1559+
1560+
# Occupy the address range right after buf so the adjacent reservation cannot
1561+
# be honored and modify_allocation has to take the slow path.
1562+
decoy = handle_return(driver.cuMemAddressReserve(old_size, 0, old_ptr + old_size, 0))
1563+
try:
1564+
with assert_no_cuda_warning():
1565+
grown = vmm_mr.modify_allocation(buf, 2 * old_size)
1566+
finally:
1567+
handle_return(driver.cuMemAddressFree(decoy, old_size))
1568+
1569+
assert buf.is_closed
1570+
assert int(grown.handle) != old_ptr
1571+
assert grown.size == 2 * old_size
1572+
# The old contents are reachable through the new mapping.
1573+
host = (ctypes.c_ubyte * old_size)()
1574+
handle_return(driver.cuMemcpyDtoH(ctypes.addressof(host), int(grown.handle), old_size))
1575+
assert bytes(host) == bytes([7]) * old_size
1576+
grown.close()
1577+
1578+
15401579
def test_vmm_allocator_rdma_unsupported_exception():
15411580
"""Test that VirtualMemoryResource throws an exception when RDMA is requested but device doesn't support it.
15421581

0 commit comments

Comments
 (0)