Skip to content

UCP/RNDV: Use local memory detection for RTR_REQ responder - #12041

Merged
tvegas1 merged 6 commits into
openucx:masterfrom
guy-ealey-morag:rtr-req-memory-detect
Oct 12, 2026
Merged

tvegas1 merged 6 commits into
openucx:masterfrom
guy-ealey-morag:rtr-req-memory-detect

Conversation

@guy-ealey-morag

@guy-ealey-morag guy-ealey-morag commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What?

The get/rndv responder (ucp_proto_rndv_rtr_req_sreq_init) now takes its buffer's memory info from ucp_memory_detect() instead of the RTR_REQ header's mem_type/sys_dev, asserting that the memory type matches. This keeps a GPU's host-staged get traffic on the NICs assigned to that GPU when using gpu-nic assignment

Why?

Currently the requester fills RTR_REQ.sys_dev from the responder's rkey, which has no sys_dev when no NIC can register the buffer. The send then has no owning GPU, and the GPU-NIC assignment doesn't restrict its lanes.

This matters only when both are true:

  • The responder's buffer can't be registered by any NIC: CUDA localized memory, any CUDA memory under MPS MLOPart, or CUDA memory without GPUDirect RDMA.
  • The responder stages through host fragments. CUDA fragments already carry the GPU's device.

Comparison with put: the RMA put/rndv receiver (ucp_rma_rndv_process_rts) has ignored rts->sys_dev and detected its buffer locally, with the same mem_type assertion, since #11553. This change makes both handlers detect locally.

Signed-off-by: Guy Ealey Morag <gealeymorag@nvidia.com>
@svc-nvidia-pr-review

Copy link
Copy Markdown

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

Comment thread src/ucp/rndv/proto_rndv.c Resolved
Comment thread src/ucp/rndv/proto_rndv.c Resolved
Comment thread src/ucp/rndv/proto_rndv.c Resolved
Signed-off-by: Guy Ealey Morag <gealeymorag@nvidia.com>
@svc-nvidia-pr-review

Copy link
Copy Markdown

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

Comment thread src/ucp/rndv/proto_rndv.c Resolved
Comment thread src/ucp/rndv/proto_rndv.c Resolved
@svc-nvidia-pr-review

Copy link
Copy Markdown

Coverage note: No test is added. The change is an internal invariant plus an assertion on an existing flow, so the existing RMA GET/rndv coverage (test/gtest/ucp/test_ucp_rma.cc plus the CUDA rndv tests) should exercise it — but the new assert is only meaningful in GPU/registered-memory configurations, so if CI for this PR runs host-only jobs, the assert is effectively untested.

@tomerg-nvidia tomerg-nvidia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a test for this?

tomerg-nvidia
tomerg-nvidia previously approved these changes Oct 8, 2026
Comment thread src/ucp/rndv/proto_rndv.c Resolved
Comment thread src/ucp/rndv/proto_rndv.c Resolved
Comment thread test/gtest/ucp/test_ucp_rma.cc Resolved
Signed-off-by: Guy Ealey Morag <gealeymorag@nvidia.com>
tomerg-nvidia
tomerg-nvidia previously approved these changes Oct 9, 2026
Comment thread test/gtest/ucp/test_ucp_rma.cc Resolved
@svc-nvidia-pr-review

Copy link
Copy Markdown

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

Signed-off-by: Guy Ealey Morag <gealeymorag@nvidia.com>
@svc-nvidia-pr-review

Copy link
Copy Markdown

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

@tvegas1
tvegas1 enabled auto-merge (squash) October 9, 2026 12:25
@svc-ucx

svc-ucx commented Oct 9, 2026

Copy link
Copy Markdown

🤖 CI Triage Agent — UCX PR (AddressSanitizer gpu on worker 2) · commit ba0a4b6c

TL;DR: The ASAN GPU gtest run failed on a single test, shm_ib/test_ucp_proto_am_rndv.am_explicit_scheme_disables_force_provenance/0 — the AM rendezvous remote-protocol estimation no longer matches expectations when RNDV_SCHEME=get_zcopy is combined with RNDV_PIPELINE_SHM_CUDA_STAGING_FORCE=y on CUDA memory with sys_dev = UNKNOWN; the fix is in this branch's rendezvous memory-info derivation (proto_rndv.c), not in the test harness.

Full analysis

Summary: make test in the AddressSanitizer GPU job exited with Error 1 because 1 of 10333 gtests failed: shm_ib/test_ucp_proto_am_rndv.am_explicit_scheme_disables_force_provenance/0, where GetParam() = shm,ib,cuda_copy,rocm_copy.

Root cause: The log shows a clean, continuous run to teardown (no SIGTERM, no timeout, no ucs_assert abort — the process survived and reported a normal gtest summary), so this is a failed EXPECT_* inside check_am_rndv_remote_proto_config(0, 0), not a crash or hang. That helper is only exercised with RNDV_PIPELINE_SHM_CUDA_STAGING_FORCE=y plus RNDV_SCHEME=get_zcopy, and its two sibling tests (am_default_remote_model_preserves_legacy_key, am_force_remote_model_remains_untagged) passed — so the differentiator is the explicit RNDV_SCHEME path, where rndv_mode != UCP_RNDV_MODE_AUTO makes ucp_proto_rndv_shm_pipeline_force_enabled() return 0 and all non-forced remote variants report UCS_MEMUNITS_INF (src/ucp/rndv/proto_rndv.c:505-512). The test selects UCP_OP_ID_AM_SEND with mem_type = UCS_MEMORY_TYPE_CUDA and sys_dev = UCS_SYS_DEVICE_ID_UNKNOWN and then requires the 1 MB threshold protocol to be am/rndv with remote_proto_config->select_param.op_id_flags == UCP_OP_ID_RNDV_RECV and zero rkey configs flagged UCP_RKEY_CONFIG_FLAG_PROTO_ESTIMATION (test/gtest/ucp/test_ucp_proto.cc:439-460). The branch commits rework exactly how mem_type/sys_dev are derived and propagated for rendezvous (52287a08 "Replace rtr_req->mem_type with mem_info.type", 4fd34f04 "Use local memory detection for RTR_REQ responder", e55bf3e2 "Assert peer-provided sys_dev matches detected memory"), feeding ucp_proto_rndv_remote_select_param_init() (src/ucp/rndv/proto_rndv.c:370-377) and ucp_proto_rndv_ctrl_get_md_map() (:111-125), where an UNKNOWN mem_sys_dev combined with the recent GPU-NIC assignment filtering (abf206ae) can collapse md_map to 0 and leave no eligible remote RNDV_RECV protocol — so am/rndv is no longer the selected 1 MB protocol. That the branch's own follow-up commit ba0a4b6c is titled "TEST/UCP: Add asserts for unknown sys_dev and md_map=0" confirms unknown-sys_dev/empty-md_map is the live defect area in this branch.

Implicated commit: 52287a08 / 4fd34f04 / e55bf3e2 (Guy Ealey Morag) on branch rtr-req-memory-detect, interacting with abf206ae "UCP/PROTO: Apply GPU-NIC assignment to RMA and rendezvous lanes (#11985)" (same author)

File: src/ucp/rndv/proto_rndv.c:370-377 (remote select-param mem_info derivation) and src/ucp/rndv/proto_rndv.c:111-125 (md_map reachability filter); expectation at test/gtest/ucp/test_ucp_proto.cc:449-460

Suggested fix: Make the rendezvous remote-protocol estimation tolerant of UCS_SYS_DEVICE_ID_UNKNOWN: in ucp_proto_rndv_ctrl_get_md_map(), do not drop lanes (and thus do not empty md_map) when mem_sys_dev == UCS_SYS_DEVICE_ID_UNKNOWN — treat unknown as "reachable from all" / skip GPU-NIC assignment filtering — and keep ucp_proto_rndv_remote_select_param_init() from narrowing CUDA mem_info to an unknown device when the local sys_dev is unknown. Reproduce locally with UCX_TLS=shm,ib UCX_RNDV_PIPELINE_SHM_CUDA_STAGING_FORCE=y UCX_RNDV_SCHEME=get_zcopy ./gtest --gtest_filter='shm_ib/test_ucp_proto_am_rndv.am_explicit_scheme_disables_force_provenance/0' and dump ucp_ep_print_info to see which protocol replaces am/rndv; if the new assertion in ucp_proto_rndv_memory_detect() (src/ucp/rndv/proto_rndv.inl:597-606) is intended to be advisory, relax it to allow CUDA vs CUDA_MANAGED mismatches rather than asserting equality.

Related: PR #12041 (this change); #11985 / abf206ae (GPU-NIC assignment for RMA and rendezvous lanes); #11685 (remote system-device namespace isolation, origin of the sys_dev proxy logic)

@tvegas1
tvegas1 merged commit d8529c7 into openucx:master Oct 12, 2026
162 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants