[michal-shalev]

What?

Add SGL datatype support to ucp_get_nbx: new get/sgl/offload and get/sgl/offload_sw protocols, param validation, and gtests (SGL tests now run for both put and get).

Why?

Completes SGL RMA at the UCP level. UCT get_sgl_zcopy was added in #11763, and put SGL already exists.

How?

Mirrors the put SGL protocols.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

Not raised (checked and fine):

• ucp_get_nbx now uses UCP_REQUEST_CHECK_PARAM_COMMON plus UCP_RMA_SGL_CHECK_PARAMS, matching ucp_put_nbx; send.rma.sgl.* is set only for the SGL path and completed_size is still initialized.
• reset via ucp_proto_get_offload_zcopy_reset rewinds the SGL iterator (offset and frag_offset), which is required for the idempotent GET replay.
• get/sgl/offload_sw does get exercised on transports without UCT_IFACE_FLAG_V2_GET_SGL_ZCOPY (self/shm), so the new sw path is not left untested.
• Residual coverage gap: no test forces PROTOS=get/sgl/offload_sw, so on IB-only runs (rc_mlx5 always advertises the v2 get-SGL cap) only the hardware path is covered; and the get variant has no emulation counterpart to put_emulation, since there is no get/sgl/am protocol.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

the diff adds ~600 lines, above the 500-line limit. can the mechanical part — extracting rma_sgl.inl out of put_offload.c and renaming UCP_PUT_SGL_CHECK* to UCP_RMA_SGL_CHECK* — go in a separate refactor-only PR, so this one only adds the get/sgl protocols and their tests?

Checked and not raised:

• ucp_get_nbx switching to UCP_REQUEST_CHECK_PARAM_COMMON + UCP_RMA_SGL_CHECK_PARAMS mirrors ucp_put_nbx; send.state.completed_size = 0 is preserved and send.rma.sgl.* is written only on the SGL path (it does not overlap rma.rkey/remote_addr in the union).
• min_sgl_frag is zero-initialized for every lane before the conditional, so put/sgl/offload keeps its previous length > 0 semantics via ucs_max(min_frag_length, 1).
• Dropping UCP_PROTO_COMMON_INIT_FLAG_MIN_FRAG and setting min_frag_offs = UCP_PROTO_COMMON_OFFSET_INVALID is required here: perf->min_length would otherwise be a byte count compared against the SGL element count.
• frag_offset read in the adjust helper is the current fragment's offset (ucp_datatype_iter_next_sgl_frags writes only next_iter), and a non-zero frag_offset implies the returned descriptor belongs to the same element, so the backward overlap stays inside the element's memh/rkey.
• ucp_proto_get_offload_zcopy_reset → ucp_proto_offload_zcopy_reset rewinds offset and type.sgl.frag_offset, which the idempotent GET replay needs.
• Returning UCS_ERR_INVALID_PARAM from the send func goes through ucp_proto_multi_handle_send_error → ucp_proto_request_abort → ucp_proto_request_zcopy_abort, so in-flight fragments still hold completion refs and the memh is dereg'd.
• The new test's lane loop follows the existing entity::has_lane_with_caps() idiom.

Residual coverage gaps:

• No test forces PROTOS=get/sgl/offload_sw, so on IB-only runs (where rc_mlx5 advertises UCT_IFACE_FLAG_V2_GET_SGL_ZCOPY) only the hardware path is exercised; the sw path is covered incidentally via self/shm.
• No test covers max_frag < min_sgl_frag (the case in the first comment above).

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

Residual coverage gap: no test forces PROTOS=get/sgl/offload_sw; on IB-only runs (rc_mlx5 advertises UCT_IFACE_FLAG_V2_GET_SGL_ZCOPY) the sw path is only hit incidentally via self/shm.

Residual coverage gap: no test covers max_frag < min_sgl_frag (e.g. RMA_ZCOPY_MAX_SEG_SIZE below cap.get.min_zcopy), where ucs_assertv(diff <= frag_offset) in ucp_proto_get_sgl_adjust_min_frag would fire — already raised in the previous round.

Residual coverage gap: the test_ucp_rma_sgl_err tests skip non-host memory because a short GPU get goes through rendezvous and asserts on error; that path stays untested.

[svc-ucx]

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

TL;DR: The newly added gtest test_ucp_rma_sgl.short_element_sync_failure forces PROTOS=get/sgl/offload (an mlx5-only HW SGL-get protocol) without checking that the hardware supports it, so it fails on the althca worker, which has no mlx5 device; the test needs the same availability guard that split_between_lanes uses.

Full analysis

Summary: 1 of 4108 gtests failed — all/test_ucp_rma_sgl.short_element_sync_failure/1 <all/host/get> — causing make: *** [test] Error 1 in "Run ./contrib/test_jenkins.sh" on the althca worker.

