Skip to content

fix(rocshmem): [gda/mlx5] fix dmabuf-backed umem registrations - #13220

Merged
omor1 merged 1 commit into
developfrom
users/omor1/rocshmem/fix/mlx5_dmabuf
Nov 6, 2026
Merged

omor1 merged 1 commit into
developfrom
users/omor1/rocshmem/fix/mlx5_dmabuf

Conversation

@omor1

@omor1 omor1 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Motivation

Registering dmabuf-backed mlx5 umem regions using DevX is broken due to incorrect API usage, resulting in runtime errors if the backing memory for the SQ, CQ, and dbrec is changed from host to device memory on a system using dmabuf.

The mlx5 provider currently places the SQ, CQ, and dbrec in host memory rather than device memory, so this feature is not currently used.

Technical Details

Registering dmabuf-backed regions using mlx5dv_devx_umem_reg_ex requires passing in the dmabuf offset as mlx5dv_devx_umem_in::addr rather than the actual address of the buffer.

Issue Tracking

Test Plan

Validate correct registration of dmabuf-backed umem regions on mlx5 hardware.

Test Result

Submission Checklist

@omor1
omor1 requested a review from a team as a code owner October 9, 2026 22:20
Copilot AI balanced review requested due to automatic review settings October 9, 2026 22:20
Registering dmabuf-backed regions using mlx5dv_devx_umem_reg_ex requires
passing in the dmabuf offset as mlx5dv_devx_umem_in::addr rather than
the actual address of the buffer.

The mlx5 provider currently places the SQ, CQ, and dbrec in host memory
rather than device memory, so this feature is not currently used.

Signed-off-by: Omri Mor <Omri.Mor@amd.com>
@therock-pr-bot

therock-pr-bot Bot commented Oct 9, 2026

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
• JIRA ID : TESTAUTO-6039
• JIRA ID - #330
• JIRA ID #330
• JIRA ID (on separate line)
ROCM-25757
• ISSUE ID : TESTUTO-3334
• ISSUE ID #3334
• ISSUE ID - TESTAUTO-3433
• ISSUE ID (on separate line)
AIRUNTIME-2352
• ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
• Closes #10
• Fixes octo-org/octo-repo#100
• Resolves: #123
• #123
• https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rocshmem/src/gda/mlx5/provider_gda_mlx5.cpp; no test file found
🔎 pre-commit ⏳ Pending ⏳ Still running…
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Oct 9, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

Copilot AI 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.

🟢 Approval recommended

The focused change matches the mlx5 DevX API contract and HSA-exported offset semantics.

0 open findings

What changed in this PR

Corrects mlx5 DevX registration for dma-buf-backed device memory.

Changes:

  • Passes the exported dma-buf offset as mlx5dv_devx_umem_in::addr.
File Description
projects/​rocshmem/​src/​gda/​mlx5/​provider_gda_mlx5.cpp Fixes dma-buf UMEM registration address semantics.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@omor1

omor1 commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

There may be additional issues dmabuf-backed umem registrations, but I'm merging this for now until those issues can be evaluated. In any case this PR is very low-risk since we do not currently place QP structures in device memory, so we do not use dmabuf-backed umem registrations.

@omor1
omor1 merged commit 12e1ea0 into develop Nov 6, 2026
115 of 128 checks passed
@omor1
omor1 deleted the users/omor1/rocshmem/fix/mlx5_dmabuf branch November 6, 2026 02:02
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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