cuda.core: don't require a CUDA context for host-only memory - #2773
Conversation
Andy-Jost
added
bug
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Since 1.2.0, Buffer.from_handle binds a default-stream deallocation token to the current context for every owning memory resource. Memory the device cannot access has no stream ordering to preserve, so skip the binding when mr.is_device_accessible is False. Such buffers record no deallocation stream and never call the driver. Fixes NVIDIA#2769 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
9730a1a to
7fcefa1
Compare
|
There was a problem hiding this comment.
I don't see why both of these Host-only MRs are needed. For test coverage we just need one or the other.
There was a problem hiding this comment.
I agree that it's not necessary to have both. The key point is that the MemoryResource does not use any CUDA abstractions and it able to be allocated and deallocated without an CUDA context.
carterbox
left a comment
carterbox
left a comment
There was a problem hiding this comment.
I have run the affected nvmath-python integrations tests against this branch, and they pass.
Only non-blocking suggestions from me.
| if not h_stream: | ||
| print( | ||
| "Warning: no deallocation stream was recorded; falling back to " | ||
| "the default stream for mr.deallocate() during Buffer " | ||
| "destruction. This is an internal cuda-core error; please " | ||
| "report it with your CUDA driver, CUDA Toolkit, and " | ||
| "cuda-python versions.", | ||
| file=sys.stderr, | ||
| ) | ||
| # No stream was recorded: host-only memory (Buffer._init records | ||
| # none) or a Buffer released before one was set. The default-stream | ||
| # token needs no CUDA context to construct. | ||
| stream = default_stream() |
There was a problem hiding this comment.
In order to retain the existing behavior, wouldn't you want to check that mr.is_device_accessible is false before deciding whether or not to print the error?
| s = Stream_accept(default_stream() if stream is None else stream) | ||
| # Host-only memory needs no CUDA context to free, so no deallocation | ||
| # stream is recorded and the driver is not called. | ||
| record_stream = mr.is_device_accessible |
There was a problem hiding this comment.
record_stream is used exactly once? Inline the expression instead?
There was a problem hiding this comment.
I agree that it's not necessary to have both. The key point is that the MemoryResource does not use any CUDA abstractions and it able to be allocated and deallocated without an CUDA context.
| def device_id(self) -> int: | ||
| raise RuntimeError("the pinned memory resource is not bound to any GPU") |
There was a problem hiding this comment.
I believe host-only MemoryResources should have device_id=-1 as below, and as documented in the release notes here: https://github.com/NVIDIA/cuda-python/pull/2750/changes#diff-793ada78d6808b82e09c2cbbc503b1d202ad6467a4b8b2846e894ffb234fa4b3
… stderr The host-only Buffer tests from NVIDIA#2773 asserted that nothing containing "Warning" reached stderr. Under the error handling policy a teardown failure is a CUDAWarning, not stderr text, so that assertion no longer checks anything. Use assert_no_cuda_warning() around allocate/close instead (marked thread_unsafe, as warning capture is process-global). The spawned-process variant checks inside the child, since warnings do not cross processes; a failure surfaces as the non-zero exit code the parent already asserts on. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
3 participants
Summary
Since 1.2.0,
Buffer.from_handlebinds a default-stream deallocation token to the current CUDA context for every owning memory resource (#2526) when no other deallocation stream is supplied. A memory resource whose memory the device cannot access has no stream ordering to preserve, yet it could no longer create aBufferin a process that never initialized CUDA. This is a regression from 1.1.x, reported against nvmath-python's NVPL-backed host APIs.Changes
mr.is_device_accessibleisFalse,Buffer.from_handlerecords no deallocation stream and does not call the driver.mr.deallocate()still receives the default stream, as in 1.1.Related Work