Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/superbench-config.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ proc_num: int
node_num: int
env: dict
mca: dict
bind_to: string
prefix: str
parallel: bool
```
Expand Down Expand Up @@ -403,6 +404,7 @@ Some attributes may only be suitable for specific mode.
| `prefix` | ✓ | ✘ | ✘ |
| `env` | ✓ | ✓ | ✓ |
| `mca` | ✘ | ✘ | ✓ |
| `bind_to` | ✘ | ✘ | ✓ |
| `parallel` | ✓ | ✘ | ✘ |
| `pattern` | ✘ | ✘ | ✓ |

Expand Down Expand Up @@ -452,6 +454,16 @@ MCA (Modular Component Architecture) frameworks, components, or modules to use i
in a flatten key-value dictionary.
Only available for `mpi` mode.

### `bind_to`

Process binding policy passed to `mpirun -bind-to`.
Only available for `mpi` mode.

Use this option when a benchmark needs to override the runner's default MPI binding behavior,
for example when the benchmark implements its own topology-aware CPU/NUMA affinity logic.

* default value: `numa`

### `parallel`

Whether run benchmarks in parallel (all ranks at the same time) or in sequence (one rank at a time).
Expand Down
5 changes: 4 additions & 1 deletion superbench/runner/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ def __validate_sb_config(self): # noqa: C901
'btl_tcp_if_exclude': 'lo,docker0',
'coll_hcoll_enable': 0,
}
if 'bind_to' not in mode:
self._sb_benchmarks[name].modes[idx].bind_to = 'numa'
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.

[BLOCKER] Existing MPI tests will break

The 4 existing MPI test cases in test_get_mode_command construct mode dicts via OmegaConf.create(...) directly — bypassing __validate_sb_config. After this change, __get_mode_command accesses mode.bind_to in the .format() call, but those test mode dicts don't include bind_to. OmegaConf raises ConfigAttributeError: Missing key bind_to for missing keys.

Verified locally:

>>> from omegaconf import OmegaConf
>>> mode = OmegaConf.create({"name": "mpi", "proc_num": 8, "mca": {}, "env": {}})
>>> mode.bind_to
# ConfigAttributeError: Missing key bind_to

Fix: Add 'bind_to': 'numa' to all 4 existing MPI test case mode dicts in test_get_mode_command, and add at least one new test case with a non-default value (e.g., 'bind_to': 'core') to verify the feature works.

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.

[SHOULD-FIX] No input validation on bind_to

The bind_to value is user-controlled (from YAML config) and interpolated directly into a shell command:

'-bind-to {bind_to} '.format(bind_to=mode.bind_to)

This command is executed via Ansible on remote nodes. An empty string produces a malformed mpirun command; a malicious string (e.g., numa; rm -rf /) could inject shell commands.

The valid set of mpirun -bind-to values is small and fixed. A whitelist validation here would close this entirely:

VALID_BIND_TO = {'none', 'hwthread', 'core', 'l1cache', 'l2cache', 'l3cache', 'socket', 'numa', 'board'}
if mode.bind_to not in VALID_BIND_TO:
    raise ValueError(f"Invalid bind_to value '{mode.bind_to}'. Must be one of: {VALID_BIND_TO}")

Note: existing fields (mca, env, prefix) have the same class of issue, but since bind_to has a small fixed valid set, it's trivial to fix here.

for key in ['PATH', 'LD_LIBRARY_PATH', 'SB_MICRO_PATH', 'SB_WORKSPACE']:
self._sb_benchmarks[name].modes[idx].env.setdefault(key, None)
if 'pattern' in mode:
Expand Down Expand Up @@ -182,13 +184,14 @@ def __get_mode_command(self, benchmark_name, mode, timeout=None):
'-tag-output ' # tag mpi output with [jobid,rank]<stdout/stderr> prefix
'-allow-run-as-root ' # allow mpirun to run when executed by root user
'{host_list} ' # use prepared hostfile or specify nodes and launch {proc_num} processes on each node
'-bind-to numa ' # bind processes to numa
'-bind-to {bind_to} ' # bind processes according to mode config
'{mca_list} {env_list} {command}'
).format(
trace=trace_command,
host_list=f'-host localhost:{mode.proc_num}' if 'node_num' in mode and mode.node_num == 1 else
f'-hostfile hostfile -map-by ppr:{mode.proc_num}:node' if 'host_list' not in mode else '-host ' +
','.join(f'{host}:{mode.proc_num}' for host in mode.host_list),
bind_to=mode.bind_to,
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.

[SHOULD-FIX] No new tests for custom bind_to values

This new format argument needs test coverage. Recommended additions:

Test case Purpose
MPI mode with bind_to: 'core' Non-default value flows into command
MPI mode with bind_to: 'none' Disabling binding works
test_validate_sb_config asserts bind_to in mode for MPI modes Default is applied when omitted

mca_list=' '.join(f'-mca {k} {v}' for k, v in mode.mca.items()),
env_list=' '.join(
f'-x {k}={str(v).format(proc_rank=mode.proc_rank, proc_num=mode.proc_num)}'
Expand Down