Root cause: The test (added in the PR's top commit) runs with "PROTOS=get/sgl/offload,reconfig", i.e. it allows only the hardware SGL-get protocol, and then relies on two mlx5-specific properties:

1. get/sgl/offload is probed with first.tl_v2_cap_flags = UCT_IFACE_FLAG_V2_GET_SGL_ZCOPY (src/ucp/rma/get_offload.c:280-285), a capability implemented only in uct/ib/mlx5/rc (commit 5122423, "UCT/IB/MLX5/RC: Implement get_sgl_zcopy"). The althca worker has no mlx5 interface — the log shows all dc_mlx5 suites [ SKIP ] (No interface for testing) — so with rc_verbs no lane advertises this cap and no protocol can be selected for an SGL get. The very first statement of the test, test_sgl(1, UCS_KBYTE) (test_ucp_rma.cc:1911), then hits ASSERT_UCS_OK(UCS_PTR_STATUS(sptr)) at test_ucp_rma.cc:1524 with the error returned by ucp_get_nbx.
2. Even if the offload proto were selected, the "short element" path only triggers when length < lpriv->min_sgl_frag, which is cap.get.min_zcopy (src/ucp/proto/proto_multi.c:879-885) — ~65 only on mlx5 (inline-response limit). On verbs it is 0/1, so the 8-byte element would be posted in the normal SGL batch, no sub-request would be created, the parent would complete with UCS_OK, and EXPECT_NE(UCS_OK, cb.status) / the "elements not posted" loop (test_ucp_rma.cc:1934-1940) would fail.

The sibling tests are guarded and therefore pass everywhere: split_between_lanes skips via offload_proto_selected(), short_elements_get_sw only excludes the offload proto (PROTOS=^get/sgl/offload), and the generic test_sgl skips when no GET_ZCOPY lane exists (which is why the tcp/... instantiation was skipped rather than failed). test_ucp_rma_sgl is instantiated with UCP_INSTANTIATE_TEST_CASE_TLS(test_ucp_rma_sgl, all, "all") (test_ucp_rma.cc:2269), so the test runs on non-mlx5 hardware.

Implicated commit: [REDACTED:Hex High Entropy String] — "UCP/TEST: Add GET SGL short element layout and failure tests", Michal Shalev (feature it tests: 1310a95, same author)

File: test/gtest/ucp/test_ucp_rma.cc:1905-1947 (test body), test_ucp_rma.cc:1911 + :1524 (first failing assert); related: src/ucp/rma/get_offload.c:280-285, src/ucp/proto/proto_multi.c:879-885

Suggested fix: Guard short_element_sync_failure on actual HW support before any assertion, e.g.:

• Add a skip condition for a lane advertising UCT_IFACE_FLAG_V2_GET_SGL_ZCOPY and cap.get.min_zcopy > 8 (the element size used), similar to test_sgl's has_lane_with_caps() check; or
• Post the wireup SGL op first and reuse the existing helper: if !offload_proto_selected(sptr) → request_wait + UCS_TEST_SKIP_R("SGL offload protocol was not selected"), exactly as split_between_lanes (test_ucp_rma.cc:1715-1719) does; or
• Move this test into a sub-class instantiated only for mlx5 TLs (rc_x/dc_x) instead of "all".

Also make the leading test_sgl(1, UCS_KBYTE) wireup call tolerant of the restricted-PROTOS case so it skips instead of asserting.

Related: PR #12191 (this branch, ucp-get-sgl); prerequisite commits 1310a95, 83c09db, e66f515; UCT capability from "UCT/IB/MLX5/RC: Implement get_sgl_zcopy (#11763)"

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 2c2d0631-767b-4d50-a6ef-aaa58163d8ba in the triage console for the audit trail.

[svc-nvidia-pr-review]

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

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Coverity coverity release on coverity_rh7) · commit 36d2668b

TL;DR: The Coverity gate failed with 4 new FORWARD_NULL defects caused by the new static callback ucp_proto_get_sgl_short_elem_completion() in this PR, which dereferences user_data unconditionally; Coverity's function-pointer model then resolves param->cb.send in the legacy ucp_tag_send_nb/ucp_tag_send_nbr/ucp_tag_send_sync_nb/ucp_stream_send_nb wrappers (where param.user_data is implicitly NULL) to it. Add a ucs_assert(user_data != NULL) (or explicit NULL guard) at the top of the callback to break the false path.

Full analysis

