-
Notifications
You must be signed in to change notification settings - Fork 103
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
API: add explicit memory mapping #1037
base: master
Are you sure you want to change the base?
Changes from 5 commits
2150add
dd2c696
c6c02b5
fe7f690
f85f87a
be38a7f
41c840f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2220,5 +2220,53 @@ ucc_status_t ucc_ee_wait(ucc_ee_h ee, ucc_ev_t *ev); | |
*/ | ||
ucc_status_t ucc_collective_triggered_post(ucc_ee_h ee, ucc_ev_t *ee_event); | ||
|
||
typedef void *ucc_mem_map_mem_h; | ||
|
||
/** | ||
* @ingroup UCC_DATATYPE | ||
*/ | ||
typedef enum { | ||
UCC_MEM_MAP_EXPORT = 0, /*!< Indicate ucc_mem_map() should export | ||
memory handles from TLs used by context */ | ||
UCC_MEM_MAP_IMPORT = 1 /*!< Indicate ucc_mem_map() should import | ||
memory handles from user memory handle */ | ||
} ucc_mem_map_flags_t; | ||
|
||
/** | ||
* @ingroup UCC_CONTEXT | ||
* @brief Routine registers or maps memory for use in future collectives. | ||
* | ||
* This routine maps a user-specified memory segment with a ucc_context_h. The | ||
* segment is considered "mapped" with the context until either the user calls | ||
* ucc_mem_unmap or ucc_context_destroy(). A handle to the mapped | ||
* memory is provided in memh. If the flag UCC_MEM_MAP_EXPORT is used, the | ||
* memory will be mapped and memory handles from TLs will be generated and | ||
* stored in the memh. If the flag UCC_MEM_MAP_IMPORT is used, the user must | ||
* provide a valid memh, otherwise behavior is undefined. | ||
* | ||
* @param [in] context Context mapped memory is associated with | ||
* @param [in] flags flags dictating the behavior of the routine | ||
* @param [in] params parameters indicating the address and length of | ||
* memory to map | ||
* @param [inout] *memh Handle for the registered memory | ||
* | ||
* @return Error code as defined by ucc_status_t. | ||
*/ | ||
|
||
ucc_status_t ucc_mem_map(ucc_context_h context, ucc_mem_map_flags_t flags, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it global function that does allgather or it just creates local handle? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. global function that does allgather There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, in this case how that should work if context is not global and we don't have any OOB function at this point? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Help me a little here. If we create a local context, can we perform communication on that context between other processes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wfaderhold21 add size parameter |
||
ucc_mem_map_params_t params, ucc_mem_map_mem_h *memh); | ||
/** | ||
* @ingroup UCC_CONTEXT | ||
* @brief Routine unmaps memory from a context | ||
* | ||
* This routine unmaps memory and all resources associated with the memory | ||
* from a context. The memh object is freed and cannot be reused. | ||
* | ||
* @param [in] *memh Handle of the registered memory | ||
* | ||
* @return Error code as defined by ucc_status_t. | ||
*/ | ||
ucc_status_t ucc_mem_unmap(ucc_mem_map_mem_h *memh); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need some collective operation to unmap? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this can be performed locally. is there a reason to make it collective? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for cuda ipc registration do we need to notify remote rank that memory is unmapped? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's a good point. i think in order to be safe, we should make this a collective operation |
||
|
||
END_C_DECLS | ||
#endif |
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.
why not to let user control map and unmap i.e. it's user responsibility to unmap all segments before context destroy?
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.
I think the problem relates to whether mapped memory is associated with a context or its own object. From my understanding, mapped memory would be used by collectives for communication purposes, meaning it would need to be associated with a context as a context contains network resources. In that case, when a user decides to destroy a context, the memory must be unmapped in order to prevent erroneous usage afterwards. @manjugv please correct me.
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.
I agree, but my question is why UCC should track it
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.
I will update the text to make it the user's responsibility