Skip to content

[TransferEngine] Clamp RDMA QP atomic depths to device limits - #3977

Draft
junyyang-amd wants to merge 1 commit into
kvcache-ai:mainfrom
Jasen2201:junyyang/ionic-qp-atomic-limits
Draft

junyyang-amd wants to merge 1 commit into
kvcache-ai:mainfrom
Jasen2201:junyyang/ionic-qp-atomic-limits

Conversation

@junyyang-amd

@junyyang-amd junyyang-amd commented Sep 9, 2026

Copy link
Copy Markdown

Description

Creating multiple RC QPs can fail on AMD Pensando Ionic devices with Failed to modify QP to RTR ... Invalid argument. These devices advertise max_res_rd_atom=16 for the whole device, while Transfer Engine currently requests a responder depth of 16 for every QP. The first QP can exhaust that budget, preventing even the second QP of an endpoint from reaching RTR.

Select the responder and initiator depths from the queried device capabilities and use them in the RTR/RTS transitions:

  • Share the responder capacity across a nominal budget of 16 QPs, with a minimum depth of 1 for nonzero capacity; cap the result at the existing default of 16 and max_qp_rd_atom.
  • Bound the initiator depth by that selected depth and max_qp_init_rd_atom; preserve zero capabilities.
  • Log the selected depths and device limits. Add nine regression cases covering Ionic, larger devices, per-QP limits, and zero capabilities.

On Ionic with all three capabilities equal to 16, this selects responder/initiator depths of 1/1 instead of 16/16. Devices with sufficient shared and per-QP capacity retain 16/16. Devices with smaller budgets may trade read concurrency for QP availability. This is a conservative per-QP allocation policy; it does not implement device-wide admission control across arbitrary endpoint/process counts or negotiate depths with heterogeneous peers.

Module

  • Transfer Engine (mooncake-transfer-engine)

Type of Change

  • Bug fix

How Has This Been Tested?

Validation against upstream main b9059252aa8db150ada8220241707d47e27ebb79 plus this patch, using rocm/atom-dev:mooncake_v0.3.14-rc1 as the build environment:

cmake -S . -B /validation/build-cpu \
  -DCMAKE_BUILD_TYPE=Release -DWITH_TE=ON -DWITH_STORE=OFF \
  -DWITH_STORE_RUST=OFF -DUSE_TENT=OFF -DUSE_CUDA=OFF -DUSE_HIP=OFF \
  -DUSE_ETCD=OFF -DUSE_HTTP=OFF -DBUILD_UNIT_TESTS=ON -DBUILD_EXAMPLES=ON
cmake --build /validation/build-cpu --target rdma_context_reprobe_test \
  rdma_endpoint_state_test rdma_endpoint_reestablish_test transfer_engine_validator -j 8
ctest --test-dir /validation/build-cpu --output-on-failure \
  -R '^(rdma_context_reprobe_test|rdma_endpoint_state_test)$'

bash scripts/code_format.sh --changed-lines --check --base upstream/main
pre-commit run --files \
  mooncake-transfer-engine/include/transport/rdma_transport/rdma_context.h \
  mooncake-transfer-engine/src/transport/rdma_transport/rdma_context.cpp \
  mooncake-transfer-engine/src/transport/rdma_transport/rdma_endpoint.cpp \
  mooncake-transfer-engine/tests/rdma_context_reprobe_test.cpp
  • CPU Transfer Engine, validator, and the three listed test binaries built successfully.
  • rdma_endpoint_state_test: 8 passed.
  • rdma_context_reprobe_test: 18 passed, including all nine added cases; 1 existing XDR test skipped because the installed RDMA headers lack extended speed support.
  • Changed-line clang-format 20.1.8 and all applicable pre-commit hooks passed.
  • HIP Transfer Engine/validator build passed with USE_HIP=ON, USE_HIP_DMABUF=ON, BUILD_UNIT_TESTS=OFF, and BUILD_EXAMPLES=ON (otherwise the same configuration). CMake reported HIP dmabuf MR registration enabled (hsa-runtime64 found).

Use the device responder budget and per-QP limits when selecting RTR and RTS atomic depths. Ionic devices advertise 16 shared responder resources, so reserving the previous fixed depth of 16 can prevent a second QP from reaching RTR.

Port the QP compatibility fix from Jasen2201/Mooncake to upstream main, with capability regression coverage. The nominal 16-QP budget is a conservative per-QP policy, not global resource admission control.
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.

1 participant