[nbellalou]

What

Preserve endpoint-fence state when lane topology changes during failover or endpoint reconfiguration.

Why

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

An asynchronous fence can remain pending while the endpoint changes configuration. Before normal RMA/atomic traffic starts using the engine, it must be safe to reconcile the lanes to flush and to keep valid deferred work across that change.

How

• Mark tracked pre-fence lanes dirty only when their underlying lane endpoint changes.
• Normalize the recorded fence lane map before a strong flush; when topology is uncertain, conservatively include current live lanes.
• Preserve valid pending fence requests across endpoint reconfiguration and restart protocol requests on the current configuration when needed.
• Add dirty-lane lifecycle, normalization, and reconfiguration-purge coverage.

Draft — merge only after PR #11924 is merged.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

Can we confirm req->send.uct.func never frees the request when it returns UCS_INPROGRESS/UCS_ERR_NO_RESOURCE/UCP_STATUS_FENCE_DEFER? The batch loop and the subsequent requeue dereference req after the call, and the address_free annotation suggests the callback may free it.

[svc-nvidia-pr-review]

UCP_STATUS_FENCE_DEFER aliases a normal error code (-99), so UCS_STATUS_IS_ERR and UCS_PTR_IS_ERR both treat it as an error. Only ucp_ep_fence_dispatch_request special-cases it. Since it flows out of a uct_pending_callback_t, can we document/assert that no generic pending-return path can observe it?

[svc-nvidia-pr-review]

blocker: this PR is well over the 500-added-line limit and bundles several independent changes (lane-generation tracking, selective-lane flush, and the non-blocking fence pending-queue). Can we split it — e.g. the flush/lane-generation refactor separate from the async fence machinery — so each is reviewable on its own?

src/ucp/rma/flush.c

The non-blocking fence path (ucp_ep_fence_strong_nb, the pending queue, UCP_STATUS_FENCE_DEFER, UCP_REQUEST_FLAG_FENCE_BLOCKED) is only reached from gtest — ucp_ep_rma_handle_fence still uses the blocking ucp_ep_fence_strong. Is this intended to be wired in this PR, or is it dead code that should be deferred until there is a real caller?

Test coverage note: The new gtests are thorough for the lane-state helpers and the fence-queue mechanics, and the EXPECTED_SIZE update is consistent with the added ucp_ep_ext_t/ucp_request_t fields. However, because the async fence path is not wired to any real RMA/AMO flow, there is no end-to-end regression coverage for the feature it implements — the tests exercise the machinery directly with synthetic stack requests only.

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Tests althca on worker 0) · commit 90485ae1

TL;DR: shm_gga/test_ucp_fence64.atomic_add_fadd/1 aborted on ucs_assertv(!(ep->flags & UCP_EP_FLAG_BLOCK_FLUSH)) in ucp_ep_flush_progress() because this PR's new ucp_ep_fence_lane_map_normalize() expands unflushed_lanes to all live lanes when the topology is dirty, which forces the blocking ucp_ep_fence_strong() path to run from inside ucp_wireup_replay_pending_requests() — exactly where flush is blocked. Fix: use the non-blocking fence engine (ucp_ep_fence_strong_nb) / defer the request when UCP_EP_FLAG_BLOCK_FLUSH is set instead of calling ucp_ep_fence_strong() inline.

Full analysis

Summary: gtest shard 1 aborted with a UCS assertion failure in ucp_ep_flush_progress() during shm_gga/test_ucp_fence64.atomic_add_fadd/1 <shm,gga/ep_based>, failing the "althca on worker 0" job (make: *** [test] Aborted, bash exit code 2).

Root cause: From the backtrace in the build log, the call chain is:
ucp_worker_progress → ucp_wireup_eps_progress → ucp_wireup_replay_pending_requests (wireup.c:1260) → ucp_proto_request_restart → ucp_proto_amo_sw_progress (amo_sw.c:397) → ucp_ep_rma_handle_fence (rma.inl:188) → ucp_ep_fence_strong (flush.c:935) → ucp_ep_flush_lanes_internal → ucp_ep_flush_progress (flush.c:83 assert).

ucp_wireup_replay_pending_requests() sets UCP_EP_FLAG_BLOCK_FLUSH for the whole replay loop (src/ucp/wireup/wireup.c:1239-1244), so any flush started from a replayed request trips the assertion. This PR's head commit made that path reachable: ucp_ep_rma_handle_fence() now calls ucp_ep_fence_normalize_lanes() first (src/ucp/rma/rma.inl:178), and ucp_ep_fence_lane_map_normalize() returns all live lanes whenever fence_lanes_dirty is set (src/ucp/rma/rma.h:50-59). A pending-request replay happens precisely after a lane/topology change, i.e. when fence_lanes_dirty is set; the shm,gga EP has ≥2 live lanes (log shows sysv/memory am + knem/memory rma_bw), so ucs_is_pow2_or_zero(unflushed_lanes | lane_map) is false and the code takes the strong-fence branch (rma.inl:188) instead of the weak fence. ucp_ep_fence_strong() is synchronous — it calls ucp_flush_wait(), which re-enters ucp_worker_progress() from inside a callbackq dispatch — so besides the assertion it is a reentrancy hazard. Note the branch already contains a non-blocking engine (ucp_ep_fence_strong_nb, flush.c:977) that ucp_ep_rma_handle_fence does not use.

