Skip to content

feat: configure GPU wired memory before starting oMLX - #5

Merged
sg-shag merged 2 commits into
mainfrom
feat/configure-gpu-wired-memory
Oct 19, 2026
Merged

sg-shag merged 2 commits into
mainfrom
feat/configure-gpu-wired-memory

Conversation

@gnezim

@gnezim gnezim commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Configure the host-wide Metal wired-memory ceiling from tfconfig.yaml before oMLX starts. Previously TF left the macOS GPU ceiling unchanged, regardless of inference memory requirements.

services.omlx.gpu_memory_gb
  → nodes.<node>.runtime.gpu_memory_gb override
  → system LaunchDaemon startup, including reboot
      read physical RAM on the inference node
      default: RAM − 10 GiB (128 → 118 GiB / 120832 MiB)
      validate → write sysctl if changed → verify readback
      drop privileges → start oMLX as node.user

Positive integer values are GiB; omitted node values inherit the service setting, and explicit null restores the calculated default. Invalid settings, unsupported hosts and sysctl failures prevent oMLX startup. Bootstrap, daemon restart and sync-with-restart share this path; dry-run remains read-only. The legacy user/process managers do not apply this privileged policy.

The system plist starts a short root setup command, then invokes oMLX through sudo -n -H -u <node.user>. No additional sysctl sudoers permission is granted. This belongs to inference-node management, not Olla routing, and does not override oMLX's memory guard (--memory-guard or --memory-guard-gb).

Evidence

  • Before: generated oMLX LaunchDaemon starts inference directly without configuring iogpu.wired_limit_mb.
    After: 60 focused tests pass, including executable shell fixtures for 128/64 GiB defaults, explicit overrides, idempotence, out-of-RAM limits, write/readback failures, argument quoting and the YAML-to-CLI dry-run path. Real sudo/sysctl mutations are replaced by fixtures.
  • make dev-lint, git diff --check, macOS plutil -lint, and /bin/sh -n pass.
  • Full suite: 374 passed, 1 failed with an ignored local copy of tfconfig.example.yaml. Existing test_cluster_status_reports_inference_health expects 13G but the example model metadata produces 11G; the same failure was reproduced against unchanged main source. The final CLI dry-run assertion was subsequently checked in the passing focused run.
  • No live-node sysctl write, daemon restart, reboot test or inference load test was performed.

Merge Danger

Door: two-way

Set the previous explicit GiB ceiling and restart to roll back the value. To restore the macOS automatic policy, remove the startup policy before resetting iogpu.wired_limit_mb=0; otherwise the next start reapplies it.

Blast Radius: inference-hosts

The next managed daemon installation/restart applies the new default even when the field is absent. The ceiling is host-wide, shared by GPU processes, and raising it can increase memory pressure. Hosts with RAM ≤10 GiB require an explicit positive limit below physical RAM. oMLX remains non-root, but the launchd entry now starts as root for setup; real launchd/sudo privilege handoff and reboot behavior still need a node canary. oMLX's independently configured memory-guard ceiling may remain lower.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bab507ce-5bb5-4e18-a47d-73d729ccd508
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sg-shag
sg-shag merged commit 163242c into main Oct 19, 2026
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants