API: add explicit memory mapping #1037
Conversation
|
@nsarka @janjust @Sergei-Lebedev @manjugv May be easier to discuss / track updates to API here. |
|
Can one of the admins verify this patch? |
src/ucc/api/ucc.h
Outdated
| * | ||
| * @return Error code as defined by @ref ucc_status_t. | ||
| */ | ||
| ucc_status_t ucc_mem_exchange_post(ucc_mem_map_mem_h *memh); |
There was a problem hiding this comment.
@wfaderhold21 Is there a need to have an explicit call for exchanging memory handles? Why can't one use allgather or other collectives?
There was a problem hiding this comment.
@manjugv Fundamentally, this is an allgather collective. This explicitly has the UCC library perform the operation rather than the User. Is it better to have the User exchange the opaque structure?
There was a problem hiding this comment.
I don't see the value add, and need for explicit collective call.
There was a problem hiding this comment.
@manjugv removed the explicit collective.
manjugv
added
Blocker-for-rel-branch
Ready-for-Review
and removed
WIP - Don't Merge
labels
|
Ping @Sergei-Lebedev @janjust |
src/ucc/api/ucc.h
Outdated
| * | ||
| * 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 |
There was a problem hiding this comment.
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.
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.
I agree, but my question is why UCC should track it
There was a problem hiding this comment.
I will update the text to make it the user's responsibility
| * @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.
is it global function that does allgather or it just creates local handle?
There was a problem hiding this comment.
global function that does allgather
There was a problem hiding this comment.
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.
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.
@wfaderhold21 add size parameter
| * | ||
| * @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.
do we need some collective operation to unmap?
There was a problem hiding this comment.
i think this can be performed locally. is there a reason to make it collective?
There was a problem hiding this comment.
for cuda ipc registration do we need to notify remote rank that memory is unmapped?
There was a problem hiding this comment.
that's a good point. i think in order to be safe, we should make this a collective operation
There was a problem hiding this comment.
I think the API change looks good. I'm interested in seeing the implementation, specifically how it will keep track of which TLs have associated memhandles. For example, will this map also create a shmem region for TL/SHM? Can we somehow use it to avoid extra memcpys in/out of shmem?
|
@manjugv we have an agreement, changes are reflected - if you approve, we can merge |
|
@janjust We will wait for Ferrol to put together an implementation, so we have API + implementation to go with it. Also, I have requested @wfaderhold21 to add an FAQ. |
What
Adds explicit mapping of user-provided memory for various TLs.