Skip to content

UCP/RMA: Activate non-blocking endpoint fences - #11926

Draft
nbellalou wants to merge 4 commits into
openucx:masterfrom
nbellalou:fence-split-4-nonblocking-fence
Draft

nbellalou wants to merge 4 commits into
openucx:masterfrom
nbellalou:fence-split-4-nonblocking-fence

Conversation

@nbellalou

@nbellalou nbellalou commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

What

Activate non-blocking endpoint fences for ordinary RMA and atomic requests.

Why

This is Part 4 of 5 of an updated split of the earlier endpoint-fence PR #11199.

Parts 1–3 provide the selective flush, asynchronous lifecycle, and topology safety needed to change the normal request path. This PR is the first one that changes ordinary RMA/atomic behavior: a request that is behind a strong endpoint fence is deferred and resumed asynchronously instead of blocking progress.

How

  • Route RMA and atomic fence admission through the asynchronous endpoint-fence engine when a strong fence is required.
  • Capture each request's fence epoch and queue a newly deferred request even before it owns a UCT lane.
  • Keep the existing cheap weak-fence fast path when one lane is sufficient.
  • Make endpoint and worker flush wait for relevant queued/in-flight fence work, without a fence flush waiting for its own triggering request.
  • Add coverage for the fresh-defer path and the activated fence flow.

RMA rendezvous ordering is completed by Part 5.

Draft — merge only after PR #11925 is merged.

Allow internal callers to flush a selected set of endpoint lanes while preserving the existing all-lane flush behavior. Track lane replacement generations so an in-flight flush safely reconciles failover topology changes.

Overlay the new lane-selection snapshot with the later memory-flush phase, keeping ucp_request_t at its existing release size.

Signed-off-by: Nathan Bellalou <nbellalou@nvidia.com>
Add an epoch-ordered per-endpoint queue and progress callback that can advance a fence with a selective remote flush before retrying blocked work. Track the in-flight flush explicitly and purge queued requests on terminal errors.

Overlay the new fence metadata with the existing RMA and atomic request storage through an anonymous union. This preserves the established send.rma and send.amo field names and limits the request size increase to eight bytes.

Signed-off-by: Nathan Bellalou <nbellalou@nvidia.com>
Track lane replacements that affect pre-fence work and conservatively normalize the fence lane mask before flushing. Keep queued fence requests across endpoint reconfiguration and restart protocol requests on the current endpoint configuration.

Signed-off-by: Nathan Bellalou <nbellalou@nvidia.com>
Gate RMA and atomic request admission on the endpoint fence epoch without progressing UCT recursively. Queue blocked requests on the endpoint, make endpoint and worker flush wait for relevant fence work, and cover fresh requests that defer before selecting a transport lane.

Signed-off-by: Nathan Bellalou <nbellalou@nvidia.com>
@svc-nvidia-pr-review

Copy link
Copy Markdown

🤖 Starting review — findings will be posted here when done.

