[michal-shalev]

What?

Split one SGL between the lanes by bytes instead of by element count, so every lane gets an equal share of the message, splitting elements where a share ends.

Why?

Elements were distributed by count, so a single element used one lane only and lost ~50% of the WRITE bandwidth, and any element count which is not a multiple of the lane count left the lanes unevenly loaded.

How?

The request keeps the total byte length of the SGL, and every lane derives its share from it like a multi-lane contiguous message, in ucp_proto_put_sgl_max_payload(). A batch of fragments is limited by its total length, so one post covers the whole lane payload.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

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

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Build build_aarch64 rhel100_aarch64) · commit 2c9bd493

TL;DR: The aarch64 build failed to compile test/gtest/ucp/test_ucp_dt.cc because ucp_datatype_iter_sgl_init() gained a new size_t length parameter but the gtest call site was not updated (6 args passed to a 7-arg function). Add the total SGL byte length argument between count and &param.

Full analysis

Summary: make failed in test/gtest — ucp/gtest-test_ucp_dt.o compile error (-Werror, "too few arguments to function ucp_datatype_iter_sgl_init"), aborting the "Build on aarch64" step with exit code 2.

Root cause: Signature/call-site mismatch introduced by this PR's series. src/ucp/dt/datatype_iter.h:118-123 now declares:

ucs_status_t ucp_datatype_iter_sgl_init(ucp_context_h context,
                                        ucp_datatype_iter_t *dt_iter,
                                        const ucp_dt_local_sgl_t *local,
                                        const ucp_dt_remote_sgl_t *remote,
                                        size_t count, size_t length,
                                        const ucp_request_param_t *param);

but the gtest still calls it with only 6 arguments (test/gtest/ucp/test_ucp_dt.cc:342-344), so &param is bound to the size_t length slot — producing both invalid conversion from 'ucp_request_param_t*' to 'size_t' and too few arguments to function. The implementation at src/ucp/dt/datatype_iter.c:286-299 assigns dt_iter->length = length, confirming the new parameter is the total SGL length in bytes. This is not aarch64-specific; it is a plain compile error that would fail any --enable-gtest build.

Implicated commit: [REDACTED:Hex High Entropy String] — "UCP/DT: Count the SGL iterator length and offset in bytes", Michal Shalev (touched src/ucp/dt/datatype_iter.h on 2026-09-14, hours before this build; PR head 2c9bd4935)

File: test/gtest/ucp/test_ucp_dt.cc:342 (declaration at src/ucp/dt/datatype_iter.h:118, definition at src/ucp/dt/datatype_iter.c:286)

Suggested fix: Pass the total byte length in the test's init_sgl_iter():

size_t total_length = std::accumulate(m_lengths.begin(), m_lengths.end(),
                                      size_t(0));
ASSERT_UCS_OK(ucp_datatype_iter_sgl_init(m_ucph.get(), &m_dt_iter,
                                         &m_local, &m_remote, count,
                                         total_length, &param));

(include <numeric>). While doing so, audit the rest of test_ucp_dt_sgl for the same semantic change — since dt_iter->length/offset are now counted in bytes rather than elements, expectations around m_dt_iter.length, offset and m_elem_index in next_frags() may also need updating. Re-run a local --enable-gtest build before pushing; a compile-only check would have caught this pre-merge.

Related: PR #11947 (this PR); earlier SGL work: #11344, #11761, #11647, #11888

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 31c93a6a-3f9b-4419-8b7a-54e0165fa211 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]

Residual coverage gap: no test exercises an SGL with zero-length elements, where ucp_dt_sgl_length() now makes dt_iter->length differ from the element count (affects proto selection and the elem_fits fallback).

Residual coverage gap: the new put_split_single_element only checks uct_comp.count, not data; the fragment-offset arithmetic (remote_addrs[i] + frag_offset) is data-verified only by the test_put_sgl(1, UCS_MBYTE) line added to put_multi_rail, which is skipped under valgrind.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

Residual coverage gap: no test uses an SGL whose elements are all zero-length, where ucp_dt_sgl_length() makes dt_iter->length differ from the element count.

Residual coverage gap: put_split_single_element only checks uct_comp.count; the fragment-offset arithmetic is data-verified only by test_put_sgl(1, UCS_MBYTE) in put_multi_rail, which is skipped under valgrind.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

Residual coverage gap: the removed lines in iter_next_frag_zero_length were the only unit coverage for ucp_datatype_iter_next_sgl_frags() returning 0 descriptors, a case both ucp_proto_put_sgl_offload_send_frag() and ..._sw_send_func() still special-case (overlaps with the previously reported all-zero-length-SGL gap).

