Repository navigation
UCP/RNDV: Skip PUT fence for ordered transports - #11673
Conversation
a633734 to
7b128fe
Compare
7b128fe to
6dbc025
Compare
|
@Artemy-Mellanox @roiedanino please converge on the review, so that we can proceed with this one. Thanks! |
129f474 to
caabfc2
Compare
UCP rendezvous PUT sends ATP on the same data lanes, where receive completion preserves PUT/ATP ordering. Remove the redundant fence so it cannot remain pending until an unrelated PUT. Keep STRONG_ORDER for fenced RDMA writes when DDP ordering requires it, and use the regular PCI-atomic fence flag for non-PUT operations. Signed-off-by: Roie Danino <rdanino@nvidia.com>
caabfc2 to
5960f15
Compare
Signed-off-by: Roie Danino <rdanino@nvidia.com>
roiedanino
changed the title
|
@Artemy-Mellanox can prioritize? it's urgent for 1.22 release |
@yosefe can you take a look - fence is really redundant? how PUT/SEND work with OOO All ordering semantics? |
Confirmed for CX‑8 OOO_ALL: SEND may arrive/scatter OOO, but the responder CQE is withheld until preceding messages, including RDMA WRITE data placement, complete. UCX processes ATP only from that RX CQE, so the same-lane PUT is complete before ATP is observed. The explicit fence is therefore redundant for PUT→SEND; strong ordering is still required for fenced WRITE→WRITE. |
|
@gleon99 can approve? |
yosefe
left a comment
yosefe
left a comment
There was a problem hiding this comment.
Seems this PR is making UCP layer assume that C+UCT transport is IB with DDP, which is not always the case
This change is indeed too broad. Although it is safe for all IB, it isn't for SRD. Maybe introduce UCT ordering cap? @yosefe @Artemy-Mellanox |
…ce-fix-master Signed-off-by: Roie Danino <rdanino@nvidia.com>
Signed-off-by: Roie Danino <rdanino@nvidia.com>
roiedanino
changed the title
|
I added a UCT capability flag for advertising the ordering guarantee of AM after PUT, advertised only by RC transport, so the fence will still be used by other transports Can you please review again? |
| return (req->flags & UCP_REQUEST_FLAG_PROTO_SEND) && | ||
| (req->send.proto_stage == | ||
| UCP_PROTO_RNDV_PUT_STAGE_FENCED_ATP); | ||
| UCP_PROTO_RNDV_PUT_STAGE_ATP); |
There was a problem hiding this comment.
maybe add srd TLS for full coverage?
Signed-off-by: Roie Danino <rdanino@nvidia.com>
Signed-off-by: Roie Danino <rdanino@nvidia.com>
Add UCT_IFACE_FLAG_PUT_AM_ORDER and select direct ATP only when every RNDV data lane provides this capability. Preserve the fenced fallback for SRD and mixed ordered/unordered lanes. Add protocol coverage for the ordered path, unordered fallback, and explicit SRD transport. --------- Signed-off-by: Roie Danino <rdanino@nvidia.com> (cherry picked from commit 9a06403)
) UCP/RNDV: Skip PUT fence for ordered transports (#11673) Add UCT_IFACE_FLAG_PUT_AM_ORDER and select direct ATP only when every RNDV data lane provides this capability. Preserve the fenced fallback for SRD and mixed ordered/unordered lanes. Add protocol coverage for the ordered path, unordered fallback, and explicit SRD transport. --------- (cherry picked from commit 9a06403) Signed-off-by: Roie Danino <rdanino@nvidia.com>
What?
Skip the redundant rendezvous PUT fence before ATP only when every data lane advertises PUT-to-AM ordering.
Why?
PR #10658 enabled
STRONG_ORDERfor fenced RDMA writes when DDP ordering is enabled. On ordered IB transports, ATP receive completion is observed only after preceding PUT data placement, so the explicit fence is unnecessary. If that fence remains pending, a later unrelated PUT can inheritSTRONG_ORDER.Other transports, such as SRD, may require the fence, so UCP must not assume IB ordering semantics globally.
How?
UCT_IFACE_FLAG_PUT_AM_ORDERtransport capability.Fixes Redmine #5125813