Summary: coverity release on coverity_rh7 exited with code 4 — cov-format-errors reported 4 new "Explicit null dereferenced (FORWARD_NULL)" issues, so the pipeline step failed (##[error]Coverity found 4 issues).

Root cause: Not a build/test failure — a static-analysis regression introduced by this PR. src/ucp/rma/get_offload.c:290-298 adds:

static void ucp_proto_get_sgl_short_elem_completion(void *request, ucs_status_t status, void *user_data)
{
    ucp_request_t *req = user_data;
    ucp_request_free(request);
    ucp_invoke_uct_completion(&req->send.state.uct_comp, status);  /* unconditional deref */
}

It is assigned to param.cb.send (get_offload.c:308) with user_data = req. Because cov-analyze runs with --enable-fnptr --fnptr-models, Coverity adds this new function as a resolution target for every indirect param->cb.send invocation — specifically ucp_request_imm_cmpl_param() at src/ucp/proto/proto_common.inl:283. The legacy wrappers build a ucp_request_param_t on the stack with a callback but no UCP_OP_ATTR_FIELD_USER_DATA, so param.user_data is zero-initialized to NULL:

• src/ucp/tag/tag_send.c:184 (ucp_tag_send_nb) → defect at :190
• src/ucp/tag/tag_send.c:198 (ucp_tag_send_nbr) → defect at :206
• src/ucp/tag/tag_send.c:222 (ucp_tag_send_sync_nb) → defect at :228
• src/ucp/stream/stream_send.c:99 (ucp_stream_send_nb) → defect at :108

Coverity therefore reports NULL user_data flowing into the new callback and being dereferenced at src/ucp/core/ucp_request.inl:1004 (ucp_invoke_uct_completion → comp). This is a false positive at runtime (those paths never dispatch to the SGL callback), but it is a hard CI gate.

Implicated commit: PR #12191 branch ucp-get-sgl, HEAD [REDACTED:Hex High Entropy String] (Michal Shalev); the SGL short-element code is touched by a33a3ff3, 7707911e, 3cc34f95 (all Michal Shalev, 2026-10-28).

File: src/ucp/rma/get_offload.c:290-298 (deref at :294/:297); reported call sites src/ucp/tag/tag_send.c:190,206,228 and src/ucp/stream/stream_send.c:108.

Suggested fix: Make the NULL impossibility explicit so Coverity prunes the modeled path. ucs_assert is compiled in under __COVERITY__ (see src/ucs/debug/assert.h:54), so it is both free in release builds and visible to the analyzer:

static void ucp_proto_get_sgl_short_elem_completion(void *request, ucs_status_t status, void *user_data)
{
    ucp_request_t *req = user_data;

    ucs_assert(req != NULL);
    ucp_request_free(request);
    ucp_invoke_uct_completion(&req->send.state.uct_comp, status);
}

Alternatives if the assert is not sufficient: (a) annotate the modeled call with /* coverity[var_deref_model] */ next to ucp_request_imm_cmpl_param() in src/ucp/proto/proto_common.inl:283 (same style as the existing /* coverity[offset_free] */ on line 282) — though this suppresses broadly; or (b) explicitly set .user_data = NULL is already the case, so instead set UCP_OP_ATTR_FIELD_USER_DATA is not applicable — prefer the assert in the new callback since it is the code this PR added.

Related: PR #12191 (this PR). No pre-existing issue found for this Coverity signature.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 95e9e4a6-4e8a-413f-b5df-cb716016555e in the triage console for the audit trail.

[svc-nvidia-pr-review]

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

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Tests roce on worker 1) · commit 54d5e142

TL;DR: The new gtest all/test_ucp_rma_sgl.short_and_long_elements aborts in flush_ep() on ucs_assertv(comp->count > 0) — a UCT RC flush op completed against a UCP ep-flush request whose completion counter was already 0, i.e. a UCP uct_completion_t is being released more times than it was referenced (stale/over-released completion) by the brand-new "short GET-SGL element via nested sub-request" path. Fix the reference accounting in ucp_proto_get_sgl_short_elem() (and make the parent request un-completable/un-abortable while sub-requests are outstanding) rather than touching the flush code.

Full analysis

Summary: roce on worker 1 gtest job aborted (core dumped) during all/test_ucp_rma_sgl.short_and_long_elements/1 <all/host/get>: uct_iface.h:1012 Assertion 'comp->count > 0' failed: comp=0x55e0e0d75498 (ucp_ep_flush_completion) count=0 status=0, make: *** [Makefile:4971: test] Aborted (core dumped).

Root cause: From the backtrace, the completion was invoked by uct_rc_ep_flush_op_completion_handler() (rc_ep.c:371) → uct_invoke_completion() while handling a TX CQE, on the completion of a UCP endpoint-flush request (ucp_ep_flush_completion) that had already reached count == 0. The crash site is ucp_test::flush_ep() → test_ucp_rma_sgl::test_sgl() (test_ucp_rma.cc:1541) called from short_and_long_elements (test_ucp_rma.cc:1769). UCP's own flush bookkeeping is one-completion-per-started-lane (ucp_ep_flush_request_update_uct_comp(), flush.c:21-48, 185-188) and a duplicate flush on the same lane would have tripped the started_lanes assertion first — so the extra decrement comes from a completion reference that outlived its owning request (the request slot was then recycled from the worker request mpool by the subsequent ucp_ep_flush_nbx).

The only code exercised exclusively by this test is the new short-element path: short_and_long_elements mixes elements below the iface cap.get.min_zcopy (1, 7, 3, 64 bytes) with normal ones, so it is the only test that drives ucp_proto_get_sgl_adjust_min_frag() → ucp_proto_get_sgl_short_elem() interleaved with real uct_ep_get_sgl_zcopy() posts (short_elements/many_short_elements use only short elements, multi_rail/with_callback only long ones — all of those passed in the same run). That function hand-rolls the parent's completion reference count outside the proto_multi framework:

