[michal-shalev]

What?

Move the PUT SGL fragment/batch send code into shared helpers in proto_multi.inl, and reuse them in the PUT sw protocol.

Why?

Prepare for GET SGL, which needs the same lane split. No functional change.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

minor: pls move this typedef to proto_multi.h next to ucp_proto_send_multi_cb_t and ucp_proto_multi_lane_send_func_t, with a short doc comment for the arrays and count. There are no other typedefs in .inl files under src/ucp, and the 7-parameter list is not self-explanatory.

[svc-nvidia-pr-review]

Placement: can we put these in a dedicated rma sgl .inl under src/ucp/rma instead? They are rma+sgl specific (req->send.rma.sgl, dt_iter->type.sgl) and only put_offload.c / get_offload.c will use them, but proto_multi.inl is also included by tag, am, stream and rndv.

[svc-nvidia-pr-review]

Assert lost its debug context:

Suggested change

	ucs_assertv(count == 1, "count=%zu", count);
	ucs_assertv(count == 1, "req=%p offset=%zu length=%zu count=%zu", req,
	req->send.state.dt_iter.offset,
	req->send.state.dt_iter.length, count);

minor: the old assert printed the dt_iter offset/length/frag_offset; count alone doesn't tell why more than one fragment was produced.

[svc-nvidia-pr-review]

Residual gap: no gtest exercises a post_func with count != 1 through ..._send_frags() with max_frag_count == 1, i.e. the new ucs_assertv(count == 1) is only an invariant check; that is acceptable for a refactor.

Residual gap: SGL zcopy offload (put/sgl/offload) needs RC/mlx5 hardware; on the tcp instantiation only the offload_sw path is covered, so the batch path depends on the IB CI jobs.

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Tests new on worker 3) · commit f042d604

TL;DR: The gtest binary aborted on a UCS rcache assertion (!(region->flags & UCS_RCACHE_REGION_FLAG_PGTABLE) at src/ucs/memory/rcache.c:396) while tcp/test_ucp_rma.put_nonblocking/2 <…/user_memh> was unmapping its memory handle — the rcache region's refcount hit 0 while the region was still in the page table, i.e. an unbalanced (double) release of the memh reference taken for a user memh; the RMA send-path refactor in this PR is the only delta in the build and must be audited for an extra ucp_memh_put*()/ucp_datatype_iter_cleanup() on the request path.

Full analysis

Summary: make test (gtest, Azure job "new on worker 3", build 142446) aborted with a core dump in tcp/test_ucp_rma.put_nonblocking/2 <tcp,cuda_copy,rocm_copy/user_memh>:
rcache.c:396 Assertion '!(region->flags & UCS_RCACHE_REGION_FLAG_PGTABLE)' failed, backtrace ucp_mem_unmap → ucp_memh_cleanup (ucp_mm.c:470) → ucp_memh_put_rcache (ucp_mm.c:441) → ucs_rcache_region_put_unsafe (rcache.inl:128) → ucs_mem_region_destroy_internal (rcache.c:396), from test_ucp_memheap::test_xfer (test_ucp_memheap.cc:120).

Root cause: Registration-cache reference-count underflow. A region inserted into the rcache page table is created with refcount = 1 plus UCS_RCACHE_REGION_FLAG_PGTABLE (src/ucs/memory/rcache.c:986-989), and that page-table reference is dropped only by ucs_rcache_region_invalidate_internal(), which first clears the flag (rcache.c:492-501). Therefore reaching refcount == 0 while the flag is still set — exactly what assert 396 caught (the refcount == 0 assert at line 394 passed) — can only happen if the region was released more times than it was acquired. The release in the trace is the user-memh path: ucp_memh_cleanup() → ucp_memh_put_rcache(context, memh->parent) for the parent reference acquired in ucp_memh_init_uct_reg() (src/ucp/core/ucp_mm.c:950). So some earlier code path in the same test already dropped that parent/memh reference (note that for a user memh ucp_memh_put() is intentionally a no-op, src/ucp/core/ucp_mm.inl:97, so any path that releases the rcache region directly, e.g. ucp_memh_put_rcache()/ucs_rcache_region_put_unsafe() on a memh it does not own, or a request whose dt_iter is cleaned up twice via the completion and the reset/abort path, produces this underflow). This is a genuine use-after-free class bug, not a timeout/infra flake — there are no large time gaps in the log, the process aborted while actively running.

Implicated commit: [REDACTED:Hex High Entropy String] — "UCP/RMA: Share SGL multi-lane send code in proto_multi.inl", Michal Shalev (the only change under test; it rewrites the put send path exercised by test_ucp_rma.put_nonblocking). Related prior work on the same files: 4af0b6b5 (#11947), [REDACTED:Hex High Entropy String] (#11888, "register user-supplied SGL memory handles on all required MDs").

File: src/ucs/memory/rcache.c:396 (assert site); real defect is in the memh ref accounting reached from src/ucp/core/ucp_mm.c:441/:470 and the RMA send path refactored in src/ucp/proto/proto_multi.inl:400-550 / src/ucp/rma/put_offload.c:261-514.

Suggested fix:

1. Audit the PR's moved code for an unbalanced release: every ucp_memh_get()/ucs_rcache_lookup*() in the shared ucp_proto_rma_sgl_offload_send_func() / ..._send_frags() and in ucp_proto_multi_zcopy_progress()'s init/abort/reset paths (ucp_proto_request_zcopy_init, ucp_proto_offload_zcopy_reset, ucp_proto_request_zcopy_abort) must have exactly one matching put; in particular make sure a request that is reset/aborted and later completed does not run ucp_datatype_iter_cleanup(dereg=1) twice, and that user-supplied memhs (param->memh, dt_iter->type.sgl.memhs[]) are never passed to ucp_memh_put_rcache()/ucs_rcache_region_put_unsafe() (only their parent, once, from ucp_memh_cleanup()).
2. Reproduce locally with the exact variant to confirm the PR introduced it:
   UCX_TLS=tcp,cuda_copy,rocm_copy ./gtest --gtest_filter='tcp/test_ucp_rma.put_nonblocking/2' with UCX_LOG_LEVEL=trace and rcache tracing (UCX_LOG_LEVEL=trace + ucs_rcache_region_trace output shows each get/put of the region) on both this branch and its merge base — that pinpoints the extra put.
3. To make such bugs fail loudly and earlier, strengthen the check at rcache.c:396 into ucs_assertv printing region/refcount/name (as line 394 does), and consider adding an assert in ucp_memh_put_rcache() that memh->parent == NULL || memh->parent == memh (i.e. that only a non-user, rcache-owned memh is being released).

Related: PR #12226 (this PR), #12191 (UCP/RMA: Add SGL support to ucp_get_nbx, same shared code), #11947 (closed, split SGL put between lanes), #11958 (open, rcache adjacent-region merging — touches the same refcount logic).

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 2603e5af-1482-4c2e-a3b8-a644dd820ead in the triage console for the audit trail.