Residual coverage gap: put_split_single_element only asserts uct_comp.count; the new fragment-offset arithmetic in the single-element case is data-verified only by test_put_sgl(1, UCS_MBYTE) in put_multi_rail.

[svc-nvidia-pr-review]

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

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Tests gpu prebuild) · commit ffb0b72d

TL;DR: The "gpu prebuild" job died because mpirun aborted immediately — OpenMPI could not find its launch agent (plm_rsh_agent: ssh : rsh not present in the doca-3.1.0-gpunetio container), so the first MPI perftest in run_mpi_tests failed with exit 243. Fix is on the CI side: install openssh-client in that container image (or force a local launcher, e.g. -mca plm isolated / empty plm_rsh_agent) — nothing to do with PR #11947's code.

Full analysis

Summary: Step "Build for tests" of job Tests gpu prebuild (build 140296) failed with Bash exited with code '243' when run_ucx_perftest 1 invoked mpirun.

Root cause: The build and install phases completed cleanly (make install, make installcheck, examples compiled). The failure is the very first MPI launch:

+ mpirun --allow-run-as-root --bind-to none ... -np 2 -x UCX_NET_DEVICES=all -x UCX_TLS=shm .../ucx_perftest ...
The value of the MCA parameter "plm_rsh_agent" was set to a path that could not be found:
  plm_rsh_agent: ssh : rsh
[swx-rdmz-ucx-gpu-02:39148] [[INVALID],INVALID] FORCE-TERMINATE AT Not found:-13 - error plm_rsh_component.c(335)

Neither ssh nor rsh exists in PATH inside the container (rdmz-harbor.../ubuntu24.04/builder:doca-3.1.0-gpunetio), so ORTE's rsh PLM component fails to initialize and mpirun force-terminates before any UCX code runs. There is no UCX/perftest error, no test output, and no timeout/hang — the process died in <0.1 s. The CI scripts never override the launcher: run_mpi_tests() builds MPIRUN with only -mca pml/osc/btl/coll options (contrib/test_jenkins.sh:971-983) and then calls run_ucx_perftest 1, which runs $MPIRUN -np 2 ... (contrib/test_jenkins.sh:670). This MPI path became part of the new prebuild job in commit 390a30e ("Build once per GPU test row and share the tree with the workers", PR #11896), which routes BUILD_ONLY_TARBALL → build_for_tests → run_mpi_tests in the prebuild job (contrib/test_jenkins.sh:1508-1523, 1624-1627); the worker jobs now skip it via PREBUILT_TARBALL. So the whole GPU row now fails on a container that lacks an MPI launch agent.