• ++req->send.state.uct_comp.count before a nested ucp_get_nbx() on the same endpoint, issued from inside the parent request's own progress/send callback (get_offload.c:316-318);
• --req->send.state.uct_comp.count on the non-pointer return path (get_offload.c:325);
• the matching release happens later in ucp_proto_get_sgl_short_elem_completion(), which does ucp_request_free(request) and then ucp_invoke_uct_completion(&req->send.state.uct_comp, status) (get_offload.c:290-299).

Nothing protects the parent from being completed/freed while those sub-request references are outstanding: if ucp_proto_rma_sgl_offload_send_frag() later returns an error, ucp_proto_multi_handle_send_error() → ucp_proto_request_abort() (proto_multi.inl:133-143) completes and frees the parent regardless of uct_comp.count, leaving the already-posted sub-requests and in-flight get_sgl_zcopy UCT ops pointing at a recycled uct_completion_t. That is exactly the signature observed: a live completion function (ucp_ep_flush_completion) with count == 0.

Implicated commit: 1310a95b "UCP/RMA: Support short elements in GET SGL via sub-requests" — Michal Shalev (test added in 1b00c0ad, PR #12191, head 54d5e142)

File: src/ucp/rma/get_offload.c:290-356 (ucp_proto_get_sgl_short_elem_completion, ucp_proto_get_sgl_short_elem, ucp_proto_get_sgl_adjust_min_frag); failing test at test/gtest/ucp/test_ucp_rma.cc:1768

Suggested fix:

1. Make the sub-request reference accounting single-sourced: set UCP_OP_ATTR_FLAG_NO_IMM_CMPL on the nested ucp_get_nbx() param so the callback is always invoked exactly once, and drop the ++/-- pair around the call (today three different code paths — PTR, immediate-OK, immediate-error — must each get the count right).
2. Guarantee the parent cannot be completed or aborted while sub-requests are outstanding: in ucp_proto_get_sgl_offload_*, route abort/error through a path that waits for uct_comp.count to drain (e.g. only uct_completion_update_status() + let the last sub-request completion finish the request), and add ucs_assertv(req->send.state.uct_comp.count == 0, ...) in the zcopy abort/complete path for SGL protocols to catch this in CI.
3. Preferably avoid re-entering the public ucp_get_nbx() from inside the parent's progress callback at all — post the short elements from the parent request itself (e.g. uct_ep_get_bcopy/the existing get/sgl/offload_sw single-element post) so that one uct_completion_t owns all fragments.
4. To confirm locally: ./gtest --gtest_filter=all/test_ucp_rma_sgl.short_and_long_elements* with UCX_LOG_LEVEL=trace_req (watch the "flush update ... comp_count" trace) and under valgrind/ASAN — the recycled-request use-after-free should show up directly.

Related: PR #12191 (this PR, "UCP/RMA: Add SGL support to ucp_get_nbx"); flush accounting context in PR #11923 ("UCP/FLUSH: Add topology-safe selective endpoint flush"); UCT side already merged in #11763 ("UCT/IB/MLX5/RC: Implement get_sgl_zcopy")

[svc-nvidia-pr-review]

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

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Build_Static build_source centos7) · commit f55427a7

TL;DR: The centos7 build_source job failed compiling src/ucp/ucp because GCC 4.8 on CentOS 7 raised -Werror=maybe-uninitialized for the buffers/lengths/remote_addrs arrays declared in proto_multi.inl and passed by address into the new SGL adjust path used by get_offload.c/put_offload.c; initialize those arrays (or at least element 0) to silence the old-compiler warning.

Full analysis

Summary: make failed with cc1: all warnings being treated as errors while compiling src/ucp/rma/get_offload.c and src/ucp/rma/put_offload.c (make[2] Error 1 → make Error 2).

Root cause: On the CentOS 7 image (old GCC, -O3 -Wall -Werror) the compiler cannot prove that the stack arrays declared in ucp_proto_rma_sgl_offload_send_frags():

proto_multi.inl:417:  void *buffers[UCP_MAX_IOV];
proto_multi.inl:418:  size_t lengths[UCP_MAX_IOV];
proto_multi.inl:419:  uint64_t remote_addrs[UCP_MAX_IOV];

are written before use. They are filled by ucp_datatype_iter_next_sgl_frags() whose fill count is only known via the returned frag_count, and then &buffers[0] / &lengths[0] / &remote_addrs[0] are handed to adjust_func (the new ucp_proto_{get,put}_sgl_offload_sw_progress adjust callbacks). After inlining into get_offload.c / put_offload.c, GCC's value tracking loses the frag_count == 0 early-return correlation and emits:

• get_offload.c:353 'remote_addrs' may be used uninitialized
• get_offload.c:340 'lengths' may be used uninitialized
• compiler_def.h:112 (via UCS_PTR_BYTE_OFFSET) 'buffers' may be used uninitialized
• the same three in put_offload.c:460 / ucp_worker.inl:103

This is a false positive from the old compiler, not a real logic bug — but -Werror makes it fatal. It is new code: proto_multi.inl was touched by the SGL series (4af0b6b5 "UCP: Split an SGL put between the lanes by bytes", 54d5e142 "Drop redundant max fragment arg from SGL send frag") and the failure appears on the merge commit f55427a7.

