Skip to content

[rocshmem] gda/mlx5: skip get_lkey() for inline RMA sends#7294

Closed
thomas-huber wants to merge 1 commit into
developfrom
users/thomas-huber/AICOMRCCL-1081-fix-mellanox-bug-in-rocshmem
Closed

[rocshmem] gda/mlx5: skip get_lkey() for inline RMA sends#7294
thomas-huber wants to merge 1 commit into
developfrom
users/thomas-huber/AICOMRCCL-1081-fix-mellanox-bug-in-rocshmem

Conversation

@thomas-huber

Copy link
Copy Markdown
Contributor

mlx5_post_wqe_rma_single() computed get_lkey(laddr) unconditionally when building the RMA WQE. For inline sends the payload is carried inside the WQE itself, so no local key is required, and get_lkey() calls LOGD_ERROR_ABORT when laddr is not in the symmetric heap or a registered buffer. The default GDA alltoallv path (ROCSHMEM_GDA_ALLTOALLV_WG_ALGO=GET) posts an 8-byte inline control message from a stack-local address, which tripped this abort and surfaced as a GPU HSA_STATUS_ERROR_EXCEPTION (0x1016).

Guard the local key as send_inline ? 0 : get_lkey(laddr), matching the wavefront variant mlx5_post_wqe_rma() which already handles inline sends correctly. Reproduces on develop, independent of any consumer (e.g. RCCL).

Motivation

The GDA/MLX5 backend aborts on any inline RMA send whose local address is not in the symmetric heap or a registered buffer. This makes the default GDA alltoallv (ROCSHMEM_GDA_ALLTOALLV_WG_ALGO=GET) unusable on Mellanox/CX7 NICs, since that path posts an inline control message from a stack-local variable. The goal is to make inline sends work regardless of where the local buffer lives, since an inline send never dereferences a local key.

Technical Details

  • src/gda/mlx5/queue_pair_mlx5.cpp, QueuePair::mlx5_post_wqe_rma_single(): change the WQE's local key from get_lkey(laddr) to send_inline ? 0 : get_lkey(laddr).
  • For an inline send the payload is embedded in the WQE, so the local key is ignored by hardware; computing it served no purpose and forced get_lkey() to validate laddr, which calls LOGD_ERROR_ABORT for non-registered/non-symmetric addresses.
  • This mirrors the already-correct wavefront variant mlx5_post_wqe_rma(), which has guarded the local key with send_inline ? 0 : get_lkey(laddr) — the single-QP variant was simply missing the same guard.
  • No behavior change for non-inline sends (the get_lkey(laddr) path is unchanged).

JIRA ID

Resolves AICOMRCCL-1081

Test Plan

  • Built the GDA/MLX5 backend and exercised the default GDA alltoallv path (ROCSHMEM_GDA_ALLTOALLV_WG_ALGO=GET), which posts an inline control message from a stack-local address.
  • Reproduced via RCCL + rocSHMEM on MI300X nodes with Mellanox CX7 NICs, running rccl-tests alltoall and alltoallv across 2 nodes (16 GPU) and 4 nodes (32 GPU).
  • Sizes swept b=1 to e=512M (alltoallv) / e=1G (alltoall).

Test Result

  • Before: alltoallv aborted with HSA_STATUS_ERROR_EXCEPTION (0x1016) on the inline control-message send.
  • After: alltoall and alltoallv pass at both 2 and 4 nodes — Out of bounds values : 0 OK, exit 0, no abort. Non-inline RMA paths unaffected.

Submission Checklist

mlx5_post_wqe_rma_single() computed get_lkey(laddr) unconditionally when
building the RMA WQE. For inline sends the payload is carried inside the
WQE itself, so no local key is required, and get_lkey() calls
LOGD_ERROR_ABORT when laddr is not in the symmetric heap or a registered
buffer. The default GDA alltoallv path (ROCSHMEM_GDA_ALLTOALLV_WG_ALGO=GET)
posts an 8-byte inline control message from a stack-local address, which
tripped this abort and surfaced as a GPU HSA_STATUS_ERROR_EXCEPTION (0x1016).

Guard the local key as `send_inline ? 0 : get_lkey(laddr)`, matching the
wavefront variant mlx5_post_wqe_rma() which already handles inline sends
correctly. Reproduces on develop, independent of any consumer (e.g. RCCL).
@abouteiller

abouteiller commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

I did the same work concurrently in #7304 can you verify its covered, but I did run the alltoallv tester after the refactor.

@omor1

omor1 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

This is also fixed in the large refactor #7217. The bug was initially introduced by #5711.

@abouteiller

Copy link
Copy Markdown
Contributor

Not needed anymore as #7304 merged

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.

3 participants