Implicated commit: 390a30e — Alexey Rivkin, "AZP/TESTS: Build once per GPU test row and share the tree with the workers (#11896)" (environment/image gap exposed by it; PR #11947 commit ffb0b72 is unrelated)

File: contrib/test_jenkins.sh:670 ($MPIRUN -np 2 ...), MPIRUN defined at contrib/test_jenkins.sh:971-983; container defined at buildlib/pr/main.yml:66-68, prebuild job at buildlib/pr/tests.yml:16-46

Suggested fix:

1. Preferred: add openssh-client to the ubuntu24.04/builder:doca-3.1.0-gpunetio image (all other test containers evidently have it) and bump the image tag in buildlib/pr/main.yml:67.
2. Immediate/robust workaround in the script — these tests are all single-node, so avoid rsh entirely by adding an explicit local launcher to MPIRUN_COMMON in run_mpi_tests():

   MPIRUN_COMMON="mpirun --allow-run-as-root --bind-to none \
       -mca plm_rsh_agent '' ... "
   

   (or -mca plm isolated), and/or guard the MPI block with command -v ssh >/dev/null || { echo '==== Not running MPI tests (no rsh agent) ===='; return; } next to the existing if mpirun --version check at contrib/test_jenkins.sh:959, so a missing launcher degrades to a skip instead of failing every GPU PR build.

Related: #11896 (introduced the gpu prebuild job); PR under test: #11947

[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 b29d3013

TL;DR: The Coverity gate failed with 2 new BAD_FREE ("free of stack-allocated memory") defects at src/ucp/rma/put_offload.c:559-560, caused by the newly added ucs_alloc_on_stack()/ucs_free_on_stack() pair in ucp_proto_put_sgl_offload_send_func() — Coverity cannot correlate the _size <= UCS_ALLOCA_MAX_SIZE branch between the alloc and the free, so it thinks ucs_free() is called on an alloca() pointer. Fix by either annotating the two frees with /* coverity[incorrect_free] */ (the file already uses that convention) or by clamping elem_count to UCP_MAX_IOV and using fixed-size stack arrays like the sibling ..._send_frags() function does.

Full analysis

Summary: coverity release on coverity_rh7 job failed at the cov-analyze/cov-format-errors gate: ##[error]Coverity found 2 issues, both BAD_FREE in src/ucp/rma/put_offload.c (lines 559 and 560). The build itself compiled cleanly (559/559 units, 100%).

Root cause: ucp_proto_put_sgl_offload_send_func() allocates the uct_rkeys / uct_memhs arrays with the hybrid macro ucs_alloc_on_stack(size, name) (src/ucs/sys/compiler.h:72), which expands to (_sz <= UCS_ALLOCA_MAX_SIZE) ? alloca(_sz) : ucs_malloc(...), and releases them with ucs_free_on_stack(ptr, size) which calls ucs_free() only when size > UCS_ALLOCA_MAX_SIZE. The two size checks are logically correlated but Coverity models them as independent conditions. The reported path proves this is a false positive: step 26/29 takes _sz <= 1200 → __builtin_alloca(_sz), then step 39 takes the contradictory uct_memhs_size > 1200 → ucs_free(uct_memhs) (incorrect_free, with freed_arg in src/ucs/debug/memtrack.c:371). The identical pattern repeats for uct_rkeys at line 560.

The defect is new to this branch: the alloc/free pair was introduced when the key arrays were made dynamically sized. The sibling function ucp_proto_put_sgl_offload_send_frags() (line 436-437) still uses plain uct_mem_h uct_memhs[UCP_MAX_IOV] fixed arrays and is not flagged — the dynamic version was needed because max_elem_count = ucs_min(lpriv->max_sgl_zcopy_count, dt_iter->length - start_index) (line 483) is not clamped to UCP_MAX_IOV, unlike max_frag_count (line 475).

Implicated commit: c58c9c66 "UCP: Decide on the SGL redirect before allocating the key arrays" (Michal Shalev, 2026-10-28), with e9c35d25 "UCP: Remove the weight scaling from the SGL element count limit" (Michal Shalev, 2026-10-28) on top — both on branch sgl-split-single-element, PR #11947. The macro itself predates this ([REDACTED:Hex High Entropy String], Thomas Vegas, 2024).

File: src/ucp/rma/put_offload.c:531-560 (defects reported at :559 and :560); macro at src/ucs/sys/compiler.h:72-86

Suggested fix: Pick one:

1. Minimal / unblocks CI now — suppress the two false positives, matching the convention already used in this very file (/* coverity[tainted_data_downcast] */ at line 570):

    /* coverity[incorrect_free] */
    ucs_free_on_stack(uct_memhs, uct_memhs_size);
    /* coverity[incorrect_free] */
    ucs_free_on_stack(uct_rkeys, uct_rkeys_size);

Note the error-path free at line 538 will likely be flagged too once this path becomes reachable in a future analysis, so annotate it as well.

2. Preferred / also removes a real risk — clamp the element count to UCP_MAX_IOV and drop the dynamic allocation entirely:

size_t max_elem_count = ucs_min(ucs_min(lpriv->max_sgl_zcopy_count, UCP_MAX_IOV),
                                dt_iter->length - start_index);
...
uct_mem_h  uct_memhs[UCP_MAX_IOV];
uct_rkey_t uct_rkeys[UCP_MAX_IOV];

This makes the function consistent with ucp_proto_put_sgl_offload_send_frags(), eliminates both BAD_FREE reports and the alloca() in a hot send path, and guards against posting more than UCP_MAX_IOV entries.

3. If the unbounded count is genuinely required, encapsulate the alloc/free in a single helper that stores the chosen allocation mode in a small struct (e.g. a was_heap flag checked by the release function) so the free decision is data-driven rather than a recomputed size comparison — Coverity can then track it without an annotation.

Related: PR #11947 (this change); macro introduced by commit [REDACTED:Hex High Entropy String] (UCS/ASYNC: Use heap to process expired timers in batch). No existing issue tracking ucs_alloc_on_stack Coverity BAD_FREE false positives was found — worth opening one, since any new user of this macro will hit the same gate failure.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 97cbde6a-f5f6-486e-a53c-10040698e137 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]

Residual coverage gap (unchanged from earlier rounds): no e2e test_ucp_rma_sgl case sends an SGL with a zero-length element mixed with non-zero ones, which is the path where the batch loop breaks on length == 0 and falls back to ..._send_frags(); it is covered only by test_ucp_dt_sgl.