Implicated commit: [REDACTED:Hex High Entropy String] — "UCP/FENCE: Preserve fence state across lane changes", Nathan Bellalou (builds on [REDACTED:Hex High Entropy String] "Add asynchronous endpoint fence engine" and acded49 "topology-safe selective endpoint flush")

File: src/ucp/rma/rma.inl:178-189 (with src/ucp/rma/rma.h:50-59 and the assertion at src/ucp/rma/flush.c:83; blocking path src/ucp/rma/flush.c:924-949)

Suggested fix:

1. Stop starting a blocking fence from a request-progress context. In ucp_ep_rma_handle_fence(), replace the ucp_ep_fence_strong(ep) call with the already-available asynchronous engine: call ucp_ep_fence_strong_nb(ep, ep->worker->fence_seq) and return UCS_ERR_NO_RESOURCE so the AMO/RMA request is re-queued as pending and resumed by ucp_ep_fence_pending_resume() when the fence flush completes. This also removes the illegal recursive ucp_worker_progress() from ucp_flush_wait().
2. Add an explicit guard for the replay window: if ep->flags & UCP_EP_FLAG_BLOCK_FLUSH, do not start any fence flush — return UCS_ERR_NO_RESOURCE (or queue on the fence pending list) so the operation is retried after ucp_wireup_replay_pending_requests() clears the flag.
3. Reduce the blast radius of the normalization: only widen unflushed_lanes to all live lanes when the recorded lane actually vanished, and keep the weak-fence fast path when the tracked work and the new operation land on a single live lane, so a dirty topology doesn't unconditionally escalate every replayed AMO to a strong fence.
4. Add a regression test that triggers a lane reconfiguration with pending AMO SW requests while UCX_FENCE_MODE=ep_based on a multi-lane shm config.

Related: PR #11925 (this PR), PR #11871 "UCP/RMA: Make EP-based fence non-blocking and failover-safe"

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 64607204-9abe-4617-a122-b786df8655e2 in the triage console for the audit trail.

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Coverity coverity devel on coverity_rh7) · commit acded494

TL;DR: The Coverity devel job failed on one new defect (FORWARD_NULL) in the newly added gtest test_ucp_fence32.replace_lane_during_selective_flush: test_flush_comp is initialized to NULL and then passed to uct_invoke_completion() (which dereferences it) with only non-fatal EXPECT_* checks in between. Guard the pointer with a fatal assertion (or explicit NULL check) before calling uct_invoke_completion().

Full analysis

Summary: Azure Pipelines "Coverity coverity devel on coverity_rh7" step aborted with ##[error]Coverity found 1 issues: — the build/analysis itself succeeded (739 TUs, 0 build errors), but the gate nerrors=1 > 0 failed the job.

Root cause: Coverity reported an explicit null dereference at test/gtest/ucp/test_ucp_fence.cc:353. The reported path is:

• line 339: test_flush_comp = NULL; (assign_zero)
• line 345: ucp_ep_flush_lanes_internal(...) is called; the mock test_flush_inprogress() (line 93-102) is what sets test_flush_comp = comp
• lines 348-350: the checks that the flush actually happened use non-fatal EXPECT_TRUE / EXPECT_EQ, so execution continues even if the mock was never invoked
• line 353: uct_invoke_completion(test_flush_comp, UCS_OK) → uct_iface.h:1009 directly dereferences comp in the trace log statement (deref_parm)

So on the path where the mock never fires, a NULL uct_completion_t* is dereferenced. This is a genuine new-code finding from the PR under test, not infrastructure flakiness — this test was added by the commit being built.

Implicated commit: [REDACTED:Hex High Entropy String] — "UCP/FLUSH: Add topology-safe selective endpoint flush", Nathan Bellalou (the commit under test; it is the only recent change to this file).

File: test/gtest/ucp/test_ucp_fence.cc:353 (and the same pattern at line 358); dereference site src/uct/base/uct_iface.h:1009

Suggested fix: Make the precondition fatal so no path reaches uct_invoke_completion() with NULL, and restore state before bailing out (an ASSERT_* returns from the test and would otherwise leave the worker critical section held and iface->ops.ep_flush patched):

    request = ucp_ep_flush_lanes_internal(
            ep, 0, &param, NULL, test_flush_completion,
            "replace_lane_during_flush", UCT_FLUSH_FLAG_LOCAL, UCS_BIT(0));
    if (!UCS_PTR_IS_PTR(request) || (test_flush_comp == NULL)) {
        ucp_ep_set_lane(ep, 0, original_lane);
        iface->ops.ep_flush = flush_func;
        UCP_WORKER_THREAD_CS_EXIT_CONDITIONAL(ep->worker);
        UCS_TEST_ABORT("selective flush did not start, comp=" <<
                       test_flush_comp);
    }
    EXPECT_EQ(1u, test_flush_call_count);
    EXPECT_EQ(original_lane, test_flush_eps[0]);

Apply the same guard before the second uct_invoke_completion(test_flush_comp, UCS_OK) at line 358 (the mock re-sets test_flush_comp on the second call, so it must be re-validated). This both silences the Coverity FORWARD_NULL and turns a would-be segfault in a broken-mock scenario into a clean test failure.

Related: PR #11925 (branch fence-split-1-selective-flush); Coverity report published as build artifact coverity_devel (cov_build_devel/output/errors/index.html) for build 134988.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id b7e0b4cd-2054-48f9-b693-6ed9d4138e8f in the triage console for the audit trail.