Skip to content

feat(rocr): Bind helper threads to the creating thread's NUMA node(s) - #14742

Merged
saleelk merged 1 commit into
developfrom
skudchad/AILIROCR-373-numa-helper-affinity
Nov 3, 2026
Merged

saleelk merged 1 commit into
developfrom
skudchad/AILIROCR-373-numa-helper-affinity

Conversation

@saleelk

@saleelk saleelk commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

When HSA_OVERRIDE_CPU_AFFINITY_DEBUG is unset or 1 (the default), ROCr helper threads (async signal handler, exception/VM-fault handler, IPC server, PC sampling) are allowed on every CPU. On multi-socket systems such as MI300A the OS can then schedule them on a NUMA node far from the application and its GPU, and performance drops for applications that pin their ranks to GPU-local cores.

This implements the behavior agreed in AILIROCR-373 and supersedes #4830.

Technical Details

All helper threads are created through os::CreateThread in os_linux.cpp, so the change is made there:

  • Default (HSA_OVERRIDE_CPU_AFFINITY_DEBUG unset or 1): helper threads get every CPU of each NUMA node the creating thread may run on. An unrestricted creator spans all nodes and so still gets every CPU, as before. If NUMA information is unavailable (no NUMA support in the kernel, unreadable sysfs), helper threads fall back to all CPUs.
  • HSA_OVERRIDE_CPU_AFFINITY_DEBUG=0: unchanged, helper threads inherit the creator's exact mask.

The node CPU lists are read from /sys/devices/system/node/node*/cpulist and the creator's mask from sched_getaffinity, both libc only. libnuma is not used because os_linux.cpp is also compiled into targets without libnuma, such as the WSL rocdxg library. Unlike #4830, there is no limit on the number of NUMA nodes, and nodes without memory are handled.

env_variables.rst is updated to describe the new default behavior.

Known limitation: a helper thread's mask is computed once, when the thread is created. The exception handler thread is created inside hsa_init, so pinning that the application applies after hsa_init (for example OpenMP binding at the first parallel region) is not reflected. Launcher pinning (srun --cpu-bind, mpirun --bind-to, taskset, numactl) happens before the process starts and is covered.

Alternatives considered: binding to the GPU's NUMA node (one async thread serves all GPUs, and ROCr cannot tell at init which GPUs the application uses), one async thread per GPU (larger redesign of signal waiting), and re-syncing the mask after creation (adds polling for an uncommon case).

Issue Tracking

JIRA ID : AILIROCR-373

Test Plan

Built libhsa-runtime64. Ran a test that starts the async signal handler and prints every thread's Cpus_allowed_list, on a 2-node system (node 0: 0-55,112-167; node 1: 56-111,168-223).

Test Result

Launch Helper threads allowed on
no pinning 0-223
taskset -c 2-3 0-55,112-167 (node 0)
taskset -c 60 56-111,168-223 (node 1)
taskset -c 2,60 0-223 (both nodes)
HSA_OVERRIDE_CPU_AFFINITY_DEBUG=0 taskset -c 2-3 2-3 (exact inheritance)

Submission Checklist

@github-actions github-actions Bot added documentation Improvements or additions to documentation project: rocr-runtime labels Oct 30, 2026
@therock-pr-bot

therock-pr-bot Bot commented Oct 30, 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/runtime/hsa-runtime/core/util/lnx/os_linux.cpp; 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.

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

LGTM.

When HSA_OVERRIDE_CPU_AFFINITY_DEBUG is unset or 1, ROCr helper threads
(e.g. the async signal handler) were allowed on every CPU, so the OS could
schedule them on a NUMA node far from the application and its GPU.

Helper threads now get every CPU of each NUMA node the creating thread may
run on. An unrestricted creator spans all nodes and so still yields every
CPU. If NUMA information is unavailable, helper threads fall back to all
CPUs. HSA_OVERRIDE_CPU_AFFINITY_DEBUG=0 still inherits the creator's exact
mask.

The node CPU lists are read from sysfs rather than through libnuma, because
os_linux.cpp is also compiled into targets without libnuma (e.g. the WSL
rocdxg library).

JIRA: AILIROCR-373
@saleelk
saleelk force-pushed the skudchad/AILIROCR-373-numa-helper-affinity branch from b741de2 to 2ebce75 Compare November 3, 2026 03:04

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

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

Re-review after implementation rework. The previous draft flagged a [Important] null check after numa_allocate_cpumask() — the libnuma dependency has been removed entirely; the new implementation reads NUMA topology directly from sysfs (/sys/devices/system/node/). That finding is resolved.

The sysfs-based FillCpuSetFromCallerNumaNodes looks correct: CPU_ALLOC/CPU_FREE(nullptr) scope-guard handles partial allocation; sched_getaffinity check gates on success; opendir/readdir traverses nodes; CPU_AND_S intersection skips nodes with no overlap against the caller's affinity; inner loop correctly bounds to cores. ReadCpuList parses the cpulist format correctly, and the zero-CPU case is caught by the caller's CPU_COUNT_S guard. Fallback to all-cores on failure preserves the original behavior.

Two gating approvals on record (shwetagkhatri, dayatsin-amd). driver-value-validation pattern flag assessed: fscanf return checked via loop condition; zero-result handled by caller — false positive for this change.

@saleelk
saleelk merged commit 840728c into develop Nov 3, 2026
408 of 413 checks passed
@saleelk
saleelk deleted the skudchad/AILIROCR-373-numa-helper-affinity branch November 3, 2026 19:53
@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.

4 participants