[rakhmets]

What?

Added a new UCP endpoint creation parameter. This parameter allows to manage the lifetime of resources used to work with a UCP remote key.
Added a new UCT remote key unpacking parameter. This parameter allows to manage the lifetime of resources used to work with the remote key.

Why?

This is another attempt to solve the problem of unclosed interprocessor memory descriptors that prevent the memory from being released by the remote side.

#11335
#11288
#11730

At the moment the only way to close the handles is to disable UCT CUDA IPC remote cache. So behavior can only be controlled at the level of the whole process. The new parameters adds the possibility of more detailed control over the lifetime of objects.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

Test coverage note (not a blocker): contrib/test_jenkins.sh swaps UCX_MEMTYPE_CACHE=n for UCX_GDR_COPY_RCACHE=n -x UCX_RCACHE_ENABLE=n, and the test now sets UCP_EP_PARAMS_FLAGS_RKEY_BOUND_LIFETIME instead of relying on the disabled memtype cache. This exercises the new flag end-to-end. The other callers of create_ucp (test_rma_cuda.c, test_ucp.c) still pass 0, so the default (unbound) path also remains covered. No additional test gap for the stated goal; the main open risk is the masked map-failure above, which is not exercised by the happy-path MPI test.

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

Non-blocking note: the CI change swaps UCX_MEMTYPE_CACHE=n for UCX_GDR_COPY_RCACHE=n -x UCX_RCACHE_ENABLE=n and exercises the new flag end-to-end. The CI comment says "other caches unrelated to that are disabled" — worth confirming that UCX_GDR_COPY_RCACHE/UCX_RCACHE_ENABLE are the only caches that previously required UCX_MEMTYPE_CACHE=n, i.e. that the memtype cache no longer needs disabling for this test to pass.

[svc-ucx]

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

TL;DR: The test_obj_size.size gtest failed because PR #11870 added a new uint8_t flags field at the end of ucp_ep_ext_t, which alignment-padded the struct from 216 to 224 bytes while the test still asserts 216; move the new field into the existing padding hole after cm_idx (or bump the expected size to 224).

Full analysis

Summary: test_obj_size.size failed in the release gtest run — EXPECT_EQ(216, sizeof(ucp_ep_ext_t)) got 224.

Root cause: The only failure in the log is a static struct-size assertion, not a runtime/hang issue:

test/gtest/common/test_obj_size.cc:53: Failure
Expected equality of these values:
  (size_t)216
  sizeof(ucp_ep_ext_t)
    Which is: 224

This PR's own commit on src/ucp/core/ucp_ep.h (30dfe8f6 — "UCP/API: Endpoint parameter controlling rkey resources.", Raul Akhmetshin) introduced a new enum ucp_ep_ext_flags { UCP_EP_EXT_FLAG_RKEY_BOUND_LIFETIME } (ucp_ep.h:520-522) and a new field to hold it:

    /* Map of system devices that require a flush operation */
    ucp_sys_dev_map_t             flush_sys_dev_map;   /* uint64_t */

    /* Flags from ucp_ep_ext_flags */
    uint8_t flags;                                     /* <-- new, ucp_ep.h:591 */
} ucp_ep_ext_t;

Because the field was appended after a uint64_t member, the compiler adds 7 bytes of tail padding to keep the struct 8-byte aligned, so a 1-byte addition costs a full 8 bytes: 216 → 224. test_obj_size.cc:53 was not updated, so the guard test correctly flags the size regression. Note the struct already has a free 7-byte padding hole: ucp_rsc_index_t cm_idx (a uint8_t, ucp_ep.h:532, confirmed via ucp_types.h:31) is immediately followed by ucs_ptr_map_key_t local_ep_id (8 bytes).

This is a deterministic build-config failure, unrelated to the test_ucp_config/gcc: error: test_ucp_config-test_ucp_config.o: No such file noise earlier in the log (that step's exit status was swallowed by the pipe into tee/grep) and unrelated to the "is not available" IB transport INFO lines, which are normal on this non-IB althca worker.

Implicated commit: 30dfe8f6 — Raul Akhmetshin, "UCP/API: Endpoint parameter controlling rkey resources." (PR #11870, branch topic/uct-rkey-unpack-flags)

File: src/ucp/core/ucp_ep.h:590-591 (new field); assertion at test/gtest/common/test_obj_size.cc:53

Suggested fix: Preferred — keep ucp_ep_ext_t at 216 bytes by relocating the new flags field into the existing padding hole next to the other single-byte member, so it costs zero bytes:

    ucp_rsc_index_t               cm_idx;        /* CM index */
    /* Flags from ucp_ep_ext_flags */
    uint8_t                       flags;
    ucs_ptr_map_key_t             local_ep_id;

and delete the trailing uint8_t flags; at ucp_ep.h:591. Then re-run make -C test/gtest test GTEST_FILTER=*test_obj_size* to confirm 216.

If the 8-byte growth is intentional/unavoidable, instead update test/gtest/common/test_obj_size.cc:53 to EXPECTED_SIZE(ucp_ep_ext_t, 224); and call out the per-endpoint memory increase in the PR description, since this test exists specifically to make such growth a deliberate decision.

Related: PR #11870 (#11870) — the PR under test; no pre-existing issue found for this assertion.

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

src/uct/cuda/cuda_ipc/cuda_ipc_md.c

Masked map-failure in uct_cuda_ipc_is_peer_accessible: on the bound_lifetime && *accessible == UCS_YES re-map path, a failed uct_cuda_ipc_map_memhandle is not reflected in the return status (*accessible stays UCS_YES, function returns UCS_OK with mapped_addr == NULL). This is the main residual correctness risk and is not exercised by the happy-path MPI test.

contrib/test_jenkins.sh

CI env-var swap (UCX_MEMTYPE_CACHE=n -> UCX_GDR_COPY_RCACHE=n -x UCX_RCACHE_ENABLE=n): confirm the memtype cache no longer needs disabling.

test/mpi/test_ucp_rkey_destroy.c

Residual test/coverage gap: the failure path of the new bound-lifetime re-map (and thus the masked-status concern) has no test coverage; only the happy path is exercised.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

src/uct/cuda/cuda_ipc/cuda_ipc_md.c

Masked map-failure in uct_cuda_ipc_is_peer_accessible: on the bound_lifetime && *accessible == UCS_YES re-map path, a failed uct_cuda_ipc_map_memhandle is not reflected in the return status — *accessible stays UCS_YES and the function returns UCS_OK with mapped_addr == NULL. The status assignment is now guarded by if (*accessible == UCS_TRY), so the UCS_YES branch's map error is dropped.

contrib/test_jenkins.sh

CI env-var swap: confirmation that the memtype cache no longer needs disabling.

test/mpi/test_ucp_rkey_destroy.c

Test coverage gap: only the happy path exercises the new bound-lifetime re-map; the failure path (and thus the masked-status concern) is untested.

[svc-ucx]

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

TL;DR: The Coverity gate failed with one new FORWARD_NULL defect: ucp_device_local_mem_list_element_pack() passes a stack uct_rkey_bundle_t with .handle = NULL into uct_md_mem_elem_pack(), and the new code in uct_cuda_ipc_md_mem_elem_pack() now dereferences that handle unconditionally (cuda_ipc_rkey_handle->cache_enabled). Guard the dereference (or reject a NULL/invalid rkey handle early) in the cuda_ipc mem_elem_pack implementation.

Full analysis

Summary: coverity devel on coverity_rh7 build 134432 failed at the "Coverity found 1 issues" gate (nerrors=1) — the build and analysis both succeeded; the job failed only because a new defect was reported.

Root cause: In src/ucp/core/ucp_device.c:230 the local-memory path builds const uct_rkey_bundle_t invalid_rkey = { .rkey = UCT_INVALID_RKEY, .handle = NULL } and passes it to uct_md_mem_elem_pack() at line 257. uct_md_mem_elem_pack() (src/uct/base/uct_md.c:759) forwards rkey_ob->handle to md->ops->mem_elem_pack, which Coverity resolves to uct_cuda_ipc_md_mem_elem_pack(). That function assigns cuda_ipc_rkey_handle = rkey_handle (cuda_ipc_md.c:795) and unconditionally dereferences it at cuda_ipc_md.c:826 (release_handle->cache_enabled = cuda_ipc_rkey_handle->cache_enabled;) — an explicit NULL dereference. The cache_enabled read is new in this PR's head commit; before it, the rkey_handle argument was not dereferenced, so passing a NULL handle from the local path was harmless. Note the same path also casts rkey (UCT_INVALID_RKEY) to uct_cuda_ipc_unpacked_rkey_t* and dereferences it at line 811/819, so the local path is not safe for this MD at all.

Implicated commit: [REDACTED:Hex High Entropy String] — Raul Akhmetshin, "UCT/API/V2: mem_elem_release supports rkey lifetime flag." (touches both ucp_device.c and cuda_ipc_md.c)

File: src/uct/cuda/cuda_ipc/cuda_ipc_md.c:826 (defect reported at src/ucp/core/ucp_device.c:257)

Suggested fix: Make uct_cuda_ipc_md_mem_elem_pack() tolerate the local/invalid rkey bundle. Either:

1. Bail out early: if ((rkey == UCT_INVALID_RKEY) || (rkey_handle == NULL)) return UCS_ERR_UNSUPPORTED; (with an ucs_assert/debug message), which also documents that cuda_ipc cannot pack a local mem element; or
2. Guard just the new field: release_handle->cache_enabled = (cuda_ipc_rkey_handle != NULL) ? cuda_ipc_rkey_handle->cache_enabled : 0;

Option 1 is preferable since key (derived from UCT_INVALID_RKEY) is dereferenced at lines 811/819 as well. Alternatively, in ucp_device_local_mem_list_element_pack() skip/reject MDs whose mem_elem_pack requires a valid remote rkey handle rather than passing a synthetic invalid_rkey. The full defect listing is in the published coverity_devel artifact (output/errors/index.html) and cov_error_log.txt.

Related: PR #11870 (this change); prior commits in the same series: d466e65 "UCT/API/V2: Rkey resource lifetime.", a47066b (#11730) "UCT/CUDA/CUDA_IPC: Unmap memory handle at ucp_device_mem_list_release."

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 2326b115-3d5e-47f5-8839-97bf3d8d8012 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]

src/uct/cuda/cuda_ipc/cuda_ipc_md.c

Masked map-failure in uct_cuda_ipc_is_peer_accessible: on the bound_lifetime && *accessible == UCS_YES re-map path, a failed uct_cuda_ipc_map_memhandle is not reflected in the return status — the status assignment is guarded by if (*accessible == UCS_TRY), so *accessible stays UCS_YES and the function returns UCS_OK with mapped_addr == NULL. map_memhandle can return a hard error (e.g. from uct_cuda_ipc_open_memhandle), so this residual failure mode is real. This is the main correctness risk worth resolving before merge.

contrib/test_jenkins.sh

CI env-var swap: confirm the memtype cache no longer needs disabling now that UCX_MEMTYPE_CACHE=n is replaced by -x UCX_TLS=^gdr_copy -x UCX_RCACHE_ENABLE=n.

test/mpi/test_ucp_rkey_destroy.c

Test-coverage gap: only the happy path of the bound-lifetime re-map is exercised; the failure path (and thus the masked-status concern) is untested.

[svc-nvidia-pr-review]

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

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Tests roce on worker 3) · commit 69359200

TL;DR: The roce worker job didn't fail on an assertion — examples/ucp_client_server hung for ~19 minutes on the first message size that triggers rendezvous (-c am -s 65534) until the Azure agent was killed; the hang is in the rkey-unpack/rendezvous path touched by this PR branch, most likely the new skip_md_map handling in ucp_ep_rkey_unpack_internal() which leaves MDs in rkey->md_map with UCT_INVALID_RKEY/cmpt=NULL.

Full analysis

Summary: Tests roce on worker 3 (build 134947) — ./contrib/test_jenkins.sh → run_ucp_client_server hung at "UCP client-server with host memory type using am API with msg_size={65534}"; the job was later terminated by the agent shutdown ("The operation was canceled", orphan ucp_client_server pids 3459283/3459945 terminated).

Root cause: Evidence from the log:

• 13:01:49.07 — client launched: taskset -c 11 ./examples/ucp_client_server -m host -c am -s 65534 -a 1.4.4.1 -p 16504
• next line: 13:20:39.84 — ##[error]The agent has received a shutdown signal
  That is an 18m50s gap with zero output, while every previous size (1/16/256/4096) completed in <1s and printed Client: iteration #1 … received FIN message. So this is a hang, not a slow test (run_client_server_app in contrib/test_jenkins.sh:201 does a bare wait ${client_pid} with no timeout, so a hung client blocks the agent until it is killed).

65534 B is the first size above the AM rendezvous threshold, i.e. the first case that exercises rkey pack/unpack + rndv GET-zcopy — exactly the code this branch changes. The branch only touches src/ucp/api/ucp.h, src/ucp/core/ucp_ep.c, src/ucp/core/ucp_rkey.c (15c130f) and src/uct/api/v2/uct_v2.h, src/uct/base/uct_md.c (f57426f, 6935920). In the new unpack code, MDs listed in skip_md_map are given rkey.rkey = UCT_INVALID_RKEY, handle = NULL, cmpt = NULL but their bit is kept in rkey->md_map (src/ucp/core/ucp_rkey.c:1006-1014), and that same md_map is what ucp_rkey_proto_resolve()/ucp_rkey_find_rma_lane() use for lane selection. A lane whose dst_md_index was skipped can therefore be selected and the rndv GET/PUT zcopy posted with an invalid rkey (or with a NULL component), which never completes → silent hang with no error printed. A secondary latent issue in the same function: unpack_params.flags is only initialized when UCP_EP_EXT_FLAG_RKEY_BOUND_LIFETIME is set (lines 977-982), so any transport reading params->flags without checking field_mask sees stack garbage.

Implicated commit: 15c130f "UCP/API: Endpoint parameter controlling rkey resources." — Raul Akhmetshin (branch head 6935920 "UCT/API/V2: mem_elem_release supports rkey lifetime flag.")

File: src/ucp/core/ucp_rkey.c:986-1037 (skip_md_map branch at 1006-1014; unpack_params init at 977-982); hang observed via contrib/test_jenkins.sh:201

Suggested fix:

1. Do not keep skipped MDs in the map used for lane/protocol selection: either clear rkey->md_map &= ~UCS_BIT(remote_md_index) for skip_md_map entries (and don't advance rkey_index), or route them through unreachable_md_map so ucp_rkey_proto_resolve() excludes those lanes. Add an assertion in ucp_rkey_find_rma_lane()/rndv lane selection that the chosen lane's rkey is not UCT_INVALID_RKEY unless the MD has no UCT_MD_FLAG_NEED_RKEY.
2. Always initialize unpack_params.flags = 0 and set UCT_RKEY_UNPACK_FIELD_FLAGS unconditionally, so no transport can read uninitialized flags.
3. Reproduce locally: UCX_TLS=rc_x ./examples/ucp_client_server -m host -c am -s 65534 (server+client) and grab a backtrace of the hung client (gdb -p) to confirm it is stuck in the rndv GET completion.
4. Independent CI hardening: wrap the client/server run in timeout inside run_client_server_app (contrib/test_jenkins.sh) so a hang fails the job in minutes with a stack dump instead of stalling the agent for 19 min.

Related: PR #11870 (this branch, "UCP/API: Endpoint parameter controlling remote key resources"); no existing issue matches this hang.

[ofirfarjun7]

AFAIU it is not bound to the rkey. We release it when cache refcount is zero not necessarily when we destroy the rkey.
Maybe we need to change the description and the naming.

[ofirfarjun7]

lets use uct_cuda_ipc_component.enable_remote_cache here.

[ofirfarjun7]

The more I think about it the more I think we should introduce ucp_ep_unpack_v2 func...