Skip to content

[Bugfix] Resolve RDMA NUMA location after memory registration - #4333

Open
jacklin78911-collab wants to merge 2 commits into
kvcache-ai:mainfrom
jacklin78911-collab:codex/rdma-registration-numa
Open

jacklin78911-collab wants to merge 2 commits into
kvcache-ai:mainfrom
jacklin78911-collab:codex/rdma-registration-numa

Conversation

@jacklin78911-collab

@jacklin78911-collab jacklin78911-collab commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Description

Untouched anonymous host memory has no NUMA node before registration. Resolving its location at that point records * in the RDMA buffer metadata, allowing NIC selection across unrelated NUMA nodes even after registration pins the pages. This is the registration-order regression reported in #4322.

Resolve the original buffer's first-page location after the first chunk's MRs are registered, then reuse that location for all chunks. Explicit locations, registration parallelism and transfer-path selection are unchanged. A failed NUMA query retains the existing * fallback; it does not trigger registration rollback.

This addresses #4322 without a new placement policy or pre-touch pass. #3657 concerns Store allocation placement; #1974 concerns GPU/PCI topology ranking; neither fixes this registration ordering.

Module

  • Transfer Engine (mooncake-transfer-engine)

Type of Change

  • Bug fix

How Has This Been Tested?

Linux CPU build from 16b7ba3c, USE_CUDA=OFF, USE_TENT=OFF:

cmake --build build --target rdma_memory_location_test rdma_context_reprobe_test rdma_buffer_range_index_test rdma_transport_submit_task_test -j4
ctest --test-dir build --output-on-failure --no-tests=error \
  -R '^(rdma_memory_location_test|rdma_context_reprobe_test|rdma_buffer_range_index_test|rdma_transport_submit_task_test)$'
  • The new test interposes the verbs/NUMA boundary and exercises the real registration, MR-map and metadata code. Untouched single-MR and chunked buffers publish * on the original code and cpu:1 with this fix.
  • All four CTest targets pass. The new test also covers explicit location preservation and rollback after a later chunk fails.
  • Hardware throughput recovery has not been measured here; the dual-host performance numbers in [Bug]: Host RDMA regression from resolving NUMA before memory registration (0.3.13.post1) #4322 belong to the issue reporter.

Checklist

  • I have performed a self-review of my own code (human review pending; draft)
  • I have formatted my code using ./scripts/code_format.sh
  • I have run pre-commit on the files changed in this PR and all hooks pass
  • I have added tests to prove my changes are effective
  • Documentation / RFC: not applicable; no API or configuration change.

AI Assistance Disclosure

  • AI tools were used

@staryxchen staryxchen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. @stmatengss please double-check, thx~

Comment on lines +526 to +529
if (entries.empty()) {
unregisterChunkMRs(chunk_addr);
return -1;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

getMemoryLocation() never returns an empty vector. A missing NUMA node and a failed numa_move_pages both come back as one entry with location *, so this entries.empty() branch cannot run. On a real query failure the code publishes * and does not roll back.
Delete the empty check and use entries[0].location directly. LaterRegistrationFailureStillRollsBack covers a later ibv_reg_mr failure, not this branch.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants