fix(oom): mark the right OOM victim, and point systemd's watchdog at a device that actually resets - #3337
Open
vpetersson-bot wants to merge 12 commits into
Open
fix(oom): mark the right OOM victim, and point systemd's watchdog at a device that actually resets#3337vpetersson-bot wants to merge 12 commits into
vpetersson-bot wants to merge 12 commits into
Conversation
A signage player that wedges under memory pressure has to recover itself. Measured on a 2 GB RK3566 board with a deliberate memory hog, the kernel's OOM killer chose: Killed process 3669 (bash) anon-rss:172kB oom_score_adj:1000 Killed process 1490 (bash) anon-rss:128kB oom_score_adj:1000 Killed process 3595 (python3) anon-rss:850944kB oom_score_adj:0 The first two are start_viewer.sh. It sets oom_score_adj=1000 on itself so that killing it stops the container and restart:always recovers — but it is a few hundred kB, so the kernel spends passes reclaiming nothing while the viewer holding the memory sits at the default score. On the same board at rest: wrapper 2,436 kB marked 1000, viewer 233,336 kB marked 0. Under load the viewer is ~990 MB. Each wasted pass is seconds of swap churn on a board already thrashing, which is how these boards end up wedged instead of self-healing. The viewer is now marked too. Same recovery path — its exit ends the wrapper's wait loop and the container stops — but the kill that happens first is the one that returns the memory. Best-effort write, since the viewer can exit between pidof and here. Host-side, a new system/resilience.yml: * Armbian re-creates zram swap at every boot (armbian-zram-config is WantedBy sysinit.target, 985 MB on this 2 GB board), silently undoing the swapoff misc.yml performs at install time. With swap back, the kernel thrashes instead of OOM-killing — the state both the R3S and the Rock Pi 4 were found wedged in. SWAP=false makes the installer's intent hold across reboots, leaving Armbian's unrelated zram /var/log alone. * systemd drives the hardware watchdog where one exists. Read the comment before trusting it: it was armed, tested against a real wedge, and did not recover the board. It is kept as a PID-1 backstop, with the health-gated alternative written down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
…a hung box
The first pass at this only considered one Armbian board. Surveying
the whole rig showed a single mechanism does not fit any of them
without help:
Pi 1-5 BCM2835 watchdog, 60 s, ALREADY held by systemd
(Pi OS default) — and those boards still wedge
x86 no /dev/watchdog at all; softdog module available
Rock Pi 4 DesignWare watchdog present but inactive
Pi (all) no softdog module — "just modprobe softdog" is not a
universal fallback
Pi (all) no PSI — /proc/pressure does not exist on Pi OS, so a
pressure-based health gate is not portable either
So: hardware watchdog where the board has one, softdog where it does
not, and a health gate built on MemAvailable as a fraction of
MemTotal, which is the one signal every board in the fleet exposes —
an absolute page count cannot span a 361 MB Pi 3A+ and an 8 GB x86.
The daemon pets only while that gate passes, which is the part
systemd's RuntimeWatchdogSec cannot do: its ping proves PID 1 is
scheduled, not that the box works. Pi OS has shipped that ping for
years on boards that wedge anyway.
Three things this found by being run on real hardware, each of which
would have shipped a protection that looks installed and does nothing:
* wd_keepalive. Debian's watchdog package pets the device whenever
the main daemon is not running. Measured on x86: daemon stopped,
device armed at 30 s, box stayed up indefinitely. Disabled and
masked, or the failure path is covered for.
* Ordering. Only one process can hold /dev/watchdog. Start the daemon
before systemd has let go — which is what a notify handler does,
since handlers flush at the end of the play — and it opens nothing,
logs alive=[none], and reports active. Observed on a Pi 4. The
re-exec is now a normal task, sequenced before the daemon starts.
* Verification. Because of the above, "service is active" proves
nothing; the play now fails unless the daemon holds an fd on the
device.
Validated end-to-end by freezing the daemon (SIGSTOP = petting stops,
the shape of a hang) on both paths:
x86, softdog, no hardware watchdog:
frozen 08:57:50, unreachable 08:58:11, back 08:58:56 uptime=24s
Pi 4, BCM2835 hardware watchdog, handed over from systemd:
frozen 09:00:38, unreachable 09:01:10, back 09:01:38 uptime=25s
Both boards were restored to their original configuration afterwards.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
vpetersson-bot
changed the title
Backing out the Armbian SWAP=false task and its handler. Two reasons, and the second is the one that matters. First, it does not match the repo. I went looking for the zram role it would have conflicted with and there isn't one: no mention of zram in the tree, on origin/master, or anywhere in git history except my own commit. What Anthias does have is the opposite — dphys-swapfile in packages.yml's "Remove deprecated apt dependencies" list, swapoff --all in misc.yml, /var/swap unlinked after it succeeds. So if a role enables zram it lives somewhere I can't see, and either way a watchdog role is the wrong place to be setting swap policy by side effect. Second, the evidence never supported it. What I measured was the OOM victim bug: the kernel reclaiming 172 kB from a marked wrapper shell while the 900 MB viewer sat at the default score. zram was present while that happened, but nothing in the measurement isolates it as a cause — and on a small board compressed swap is a real capacity win, which is presumably why Armbian ships it on. Backing it out exposed a bug it had been hiding. The health gate read MemAvailable, which deliberately excludes free swap, so on any board running zram it would have counted a box with plenty of compressed headroom as starved and rebooted it. That only looked correct while assuming the swap had been turned off. It now counts SwapFree as headroom, which is also the conservative direction: zram pages are compressed, so free swap is cheaper than the same count of free RAM pages, and the gate holds on slightly longer than a RAM-only reading would. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate findings affect watchdog setup and configuration correctness.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
ansible/roles/system/tasks/resilience.yml:153
- This disables
RuntimeWatchdogSec, so after the handover the health-gatedwatchdogdaemon is the sole process holding the device; a healthy memory check can keep petting even if PID 1 is wedged. That contradicts the PR's stated retained protection for a dead/stalled PID 1. Either add an explicit PID 1 liveness check to the gate or update the stated recovery scope, because this drop-in can no longer provide both protections.
RuntimeWatchdogSec=0
- Files reviewed: 6/7 changed files
- Comments generated: 2
- Review effort level: Lite
…ebooting it The Rock Pi 4 came back, so I ran the same SIGSTOP proof on the third hardware class. It failed, and in the worst way. Rock Pi 4B (RK3399), health-gated daemon holding /dev/watchdog at a 30 s timeout. Daemon SIGSTOPped 10:26:46; board unreachable by 10:27:12 — the watchdog fired on schedule — and then stayed dark. No ping, no SSH, nine minutes later. It needed a power cycle. Same Synopsys DesignWare IP as the NanoPi R3S, which did exactly this in the earlier round. I had attributed that one to systemd continuing to pet from PID 1; this run rules that out, because the daemon here had definitively stopped petting. The likelier reading of both is that the DesignWare reset halts these SoCs rather than resetting them. So on Rockchip the watchdog does not recover the player, it switches it off — worse than the wedge for an unattended screen, since a wedged board might still be talked down and a halted one cannot. The role now identifies the driver and refuses to arm one measured to behave this way, logging why instead. That deliberately leaves the two boards that suffer most without a layer 2. Arming it anyway because it "should" reset is how a recovery feature becomes a fleet of dark screens. softdog is the obvious next thing to try there — it reboots through the kernel's ordinary restart path rather than asserting the SoC reset, and plain `reboot` works fine on both boards — but that is untested, so it is written down rather than shipped. Validated reboot paths remain x86 (softdog) and Pi (BCM2835), both proven end-to-end. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate watchdog configuration and fail-safe issues remain.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
ansible/roles/system/defaults/main.yml:12
- This advertised override is never referenced: the resilience task always writes
RuntimeWatchdogSec=0. A board override ofsystem_runtime_watchdog_secis therefore silently ignored, and its description promises behavior this role cannot provide. Remove the unused setting or wire it into an intentional systemd configuration path.
system_runtime_watchdog_sec: 30
ansible/roles/system/tasks/resilience.yml:203
- On a fresh Debian/Raspberry Pi OS host,
/etc/systemd/system.conf.dis not guaranteed to exist, andcopydoes not create missing parent directories. This makes the handover task fail before the watchdog is configured on otherwise supported hosts. Create the directory first, as the Docker drop-in task does, or write to an existing systemd configuration path.
- name: Release the watchdog device from systemd
ansible.builtin.copy:
dest: /etc/systemd/system.conf.d/anthias-watchdog.conf
ansible/roles/system/templates/watchdog.conf:29
watchdog-refresh-use-settimeoutselects the ioctl used to refresh the watchdog; it does not disable graceful shutdown or make a failed health check use a direct reset. The rationale above this line therefore documents a protection that this setting does not implement. Use the watchdog package's actual failure behavior for that guarantee, or correct the comment to describe the refresh ioctl.
# Don't attempt a graceful shutdown on failure. Under the memory
# pressure this exists for, `shutdown` is exactly the kind of thing
# that hangs — let the device reset instead of waiting on userspace
# that has already proven it cannot make progress.
watchdog-refresh-use-settimeout = yes
- Files reviewed: 6/7 changed files
- Comments generated: 2
- Review effort level: Lite
softdog does recover a Rockchip board, so these boards get a layer 2
after all. Same Rock Pi 4B, same SIGSTOP test, back to back:
DesignWare hardware watchdog
SIGSTOPped 10:26:46 -> unreachable 10:27:12 -> still dark nine
minutes later, power cycle required
softdog
SIGSTOPped 10:48:47 -> unreachable 10:49:23 -> back 10:49:53,
uptime=20s
The hardware block asserts a SoC reset these boards do not come back
from; softdog reboots through the kernel's ordinary restart path
instead. So an untrusted hardware watchdog is now treated exactly like
a missing one and falls through to softdog, rather than the board
being left with no protection at all.
Which device to pet stops being obvious once both exist. /dev/watchdog
is an alias for whichever registered first — on Rockchip that is the
DesignWare block, so pointing the daemon at it would arm precisely the
device we fell back to avoid. Resolution is by driver identity now,
verified against all three real layouts:
Rock Pi 4 (DesignWare + softdog), want softdog -> /dev/watchdog1
Pi 4 (BCM2835 only), want hardware -> /dev/watchdog0
Pi 4, want softdog -> fails, rather than
returning the hardware device
That last case matters: no Raspberry Pi kernel ships softdog, so the
resolver has to refuse rather than silently arm something else.
This also gives RuntimeWatchdogSec=0 a second job. On Rockchip, systemd
arming the hardware watchdog is the failure mode, not the protection.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate findings affect viewer OOM targeting and safe, repeatable watchdog provisioning.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
ansible/roles/system/defaults/main.yml:12
- This variable is never consumed: the handover task writes the literal
RuntimeWatchdogSec=0, and there are no other references tosystem_runtime_watchdog_sec. Inventory overrides are therefore silently ignored, while this comment promises a configurable PID 1 watchdog. Remove this dead setting/documentation or implement an explicit fallback without making systemd compete with the health-gated daemon for the device.
system_runtime_watchdog_sec: 30
ansible/roles/system/tasks/resilience.yml:231
- On a clean Debian/Raspberry Pi host,
/etc/systemd/system.conf.dis not guaranteed to exist, andcopydoes not create parent directories, so first provisioning can fail here before the watchdog handover. Create this directory first, as the Docker drop-in task already does atansible/roles/system/tasks/docker.yml:95-103.
ansible.builtin.copy:
dest: /etc/systemd/system.conf.d/anthias-watchdog.conf
ansible/roles/system/tasks/resilience.yml:76
- The identity read is explicitly allowed to fail, but
default('')makes the trust expression evaluate to true when it does:designwareis not contained in an empty identity. An unreadable or empty driver identity can therefore arm an unknown hardware watchdog, contrary to the fail-safe behavior described above. Treat a failed or empty identity as untrusted and only select hardware after a positive identity check.
system_watchdog_hw_reboots: >-
{{ 'designware' not in
(system_watchdog_identity.content | default('') | b64decode | lower) }}
ansible/roles/system/tasks/resilience.yml:243
- This handover is gated on finding a replacement device. If
modprobe softdogor the resolver fails on a DesignWare host,system_watchdog_device.stat.existsis false, so this drop-in is skipped and any existing systemd owner keeps arming/petting the hardware watchdog that was classified as unsafe. Release the existing hardware owner based on raw device presence even when no safe replacement was found, while leaving the watchdog daemon disabled.
when: system_watchdog_device.stat.exists
- Files reviewed: 6/7 changed files
- Comments generated: 1
- Review effort level: Lite
…ence Five review findings, all real, and the third is the one that would have quietly disabled this in production. * The decision keyed off "does /dev/watchdog exist", but softdog registers one of its own. On an x86 host the first run loads and persists softdog; the second run then sees a device, calls it hardware, resolution finds no non-software device, and every watchdog task is skipped from then on. Upgrades would have stopped enforcing the configuration they had installed. Classification is now by driver identity, and a software identity means the softdog path. * A failed or empty identity read evaluated as trusted, because 'designware' is not in ''. That armed an unidentifiable hardware watchdog — the inverse of the fail-safe this exists for. Hardware is now used only on a positive identification. * The systemd handover was gated on having found a safe replacement. If softdog cannot load on a DesignWare host, that left systemd free to keep arming the hardware watchdog already classified as unsafe — the exact path that leaves a board dark. It is now gated on a device existing at all. * copy does not create parent directories, so a clean host without /etc/systemd/system.conf.d would fail first provisioning before the handover. Created first, as the Docker drop-in already does. * system_runtime_watchdog_sec was dead: the handover writes a literal 0 and nothing read the variable, so the documented knob silently did nothing. Removed, with a note on why no runtime knob is offered — it would only let an operator put systemd back into competition with the daemon for a device only one of them can hold. tests/ansible/watchdog_classification.yml pins the decision against the five layouts that actually exist, including the x86 second-run case, so this cannot regress unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
There was a problem hiding this comment.
🟡 Changes recommended
The unresolved critical watchdog path and additional correctness issues should be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
ansible/roles/system/tasks/resilience.yml:171
- This re-check is unused and it checks
/dev/watchdog, not the resolved/dev/watchdogNemitted by the resolver. Aftermodprobe softdog, sysfs registration can be visible before udev has created the numbered node; the command then succeeds, the role proceeds with a nonexistentwatchdog-device, and the later service/FD check aborts provisioning instead of retrying. Stat the resolved path (with a short retry) and use that result to gate the daemon setup.
- name: Re-check for a watchdog device after the softdog attempt
ansible.builtin.stat:
path: /dev/watchdog
register: system_watchdog_device_raw
ansible/roles/system/tasks/resilience.yml:309
- This check only looks for any fd whose target contains
watchdog, so it would also pass if the daemon opened/dev/watchdog0(the unsafe DesignWare alias) instead of the resolved/dev/watchdog1. Compare the fd target withsystem_watchdog_dev_path; otherwise the verification can report protection while the wrong watchdog is armed.
ansible.builtin.shell: >-
set -o pipefail;
ls -l /proc/$(pgrep -x watchdog | head -1)/fd 2>/dev/null
| grep -c watchdog
ansible/roles/system/tasks/resilience.yml:293
- Please avoid suppressing
ansible-lint'sno-handlerrule here. This task is deliberately ordered so systemd releases the device before the daemon starts; model that ordering with an explicitly flushed handler or another lint-compliant structure instead of hiding the rule, so later edits cannot accidentally move the handover behind daemon startup.
when: system_watchdog_handover.changed # noqa: no-handler
tests/ansible/watchdog_classification.yml:13
- The documented invocation uses an explicit comma inventory (
-i localhost,) but the play has noconnection: local; Ansible will normally try SSH to localhost instead of evaluating these cases in-process, so this classification check fails on hosts without an SSH daemon. Addconnection: localto the play (or include-c localin the command).
hosts: localhost
- Files reviewed: 7/8 changed files
- Comments generated: 1
- Review effort level: Lite
Five more review findings. The first is a real hole in the safety argument this role makes. * RebootWatchdogSec makes systemd open a watchdog again while stopping services, and the drop-in never set WatchdogDevice — so systemd took /dev/watchdog, which on a Rockchip host is the DesignWare block this role rejects for halting instead of rebooting. A hung shutdown would have fired precisely the device we went out of our way to avoid. The drop-in now pins WatchdogDevice to the vetted path, and where no device was vetted it sets RebootWatchdogSec=0 rather than letting the default pick one. * The post-modprobe re-check stat'd /dev/watchdog rather than the resolved /dev/watchdogN, and sysfs registration can beat udev to creating the numbered node — so the role could proceed with a watchdog-device that did not exist yet and fail later at verification. It now waits on the resolved path with a short retry. * The "is the daemon holding it" check counted any fd containing "watchdog", so it would have passed with the daemon on the unsafe DesignWare alias — reporting protection while the wrong device was armed. It matches the resolved path exactly now. * Dropped the `# noqa: no-handler`. The ordering it was hiding matters enough to be explicit: the handover is a handler, flushed by a named meta task before anything claims the device, so a later edit cannot quietly move daemon startup ahead of it. * The classification test asserted its cases through whatever connection ansible chose; it pins `connection: local` now, so it does not depend on the runner having an SSH daemon. The drop-in moved from inline content to a template, since it now has a conditional and Jinja inside a block scalar is a trap. Both rendered branches verified: with a vetted device it emits WatchdogDevice plus the reboot timeout, without one it emits RebootWatchdogSec=0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate watchdog validation and device-path issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
ansible/roles/system/tasks/resilience.yml:191
system_watchdog_dev_pathis a numeric/dev/watchdogNselected once during provisioning and then persisted into both the watchdog and systemd configurations. Those numbers are assigned by registration order; with Rockchip's DesignWare device plus the explicitly loaded softdog, a different boot order can make the saved path refer to the rejected DesignWare device, silently defeating the fallback. Resolve the driver identity at watchdog service startup or persist a stable device identity before arming it.
system_watchdog_dev_path: "{{ system_watchdog_path.stdout | default('') | trim }}"
- Files reviewed: 8/9 changed files
- Comments generated: 1
- Review effort level: Lite
Two findings on the resolved device path:
* /dev/watchdogN numbering is registration order. On a Rockchip host
the DesignWare block and the softdog this role loads can swap
numbers on a later boot, and a config pinned to the number would
then arm the device measured to halt the board — silently undoing
the fallback. A udev rule matches ATTR{identity}, which boot order
cannot change, and gives it /dev/anthias-watchdog; the daemon and
systemd are pointed at that. No identity match means no symlink, so
a host whose driver changed arms nothing rather than the wrong
thing, and the role keeps the numeric path it validated this run.
* the systemd drop-in branched on the resolver having printed a path,
which is set before the node is stat'd. If the retries expired the
branch still wrote WatchdogDevice and RebootWatchdogSec for a device
the role never validated. It now branches on the stat.
Verified on the Pi 4 testbed: the rule creates the symlink, survives a
reboot, and matches nothing when the identity differs; the daemon
configured with the symlink opens it ("alive=/dev/anthias-watchdog",
identity "Broadcom BCM2835 Watchdog timer") and the role's fd check
sees the resolved /dev/watchdog0, returning 1. Before the systemd
handover the same daemon reported active while logging "cannot open
... Device or resource busy" and the fd check returned 0 — the check
catches exactly that. Testbed restored (package purged, files removed).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
There was a problem hiding this comment.
🟡 Changes recommended
Critical watchdog ordering, device selection, cleanup, and keepalive verification issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
ansible/roles/system/tasks/resilience.yml:370
- If a host was configured previously and a later run has no resolvable watchdog (for example,
softdogfails to load or the hardware node disappears), this condition skips the drop-in update and the daemon setup without removing the old state. The enabledwatchdogservice and staleRebootWatchdogSec/WatchdogDeviceconfiguration can then remain active and reference a device that this run explicitly failed to validate. Add an explicit disable/remove path for the previously managed watchdog state when no safe device exists.
# Gated on a device existing at all, NOT on having found a safe one.
# If softdog can't load on a DesignWare host we end up with no
# daemon — but systemd must still be stopped from arming the
# hardware watchdog that was just classified as unsafe, which is the
# failure that leaves a board dark.
when: system_watchdog_hw.stat.exists or system_watchdog_device.stat.exists
- Files reviewed: 9/10 changed files
- Comments generated: 4
- Review effort level: Lite
Four findings on the same path: * handlers run in definition order, and "Restart watchdog daemon" was defined before the systemd re-exec — so first provisioning restarted the daemon while systemd still held the device. The re-exec is now defined first. * the numeric-path fallback reintroduced the bug the udev rule exists to prevent: a later boot can renumber /dev/watchdogN onto the device measured to halt the board. If the identity-backed name cannot be proven, arm nothing. An unprotected board is recoverable by hand; a halting one is not. * a host that had a safe device on an earlier run and has none now kept that run's daemon petting a rejected device. Stop and disable it, drop the stale rule, and write the systemd drop-in unconditionally so its no-device branch clears WatchdogDevice. * masking wd_keepalive had failed_when: false, so a failed mask left it petting the device after the health daemon died while the fd check still reported protection. Only an absent unit is tolerated now, and service_facts verifies it is masked or gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
There was a problem hiding this comment.
🟡 Changes recommended
Address unsafe unknown-driver watchdog fallback, apply OOM priority to memory-heavy viewer descendants, and correct the watchdog-path comment.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
ansible/roles/system/tasks/resilience.yml:343
- The Debian/Raspberry Pi
watchdogpackage gates its service throughrun_watchdogin/etc/default/watchdog, which defaults to0on a fresh install. This task only setsrun_wd_keepalive=0, sostate: restartedcan return without starting the daemon; the laterpgrep/fd check then fails and no watchdog is installed. Setrun_watchdog=1here before enabling the service, while retaining the keepalive setting at 0.
- name: Stop wd_keepalive covering for a failed health daemon
ansible.builtin.lineinfile:
path: /etc/default/watchdog
regexp: '^#?\s*run_wd_keepalive='
line: run_wd_keepalive=0
ansible/roles/system/tasks/resilience.yml:197
- The comment says an identity containing a quote is treated as unusable and that the role "keeps the numeric path", but the subsequent conditions require
system_watchdog_ident_exactto be non-empty and otherwise setsystem_watchdog_arm_pathto'', so the numeric path is not kept and no watchdog is armed. Please make the comment match the actual fail-closed behavior.
# Verbatim, because the udev rule compares it byte for byte. A
# double quote would break out of the rule's quoting, so an
# identity containing one is treated as unusable and the role
# keeps the numeric path (what it did before the rule existed).
ansible/roles/system/tasks/resilience.yml:309
- This branch only disables the
watchdogservice when no vetted device is found; thewd_keepalivestop/mask/assert block below is guarded bywhen: system_watchdog_device.stat.existsand is therefore skipped on this path. If a distro or an older installation already haswd_keepaliverunning, it will continue holding/petting the rejected watchdog after this role claims to have failed closed, defeating the health-gated recovery (and potentially keeping the unsafe DesignWare device armed). Move the keepalive teardown and verification outside the safe-device condition, while still tolerating an absent unit.
- name: Stop the daemon when this run found nothing safe to arm
ansible.builtin.systemd:
name: watchdog
enabled: false
state: stopped
register: system_watchdog_teardown
failed_when:
- system_watchdog_teardown is failed
- not (system_watchdog_teardown.msg | default('') is search('(?i)could not find|not found|no such file'))
when: not system_watchdog_device.stat.exists
- Files reviewed: 9/10 changed files
- Comments generated: 1
- Review effort level: Lite
…start Three more findings: * the classification was a denylist, so any driver that wasn't DesignWare or softdog was armed without anyone having watched it reset a board — including drivers that don't exist yet, on the generic arm64 target. A watchdog that halts turns every wedge into a physical visit, which is what DesignWare did to two boards here. Now an allowlist (system_watchdog_hw_allowlist, BCM2835 measured), with softdog — whose reboot path is measured and driver-independent — for everything else. * the unit's ExecStart is `[ $run_watchdog != 1 ] || exec /usr/sbin/watchdog`, so an install where that was ever answered "no" would provision with a service that reports active and runs nothing. Set it explicitly rather than trusting the debconf default. * the wd_keepalive stop/mask/verify was gated on having a safe device, so on the fail-closed path an already-running keepalive would go on petting the rejected device. It now runs on every host. Also corrects the stale comment that said an unusable identity keeps the numeric path; it arms nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
oom_score_adj is inherited at fork, so every helper the viewer spawns after the write is already covered — but one running when the wrapper executes (the per-clip gst_fbdev_player.py, hundreds of MB during playback) kept the default 0 and was the last thing the kernel would consider. Walk the tree once via /proc (the armhf viewer image ships no procps-ng); verified the walk reaches two levels of children. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
systemd already has this: RuntimeWatchdogSec= makes PID 1 open /dev/watchdog, program the timeout and ping at half of it, for a hardware device and softdog alike. Raspberry Pi OS ships it enabled (RuntimeWatchdogSec=1m in /usr/lib/systemd/system.conf.d/40-rpi-enable-watchdog.conf) — so every Pi in the fleet was already protected, and the first thing this role did was set RuntimeWatchdogSec=0 to take the device away from it and hand it to the Debian watchdog daemon. Gone with the daemon: the watchdog package, /etc/watchdog.conf, the 81-line health-gate script, wd_keepalive masking and verification, run_watchdog, the device handover and its flush_handlers ordering, the fd check, and the teardown path for all of it. What is left is the part systemd has no opinion about — WHICH device to open — because the Rockchip DesignWare block halts instead of resetting (measured), so it must not be armed and softdog must be pointed at by a name that boot order can't renumber. What is lost with the health gate: a board thrashing while PID 1 is still scheduled will not be reset, because systemd keeps petting happily. There was never a measurement showing that gate firing; the wedges reproduced here were the viewer container being unkillable, which bin/lib/viewer/common.sh fixes directly. If pressure-based recovery is wanted, the kernel-native routes are the place to look. Also fixes a bug the daemon version shipped with: the resolver's output was split on '\t' inside a YAML block scalar, where that is a literal backslash-t, so the identity came back empty, the udev rule was never written, and — after the fail-closed change — every board would have provisioned with no watchdog at all. The resolver now prints two lines. Caught by running the role against a Pi 4, not by the test playbook, which passed throughout. Verified on the Pi 4: role run end-to-end, systemd reports WatchdogDevice=/dev/anthias-watchdog RuntimeWatchdogUSec=30s, PID 1 holds the resolved node, the device reports timeout=30 state=active, and a second run is idempotent (changed=0). Board restored to stock afterwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
vpetersson-bot
changed the title
|
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
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.
Two independent changes to how a wedged board recovers.
1. The OOM killer was picking the wrong victim
bin/lib/viewer/common.shmarked only the wrapper shell as the preferred OOM victim. Measured on a 2 GB RK3566 board under a deliberate memory hog: the kernel chose the wrapper twice in a row, reclaiming 172 kB and 128 kB —oom_score_adj:1000on a few-hundred-kB shell outranks a 900 MB Qt process at the default score — then had to invoke the OOM killer again for the actual allocation. The viewer process (and any helper already running under it) is now marked too, so the kill that happens first is the one that returns the memory: 2,632 kB reclaimed before, 105,468 kB after.2. Arm a watchdog, using the one already in the kernel
systemd does the petting.
RuntimeWatchdogSec=makes PID 1 open/dev/watchdog, program the timeout and ping at half of it — for a hardware device and forsoftdogalike. Raspberry Pi OS already ships this enabled (RuntimeWatchdogSec=1min/usr/lib/systemd/system.conf.d/40-rpi-enable-watchdog.conf), so no daemon, no petting script, and no handover is needed anywhere.What systemd has no opinion about is which device to open, and that is the whole reason this role exists:
So the role classifies the device by driver identity against an allowlist of drivers someone has watched reset a board, loads
softdogfor everything else, gives the chosen device a stable name via udev (/dev/watchdogNis registration order and can renumber onto the rejected device after a reboot), and points systemd'sWatchdogDeviceat that name. If the identity-backed name can't be produced, it arms nothing and fails loudly — an unprotected board is recoverable by hand, a board that halts on every wedge is not.What this deliberately does not do
It does not gate the petting on a health check. An earlier revision of this PR installed the Debian
watchdogdaemon with a memory-pressure test binary, so a board that was thrashing while PID 1 was still scheduled would reset. That cost ~870 lines, a third-party daemon, taking the device away from systemd, and a keepalive to keep masked — and there is no measurement showing it ever fires. The wedges reproduced here were the viewer container being unkillable, which change 1 fixes directly. The cost of dropping it is stated plainly: a thrashing board with a live PID 1 will not be reset by this. If we want pressure-based recovery later, the kernel-native routes are the place to look (PSI and systemd-oomd where the board has PSI — Raspberry Pi OS does not — orhung_task_panicwhere it does not).Verification
Run against a Pi 4 testbed end-to-end, not just rendered: systemd reports
WatchdogDevice=/dev/anthias-watchdog,RuntimeWatchdogUSec=30s, PID 1 holds the resolved node, the device reportstimeout=30 state=active, and a second run is idempotent. The udev rule was separately shown to create the symlink, survive a reboot, and match nothing when the identity differs. Board restored to stock afterwards.That hardware run is also what caught a bug the daemon revision shipped with: the resolver's output was split on
'\t'inside a YAML block scalar, where that is a literal backslash-t, so the driver identity came back empty, the udev rule was never written, and every board would have provisioned with no watchdog at all.tests/ansible/watchdog_classification.ymlpassed the whole time — it pins the decision expressions, not the plumbing that feeds them.🤖 Generated with Claude Code