Repository navigation
UCP/UCT/UCS: Add memory registration flags - #11553
Conversation
Add memory flags to UCS memory information and propagate them through UCT mem_query_v2 so providers can report whether a memory region is safe to register. Use the CAN_REGISTER flag in UCP memory registration and protocol selection paths to avoid selecting registration-dependent protocols for non-registerable memory. Mark CUDA async allocations classified as CUDA managed as non-registerable, and require CAN_REGISTER for IB memory domains that need registration. Add focused gtests for memtype cache flag handling, IB registration failure for CUDA async memory, and UCP protocol filtering.
|
Also update NVIDIA copyright years on changes files |
| ucp_md_index_t i, md_index; | ||
| uint8_t sg_count; | ||
|
|
||
| if (!mem_buffer::is_mem_type_supported(UCS_MEMORY_TYPE_CUDA)) { |
There was a problem hiding this comment.
remove if we check for async after?
There was a problem hiding this comment.
is_async_supported doesn't check we have a gpu, just the compile-time cuda library version
There was a problem hiding this comment.
i see, can we merge the two checks in is_async_supported?
There was a problem hiding this comment.
since all uses of is_async_supported (even in master) first check for cuda mem type supported, we can call is_mem_type_supported(UCS_MEMORY_TYPE_CUDA) inside is_async_supported(), and only keep this one everywhere.
Add the mem_flags to the rcache key. Add more tests. Apply CR fixes.
| ucp_memory_detect(context, address, length, &info); | ||
| ucp_memh_init(memh, context, memh_flags, uct_flags, method, mem_type, | ||
| info.sys_dev); | ||
| ucp_memh_init(memh, context, memh_flags, uct_flags, method, mem_info->type, |
There was a problem hiding this comment.
can we simply pass mem_info here, instead of type, sys_dev and mem_flags?
brminich
left a comment
There was a problem hiding this comment.
maybe we do not keep non-registrable memory in rcache?
| .parent = &ucp_mem_dummy_handle.memh, | ||
| .mem_type = UCS_MEMORY_TYPE_HOST, | ||
| .sys_dev = UCS_SYS_DEVICE_ID_UNKNOWN, | ||
| .super.mem_flags = UCS_MEM_FLAG_REGISTRABLE, |
There was a problem hiding this comment.
is there a reason to keep flags separately in rcache?
| reg_mem_info.type = internal_mem_info.type; | ||
| reg_mem_info.sys_dev = internal_mem_info.sys_dev; | ||
| reg_mem_info.mem_flags = internal_mem_info.mem_flags; | ||
| ucp_mem_info_update_for_type(®_mem_info, mem_type); |
There was a problem hiding this comment.
why do we need it here? Is there a use case that mem_type can be different from internal_mem_info.type?
| } | ||
|
|
||
| ucp_memory_detect(context, address, length, mem_info); | ||
| ucp_mem_info_update_for_type(mem_info, mem_type); |
There was a problem hiding this comment.
why do we need it ? what is the use case that we havee to change the info filled by detect function?
| ucp_mem_info_detect_for_type(context, buffer, length, mem_type, | ||
| &mem_info); |
There was a problem hiding this comment.
why do we need to redetect here?
|
|
||
| status = ucs_rcache_get(rcache, address, length, alignment, | ||
| PROT_READ | PROT_WRITE, ®_ctx, &rregion); | ||
| PROT_READ | PROT_WRITE, mem_info->mem_flags, |
There was a problem hiding this comment.
do we really need to pass flags? Can we assume that we always need registrable?
There was a problem hiding this comment.
actually that's interesting, so we would never cache memory without REGISTRABLE flag?
| mem_info = ucp_memory_info_from_memh(memh); | ||
| status = ucp_memh_get(context, address, length, &mem_info, cache_md_map, |
|
|
||
| status = ucs_rcache_get(rcache, address, length, alignment, | ||
| PROT_READ | PROT_WRITE, ®_ctx, &rregion); | ||
| PROT_READ | PROT_WRITE, mem_info->mem_flags, |
There was a problem hiding this comment.
actually that's interesting, so we would never cache memory without REGISTRABLE flag?
Also fix CR comments
…m v1 Also assert on different mem_type from detected mem_type.
| *md_idx_p = context->alloc_md[alloc_mem_type].md_index; | ||
| mem_info_p->type = alloc_mem_type; | ||
| mem_info_p->flags = context->alloc_md[alloc_mem_type].mem_flags; |
| CUdevice cuda_device = CU_DEVICE_INVALID; | ||
| CUpointer_attribute attr_type[] = {CU_POINTER_ATTRIBUTE_MEMORY_TYPE, | ||
| CU_POINTER_ATTRIBUTE_IS_MANAGED, | ||
| CU_POINTER_ATTRIBUTE_DEVICE_ORDINAL, |
There was a problem hiding this comment.
why do we need cuda_device?
There was a problem hiding this comment.
also can you avoid doing this query and cache the result from the previous query in the mem_query function?
There was a problem hiding this comment.
you're right, removed this function and reused the result from the earlier query
| return UCS_MEM_FLAG_REGISTRABLE; | ||
| } | ||
|
|
||
| if (!md->config.dmabuf_supported) { |
There was a problem hiding this comment.
why? what if it is cuda_managed memory?
There was a problem hiding this comment.
cuda_managed is handled above
Don't query twice the pointer for asyncness Refactoring tests
* UCP/UCT/UCS: Add memory registration flags Add memory flags to UCS memory information and propagate them through UCT mem_query_v2 so providers can report whether a memory region is safe to register. Use the CAN_REGISTER flag in UCP memory registration and protocol selection paths to avoid selecting registration-dependent protocols for non-registerable memory. Mark CUDA async allocations classified as CUDA managed as non-registerable, and require CAN_REGISTER for IB memory domains that need registration. Add focused gtests for memtype cache flag handling, IB registration failure for CUDA async memory, and UCP protocol filtering.
What?
Add memory registration flags to UCS memory information and propagate them through UCT mem_query_v2 and UCP memory/protocol selection paths.
This adds a REGISTERABLE memory flag, lets CUDA async allocations classified as CUDA managed report that they are not registerable, and makes UCP avoid registration-dependent MDs/protocols when that flag is missing.
Why?
Some CUDA memory can be detected as a memory type that normally has registration-capable transports, while the specific allocation cannot actually be registered on IB MDs. Without carrying this per-allocation information into UCP, protocol selection can choose zcopy paths that require registration and later fail.
How?