Add UtilizationMonitor: CPU, memory and disk usage from /proc - #12
Conversation
Reads live utilisation to sit alongside SensorMonitor's temperatures: aggregate and per-core CPU, memory and swap, per-device disk busy percentage, and per-filesystem capacity. Kept separate from SensorMonitor because every figure here except memory is a RATE derived from two samples. SensorMonitor is stateless between polls, so folding rate state into it would turn its start/stop from a power optimisation into a correctness requirement. Decisions that are not obvious, each with the reason it is that way: - A rate that has only one sample reports -1.0, not 0.0. Reporting 0.0 is indistinguishable from a genuinely idle machine, so a panel would render a confident "CPU 0%" for one interval on every open. - iowait counts as IDLE. The CPU executes nothing during it; counting it as busy makes a box blocked on slow storage report 100% CPU and sends the reader after the wrong problem. - Memory used is MemTotal - MemAvailable. MemFree excludes reclaimable page cache, so a healthy Linux box would read ~95% full forever. - No swap configured reports -1.0, not 0.0, so a caller can omit the row rather than claim the swap is merely empty. - Disk busy divides by the MONOTONIC clock, not the poll interval, so a late timer cannot inflate the result past 100%. - Whole devices only, decided by /sys/block/<name> rather than by trailing-digit patterns, which gets nvme0n1 wrong. A partition's busy time is its parent's counted again. - start() discards rate state, so a panel closed for an hour cannot report one interval measured across that hour when it reopens. Verified on cixmini (CIX Sky1, 12 cores, 62 GiB) against the real /proc, not only fixtures. That run found a defect the fixtures could not: four loop-mounted ISOs under /mnt each reported 100% full and double-counted bytes already charged to the filesystem holding the image. Loop-backed and read-only image filesystems are now excluded, with the verbatim /proc/mounts lines kept as a regression test. Cross-checked against df on the same host: / 16.1% vs df 17%, /boot/efi 3.7% vs df 4% -- the difference is df excluding root-reserved blocks. 16 fixture tests, all passing under valac 0.56.19 on aarch64.
The test cleaned up its fixture tree by shelling out through Posix, but the meson target for it declares only gio_dep and gee_dep. It compiled by hand because the manual invocation passed --pkg posix; under `meson test` it would have failed with "The name `Posix' does not exist". Replaced with a small GLib recursive remove, so the source needs exactly the packages the target declares. Rebuilt with only --pkg gio-2.0 and --pkg gee-0.8: 16/16 pass. The pre-existing sensor_test leaks its fixture tree rather than removing it, so there was no precedent to follow either way. Found by the local upstream-reviewer simulation, which also withdrew a false finding in the same round: it had called the unmap handler's hidden_for_unavailable early return a leak, when that return is the documented mechanism keeping polling alive so a hidden indicator can observe its sensors coming back.
…eudo-fs Replaces the hand-maintained pseudo-fs fstype switch in is_real_filesystem() with GLib.UnixMountEntry.@for(mount_point). is_system_internal(), which gio-unix-2.0 already provides and which maintains a broader list internally (covers /dev/loop, devpts, ...). meson.build already declares giounix_dep so no dependency change. Verified empirically against /proc/mounts on ncz-33e4c9: glib returns true for /proc, /sys, /run, /dev, /mnt/argonas-{archives,git,projects} (autofs parent), the docker overlay / nsfs mounts, and the kernel debug/tracing/security/cgroup2 mounts; false for / (btrfs, see below), /boot/efi (vfat), and /mnt/argonas-projects (nfs) as well as for the mounted loop images under /mnt. The iso9660/udf/erofs and /dev/loop product rules above catch the loop images regardless of what glib says. Two findings drove the device-shape override that survives this change: 1. glib's hard-coded system_mount_paths list contains "/" itself (Nautilus hides its root mount because the file manager already has a "Filesystem root" entry), and the btrfs subvol rule on top of that flags any subvolume with a non-"/" root path. is_system_internal() therefore returns true for "/" on hosts that mount the rootfs as a btrfs subvolume (this host's / lives at /@rootfs). Without an override, the panel would silently lose the root disk -- the one the user most wants to see. The device path (starts with /dev/, or contains ":" for a network share) is the strongest "this is a real disk" signal glib has no business overriding, so it is applied before consulting is_system_internal(): when the device path looks like a real disk, include without asking glib. 2. GUnixMountEntry has no override hook and always reads the REAL /proc/mounts, so the fixture tests in tests/utilization_test.vala (which write a synthetic /proc/mounts under proc_root) cannot drive it -- glib would query the host and ignore the fixture. The old fstype switch is kept as fstype_is_pseudo() and consulted only when proc_root is set, mirroring how SensorMonitor.sysfs_root keeps its fixture tree out of the host's sysfs. This is why is_real_filesystem has two code paths: proc_root == "" calls glib, proc_root != "" falls back to fstype_is_pseudo(). All 16 tests/utilization_test.vala cases still pass against the fixture tree. The smoke probe against the real /proc on this host returns /, /boot/efi, and /mnt/argonas-projects (nfs); isov9, sqx, isoA, isoB, isoC, sqz, /proc, /sys, /run, /dev are all correctly absent.
The mount fix in e081961 added a GLib.UnixMountEntry call to utilization.vala (gio-unix-2.0). The main libsingularity_system library target already declared giounix_dep and needed no change -- but the standalone utilization-test executable, a separate meson target with its own dependency list, still only declared gio_dep and gee_dep. valac compiles the whole file regardless of which branch a given test exercises, so the missing package broke the build even though every fixture test sets proc_root and never reaches the GUnixMountEntry path. Caught by the full container build failing at [137/635] Compiling Vala source .../utilization-test.p/...: error: The name `UnixMountEntry' does not exist in the context of `GLib' which the earlier standalone `valac --pkg gio-unix-2.0 ...` compile (used to verify e081961) didn't catch, because that manual invocation passed the package by hand instead of going through this target's own declaration.
…t non-dev real filesystems Three real bugs from Codex review on PR singularityos-lab#12, each verified against the code before fixing: - guest/guest_nice were summed into CpuSample.total on top of user/nice, which already include them per the kernel's own accounting -- doubled guest time on any host running VMs. Now excluded from the sum entirely. - Per-CPU deltas were matched by array position, not cpuN label, unlike the disk path a few lines down which already keys off device name. CPU hotplug (offlining a lower-numbered core, common on big.LITTLE/power- gated ARM parts) shifts which index each remaining core lands at between polls, silently diffing one core's counters against another's. Now matched by label via a Gee.HashMap, same shape as disks. - is_real_filesystem()'s final fallthrough was an unconditional , so a mount that is neither /dev/-prefixed nor colon-bearing -- a CIFS //server/share or a ZFS pool/dataset -- was rejected even after surviving both the fast-path device check and the pseudo-filesystem checks. Now admits real filesystems that reach that point instead of defaulting to reject. Added a regression test per fix.
|
Fixed the 3 findings from the automated review — all confirmed real:
Added a regression test per fix. |
|
Build and tests pass here, but one blocker remains. |
poll() called query_filesystem_info() synchronously for every mount every two seconds. GIO's synchronous variant blocks until the filesystem answers, so a slow or unreachable NFS/CIFS mount stalls the main loop for as long as the RPC takes to time out -- freezing the whole shell, not just the panel. Capacity is now probed with query_filesystem_info_async() off the periodic poll, on its own slower cadence: mount capacity does not change second to second, and there is no reason to pay for it at CPU/memory sampling rate. CPU and memory sampling read /proc and stay synchronous in poll(), which is cheap and cannot block. An in-flight set coalesces refreshes so a mount that is slow to answer never has a second probe queued behind the first, and a probe that errors leaves the previous known value in place instead of dropping the row or blocking. refresh_filesystems() exposes an on-demand refresh, and start() kicks an immediate first cycle so a freshly opened panel does not wait a full interval for its first bar. Adds dispose() calling stop(), matching SensorMonitor, and a Cancellable that stop()/dispose() cancel so an in-flight probe cannot outlive the monitor. Tests: existing filesystem tests drive the async path; new coverage asserts poll() does not populate capacity synchronously (a nonexistent mount errors gracefully and never reaches the known set) and that dispose() stops the monitor idempotently.
|
Both addressed in f02c4cb. Main-loop blocking. You're right that this is a freeze risk and not a theoretical one — GIO's synchronous Capacity now uses A few details worth flagging:
dispose(). Added, calling Tests. The existing filesystem tests now drive the async path. Two new ones: that One caveat on my end: I verified |
|
Heads-up that the main-loop blocker is addressed, in case it got lost behind You were right that it wasn't theoretical: GIO's synchronous No rush from my side — flagging it only because |
…t non-dev real filesystems Three real bugs from Codex review on PR #12, each verified against the code before fixing: - guest/guest_nice were summed into CpuSample.total on top of user/nice, which already include them per the kernel's own accounting -- doubled guest time on any host running VMs. Now excluded from the sum entirely. - Per-CPU deltas were matched by array position, not cpuN label, unlike the disk path a few lines down which already keys off device name. CPU hotplug (offlining a lower-numbered core, common on big.LITTLE/power- gated ARM parts) shifts which index each remaining core lands at between polls, silently diffing one core's counters against another's. Now matched by label via a Gee.HashMap, same shape as disks. - is_real_filesystem()'s final fallthrough was an unconditional , so a mount that is neither /dev/-prefixed nor colon-bearing -- a CIFS //server/share or a ZFS pool/dataset -- was rejected even after surviving both the fast-path device check and the pseudo-filesystem checks. Now admits real filesystems that reach that point instead of defaulting to reject. Added a regression test per fix.
Adds
UtilizationMonitor/UtilizationReading/CapacityReading(CPU, memory, disk from /proc), the dependency singularity-shell#21 needs.Split out of the shell PR's dependency branch per your comment on #21: only the monitor API, no night-light or styling commits.
Validated: rebased clean on current main (74d66c3), libsingularity's own targets (including this code) compile with zero errors in the project's own build container. Full-superproject container build hit an unrelated, pre-existing failure in singularity-edit's vendored tree-sitter build, unaffected by this change.