Skip to content

fix(rocr): honor NUMA memory policy for host allocations - #14056

Merged
shwetagkhatri merged 5 commits into
developfrom
users/cfreehil/ROCM-33395-honor-mempolicy
Nov 6, 2026
Merged

shwetagkhatri merged 5 commits into
developfrom
users/cfreehil/ROCM-33395-honor-mempolicy

Conversation

@cfreeamd

@cfreeamd cfreeamd commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

bind_mem_to_numa() unconditionally issues an mbind(MPOL_PREFERRED | MPOL_F_STATIC_NODES, <owning CPU agent's NUMA node>) on every host-pool allocation. An explicit-range mbind outranks the process/task memory policy, so numactl -m / set_mempolicy() are silently inert for ROCr host buffers: the runtime always wins, and HPC jobs that pin memory by policy cannot place GPU-accessed host memory (kernarg, completion signals, RPC buffers) where they asked. This is the explicit-policy half of ROCM-32080 (the no-policy default half — callers selecting CPU-agent-0 host pools — is fixed upstream in llvm-project PR #233099; ROCr's per-agent binding is correct there).

Technical Details

Defer to an explicitly-set process memory policy: when the process has one, skip the per-agent mbind and let the policy govern host placement. Behavior is unchanged when no policy is set.

  • Skip the per-agent mbind when get_mempolicy() reports a non-default policy (masking off the MPOL_F_STATIC_NODES / MPOL_F_RELATIVE_NODES mode flags before the comparison). Returning 0 lets the allocation proceed under the process policy and is also correct for the NoSubstitute path.
  • HSA_FORCE_MBIND=1 restores the legacy per-agent binding (escape hatch).
  • Env var plumbed the thunk-idiomatic way (matching HSA_ZFB / PM4_TARGET_XCC): hsakmt_force_mbind global in globals.c, read once at init in openclose.c:init_vars_from_env() via hsakmt_safe_env_to_int, declared extern in libhsakmt.h — no inline getenv in the alloc path.

Note: cpuset.mems is kernel-enforced and mbind cannot escape it, so no ROCr change is needed for the cgroup case.

Issue Tracking

JIRA ID : ROCM-33395

Relates to ROCM-32080. Complements upstream llvm-project PR #233099 (the no-policy default case).

Test Plan

Validated on b06-2 (2-socket EPYC 9655 + 8× MI350X gfx950, ROCm 10.2), node-1 GPU with the process pinned to node 1, tracing allocations with strace -f -e trace=mbind. Four cases covering the honor path, the legacy override, and no-regression:

  • Stock + numactl -m 1
  • Patched + numactl -m 1
  • Patched + numactl -m 1 + HSA_FORCE_MBIND=1
  • Patched, no policy

Test Result

Case Placement mbind Expected
Stock + policy node 0 present bug reproduced
Patched + policy node 1 none policy honored
Patched + policy + HSA_FORCE_MBIND=1 node 0 present legacy restored
Patched, no policy node 0 present no regression

All four cases PASS.

Submission Checklist

…ions

bind_mem_to_numa() unconditionally mbind()s host-pool allocations to the
owning CPU agent's NUMA node, overriding an explicitly-set process memory
policy (numactl -m / set_mempolicy). Skip the per-agent mbind when the
process has set an explicit policy (get_mempolicy() != MPOL_DEFAULT) so the
policy governs host placement. HSA_FORCE_MBIND=1 restores the legacy
per-agent binding. No change when no policy is set.

Relates to ROCM-32080; tracked by ROCM-33395.
Copilot AI balanced review requested due to automatic review settings October 22, 2026 18:48
@therock-pr-bot

therock-pr-bot Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ 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/rocr-runtime/libhsakmt/src/fmm.c, projects/rocr-runtime/libhsakmt/src/globals.c, projects/rocr-runtime/libhsakmt/src/libhsakmt.h, projects/rocr-runtime/libhsakmt/src/openclose.c; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

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

🙋 Wish to Override Policy?

@therock-pr-bot

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.

🟡 Changes recommended

The new shortcut bypasses required placement checks and binding for NoSubstitute allocations, including APU device allocations.

1 open issue
What changed in this PR

This PR updates ROCr host-memory placement to honor explicit NUMA memory policies instead of always binding allocations to the owning CPU agent’s node.

Changes:

  • Detect explicit memory policies and skip per-agent binding when one is set.
  • Add HSA_FORCE_MBIND to restore legacy binding behavior.
  • Preserve existing placement behavior when no explicit policy is set.
File Description
projects/​rocr-runtime/​libhsakmt/​src/​openclose.c Read the HSA_FORCE_MBIND environment setting.
projects/​rocr-runtime/​libhsakmt/​src/​libhsakmt.h Declare the shared binding override.
projects/​rocr-runtime/​libhsakmt/​src/​globals.c Define and document the binding override.
projects/​rocr-runtime/​libhsakmt/​src/​fmm.c Detect explicit NUMA policies and conditionally bypass binding.

Review effort: Balanced

@cfreeamd cfreeamd left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 AI-assisted pre-review (for @cfreeamd)

fmm.c, globals.c, libhsakmt.h, openclose.c all look correct.

@dayatsin-amd

Copy link
Copy Markdown
Contributor

LGTM, nit pick. Please adjust your AI agent to use the "new" PR description format. You can use this PR as an example:
#13908

Claude Agent and others added 2 commits October 31, 2026 21:32
The early return that defers host placement to an explicit process memory
policy ran for every allocation, including strict (NoSubstitute) binds. The
udmabuf APU path forces NoSubstitute=1 to pin host memory to the GPU's nearest
CPU node for hardware locality; deferring there silently dropped that bind and
returned success, violating the no-fallback contract.

Gate the deferral on !NoSubstitute so only substitutable host-pool allocations
(kernarg / signals / RPC) honor the policy, while strict binds keep their hard
per-agent mbind and error handling. Rename the helper to reflect that Linux
mempolicy is per-thread (numactl-set policy is inherited by every thread).

Relates to ROCM-32080; tracked by ROCM-33395.

@shwetagkhatri shwetagkhatri 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.

Accepted and LGTM. Both points are addressed - the early return is now gated on NoSubstitute, and the helper is renamed to thread_has_explicit_mempolicy. Thanks Chris.

@shwetagkhatri
shwetagkhatri merged commit 7c96e6c into develop Nov 6, 2026
213 of 214 checks passed
@shwetagkhatri
shwetagkhatri deleted the users/cfreehil/ROCM-33395-honor-mempolicy branch November 6, 2026 19:38
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.

4 participants