Make the daemon fail loudly and put it under a systemd watchdog

[richardkiene]

This is the big one, so I'll try to explain the thinking rather than just list the diff.

The problem I wanted to solve

I'm using nvfd in curve mode on a workstation card whose stock fan curve stalls at ~60% and lets the die throttle. The thing that worried me about running any fan daemon in curve mode is what happens when the daemon isn't there: nvmlDeviceSetFanSpeed_v2 puts the card in manual fan policy, and if the process dies while the GPU is idle the fan is frozen at the 30% floor when the next job lands. So I looked at how the daemon handles errors, and found it mostly doesn't:

• an unparseable config.json was treated as "everything on auto"
• a missing or corrupt curve.json fell back to a hardcoded curve
• an unknown mode string fell back to that same curve
• a failed nvmlDeviceSetFanSpeed_v2 was ignored, so the unit sat active while writing to stderr every five seconds
• a daemon hung inside an NVML call was never restarted

None of those is visible from systemctl status. The daemon looks healthy right up until you notice the GPU is hot.

What this changes

The daemon treats every error as fatal. Unreadable config or curve, manual mode without a sane speed, unknown mode, a temperature read failing, a fan command NVML rejects — each one logs the reason to syslog, resets the fans to auto so nothing stays pinned, and exits non-zero. Restart=on-failure brings it back, and StartLimitIntervalSec=60/StartLimitBurst=5 mean that if it keeps dying the unit parks itself in failed where monitoring can see it, instead of restarting forever.

A systemd watchdog. The unit becomes Type=notify with WatchdogSec=30; the daemon sends READY=1 once it's up and WATCHDOG=1 after every 5 s poll. I didn't want to add a libsystemd link dependency for three strings, so src/notify.c is a forty-line sd_notify over an AF_UNIX datagram (handles the @ abstract-namespace form too). The daemon also reads WATCHDOG_USEC and refuses to start if someone's drop-in sets the watchdog shorter than twice the poll interval, since that would just get it killed. This works under ProtectSystem=strict — the kernel's read-only check doesn't apply to sockets, and systemd's own systemd-resolved unit combines the two.

Startup sweep. On its first poll the daemon hands every GPU that isn't configured for manual or curve mode back to the driver, so if a previous instance died without cleaning up, those fans aren't left where it put them. If a GPU can't be reset on that sweep (a card without controllable fans, say) it's logged and skipped rather than fatal — this instance never touched it, and it shouldn't stop the others being managed.

Curve re-read every poll. config.json was already re-read each loop; curve.json was cached until SIGHUP, which meant nvfd curve 60 70 silently did nothing until you remembered systemctl reload. Now both are read every 5 s. SIGHUP stays as a logged no-op so a stray HUP doesn't kill the process; ExecReload= and the README lines about it are gone.

Readers that say what's wrong. config_read() now distinguishes "no file yet" (an empty object, fine) from "file exists but is unusable" (NULL plus config_last_error()), and config_write_gpu refuses to overwrite a file it couldn't parse. curve_load() validates the curve properly: top-level object, integer keys 0–100 with a leading digit, integer values 0–100, no duplicate temperatures ("30" and "030" both parse to 30 and divided by zero in curve_interpolate), at most MAX_CURVE_POINTS, at least one point. Both use JSON_REJECT_DUPLICATES. The legacy plain-text migration only writes speeds the daemon will accept, and refuses otherwise instead of guessing.

CLI and TUI follow suit. nvfd curve and nvfd <n> curve check the curve loads before switching a GPU into curve mode, so the daemon is never handed a config it'll die on; the dashboard does the same before writing "curve". Exit status is non-zero when a command fails. The TUI and nvfd status exit with the parse error instead of rendering a blank config, and the curve editor won't open on an invalid file rather than overwriting it from the default on save. curve_default_interpolate ended up with no callers and is removed. fan_get_count returns -1 on error so fan_reset_to_auto can't "reset" zero fans and report success.

Testing

Running on an RTX 6000 Ada (Ubuntu 22.04.5, systemd 249, driver 595.84) in curve mode. systemctl status shows active (running) only after READY=1, journalctl -u nvfd is quiet, and the fan tracks curve edits within one poll. I bisected the hardening directives separately — see #14, which is independent of this one but you'll want both for the unit to actually start.

This touches src/fan.c alongside #15; they merge cleanly in either order (I have them together on my fork's main). It's a lot of change for one PR and I'm happy to split it if you'd prefer to take the watchdog and the reader validation separately — say the word.

[ricky-chaoju]

Thanks for the write-up, it made this a lot easier to follow. I ran the branch on a 5090 under systemd 255 with the real device nodes and it behaves as you describe: the unit comes up as Type=notify, stays running well past WatchdogSec=30 with no restarts, a bad curve file takes it to failed after five tries, the WatchdogSec guard trips with a clear message, and SIGHUP is the quiet no-op you intended. No need to split it, it hangs together.

I've left the rest inline. The migration one I'd like fixed before this goes in; the other two are take-or-leave. One more that's outside the diff: draw_curve_info (src/dashboard.c:341) is the only caller you didn't move off curve_read(), so an invalid curve file now prints the parse error to stderr with curses up, and it still says "(no curve file - using default)" though that fallback is gone.

[ricky-chaoju]

This makes a migration failure fatal for every command, and main() bails before dispatch, so a v1.x box whose /etc/infinirc_gpu_fan_control.conf holds something outside this set can't run nvfd at all any more. I built both binaries and compared: for 20, 0, abc and 60%, main migrates and carries on, while this branch returns 1 from list, auto, status, curve reset and 0 auto alike. The legacy file is never removed and config.json never gets written, so it repeats on every run and the only way out is deleting the file by hand.

Since fan_set_speed clamps anything below 30 anyway, could this clamp into range and warn instead of refusing?

[richardkiene]

Updated migration so legacy values no longer block every command. Numeric values are clamped to 30–100; numeric prefixes such as 60% preserve legacy behavior with a warning; nonnumeric values migrate to auto with a warning. Added regression coverage for these cases.

[ricky-chaoju]

Same thought here: fan_set_speed clamps below 30 a moment later, so the daemon is dying over a value it would have accepted anyway. Clamping and logging would keep the fail-loud behaviour for the cases that really are broken.

[richardkiene]

Out-of-range manual speeds are now clamped to 30–100 and logged at LOG_WARNING. Missing or noninteger speeds remain fatal.

[ricky-chaoju]

Would you mind keeping this line? systemctl reload nvfd now fails outright with "Job type reload is not applicable". The READMEs are updated so nobody new will be caught out, but anyone with an existing script calling it gets a hard error, and SIGHUP is already a harmless logged no-op, so it costs nothing to leave in.

[richardkiene]

Restored ExecReload=/bin/kill -HUP $MAINPID, retaining compatibility with existing reload scripts while SIGHUP remains a logged no-op.

[richardkiene]

Also moved draw_curve_info() off curve_read(). Missing and invalid curve files are now distinguished within curses, without writing the parse error to stderr or claiming a default fallback.