Implicated commit: [REDACTED:Hex High Entropy String] (Michal Shalev, merge of master into ucp-get-sgl); the warning originates from the SGL offload work in 4af0b6b / 54d5e14 (Michal Shalev)

File: src/ucp/proto/proto_multi.inl:417-419 (triggered from src/ucp/rma/get_offload.c:340,353 and src/ucp/rma/put_offload.c:460)

Suggested fix: Give the compiler a definite initialization at the declarations in ucp_proto_rma_sgl_offload_send_frags(), e.g.:

void *buffers[UCP_MAX_IOV]        = {NULL};
size_t lengths[UCP_MAX_IOV]       = {0};
uint64_t remote_addrs[UCP_MAX_IOV] = {0};

Only the first element is strictly needed for the adjust_func (max_frag_count == 1) path, so initializing just buffers[0]/lengths[0]/remote_addrs[0] before the ucp_datatype_iter_next_sgl_frags() call is the zero-cost variant. Alternatively wrap the declarations with UCX's existing "pretend-initialized" idiom (UCS_V_INITIALIZED) rather than disabling -Wmaybe-uninitialized. Verify with the centos7 container, since newer GCC used by the other legs does not reproduce this.

Related: PR #12191 (ucp-get-sgl); predecessor PR #11947 ("UCP: Split an SGL put between the lanes by bytes") introduced the array-passing pattern. No existing issue found for this warning.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 5810bc80-784e-48fd-90b3-51a201f6da3d in the triage console for the audit trail.

[svc-nvidia-pr-review]

A short element falls back to a full nested UCP request per element: parameter checks, protocol selection, memory detection and registration, plus a request from the mpool. That is the dominant cost exactly in the many-small-elements case this protocol targets (many_short_elements posts 1024, various_counts up to 4096). Can we post the short element on the same lane with the parent's uct_comp instead (e.g. uct_ep_get_bcopy like get/bcopy does), or at least pass the element's memh so it is not re-registered?

[svc-nvidia-pr-review]

This relies on cap.get.min_zcopy applying to every SGL element, but uct_ep_get_sgl_zcopy only documents the count limit. Please document the per-element minimum length in uct_v2.h, so the transports and this protocol agree on the contract.

[svc-nvidia-pr-review]

Residual coverage gap: no test forces PROTOS=get/sgl/offload_sw; on IB-only runs the sw path is only hit incidentally via self/shm.

Residual coverage gap: no test covers max_frag < min_sgl_frag (via RMA_ZCOPY_MAX_SEG_SIZE below cap.get.min_zcopy, or asymmetric lanes), where ucs_assertv(diff <= frag_offset) in ucp_proto_get_sgl_adjust_min_frag would fire and a release build would walk the buffer pointer before the element start.

test_ucp_rma_sgl_err skips non-host memory because a short GPU get goes through rendezvous and asserts on force close.

PR size is above the 500-added-line limit; the split request was already made in a previous round, so not repeated.

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Build_Static build_source centos7) · commit 1c61c0c3

TL;DR: The CentOS 7 (gcc 4.8) build_source job failed on -Werror=maybe-uninitialized in src/ucp/rma/put_offload.c because the warning-silencing initialization added in HEAD commit 1c61c0c3 is guarded by if (adjust_func != NULL), and the PUT SGL sw protocol passes adjust_func == NULL; initialize buffers[0]/lengths[0]/remote_addrs[0] unconditionally.

Full analysis

Summary: Compilation of src/ucp/ucp failed — make[2]: *** [rma/libucp_la-put_offload.lo] Error 1 with three -Werror=maybe-uninitialized errors for buffers, lengths, remote_addrs in ucp_proto_put_sgl_offload_sw_progress.

Root cause: In ucp_proto_rma_sgl_offload_send_frags() the arrays buffers[UCP_MAX_IOV], lengths[UCP_MAX_IOV] and remote_addrs[UCP_MAX_IOV] are filled by ucp_datatype_iter_next_sgl_frags() and only read when frag_count != 0. The old gcc (4.8.5, CentOS 7 container) cannot prove that through the inlining, so it reports a false-positive maybe-uninitialized. HEAD commit 1c61c0c3 ("UCP/PROTO: Silence maybe-uninitialized warning in SGL send frags") added a workaround, but wrapped it in if (adjust_func != NULL) (proto_multi.inl:429-435). The PUT SGL software protocol path — ucp_proto_put_sgl_offload_sw_send_func() → ucp_proto_rma_sgl_offload_send_frags(req, lpriv, 1, next_iter, ucp_proto_put_sgl_offload_sw_post, NULL) (put_offload.c:488-489) — passes adjust_func == NULL, so the arrays stay uninitialized in that inlined instantiation and the warning still fires at put_offload.c:460 (uct_iov_t iov = { .buffer = buffers[0], ... }) and ucp_worker.inl:103 for remote_addrs. The partial workaround is therefore incomplete; only the adjust_func != NULL instantiation (used by GET/other protos) was fixed.

Implicated commit: [REDACTED:Hex High Entropy String] — Michal Shalev, "UCP/PROTO: Silence maybe-uninitialized warning in SGL send frags"

File: src/ucp/proto/proto_multi.inl:429-435 (warning reported at src/ucp/rma/put_offload.c:460 and src/ucp/core/ucp_worker.inl:103, declarations at proto_multi.inl:417-419)

Suggested fix: Drop the adjust_func != NULL guard and initialize the first element unconditionally, so every inlined instantiation is covered:

    /* Silence old-compiler maybe-uninitialized warning: the arrays are filled
     * by ucp_datatype_iter_next_sgl_frags() when frag_count > 0 */
    buffers[0]      = NULL;
    lengths[0]      = 0;
    remote_addrs[0] = 0;

    ucs_assert(max_frag_count <= UCP_MAX_IOV);
    ucs_assert((adjust_func == NULL) || (max_frag_count == 1));

(Equivalently, declare them as void *buffers[UCP_MAX_IOV] = {NULL}; etc.) The cost is negligible since only index 0 is written, and it keeps the CentOS 7 / gcc 4.8 build green. Re-run the Build_Static build_source centos7 job after the change, since that is the only configuration that trips this gcc false positive.

Related: PR #12191 (branch ucp-get-sgl); precursor PR #11947 "UCP: Split an SGL put between the lanes by bytes" (4af0b6b5) introduced the shared ucp_proto_rma_sgl_offload_send_frags helper. No existing issue found for this warning.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 75f9de41-7215-4e1c-a002-4a55f705e763 in the triage console for the audit trail.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

minor: this doubles ~20 parameter-validation tests (invalid_rkey, missing_remote_*, memhs_*, rkeys_*, count_mismatch, ...) that all exercise the same UCP_RMA_SGL_CHECK_PARAMS macro, times 2 memory types times every TL instantiation. maybe keep them put-only with require_put() and leave one get case to verify ucp_get_nbx wires the checks in?

[svc-nvidia-pr-review]

Previously raised, still open: PR size is above the 500-added-line limit / split request.

Previously raised, still open: no coverage for max_frag < min_sgl_frag (RMA_ZCOPY_MAX_SEG_SIZE below cap.get.min_zcopy, or asymmetric lanes), where ucs_assertv(diff <= frag_offset) in ucp_proto_get_sgl_adjust_min_frag() fires and a release build walks the buffer pointer before the element start.

Previously raised, still open: test_ucp_rma_sgl_err skips non-host memory because a short GPU get goes through rendezvous and asserts on force close.

Residual gap: a short element recurses into full protocol selection, so on GPU memory without GPUDirect each 8-byte element becomes a rendezvous operation — the same path the test_ucp_rma_sgl_err TODO says asserts on force close. Only the host path is exercised.

Residual gap: for interleaved long/short element patterns the offload path posts one UCT op per long run plus one nested UCP request per short element; no test measures or bounds that behavior.

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Build build_aarch64 ctyunos2507_aarch64) · commit 3c2de12b

TL;DR: The aarch64 build fails with -Werror=maybe-uninitialized on elem_indices[0] in ucp_proto_rma_sgl_offload_send_frags(); the HEAD commit that added "silencing" initializers for buffers[0]/lengths[0]/remote_addrs[0] forgot elem_indices[0]. Add elem_indices[0] = 0; to that same block.

Full analysis

Summary: make of src/ucp failed on two objects (rma/libucp_la-get_offload.lo and rma/libucp_la-put_offload.lo) with GCC -Werror=maybe-uninitialized.

Root cause: In ucp_proto_rma_sgl_offload_send_frags() the local array size_t elem_indices[UCP_MAX_IOV] (proto_multi.inl:420) is only written by the inlined ucp_datatype_iter_next_sgl_frags(). With -O3 on aarch64 GCC cannot prove element 0 is written before it is read at:

• proto_multi.inl:446 → ucp_proto_get_sgl_adjust_min_frag() → ucp_proto_get_sgl_short_elem() (get_offload.c:318, req->send.rma.sgl.rkeys[elem_index])
• proto_multi.inl:456 → sgl_memhs[elem_index] (reached from ucp_proto_put_sgl_offload_sw_send_func, put_offload.c:488)

The block at proto_multi.inl:429-435 exists precisely to silence this class of warning for the max_frag_count == 1 case, but it initializes only buffers[0], lengths[0] and remote_addrs[0] — elem_indices[0] was omitted. Both failing call sites pass max_frag_count == 1, so they hit exactly this gap. Build log timestamps show continuous compile activity (no hang), so this is a pure compile error, not a timeout.

Implicated commit: 3c2de12b — "UCP/PROTO: Init single SGL fragment arrays to silence maybe-uninitialized", Michal Shalev (the PR HEAD; follow-up to 1c61c0c3 which attempted the same fix)

File: src/ucp/proto/proto_multi.inl:420 (declaration), lines 429-435 (incomplete initialization); reads at proto_multi.inl:446, proto_multi.inl:456, src/ucp/rma/get_offload.c:318

Suggested fix: Complete the silencing block in ucp_proto_rma_sgl_offload_send_frags():

    if (max_frag_count == 1) {
        /* Silence compiler warning, a single fragment is read by the adjust
         * and post functions */
        buffers[0]      = NULL;
        lengths[0]      = 0;
        remote_addrs[0] = 0;
        elem_indices[0] = 0;   /* <-- add this */
    }

Note the two reads are reached from different paths (adjust_func at line 446 for get, and the frag_index loop at line 456 for put), and both call sites pass max_frag_count == 1, so this single addition clears both errors. If a future caller passes max_frag_count > 1 and GCC still complains, prefer initializing elem_indices[0] = 0; unconditionally before the ucp_datatype_iter_next_sgl_frags() call rather than inside the == 1 guard. Verify locally with an -O3 -Werror aarch64/GCC build, since x86 GCC does not reproduce this.

Related: PR #12191 (branch ucp-get-sgl); prior attempts in this PR: 1c61c0c3, 3c2de12b. Context commit: 4af0b6b5 (#11947, "UCP: Split an SGL put between the lanes by bytes").

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

these helpers are RMA-SGL specific (req->send.rma.sgl, dt_iter->type.sgl) and are only used by put_offload.c/get_offload.c, but proto_multi.inl is also included by the AM, tag, stream and rndv protocols. can they live in src/ucp/rma/rma_sgl.inl (or rma.inl) instead, so the generic multi-lane header does not need to know the RMA request layout?

[svc-nvidia-pr-review]

PR size is above the 500-added-line limit — please consider splitting it.

No coverage for max_frag < min_sgl_frag (e.g. RMA_ZCOPY_MAX_SEG_SIZE below cap.get.min_zcopy, or asymmetric lanes), where ucs_assertv(diff <= frag_offset) in ucp_proto_get_sgl_adjust_min_frag() fires and a release build walks the buffer pointer before the element start.

No test forces PROTOS=get/sgl/offload_sw.

test_ucp_rma_sgl_err skips non-host memory because a short GPU get goes through rendezvous and asserts on force close.

No get/sgl/am emulation counterpart to put/sgl/am/bcopy. Concrete consequence: mm (posix/sysv) advertises only PUT/GET_BCOPY and self only GET_BCOPY, so on such a lane set neither get/sgl/offload nor get/sgl/offload_sw can be selected and the request lands on ucp_proto_reconfig_progress(), which logs cannot find remote protocol, calls ucp_ep_set_lanes_failed_schedule() when error handling is enabled, and aborts with UCS_ERR_CANCELED — i.e. a valid API call can fail the endpoint rather than return an error. The new tests cannot catch this because test_sgl() skips when there is no UCT_IFACE_FLAG_GET_ZCOPY lane.

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Tests roce on worker 0) · commit 1187eaa5

TL;DR: The new all/test_ucp_rma_sgl.short_and_long_elements/1 <all/host/get> test aborts on uct_iface.h:1012 Assertion 'comp->count > 0' failed: comp=... (ucp_ep_flush_completion) count=0 — an extra UCT completion is delivered to the endpoint‑flush completion after it already reached 0, caused by the completion‑count bookkeeping of the brand‑new GET‑SGL "short element via sub‑request" path in this PR. Fix the uct_comp.count accounting in ucp_proto_get_sgl_short_elem() / the UCS_INPROGRESS → UCS_OK remapping in ucp_proto_rma_sgl_offload_send_frags() so the parent SGL request cannot be completed while a nested ucp_get_nbx() is still outstanding on the RC QP.

Full analysis

Summary: gtest job "roce on worker 0" aborted (core dumped, make: *** [Makefile:4971: test] Aborted) in the new test_ucp_rma_sgl.short_and_long_elements GET test, inside flush_ep(sender()).

Root cause: From the log, the failure is a hard assertion, not a timeout (progress is continuous up to the abort; the later timestamps are only the crash handler printing the backtrace):

