Skip to content
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d72bcfb
UCT/CUDA_COPY: Skip dmabuf probe for async managed memory
pentschev Sep 8, 2026
65e7d2c
Merge remote-tracking branch 'upstream/master' into cuda-async-skip-d…
pentschev Sep 8, 2026
1d53937
UCT/CUDA_COPY: Address async managed memory review
pentschev Sep 8, 2026
458aeca
GTEST/CUDA: Cover managed memory registration flags
pentschev Sep 8, 2026
31617b6
UCT/CUDA_COPY: Detect managed memory pools
pentschev Sep 8, 2026
0fb53b3
UCT/CUDA_COPY: Preserve managed pool device locality
pentschev Sep 8, 2026
1b27615
UCT/CUDA_COPY: Handle managed pool CPU devices
pentschev Sep 8, 2026
12b3116
UCT/CUDA_COPY: Keep managed-pool detection resilient
pentschev Sep 8, 2026
07409ae
UCT/CUDA_COPY: Use mempool attributes for detection
pentschev Sep 8, 2026
cfc5a8e
UCT/CUDA_COPY: Detect managed memory pools by capability
pentschev Sep 9, 2026
a041773
Merge remote-tracking branch 'upstream/master' into cuda-async-skip-d…
pentschev Sep 9, 2026
ad87c78
GTEST/CUDA: Reuse managed default pool lookup
pentschev Sep 9, 2026
13e872f
UCT/CUDA_COPY: Track queried attribute count
pentschev Sep 9, 2026
2b73283
UCT/CUDA_COPY: Clarify managed pool detection
pentschev Sep 9, 2026
3773519
GTEST/CUDA: Align managed pool test variables
pentschev Sep 9, 2026
15e7781
GTEST/CUDA: Clear managed pool probe errors
pentschev Sep 9, 2026
16a911b
GTEST/CUDA: Share async release path
pentschev Sep 9, 2026
f473f14
UCT/CUDA_COPY: Register managed memory without dmabuf
pentschev Sep 9, 2026
6ece985
Merge remote-tracking branch 'upstream/master' into cuda-async-skip-d…
pentschev Sep 10, 2026
a7cbb96
UCT/CUDA_COPY: Preserve device symbol flags
pentschev Sep 10, 2026
437da17
UCT/CUDA_COPY: Simplify managed memory checks
pentschev Sep 10, 2026
fae7ff1
UCT/CUDA_COPY: Initialize memory query result
pentschev Sep 10, 2026
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
14 changes: 11 additions & 3 deletions src/uct/cuda/cuda_copy/cuda_copy_md.c
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,11 @@ uct_cuda_copy_md_is_registrable(uct_cuda_copy_md_t *md,
return 0;
}

/* Managed memory is registered through ODP and is not dmabuf-exportable. */
if (mem_info->type == UCS_MEMORY_TYPE_CUDA_MANAGED) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

does this also flip CUDA device symbols to registrable? ucm/cuda/cudamem.c reports symbols (cudaGetSymbolAddress, cuModuleGetGlobal) as UCS_MEMORY_TYPE_CUDA_MANAGED with mem_flags=0, and uct_cuda_copy_md_mem_query() takes the managed type from the memtype cache, so this branch would report them REGISTRABLE and also overwrite the cached mem_flags via ucs_memtype_cache_update().

Rationale: mem_query() does addr_mem_info.type = cached_mem_info.type when the cached type is CUDA_MANAGED (the "device symbols" workaround), then computes flags from addr_mem_info.type and writes them back to the cache. Before this change the symbol range went through the dmabuf probe and stayed non-registrable when export failed.

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.

I think this comment is important. @pentschev #11646 made cuda symbols managed memory as a workaround, and the change we suggested made it so they will now be registerable if queried. The fix needs to make sure it doesn't set registerable to device symbols

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this is fixed now. mem_query now retains the driver-reported memory type for flag detection before applying the cached CUDA-managed type used to report device symbols. Therefore device symbols continue through the normal non-managed path, remain non-registrable, and their cached zero flags are preserved. I also extended test_cuda_get_symbol_address to verify this after mapping the symbol.

return UCS_MEM_FLAG_REGISTRABLE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
return UCS_MEM_FLAG_REGISTRABLE;
return 1;

minor: the function is a boolean predicate, the other branches return 0/1.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

}

/* Host-located CUDA VMM is registerable even if dmabuf export fails. */
if (is_host_located || (mem_info->sys_dev == UCS_SYS_DEVICE_ID_UNKNOWN) ||
!md->config.dmabuf_supported) {
Expand Down Expand Up @@ -984,6 +989,7 @@ ucs_status_t uct_cuda_copy_md_mem_query(uct_md_h tl_md, const void *address,
CUdevice avail_cuda_device = CU_DEVICE_INVALID;
ucs_memory_info_t cached_mem_info;
ucs_memory_info_t addr_mem_info;
ucs_memory_info_t detected_mem_info = {};

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.

nit: initialized variables go above unintialized variables.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

ucs_status_t cache_status;
ucs_status_t status;

Expand All @@ -1009,8 +1015,10 @@ ucs_status_t uct_cuda_copy_md_mem_query(uct_md_h tl_md, const void *address,
return status;
}

/* CUDA reports device symbols as device memory, so preserve the type
* explicitly provided by the UCM allocation event. */
/* Preserve the driver-reported type for flags. CUDA reports device
* symbols as device memory, so use the UCM event type for callers. */
detected_mem_info = addr_mem_info;

if ((cache_status == UCS_OK) &&
(cached_mem_info.type == UCS_MEMORY_TYPE_CUDA_MANAGED)) {
addr_mem_info.type = cached_mem_info.type;
Expand Down Expand Up @@ -1064,7 +1072,7 @@ ucs_status_t uct_cuda_copy_md_mem_query(uct_md_h tl_md, const void *address,

if (address != NULL) {
addr_mem_info.mem_flags = uct_cuda_copy_md_detect_mem_flags(
md, &addr_mem_info, is_async_managed, is_host_located,
md, &detected_mem_info, is_async_managed, is_host_located,
dmabuf_queried ? &dmabuf : NULL);
ucs_memtype_cache_update(addr_mem_info.base_address,
addr_mem_info.alloc_length, addr_mem_info.type,
Expand Down
13 changes: 13 additions & 0 deletions test/apps/test_cuda_get_symbol_address.cu
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include <ucp/api/ucp.h>
#include <ucm/api/ucm.h>
#include <ucs/memory/memtype_cache.h>
#include <cuda_runtime.h>


Expand Down Expand Up @@ -68,6 +69,7 @@ static int check_symbol_memory_type(ucp_context_h context, void *address)
{
ucp_mem_map_params_t map_params;
ucp_mem_attr_t mem_attr;
ucs_memory_info_t cached_mem_info;
ucp_mem_h memh;
ucs_status_t status;
int num_errors = 0;
Expand All @@ -94,6 +96,17 @@ static int check_symbol_memory_type(ucp_context_h context, void *address)
++num_errors;
}

status = ucs_memtype_cache_lookup(address, sizeof(device_int),
&cached_mem_info);
if (status != UCS_OK) {
printf("memtype cache lookup failed: %s\n", ucs_status_string(status));
++num_errors;
} else if (cached_mem_info.mem_flags & UCS_MEM_FLAG_REGISTRABLE) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

does this hold when dmabuf is not supported? there uct_cuda_copy_md_is_registrable() returns 1 for the symbol (driver type is cuda with a real sys_dev), so the app fails on such a machine. maybe check the flags only when dmabuf is supported?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this expects dmabuf export to fail for the symbol, but when the build or device has no dmabuf support (CUDA < 11.7, or UCX_CUDA_COPY_DMABUF=n), uct_cuda_copy_md_is_registrable() returns 1 and this check fails. can we make the expectation independent of dmabuf support?

Rationale: uct_cuda_copy_md_is_dmabuf_supported() compiles the attribute query out below CUDA 11.7, so md->config.dmabuf_supported == 0 and is_registrable() returns 1 at the !md->config.dmabuf_supported short-circuit, i.e. the cached symbol flags would contain UCS_MEM_FLAG_REGISTRABLE. buildlib/pr/cuda/test_malloc_hook.sh runs this app unconditionally.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed the zero-flags assertion.

printf("unexpected cached symbol memory flags 0x%x\n",
cached_mem_info.mem_flags);
++num_errors;
}

status = ucp_mem_unmap(context, memh);
if (status != UCS_OK) {
printf("ucp_mem_unmap() failed: %s\n", ucs_status_string(status));
Expand Down
63 changes: 56 additions & 7 deletions test/gtest/common/mem_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,31 @@

#endif

#if HAVE_CUDA && CUDART_VERSION >= 13000
static cudaError_t
mem_buffer_cuda_get_default_managed_pool(cudaMemPool_t *pool_p)
{
cudaMemLocation location = {};
int device;
cudaError_t cuda_status;

cuda_status = cudaGetDevice(&device);
if (cuda_status != cudaSuccess) {
return cuda_status;
}

location.type = cudaMemLocationTypeDevice;
location.id = device;
cuda_status = cudaMemGetDefaultMemPool(pool_p, &location,
cudaMemAllocationTypeManaged);
if (cuda_status != cudaSuccess) {
(void)cudaGetLastError(); /* do not leak the error to other tests */
}

return cuda_status;
}
#endif

#if HAVE_ROCM
#include <hip_runtime.h>
#include <hip_version.h>
Expand Down Expand Up @@ -473,7 +498,22 @@ void *mem_buffer::allocate(size_t size, ucs_memory_type_t mem_type, bool async)
}
return ptr;
case UCS_MEMORY_TYPE_CUDA_MANAGED:
CUDA_CALL(cudaMallocManaged(&ptr, size), ": size=" << size);
if (async) {
#if CUDART_VERSION >= 13000
cudaMemPool_t pool;

CUDA_CALL(mem_buffer_cuda_get_default_managed_pool(&pool), "");
CUDA_CALL(cudaMallocFromPoolAsync(&ptr, size, pool, 0),
": size=" << size);
CUDA_CALL(cudaStreamSynchronize(0), "");
#else
UCS_TEST_ABORT("asynchronous allocation for " +
std::string(ucs_memory_type_names[mem_type]) +
" memory type is not supported");
#endif
} else {
CUDA_CALL(cudaMallocManaged(&ptr, size), ": size=" << size);
}
return ptr;
#endif
#if HAVE_ROCM
Expand Down Expand Up @@ -549,6 +589,7 @@ void mem_buffer::release(void *ptr, ucs_memory_type_t mem_type, bool async)
break;
#if HAVE_CUDA
case UCS_MEMORY_TYPE_CUDA:
case UCS_MEMORY_TYPE_CUDA_MANAGED:
if (async) {
#if CUDART_VERSION >= 11020
cudaStreamSynchronize(0);
Expand All @@ -562,9 +603,6 @@ void mem_buffer::release(void *ptr, ucs_memory_type_t mem_type, bool async)
CUDA_CALL(cudaFree(ptr), ": ptr=" << ptr);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This case is now identical to the UCS_MEMORY_TYPE_CUDA one above (sync stream, cudaFreeAsync(ptr, 0) when async, cudaFree(ptr) otherwise) — only the version threshold differs. Can we just add case UCS_MEMORY_TYPE_CUDA_MANAGED: to it instead of duplicating? cudaFreeAsync exists since CUDART 11.2, so the 13000 guard is not needed for the free path. Root AGENTS.md asks not to duplicate similar implementation patterns.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

break;
case UCS_MEMORY_TYPE_CUDA_MANAGED:
CUDA_CALL(cudaFree(ptr), ": ptr=" << ptr);
break;
#endif
#if HAVE_ROCM
case UCS_MEMORY_TYPE_ROCM:
Expand Down Expand Up @@ -851,10 +889,21 @@ bool mem_buffer::is_async_supported(ucs_memory_type_t mem_type)
if (!is_mem_type_supported(UCS_MEMORY_TYPE_CUDA)) {
return false;
}
return mem_type == UCS_MEMORY_TYPE_CUDA;
#else
return false;

if (mem_type == UCS_MEMORY_TYPE_CUDA) {
return true;
}

#if CUDART_VERSION >= 13000
if (mem_type == UCS_MEMORY_TYPE_CUDA_MANAGED) {
cudaMemPool_t pool;

return mem_buffer_cuda_get_default_managed_pool(&pool) == cudaSuccess;
}
#endif
#endif

return false;
}

mem_buffer::mem_buffer(size_t size, ucs_memory_type_t mem_type) :
Expand Down
2 changes: 1 addition & 1 deletion test/gtest/common/mem_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class mem_buffer {
/* Allocation mode. */
enum class alloc_mode {
DEFAULT, /* Default allocation mode, using cudaMalloc */

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: DEFAULT is also used for cudaMallocManaged now, so the comment is stale.

Suggested change
DEFAULT, /* Default allocation mode, using cudaMalloc */
DEFAULT, /* Default synchronous CUDA allocation mode */

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: the DEFAULT comment is now stale as well — that path also uses cudaMallocManaged.

Suggested change
DEFAULT, /* Default allocation mode, using cudaMalloc */
DEFAULT, /* Default CUDA allocation mode */
ASYNC /* Asynchronous CUDA allocation mode */

ASYNC /* Asynchronous allocation mode, using cudaMallocAsync */
ASYNC /* Asynchronous CUDA allocation mode */
};

mem_buffer(size_t size, ucs_memory_type_t mem_type);
Expand Down
77 changes: 77 additions & 0 deletions test/gtest/uct/cuda/test_switch_cuda_device.cc
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,45 @@ class test_mem_alloc_device : public test_switch_cuda_device {
EXPECT_TRUE(mem_attr.mem_flags & UCS_MEM_FLAG_REGISTRABLE);
}

void query_managed_registrable(void *address, size_t size)
{
CUpointer_attribute attr_type = CU_POINTER_ATTRIBUTE_CONTEXT;
uct_md_mem_attr_v2_t mem_attr = {};
CUcontext cuda_mem_ctx = nullptr;

ASSERT_EQ(CUDA_SUCCESS,
cuPointerGetAttribute(&cuda_mem_ctx, attr_type,
(CUdeviceptr)address));
EXPECT_NE(nullptr, cuda_mem_ctx);

mem_attr.field_mask = UCT_MD_MEM_ATTR_V2_FIELD_MEM_TYPE |
UCT_MD_MEM_ATTR_V2_FIELD_MEM_FLAGS;
EXPECT_UCS_OK(uct_md_mem_query_v2(md(), address, size, &mem_attr));
EXPECT_EQ(UCS_MEMORY_TYPE_CUDA_MANAGED, mem_attr.mem_type);
EXPECT_TRUE(mem_attr.mem_flags & UCS_MEM_FLAG_REGISTRABLE);
}

void test_async_managed_mem_pool_registrable()
{
constexpr size_t size = 192;

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: extra spaces, nothing to align with here.

Suggested change
constexpr size_t size = 192;
constexpr size_t size = 192;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

uct_md_mem_attr_v2_t mem_attr = {};

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: pls align = on the column (longest LHS gets one space before =).

Suggested change
uct_md_mem_attr_v2_t mem_attr = {};
constexpr size_t size = 192;
uct_md_mem_attr_v2_t mem_attr = {};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


if (!mem_buffer::is_async_supported(UCS_MEMORY_TYPE_CUDA_MANAGED)) {
UCS_TEST_SKIP_R("asynchronous CUDA managed memory is not "
"supported");
}

mem_buffer buffer(size, UCS_MEMORY_TYPE_CUDA_MANAGED,
mem_buffer::alloc_mode::ASYNC);
buffer.memset(0);

mem_attr.field_mask = UCT_MD_MEM_ATTR_V2_FIELD_MEM_TYPE |
UCT_MD_MEM_ATTR_V2_FIELD_MEM_FLAGS;
EXPECT_UCS_OK(uct_md_mem_query_v2(md(), buffer.ptr(), size, &mem_attr));

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: these lines duplicate the tail of query_managed_registrable(); can we split the query + expectations into a small helper and call it from both, keeping only the CU_POINTER_ATTRIBUTE_CONTEXT assert in query_managed_registrable()?

EXPECT_EQ(UCS_MEMORY_TYPE_CUDA_MANAGED, mem_attr.mem_type);
EXPECT_TRUE(mem_attr.mem_flags & UCS_MEM_FLAG_REGISTRABLE);

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: can we reuse query_managed_registrable() here? the only difference is the context check, which can move to its caller.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

}

private:
std::vector<ucs_sys_device_t> m_sys_dev;

Expand Down Expand Up @@ -441,6 +480,44 @@ UCS_TEST_P(test_mem_alloc_device, no_current_context_user_mem_registrable,
EXPECT_EQ(CUDA_SUCCESS, cuMemFree(dptr));
}

UCS_TEST_P(test_mem_alloc_device, user_managed_mem_registrable,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

with PREF_LOC=cpu (which is also the default) the managed branch sets sys_dev to unknown, so is_registrable() returns 1 before reaching the new CUDA_MANAGED check - this test and uct_alloc_managed_mem_registrable pass without the fix too. can we use gpu here?

Suggested change
UCS_TEST_P(test_mem_alloc_device, user_managed_mem_registrable,
UCS_TEST_P(test_mem_alloc_device, user_managed_mem_registrable,
"CUDA_COPY_PREF_LOC=gpu")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

"CUDA_COPY_PREF_LOC=cpu")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

user_managed_mem_registrable, CUDA_COPY_PREF_LOC=cpu: with PREF_LOC=cpu the managed buffer gets sys_dev=UNKNOWN, so is_registrable() already returned true before this fix and the new branch is not exercised; can we add a PREF_LOC=gpu variant here too? that would also cover the fix on CUDA < 13, where all three async_managed_mem_pool_* tests are compiled out/skipped.

{
constexpr size_t size = 4 * UCS_MBYTE;
mem_buffer buffer(size, UCS_MEMORY_TYPE_CUDA_MANAGED);

query_managed_registrable(buffer.ptr(), size);
}

UCS_TEST_P(test_mem_alloc_device, uct_alloc_managed_mem_registrable,
"CUDA_COPY_PREF_LOC=cpu")
{
ASSERT_UCS_OK(allocate(UCS_MEMORY_TYPE_CUDA_MANAGED));

query_managed_registrable(mem.address, mem.length);

EXPECT_UCS_OK(uct_mem_free(&mem));
}

UCS_TEST_P(test_mem_alloc_device, async_managed_mem_pool_registrable,
"CUDA_COPY_DMABUF=try")
{
test_async_managed_mem_pool_registrable();
}

UCS_TEST_P(test_mem_alloc_device, async_managed_mem_pool_gpu_pref_loc,
"CUDA_COPY_DMABUF=try", "CUDA_COPY_PREF_LOC=gpu")
{
test_async_managed_mem_pool_registrable();
}

UCS_TEST_P(test_mem_alloc_device,
async_managed_mem_pool_registrable_cuda_type,
"CUDA_COPY_DMABUF=try", "CUDA_COPY_ASYNC_MEM_TYPE=cuda")
{
test_async_managed_mem_pool_registrable();
}

UCS_TEST_P(test_mem_alloc_device, no_current_context_vmm_mem_registrable,
"CUDA_COPY_ASYNC_MEM_TYPE=cuda")
{
Expand Down
Loading