Repository navigation
UCT/CUDA_IPC: Implement uct_ep_put_sgl_zcopy - #11359
Merged
Merged
Conversation
michal-shalev
marked this pull request as draft
April 19, 2026 16:18
michal-shalev
force-pushed
the
uct-cuda-ipc-sgl
branch
2 times, most recently
from
April 26, 2026 15:17
81ed122 to
2101677
Compare
michal-shalev
added
Ready for Review
and removed
WIP-DNM
michal-shalev
marked this pull request as ready for review
April 26, 2026 15:18
michal-shalev
force-pushed
the
uct-cuda-ipc-sgl
branch
2 times, most recently
from
April 26, 2026 16:08
e66fb59 to
f56085d
Compare
michal-shalev
marked this pull request as draft
April 26, 2026 16:54
michal-shalev
force-pushed
the
uct-cuda-ipc-sgl
branch
3 times, most recently
from
June 24, 2026 11:55
325279b to
50b7f6f
Compare
michal-shalev
added
Ready for Review
and removed
WIP-DNM
michal-shalev
force-pushed
the
uct-cuda-ipc-sgl
branch
3 times, most recently
from
July 2, 2026 10:17
be7b14d to
e4b5a14
Compare
michal-shalev
force-pushed
the
uct-cuda-ipc-sgl
branch
from
July 2, 2026 10:18
e4b5a14 to
24af44d
Compare
michal-shalev
marked this pull request as ready for review
July 2, 2026 10:18
brminich
reviewed
Jul 9, 2026
Comment on lines
+304
to
+307
| if (ucs_unlikely(count == 0)) { | ||
| ucs_trace_data("Zero count put_sgl_zcopy: skip it"); | ||
| return UCS_OK; | ||
| } |
Contributor
There was a problem hiding this comment.
maybe we do not need this check separately - it is covered by the check on line 320 below. you can move trace there
| mapping->count = count; | ||
| mapping->entries = (uct_cuda_ipc_sgl_entry_t *)(mapping + 1); | ||
|
|
||
| cuda_dsts = ucs_malloc(count * sizeof(CUdeviceptr), "cuda_ipc_sgl_dsts"); |
Contributor
There was a problem hiding this comment.
can we combine with mapping allocation? i.e. have one malloc covering both?
| uct_cuda_ipc_sgl_unmap(mapping, mapping->count, | ||
| cuda_ipc_event->cuda_device, | ||
| iface->config.enable_cache); | ||
| ucs_free(mapping); |
Contributor
There was a problem hiding this comment.
maybe we can free in uct_cuda_ipc_sgl_unmap? they are always used together
Sergei-Lebedev
approved these changes
Jul 9, 2026
brminich
approved these changes
Jul 9, 2026
| return status; | ||
| } | ||
|
|
||
| mapping = ucs_malloc(sizeof(*mapping) + |
Contributor
There was a problem hiding this comment.
for the next PR, we can alloca if batch is not big
Comment on lines
+337
to
+344
| #if CUDA_VERSION >= 13000 | ||
| if (cuda_ipc_event->sgl_mapping != NULL) { | ||
| uct_cuda_ipc_sgl_mapping_destroy(cuda_ipc_event->sgl_mapping, | ||
| cuda_ipc_event->cuda_device, | ||
| iface->config.enable_cache); | ||
| return; | ||
| } | ||
| #endif |
Contributor
There was a problem hiding this comment.
for the next PR, we can keep CUDA > 13 macro guard only inside uct_cuda_ipc_sgl_mapping_destroy and cuda_ipc_event->sgl_mapping will always be NULL for older cuda
This would reduce number of ifdefs thru the code a bit
|
|
||
| _UCT_INSTANTIATE_TEST_CASE(test_cuda_ipc_rma_device, cuda_ipc) | ||
|
|
||
| class test_cuda_ipc_put_sgl : public test_cuda_ipc_rma { |
Contributor
There was a problem hiding this comment.
next PRs, we can make it generic UCT test for all tls supporting put_SGL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Implement
uct_ep_put_sgl_zcopyfor thecuda_ipctransport.Why?
Allow UCP SGL put offload to run over
cuda_ipc, replacing N separate per-buffer copies with a single batched submission and reducing per-element overhead on multi-buffer transfers.How?
On CUDA 13+, issue one
cuMemcpyBatchAsyncfor the whole SGL, with all mappings tracked in a per-eventuct_cuda_ipc_sgl_mapping_tand unmapped on completion. On older CUDA, fall back to per-elementuct_cuda_ipc_ep_put_zcopy, arming the user completion only on the last element.