[ RUN ] all/test_ucp_rma_sgl.short_and_long_elements/1 <all/host/get>
uct_iface.h:1012 Assertion `comp->count > 0' failed: comp=0x55b82729a998 (ucp_ep_flush_completion) count=0 status=0
  uct_invoke_completion()            src/uct/base/uct_iface.h:1012
  uct_rc_ep_flush_op_completion_handler()  src/uct/ib/rc/base/rc_ep.c:371
  uct_rc_txqp_completion_inl_resp()   src/uct/ib/rc/base/rc_ep.h:540
  uct_rc_mlx5_iface_poll_tx()         rc_mlx5.inl:2111
  ucp_test::flush_ep()                test/gtest/ucp/ucp_test.cc:203
  test_ucp_rma_sgl::test_sgl()        test/gtest/ucp/test_ucp_rma.cc:1541

A UCP EP‑flush request counts exactly one UCT completion per lane whose uct_ep_flush() returned UCS_INPROGRESS (ucp_ep_flush_request_reset() / ucp_ep_flush_progress() in src/ucp/rma/flush.c). Getting a completion when count == 0 means one more UCT completion was delivered than was accounted for — i.e. a stale/duplicate RC flush op, or a completion landing on a request whose owner already finished.

Everything in this path is new in this PR (branch ucp-get-sgl, PR #12191). The test vector is {1, 4K, 7, 64, 2K, 3, 512, 100, 1K, 65K, 1}, so it is the only test that exercises the "short element" branch, which issues a nested ucp_get_nbx() on the same endpoint from inside the parent SGL request's progress and hand-manages the parent's completion counter:

/* src/ucp/rma/get_offload.c:301-328 */
++req->send.state.uct_comp.count;
status_ptr = ucp_get_nbx(req->send.ep, buffer, length, remote_addr, ...);
if (UCS_PTR_IS_PTR(status_ptr)) return UCS_INPROGRESS;
--req->send.state.uct_comp.count;   /* immediate completion */

and that UCS_INPROGRESS is then converted to UCS_OK so the generic engine deliberately does not count the fragment:

/* src/ucp/proto/proto_multi.inl:446-452 */
status = adjust_func(req, lpriv, elem_indices[0], &buffers[0], &lengths[0], &remote_addrs[0]);
if (ucs_unlikely(status != UCS_OK))
    return (status == UCS_INPROGRESS) ? UCS_OK : status;   /* vs. ucp_proto_multi_progress(): UCS_OK => no ++count */

The 1‑byte trailing element goes through this sub‑request path and is posted as a tiny RDMA READ, which is exactly why the completing CQE is an inline‑scatter one (uct_rc_txqp_completion_inl_resp, which pops all outstanding ops with sn <= hw_ci, including the flush op). Once the parent/sub‑request reference counting is off by one, the parent SGL request is completed and released while a UCT op is still outstanding on the RC QP, and the late TX CQE then completes a flush op whose completion was already satisfied — the observed count=0 abort. (The UCT side of the feature is also new and relevant: uct_rc_mlx5_base_ep_sgl_zcopy_post() in src/uct/ib/mlx5/rc/rc_mlx5.inl:508-599 posts N WQEs, signals only the last one, and registers a single send op at txwq->sig_pi, which the flush op's sn also depends on.)

Implicated commit: 1310a95b "UCP/RMA: Support short elements in GET SGL via sub-requests" (Michal Shalev) — introduced the sub-request path; related follow-ups in the same series: 3cc34f95, 6c6b9d21, 1187eaa5 (HEAD of this build), 51224230 "UCT/IB/MLX5/RC: Implement get_sgl_zcopy".

File: src/ucp/rma/get_offload.c:291-328 (ucp_proto_get_sgl_short_elem[_completion]), with src/ucp/proto/proto_multi.inl:446-452 and src/uct/ib/mlx5/rc/rc_mlx5.inl:508-599; assertion site src/uct/base/uct_iface.h:1012.

Suggested fix:

1. Make the short-element sub-request hold a reference that cannot be released early: keep the ++uct_comp.count and let only the sub-request callback (or an explicit error path) drop it. For the immediate-completion case, instead of --count inline, pass UCP_OP_ATTR_FLAG_NO_IMM_CMPL to the nested ucp_get_nbx() so the sub-request always returns a request pointer and always completes via ucp_proto_get_sgl_short_elem_completion() — one increment, one decrement, no in-line window where the parent can reach 0.
2. Audit the error path: on a hard error from ucp_get_nbx() the code does --count and returns the error, which then goes to ucp_proto_request_abort(); verify the parent cannot be aborted/freed while earlier fragments (SGL zcopy ops) are still outstanding.
3. Add a debug assertion in ucp_proto_request_zcopy_completion / before ucp_request_put() for SGL GET that no sub-request is still in flight, which will convert this latent corruption into a local failure instead of a flush-completion abort.
4. While there, double-check the single-send-op + single-signaled-WQE accounting in uct_rc_mlx5_base_ep_sgl_zcopy_post() (res_count, sig_pi) against uct_rc_txqp_add_flush_comp(..., ep->tx.wq.sig_pi), since the flush op's sn is derived from it.

Related: PR #12191 (this PR); feature predecessors #11763 (get_sgl_zcopy), #11969 ("fix psn track for sgl"), #11947 (SGL put lane split).

[qierli]

Hi @michal-shalev, thanks for this. We run SGL gets in production for NIXL KV-cache reads (on our own UCX patch) and tested this PR at 1187eaa on B300 with rc_mlx5:

• On an endpoint with no GET_ZCOPY lane (e.g. UCX_TLS=tcp), an SGL get fails the whole endpoint: cannot find remote protocol ... get into sgl host memory from host, then the error callback fires with UCS_ERR_ENDPOINT_TIMEOUT. Contig gets there work through get/am. Could SGL get have an AM fallback like put/sgl/am, or fail only the request (e.g. UCS_ERR_UNSUPPORTED)? Happy to send a follow-up PR.
• Perf against get/sgl/offload_sw: get/sgl/offload is faster for small elements (102K × 4 KiB, GPU→GPU: 45 vs 55 ms) but about 10% slower for 40–120 KiB elements read from GPU memory (48K × 40 KiB: 121 vs 111 ms).

Are you planning the NIXL side (READs through sglXfer, like ai-dynamo/nixl#1835)? If not, we have it drafted against this branch and can send it.