feat: opt-in CPU/GPU clock-cap add-on (from #91 B) - #98
Open
MikeGibbsOnyx wants to merge 12 commits into
Open
MikeGibbsOnyx wants to merge 12 commits into
MikeGibbsOnyx wants to merge 12 commits into
Conversation
Add a CPU panel (usage, temperature, power, model/cores with sparklines) as the single home for CPU stats on the device page, and restructure the Resources section into two independent columns: left: GPU + CPU (CPU directly beneath GPU) right: [RAM] + Storage + Network [+ Tailnet] On GB10 devices (DGX Spark / GX10) the CPU and GPU share one package and power envelope, and the CPU is often the part that runs hot first — this makes that visible at the device level. Layout: each column is a flex stack so panels take natural heights (no row-stretch dead space under the GPU panel). The bottom panel of each column gets flex-grow, so whichever side is shorter stretches its lowest panel to match — both columns always end at the same height regardless of content. CPU stats previously lived as a temp row inside the GPU panel (Sparks) and RAM panel (hosts); those rows are removed in favor of the dedicated panel so CPU is shown in one place. NetworkPanel and TailscalePanel gain an optional className prop to support the grow alignment.
Add a 'Clock Cap' row to the CPU and GPU panels showing the active frequency ceilings, so a capped device is visible at a glance. - CPU: per-frequency-domain max_perf read from sysfs (local) or a single SSH command (remote). Domains are grouped by cpuinfo_max_freq and labelled X925/A725 on GB10; a domain is 'capped' when its max_perf ceiling is below the hardware max. - GPU: nvidia-smi does not expose the active -lgc range, so the lock is read from the gpu-clock-lock.service unit (path configurable via GPU_CLOCK_LOCK_UNIT, defaulting to the common convention). - GPU SM-clock bar now scales to the cap when one is set, so it reflects the headroom that actually exists (2177/2200 reads ~full, not 72%). - Caps are cached 60s with an in-flight guard so the parallel CPU/GPU polls share one read. Verified on a 3x GB10 fleet (2x DGX Spark + 1x GX10): X925 2600/3900 capped, A725 2808/2808, GPU 2200 lock — local and remote-over-SSH.
…dware-bounded slider, live apply + boot-unit persist - server: pure clockControl.js (domain enum, bounds parsing from cpuinfo_min/ max_freq and nvidia-smi -q -d CLOCK Default Applications Clock, clamping, validation, helper argv, exit-code interpretation), SystemCollector applyClockCap with SSH root helper (primary) and local container-root fallback (secondary), volatile override merge so live-only applies are shown truthfully for the cache TTL - API: GET /api/sparks/:id/clocks/bounds, POST /api/sparks/:id/clocks (opt-in per Spark via clockControlEnabled, 403 when off, server-side clamping against re-read hardware bounds, rate-limited) - scripts: sparkdash-set-clock (POSIX sh root helper) + one-time scoped sudoers installer - client: ClockCapControl dialog (slider + number input bounded by discovered range, presets, Apply this-boot vs Save, honest warnings), wired into GPU and CPU panels, Allow clock control toggle in Edit Spark - tests: clockControl.test.js (pure logic), SystemCollector.clockControl (apply paths, override merge, bounds), ClockCapControl.test.tsx (vitest)
…-domain unit preservation, systemd $-safe ExecStart, README - _dropConvergedOverrides: gpu override drops only when a fresh read actually reports the applied value (unit file is blind to a live-only -lgc), so the UI stays honest past the cache TTL - _persistClockUnitLocal: both CPU domains share cpu-clock-cap.service, so a one-domain Save now carries the sibling domain's effective cap (volatile override, else live max_perf) instead of clobbering it; explicit per-core commands (no shell loop variables — systemd expands $VAR in ExecStart); unreadable sibling cores are omitted, not guessed - SparkMonitor.updateConfig calls clearClockCapsOverride (D5 hot config) - helper cpu_unit_body: same systemd $-safe per-core ExecStart - README: Clock control section, TOC/Features/REST API/Scripts rows - tests: gpu override survival/convergence + 5 sibling-persist regressions
…llback warning - parseCpuCoreMaxKhz / expandCpuToken / parseCpuBootUnitDefaults: derive the Boot default preset per CPU domain from what cpu-clock-cap.service actually installs (systemd brace ranges and explicit per-core echoes; unknown cores skipped, never guessed). GPU boot default = the -lgc value parsed from gpu-clock-lock.service. Nothing hardcoded. - _getClockBounds now also returns cpuBootDefaults + gpuBootDefaultMHz (local: read through the host-root bind; remote: the bounds command dumps both boot units as parts[2]/[3]) - bounds route reports the D8 GPU fallback use in warnings; the dialog surfaces bounds warnings next to the range - tests: brace-range expansion, measured-unit parsing (both ExecStart styles), preset exposure; fixture corrected to the real spark-1 topology (cores 0-4 and 10-14 = X925, 5-9 and 15-19 = A725)
…ibling-preserving helper persist Gate 1: SparkMonitor.snapshot() now emits clockControlEnabled (beside comfyMonitoring), and src/App.tsx threads the flag through placeholderSnapshot's roleFields + the WS-down merge block, so the opt-in toggle actually reaches the browser. Pinned by snapshot-contract tests in monitor-lifecycle.test.js. Gate 2: the helper argv chain and the availability probe no longer gate on "sudo -n true" (impossible under the shipped argumentless scoped sudoers grant). The probe exercises exactly the granted command: test -x (missing -> 127/not-installed), then an argumentless "sudo -n <helper>" whose usage print exits 1 = installed-and-allowed; sudo refusals hit the sparkdash-probe-sentinel -> 126/passwordless-sudo-hint. interpretHelperExit anchors on the argv sentinels instead of bare 126/127 digits in the message; the bounds API passes the probe's precise reason through to the domain rows. README + installer verify hint document the encoding. Gate 3: scripts/sparkdash-set-clock cpu_unit_body regenerates BOTH CPU domains (sibling values read from each core's current max_perf, unreadable cores omitted) so a single-domain Save can no longer wipe the sibling's boot cap. The container persist path (_persistClockUnitLocal) now wraps the CPU ExecStart in /bin/sh -c and matches the helper's Description/After/blank-line unit body, so both paths produce byte-identical units for the same request. New clockHelperScript.test.js drives the real script (sh) against a stubbed sysfs tree.
…oks, normalized exec-list shape cpu-big = the >=3 MHz group per the parseCpuClockCaps convention (fixtures had the groups swapped); SPARKDASH_SMI hook shadows the absolute nvidia-smi path; write_core now honors SPARKDASH_CPU_SYS; the helper's per-core command list is built as 'cmd; cmd' with no leading/trailing separators so the generated unit is byte-identical to the container persist path.
…-applied honesty Per the card + orchestrator amendments, measured on the live box: - GPU ceiling is now parsed from "Max Clocks → Graphics" (3003 on this silicon), with the fallback chain Max Clocks → Applications Clocks → Default Applications Clocks → GPU_CLOCK_MAX_MHZ. "Default Applications Clocks" is the boot-default boost point (2418), NOT the hardware limit — using it made 2600/2800/3000 unreachable. The section parser anchors on the exact header line (indent-4, no trailing colon; keys indent-8 with a padded fixed-column colon) and scans only strictly-deeper children, so the live transient "Clocks" sample (2184) can never win. Accepts the singular header spelling some drivers print. The deprecated reader is kept as the reported boot-default datapoint only. - POST /api/sparks/:id/clocks responses carry additive requestedMHz / appliedMHz / snapped (src/api/types.ts mirrors, response shape stays backward compatible). The driver quantises -lgc silently (a requested 2000 lands on 1976), so the apply paths now capture nvidia-smi's own "GPU clocks set to (min, max)" confirmation on BOTH the helper and the container path, and sysfs max_perf writes are read back; the strictest observed value is reported. When no observation is possible the response repeats the request and adds an explicit "could not be read back" warning — it never claims an unobserved confirmation. The D5 volatile override records the APPLIED value, never the requested one. - buildClockCapDomains emits the wire-ready 200 MHz grid (grid.min/max/ step/candidates/bandApplied) per domain: pure arithmetic band [0.30, 0.80]-of-ceiling (align up/down on 200), NO hardware detenting (--query-supported-clocks=graphics prints [N/A] on driver 580.173.02); a degenerate band falls back to the plain hard bounds with no candidate chips instead of crashing. - Tests: the orchestrator-pinned -q -d CLOCK fixture parsed verbatim (ceiling=3003; 2184 and 2418 asserted non-ceiling), the three pinned per-domain candidate lists, band edges, degenerate-band fallback, the captured set-to stdout shapes for both apply paths, and the additive response fields. 90/90 green.
…ion, helper done gate, clean cap removal
…ify button position - Slider (200 MHz grid) and manual-entry field are now two distinct labelled elements (own <label> each), not one combined flex widget. They still share one value state, so moving either updates the other. - Closed state: wrap [Modify][value chip] in one right-aligned flex group and give the chip a fixed width (w-28), so the Modify button sits at the same x-position on every row — the two CPU cluster rows (Big/Little) line up regardless of label or chip-text width. - Tests: +2 (separate labelled controls; Modify+chip one fixed group).
Move the privileged helper under scripts/sparkdash-clock-addon/. Document sudoers honestly: a command with no argv list matches any argv for that binary. Core sparkDash stays unarmed until helper + sudoers + clockControlEnabled. Co-authored-by: Philip Eriksson <35528685+PhilipEriksson@users.noreply.github.com>
MikeGibbsOnyx
commented
Sep 13, 2026
MikeGibbsOnyx
left a comment
MikeGibbsOnyx
left a comment
Collaborator
Author
There was a problem hiding this comment.
Field-testing the add-on end-to-end on a non-local Spark (DGX Spark, 20 cores) before merge. Helper installed, sudoers drop-in present, clockControlEnabled armed — then live applies only (no persist, no boot units written). Everything was reverted at the end.
What worked
- Arming flips
writable: trueon all three domains; unarmed POST still fails closed. - cpu-little 2400 → applied 2392,
max_perf= 2392000 on cpu0-4,10-14; big cores untouched. - Two-column Resources + dedicated CPU panel render correctly;
A725 capMHz 2392 / 2808 capped: true. - Removal → all little cores back to 2808000; panel converges.
- GPU
-lgc 0,2400→ driver confirms GPU clocks set to "(gpuClkMin 0, gpuClkMax 2400)";-rgcrestores. - No
AuthGate, existing middleware unchanged.
Four things I'd like addressed before this is called done. Details inline.
| // A removal has no value to verify — skip the read-back entirely so a | ||
| // CPU sysfs echo of the hardware maximum can never masquerade as the | ||
| // "confirmed" cap (the container path applies the same rule). | ||
| const verified = appliedMHz == null ? null : await this._verifyAppliedClockCap(domain, helperOut); |
Collaborator
Author
There was a problem hiding this comment.
CPU applies over the helper always warn "could not be read back".
Observed on real hardware: POST cpu-little 2400 returned
appliedMHz: 2400, warnings: ["the applied value could not be read back from the hardware; it is reported as requested"]
but max_perf on every little core read 2392000 immediately. The value is observable — the helper path just never looks. _verifyAppliedClockCap parses the helper's stdout, and for CPU domains the helper is silent by design (sysfs writes only), so verified is always null and the warning fires unconditionally.
The container path gets this right (readBackKhz at line 674 / 708 — a real sysfs read-back after the write). The remote path needs the same: after a CPU apply, issue one follow-up read of the domain's max_perf files over SSH and feed those kHz values into the same Math.min(...) / 1000 reduction. Bonus: it also gives you the quantise warning for free (requested 2400 → hardware 2392), which today never fires on remote CPU applies.
As shipped, the honest-but-useless warning devalues the field for the case where it should fire (an actual failed write).
| unlock=$3 | ||
| base="$CPU_SYS/$cpu_n/cpufreq" | ||
| if [ "$unlock" -eq 1 ]; then | ||
| khz=$(cat "$base/cpuinfo_max_freq" 2>/dev/null || true) |
Collaborator
Author
There was a problem hiding this comment.
--unlock is not a true restore: cpuinfo_max_freq can be lower than a core's own max_perf.
Measured on the Spark I tested (little cores = 2808, big cores cpuinfo = 3900):
cpu15 3978000 cpu16 3978000 cpu17 3978000 cpu18 3978000
cpu19 4004000 cpu5 3900000
Four big cores were running with a hardware max above their cpuinfo_max_freq, and one above it by 104 MHz. So --unlock writing cpuinfo_max_freq over each core silently lowers cpu15-19 from 3978/4004 to 3900 — "remove cap" is not a no-op restore, and a cap/uncap cycle will drift those cores down permanently until reboot.
Same defect in the container mirror (SystemCollector.js line ~695).
Two things this needs:
- Record the pre-cap value and restore it. Read the domain's current
max_perfset before the first write and hand that back on--unlock, instead of assumingcpuinfo_max_freqis the ceiling. Stateless variant: treat "already equal tocpuinfo_max_freq" as uncapped and only write where a cap is actually present. - The bounds math has the same blind spot. Domain ceilings are derived from
cpuinfo_max_freq, so the UI advertises X925 max = 3900 while the hardware will actually hold 4004. Either report the max observedmax_perf, or note the discrepancy — otherwise the "hardware bounds" shown to the operator are wrong for exactly the cores they're about to cap.
| } | ||
| } | ||
| if (persist) this._clockCapsOverride = {}; // persisted → drop all volatile state | ||
| else if (domain === "gpu") this._clockCapsOverride.gpu = finalMHz; |
Collaborator
Author
There was a problem hiding this comment.
A removal leaves the panel showing the old cap for the full caps-cache TTL (~60 s).
Timeline after POST {domain:"gpu", maxMHz:null} returned ok:true, appliedMHz:null:
t+0s gpuLock {minMHz: 0, maxMHz: 2400} ← still capped in the UI
t+12s gpuLock None
Hardware was already unlocked (nvidia-smi Max Clocks back to 3003) when the UI still said 2400. Same shape on CPU: A725 capMHz 2392 capped: true moments after all little cores read 2808000.
Cosmetic rather than dangerous — the values come from the fresh read, not from the override — but after an unlock the operator sees a cap that isn't there. An unlock has no value to display; it should clear the domain's override immediately and short-circuit the cache for that row rather than waiting for TTL expiry.
| if [ "$UNLOCK" -eq 1 ]; then | ||
| "$SMI" -rgc | ||
| else | ||
| "$SMI" -lgc "0,$MAX_MHZ" |
Collaborator
Author
There was a problem hiding this comment.
Document that clocks.max.gr does not reflect -lgc on this driver — otherwise a reviewer will read the GPU cap as a silent failure.
After a successful -lgc 0,2400 (driver confirmed set to "(gpuClkMin 0, gpuClkMax 2400)", rc 0):
$ nvidia-smi --query-gpu=clocks.gr,clocks.max.gr --format=csv,noheader
383 MHz, 3003 MHz ← max still reads the unlocked hardware ceiling
$ nvidia-smi -q -d CLOCK | grep -A3 "Max Clocks"
Graphics : 3003 MHz
clocks_throttle_reasons.active stays 0x0 too. On this platform the driver's own set-to line is the only trustworthy evidence that a lock landed — clocks.max.gr and the throttle-reason mask cannot be used as a post-apply assertion, here or in any future smoke script. A line in this README plus a short note next to parseGpuSetClocksReply would save the next person the debugging trip.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Split of #91 part B — clock caps as an opt-in host add-on, stacked on part A.
Original work by @PhilipEriksson. Packaging/docs: helper lives in
scripts/sparkdash-clock-addon/.Privilege model
sparkDash stays a dashboard until all three are true on that Spark:
sudo ./scripts/sparkdash-clock-addon/install-clock-helper.sh)/etc/sudoers.d/sparkdash-clockpresentclockControlEnabled, default off)Sudoers
NOPASSWD: /usr/local/bin/sparkdash-set-clockwith no argv list means that binary, any argv (sudoers semantics — not “argumentless only”). The helper still accepts only a tight flag set. Probe uses an argumentless run so a health check cannot apply a cap.Live apply is the default. Persist-to-boot is a second labeled action.
This PR vs
mainincludes the CPU panel (A). Prefer merge A first, then rebase B — or take B as the combined tree.Test plan
node --test server/collectors/__tests__/clockControl.test.js server/collectors/__tests__/clockHelperScript.test.js server/collectors/__tests__/SystemCollector.clockCaps.test.js server/collectors/__tests__/SystemCollector.clockControl.test.jsPlease do not merge until Mia + a second reviewer sign off. Do not install the helper on production dens from this PR alone.