-
Notifications
You must be signed in to change notification settings - Fork 449
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.
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
UCT/API: introduce sys_dev field for mem_alloc #10448
base: master
Are you sure you want to change the base?
UCT/API: introduce sys_dev field for mem_alloc #10448
Conversation
src/uct/api/uct.h
Outdated
UCT_MEM_ALLOC_PARAM_FIELD_NAME = UCS_BIT(4) | ||
UCT_MEM_ALLOC_PARAM_FIELD_NAME = UCS_BIT(4), | ||
|
||
/** Enables @ref uct_mem_alloc_params_t::sys_device */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is called sys_dev
in uct_mem_alloc_params_t
. Pls align one of them with the other.
also UCT_MEM_ALLOC_PARAM_FIELD_SYS_DEVICE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UCT_MEM_ALLOC_PARAM_FIELD_SYS_DEV
src/uct/api/uct.h
Outdated
* Index of the system device on which memory is to be allocated. | ||
* Eg: UCS_SYS_DEVICE_ID_UNKNOWN to allocate on host memory, or a specific | ||
* index to allocate memory on GPU device. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just
"System device on which memory is to be allocated, or UCS_SYS_DEVICE_ID_UNKNOWN to allow allocating on any device".
The host memory comment is misleading because mem_type field is used to select host memory
@@ -539,10 +539,11 @@ ucs_status_t uct_mem_alloc_check_params(size_t length, | |||
|
|||
ucs_status_t uct_md_mem_alloc(uct_md_h md, size_t *length_p, void **address_p, | |||
ucs_memory_type_t mem_type, unsigned flags, | |||
const char *alloc_name, uct_mem_h *memh_p) | |||
const char *alloc_name, ucs_sys_device_t sys_dev, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move sys_dev after mem_type
alloc_length = length; | ||
log_level = (flags & UCT_MD_MEM_FLAG_HIDE_ERRORS) ? UCS_LOG_LEVEL_DEBUG : | ||
UCS_LOG_LEVEL_ERROR; | ||
|
||
ucs_trace("allocating %s: %s memory length %zu flags 0x%x", alloc_name, | ||
ucs_memory_type_names[mem_type], alloc_length, flags); | ||
ucs_trace("allocating %s: %s memory length %zu sys_dev %s flags 0x%x", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
print type before sys_dev
@@ -1160,7 +1160,7 @@ ucs_status_t | |||
uct_ib_mlx5_devx_device_mem_alloc(uct_md_h uct_md, size_t *length_p, | |||
void **address_p, ucs_memory_type_t mem_type, | |||
unsigned flags, const char *alloc_name, | |||
uct_mem_h *memh_p); | |||
ucs_sys_device_t sys_dev, uct_mem_h *memh_p); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- fix indentation
- move sys_dev_after mem_type
What? Why?
As we move towards supporting multi-device configurations and relaxing GPU context setting requirements, UCP and other UCT users need to be able to indicate on which system device the allocation is being requested on without having to rely on inferring the same from context set against calling thread. This patch extends allocation params struct to include system device. In this initial implementation all UCT allocators ignore the system device argument but eventually different transports can convert the given system device into a bus_id and allocate against that specific device.