Repository navigation
UCT/CUDA/COPY: Qualify cuda_copy for RNDV with peer-failure EH #11319
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
Changes from 24 commits
7e53a1f
b020c4f
a02e613
5b6552d
29e6fb6
c64e5c8
7650e78
7c04647
6e39794
f1f8121
fe8d447
c040464
2b51f27
4db34a7
f73b217
6b34770
5ea2356
ef3eef8
5eda833
9b4825f
f9e0c0f
5f041ec
00e37ec
13b658e
85bf24e
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 |
|---|---|---|
|
|
@@ -390,6 +390,15 @@ static ucp_md_map_t ucp_request_get_invalidation_map(ucp_ep_h ep) | |
| lane = key->rma_bw_lanes[i]; | ||
|
|
||
| if (!ucp_ep_is_lane_p2p(ep, lane)) { | ||
| /* Same-worker PEER selection may include MDs without RMA | ||
| * invalidation, while capable MDs still need it. */ | ||
| if ((key->flags & UCP_EP_CONFIG_KEY_FLAG_SELF) && | ||
|
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. maybe check only
Contributor
Author
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'm keeping the SELF/PEER guard because lane selection relaxes INVALIDATE_RMA only for that case; FAILOVER was explicitly kept under full requirements. Removing it would silently skip invalidation if an unsupported lane appeared on a cross-worker or FAILOVER endpoint. If selection policy changes later, both paths should change together. |
||
| (key->err_mode == UCP_ERR_HANDLING_MODE_PEER) && | ||
| !(ucp_ep_md_attr(ep, lane)->flags & | ||
| UCT_MD_FLAG_INVALIDATE_RMA)) { | ||
| continue; | ||
| } | ||
|
|
||
| ucs_assert(ucp_ep_get_iface_attr(ep, lane)->cap.flags & | ||
| UCT_IFACE_FLAG_GET_ZCOPY); | ||
| ucs_assert(ucp_ep_md_attr(ep, lane)->flags & | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2200,6 +2200,7 @@ ucp_wireup_try_select_lanes(ucp_ep_h ep, unsigned ep_init_flags, | |
| unsigned *addr_indices, ucp_ep_config_key_t *key, | ||
| ucp_rsc_index_t *dst_md_storage) | ||
| { | ||
| unsigned select_ep_init_flags; | ||
| ucs_status_t status; | ||
|
|
||
| ucp_ep_config_key_reset(key); | ||
|
|
@@ -2208,7 +2209,14 @@ ucp_wireup_try_select_lanes(ucp_ep_h ep, unsigned ep_init_flags, | |
| key->dst_version = remote_address->dst_version; | ||
| key->dst_md_cmpts = dst_md_storage; | ||
|
|
||
| status = ucp_wireup_select_lanes(ep, ep_init_flags, *tl_bitmap, | ||
| select_ep_init_flags = ep_init_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. this repeats the
Contributor
Author
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. Not moving the relaxation into |
||
| if ((key->err_mode == UCP_ERR_HANDLING_MODE_PEER) && | ||
| (remote_address->uuid == ep->worker->uuid)) { | ||
| /* Same-worker PEER EPs do not require peer-failure transports. */ | ||
| select_ep_init_flags &= ~UCP_EP_INIT_ERR_MODE_PEER_FAILURE; | ||
| } | ||
|
|
||
| status = ucp_wireup_select_lanes(ep, select_ep_init_flags, *tl_bitmap, | ||
|
rakhmets marked this conversation as resolved.
Resolved
|
||
| remote_address, addr_indices, key, 1); | ||
| if (status != UCS_OK) { | ||
| return status; | ||
|
|
||
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.
minor: can we key the skip on the MD capability instead of re-deriving the wireup relaxation here? It keeps the check tied to the precondition of the assert below, and
UCP_EP_CONFIG_KEY_FLAG_SELFis also set for the CM intermediate config, which is selected against the local worker address (wireup_cm.c:190), so it is not by itself a reliable "same worker" indicator.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.
The preliminary CM key can be marked SELF, but it selects lanes with the original peer-failure flags, so with RNDV enabled it still requires INVALIDATE_RMA. The proposed check would not distinguish that key and would also skip unsupported FAILOVER lanes. I’m keeping the exception limited to SELF+PEER, the case whose selection requirements this PR relaxes.