#define ucp_trace_req(_sreq, _message, ...) \
ucs_trace_req("req %p: " _message, (_sreq), ## __VA_ARGS__)

#define UCP_STATUS_FENCE_DEFER ((ucs_status_t)(UCS_ERR_LAST + 1))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new UCP_STATUS_FENCE_DEFER static assertion open-codes a #ifdef __cplusplus / static_assert / _Static_assert split. The project already provides UCS_STATIC_ASSERT(_cond) (in ucs/sys/compiler_def.h) which compiles in both C and C++. Suggested: use UCS_STATIC_ASSERT((UCP_STATUS_FENCE_DEFER < UCS_OK) && (UCP_STATUS_FENCE_DEFER >= UCS_ERR_LAST)).

#define ucp_trace_req(_sreq, _message, ...) \
ucs_trace_req("req %p: " _message, (_sreq), ## __VA_ARGS__)

#define UCP_STATUS_FENCE_DEFER ((ucs_status_t)(UCS_ERR_LAST + 1))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UCP_STATUS_FENCE_DEFER = -99, a value in the reserved negative ucs_status_t range that satisfies UCS_STATUS_IS_ERR(). It flows out of proto-progress functions into ucp_request_try_send. This works today only because every generic error path either special-cases it first or is unreachable for these requests. Consider adding a short comment at the definition documenting that any generic UCS_STATUS_IS_ERR handler on the RMA/AMO progress path must special-case it, so a future status-range change or new caller does not silently treat a fence-defer as a fatal/abort error.

@svc-nvidia-pr-review

Copy link
Copy Markdown

PR exceeds the hard size limit: REVIEW.md sets a hard limit of 500 added lines, but this PR adds well over 1,000 lines. The change also bundles several separable pieces: (1) the amo/rma request-union memory rework, (2) the lane-generation / selective-lane flush machinery, and (3) the deferred fence queue + async strong fence. Please split into reviewable PRs (e.g., the request-union refactor and the selective-flush lane-state helpers can land independently of the fence-queue feature).

@svc-ucx

svc-ucx commented Sep 8, 2026

Copy link
Copy Markdown

🤖 CI Triage Agent — UCX PR (Tests BlueField on worker 0) · commit bd6dcbc5

TL;DR: shm_rc_dc/test_ucp_rma_order.put_ordering/1 <self,shm,rc,dc/ep_based> aborted on ucs_assertv(!req->send.flush.started_lanes) in ucp_ep_flush_request_resched(); the new selective (lane-masked) fence flush marks lanes that are excluded by lanes.lane_mask as "started", so the old assertion — which assumes started_lanes != 0 means a UCT flush was really issued before wireup completed — now fires spuriously. Fix: don't fold mask-excluded lanes into started_lanes (or qualify the assertion with & req->send.flush.lanes.lane_mask).

Full analysis

Summary: BlueField gtest run crashed (core dumped, make: *** [Makefile:4713: test] Aborted) in shm_rc_dc/test_ucp_rma_order.put_ordering/1 with flush.c:349 Assertion '!req->send.flush.started_lanes' failed: req=0xaaaae16d2300 flush started_lanes=0x1.

Root cause: The backtrace shows the assertion is hit while wireup replays a pending flush request on a blocked EP: ucp_wireup_replay_pending_requests() → ucp_request_send() → ucp_ep_flush_progress_pending() (flush.c:392) → ucp_ep_flush_request_resched() (flush.c:349). In ucp_ep_flush_request_resched() the UCP_EP_FLAG_BLOCK_FLUSH branch asserts that no lane has been started yet, because a flush submitted before wireup completes can only ever get UCS_ERR_NO_RESOURCE from the wireup EPs. That invariant was broken by this PR's selective/lane-masked flush: in ucp_ep_flush_progress() (src/ucp/rma/flush.c:151-155) lanes that are not in req->send.flush.lanes.lane_mask are accounted for via ucp_ep_flush_request_update_uct_comp(req, -1, UCS_BIT(lane)), which ORs the bit into started_lanes even though uct_ep_flush() was never called on that lane. The EP-based fence path (ucp_ep_fence_strong_nb(), flush.c:1052-1057, via ucp_ep_rma_handle_fence() in src/ucp/rma/rma.inl:225-241) always submits such a masked flush (lane_mask = ep->ext->unflushed_lanes), and the test runs with FENCE_MODE=ep_based, MAX_RMA_LANES=2 over self,shm,rc,dc, so lane 0 is excluded → started_lanes=0x1 (exactly the reported value, a single non-RMA lane bit). Once the EP enters BLOCK_FLUSH (wireup/lane reconfig) and the still-pending fence flush is replayed, the stale assertion aborts. This is a code/assertion inconsistency introduced by the PR, not flaky hardware.

Implicated commit: bd6dcbc "UCP/RMA: Activate non-blocking endpoint fences" (Nathan Bellalou) — exposes the bug introduced by acded49 "UCP/FLUSH: Add topology-safe selective endpoint flush" (same author/series)

File: src/ucp/rma/flush.c:349 (assertion) and src/ucp/rma/flush.c:151-155 (started_lanes polluted by mask-excluded lanes)

Suggested fix: Make started_lanes mean "flush actually started on this lane" again, or teach the resched assertions about the lane mask. Concretely, either:

  • keep a separate mask (e.g. req->send.flush.skipped_lanes) for lanes decremented because they are outside lanes.lane_mask / have uct_ep == NULL, and use started_lanes | skipped_lanes only in the loop condition at flush.c:146; or
  • minimally, relax the checks in ucp_ep_flush_request_resched() to consider only requested lanes:
    ucs_assertv(!(req->send.flush.started_lanes & req->send.flush.lanes.lane_mask), ...) and likewise mask req->send.flush.started_lanes & ucp_ep_config(ep)->p2p_lanes & lane_mask in the else-branch (flush.c:353-366).

Add a regression variant that triggers ucp_ep_fence_strong_nb() while the EP still has unconnected p2p/wireup lanes (the test_ucp_rma_order ep_based variant with MAX_RMA_LANES=2 already reproduces it).

Related: PR #11926 (branch fence-split-4-nonblocking-fence); no existing issue found for this assertion.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants