From 9b6911d2e66a9539aa2af8e808956f978fde3316 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 13 Aug 2026 18:37:36 +0200 Subject: [PATCH 01/11] basic/psi-util: add lookup table for PressureType --- src/basic/psi-util.c | 16 +++++++++------- src/basic/psi-util.h | 3 +++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/basic/psi-util.c b/src/basic/psi-util.c index f2a93e674f..d1f5838bb1 100644 --- a/src/basic/psi-util.c +++ b/src/basic/psi-util.c @@ -19,7 +19,7 @@ int read_resource_pressure(const char *path, PressureType type, ResourcePressure _cleanup_fclose_ FILE *f = NULL; unsigned field_filled = 0; ResourcePressure rp = {}; - const char *t, *cline; + const char *cline; char *word; int r; @@ -27,12 +27,7 @@ int read_resource_pressure(const char *path, PressureType type, ResourcePressure assert(IN_SET(type, PRESSURE_TYPE_SOME, PRESSURE_TYPE_FULL)); assert(ret); - if (type == PRESSURE_TYPE_SOME) - t = "some"; - else if (type == PRESSURE_TYPE_FULL) - t = "full"; - else - return -EINVAL; + const char *t = ASSERT_PTR(pressure_type_to_string(type)); r = fopen_unlocked(path, "re", &f); if (r < 0) @@ -131,6 +126,13 @@ static const char* const pressure_resource_table[_PRESSURE_RESOURCE_MAX] = { DEFINE_STRING_TABLE_LOOKUP(pressure_resource, PressureResource); +static const char* const pressure_type_table[_PRESSURE_TYPE_MAX] = { + [PRESSURE_TYPE_SOME] = "some", + [PRESSURE_TYPE_FULL] = "full", +}; + +DEFINE_STRING_TABLE_LOOKUP(pressure_type, PressureType); + int is_pressure_supported(void) { static thread_local int cached = -1; int r; diff --git a/src/basic/psi-util.h b/src/basic/psi-util.h index 458c7f63ea..c82a1da134 100644 --- a/src/basic/psi-util.h +++ b/src/basic/psi-util.h @@ -7,6 +7,8 @@ typedef enum PressureType { PRESSURE_TYPE_SOME, PRESSURE_TYPE_FULL, + _PRESSURE_TYPE_MAX, + _PRESSURE_TYPE_INVALID = -EINVAL, } PressureType; typedef enum PressureResource { @@ -50,6 +52,7 @@ static inline const PressureResourceInfo* pressure_resource_get_info(PressureRes } DECLARE_STRING_TABLE_LOOKUP(pressure_resource, PressureResource); +DECLARE_STRING_TABLE_LOOKUP(pressure_type, PressureType); /* Default parameters for pressure watch logic in sd-event and PID 1 */ #define PRESSURE_DEFAULT_TYPE "some" From aa1045f862fc955e99691ceacce65ff0a2237058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 16 Aug 2026 12:28:00 +0200 Subject: [PATCH 02/11] test-procfs-util: convert to new ASSERT macros Replace assert_se()/assert() with the typed ASSERT_* macros, matching the conversions done across the rest of src/test/. --- src/test/test-procfs-util.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/test/test-procfs-util.c b/src/test/test-procfs-util.c index ac77898737..9a368f9f18 100644 --- a/src/test/test-procfs-util.c +++ b/src/test/test-procfs-util.c @@ -15,37 +15,35 @@ int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); - assert_se(procfs_cpu_get_usage(&nsec) >= 0); + ASSERT_OK(procfs_cpu_get_usage(&nsec)); log_info("Current system CPU time: %s", FORMAT_TIMESPAN(nsec/NSEC_PER_USEC, 1)); - assert_se(procfs_memory_get_used(&v) >= 0); + ASSERT_OK(procfs_memory_get_used(&v)); log_info("Current memory usage: %s", FORMAT_BYTES(v)); - assert_se(procfs_tasks_get_current(&v) >= 0); + ASSERT_OK(procfs_tasks_get_current(&v)); log_info("Current number of tasks: %" PRIu64, v); pid_max = TASKS_MAX; r = procfs_get_pid_max(&pid_max); if (r == -ENOENT || ERRNO_IS_NEG_PRIVILEGE(r)) return log_tests_skipped_errno(r, "can't get pid max"); - assert(r >= 0); + ASSERT_OK(r); log_info("kernel.pid_max: %"PRIu64, pid_max); threads_max = TASKS_MAX; r = procfs_get_threads_max(&threads_max); if (r == -ENOENT || ERRNO_IS_NEG_PRIVILEGE(r)) return log_tests_skipped_errno(r, "can't get threads max"); - assert(r >= 0); + ASSERT_OK(r); log_info("kernel.threads-max: %"PRIu64, threads_max); limit = MIN(pid_max - (pid_max > 0), threads_max); - - assert_se(r >= 0); log_info("Limit of tasks: %" PRIu64, limit); - assert_se(limit > 0); + ASSERT_GT(limit, 0U); /* This call should never fail, as we're trying to set it to the same limit */ - assert(procfs_tasks_set_limit(limit) >= 0); + ASSERT_OK(procfs_tasks_set_limit(limit)); if (limit > 100) { log_info("Reducing limit by one to %"PRIu64"…", limit-1); @@ -53,22 +51,22 @@ int main(int argc, char *argv[]) { r = procfs_tasks_set_limit(limit-1); if (r == -ENOENT || ERRNO_IS_NEG_FS_WRITE_REFUSED(r)) return log_tests_skipped_errno(r, "can't set tasks limit"); - assert_se(r >= 0); + ASSERT_OK(r); - assert_se(procfs_get_pid_max(&v) >= 0); + ASSERT_OK(procfs_get_pid_max(&v)); /* We never decrease the pid_max, so it shouldn't have changed */ - assert_se(v == pid_max); + ASSERT_EQ(v, pid_max); - assert_se(procfs_get_threads_max(&v) >= 0); - assert_se(v == limit-1); + ASSERT_OK(procfs_get_threads_max(&v)); + ASSERT_EQ(v, limit - 1); - assert_se(procfs_tasks_set_limit(limit) >= 0); + ASSERT_OK(procfs_tasks_set_limit(limit)); - assert_se(procfs_get_pid_max(&v) >= 0); - assert_se(v == pid_max); + ASSERT_OK(procfs_get_pid_max(&v)); + ASSERT_EQ(v, pid_max); - assert_se(procfs_get_threads_max(&v) >= 0); - assert_se(v == limit); + ASSERT_OK(procfs_get_threads_max(&v)); + ASSERT_EQ(v, limit); } return 0; From efa45f5f470de4991398655b61cf6abe7843d755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 13 Aug 2026 18:41:01 +0200 Subject: [PATCH 03/11] basic/procfs-util: add procfs_cpu_get_ticks() --- src/basic/procfs-util.c | 33 +++++++++++++++++++++++++++++++++ src/basic/procfs-util.h | 13 +++++++++++++ src/test/test-procfs-util.c | 8 ++++++++ 3 files changed, 54 insertions(+) diff --git a/src/basic/procfs-util.c b/src/basic/procfs-util.c index 9ddffe2d37..f11c849baf 100644 --- a/src/basic/procfs-util.c +++ b/src/basic/procfs-util.c @@ -171,6 +171,39 @@ int procfs_cpu_get_usage(nsec_t *ret) { return 0; } +int procfs_cpu_get_ticks(ProcfsCpuTicks *ret) { + _cleanup_free_ char *first_line = NULL; + int r; + + assert(ret); + + r = read_one_line_file("/proc/stat", &first_line); + if (r < 0) + return r; + + const char *p = first_word(first_line, "cpu"); + if (!p) + return -EINVAL; + + /* All eight fields exist on any kernel we support: iowait was added in 2.5.41, steal in 2.6.11. + * "guest" and "guest_nice" are not returned, the kernel already includes them in "user" and + * "nice". */ + ProcfsCpuTicks t; + if (sscanf(p, "%"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64, + &t.user, + &t.nice, + &t.system, + &t.idle, + &t.iowait, + &t.irq, + &t.softirq, + &t.steal) < 8) + return -EINVAL; + + *ret = t; + return 0; +} + int convert_meminfo_value_to_uint64_bytes(const char *s, uint64_t *ret) { _cleanup_free_ char *w = NULL; uint64_t v; diff --git a/src/basic/procfs-util.h b/src/basic/procfs-util.h index cf091d1e07..15a07871d1 100644 --- a/src/basic/procfs-util.h +++ b/src/basic/procfs-util.h @@ -11,6 +11,19 @@ int procfs_tasks_get_current(uint64_t *ret); int procfs_cpu_get_usage(nsec_t *ret); +typedef struct ProcfsCpuTicks { + uint64_t user; + uint64_t nice; + uint64_t system; + uint64_t idle; + uint64_t iowait; + uint64_t irq; + uint64_t softirq; + uint64_t steal; +} ProcfsCpuTicks; + +int procfs_cpu_get_ticks(ProcfsCpuTicks *ret); + int procfs_memory_get(uint64_t *ret_total, uint64_t *ret_used); static inline int procfs_memory_get_used(uint64_t *ret) { return procfs_memory_get(NULL, ret); diff --git a/src/test/test-procfs-util.c b/src/test/test-procfs-util.c index 9a368f9f18..14def54ee0 100644 --- a/src/test/test-procfs-util.c +++ b/src/test/test-procfs-util.c @@ -18,6 +18,14 @@ int main(int argc, char *argv[]) { ASSERT_OK(procfs_cpu_get_usage(&nsec)); log_info("Current system CPU time: %s", FORMAT_TIMESPAN(nsec/NSEC_PER_USEC, 1)); + ProcfsCpuTicks ticks; + ASSERT_OK(procfs_cpu_get_ticks(&ticks)); + log_info("Current CPU ticks: user=%" PRIu64 " nice=%" PRIu64 " system=%" PRIu64 " idle=%" PRIu64 + " iowait=%" PRIu64 " irq=%" PRIu64 " softirq=%" PRIu64 " steal=%" PRIu64, + ticks.user, ticks.nice, ticks.system, ticks.idle, + ticks.iowait, ticks.irq, ticks.softirq, ticks.steal); + ASSERT_GT(ticks.idle, 0U); + ASSERT_OK(procfs_memory_get_used(&v)); log_info("Current memory usage: %s", FORMAT_BYTES(v)); From ff54321e3c10f5e1d67a68b55d4dab78f6957636 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 15 Aug 2026 20:29:19 +0200 Subject: [PATCH 04/11] basic/procfs-util: stop counting guest time twice The kernel reports guest usage in guest/guest-nice and also includes it in user/nice. By adding the guest and guest-nice values, we were counting those ticks twice. C.f. https://gitlab.com/procps-ng/procps/-/blob/master/library/stat.c?ref_type=heads#L612-615 > note: we exclude guest tics from xtot since ... > 'user' already includes 'guest' > 'nice' already includes 'gnice' > (see linux sources: ./kernel/sched/cputime.c, kcpustat_cpu_fetch) Rewrite procfs_cpu_get_usage() to use procfs_cpu_get_ticks() and fix the accounting error. --- src/basic/procfs-util.c | 43 ++++++++++++----------------------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/src/basic/procfs-util.c b/src/basic/procfs-util.c index f11c849baf..af3f14ae3f 100644 --- a/src/basic/procfs-util.c +++ b/src/basic/procfs-util.c @@ -128,46 +128,27 @@ static uint64_t calc_gcd64(uint64_t a, uint64_t b) { } int procfs_cpu_get_usage(nsec_t *ret) { - _cleanup_free_ char *first_line = NULL; - unsigned long user_ticks, nice_ticks, system_ticks, irq_ticks, softirq_ticks, - guest_ticks = 0, guest_nice_ticks = 0; - uint64_t sum, gcd, a, b; - const char *p; + ProcfsCpuTicks ticks; int r; assert(ret); - r = read_one_line_file("/proc/stat", &first_line); + r = procfs_cpu_get_ticks(&ticks); if (r < 0) return r; - p = first_word(first_line, "cpu"); - if (!p) - return -EINVAL; + uint64_t ticks_per_second = sysconf_clock_ticks_cached(), + sum = ticks.user + ticks.nice + ticks.system + ticks.irq + ticks.softirq, + gcd = calc_gcd64(NSEC_PER_SEC, ticks_per_second); - if (sscanf(p, "%lu %lu %lu %*u %*u %lu %lu %*u %lu %lu", - &user_ticks, - &nice_ticks, - &system_ticks, - &irq_ticks, - &softirq_ticks, - &guest_ticks, - &guest_nice_ticks) < 5) /* we only insist on the first five fields */ - return -EINVAL; + /* Reduce NSEC_PER_SEC/ticks_per_second first to avoid overflow of ticks * NSEC_PER_SEC, and split + * the multiplication into whole and remainder parts, so that the result stays exact. + * + * Note that in most cases b == 1 and the second term is trivial. */ + uint64_t a = NSEC_PER_SEC / gcd, + b = ticks_per_second / gcd; - uint64_t ticks_per_second = sysconf_clock_ticks_cached(); - - sum = (uint64_t) user_ticks + (uint64_t) nice_ticks + (uint64_t) system_ticks + - (uint64_t) irq_ticks + (uint64_t) softirq_ticks + - (uint64_t) guest_ticks + (uint64_t) guest_nice_ticks; - - /* Let's reduce this fraction before we apply it to avoid overflows when converting this to μsec */ - gcd = calc_gcd64(NSEC_PER_SEC, ticks_per_second); - - a = NSEC_PER_SEC / gcd; - b = ticks_per_second / gcd; - - *ret = DIV_ROUND_UP((nsec_t) sum * (nsec_t) a, (nsec_t) b); + *ret = (nsec_t) (sum / b * a + DIV_ROUND_UP(sum % b * a, b)); return 0; } From b298d68dd0f5c7e4ae5738c2bfe105889f1327e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Aug 2026 09:35:31 +0200 Subject: [PATCH 05/11] shared/blockdev-list: add flags to ignore virtual devices and partitions --- src/shared/blockdev-list.c | 24 ++++++++++++++++++++++++ src/shared/blockdev-list.h | 18 ++++++++++-------- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/shared/blockdev-list.c b/src/shared/blockdev-list.c index 3c3ec59f06..f6b70babfc 100644 --- a/src/shared/blockdev-list.c +++ b/src/shared/blockdev-list.c @@ -10,6 +10,7 @@ #include "device-util.h" #include "devnum-util.h" #include "errno-util.h" +#include "path-util.h" #include "string-util.h" #include "strv.h" #include "terminal-util.h" @@ -166,6 +167,29 @@ int blockdev_list_one( goto no_match; } + if (FLAGS_SET(flags, BLOCKDEV_LIST_IGNORE_VIRTUAL)) { + const char *devpath; + + r = sd_device_get_devpath(dev, &devpath); + if (r < 0) { + log_device_warning_errno(dev, r, "Failed to acquire device path of discovered block device '%s', ignoring: %m", node); + goto skipped; + } + + if (path_startswith(devpath, "/devices/virtual/")) + goto no_match; + } + + if (FLAGS_SET(flags, BLOCKDEV_LIST_IGNORE_PARTITIONS)) { + r = device_is_devtype(dev, "partition"); + if (r < 0) { + log_device_warning_errno(dev, r, "Failed to check if device is a partition '%s', ignoring: %m", node); + goto skipped; + } + if (r > 0) + goto no_match; + } + if (FLAGS_SET(flags, BLOCKDEV_LIST_IGNORE_ZRAM)) { r = device_sysname_startswith(dev, "zram"); if (r < 0) { diff --git a/src/shared/blockdev-list.h b/src/shared/blockdev-list.h index 7804bbe052..3d0c17954a 100644 --- a/src/shared/blockdev-list.h +++ b/src/shared/blockdev-list.h @@ -4,14 +4,16 @@ #include "forward.h" typedef enum BlockDevListFlags { - BLOCKDEV_LIST_SHOW_SYMLINKS = 1 << 0, /* Pick up symlinks to block devices too */ - BLOCKDEV_LIST_REQUIRE_PARTITION_SCANNING = 1 << 1, /* Only consider block devices with partition scanning */ - BLOCKDEV_LIST_IGNORE_ZRAM = 1 << 2, /* Ignore ZRAM */ - BLOCKDEV_LIST_REQUIRE_LUKS = 1 << 3, /* Only consider block devices with LUKS superblocks */ - BLOCKDEV_LIST_IGNORE_ROOT = 1 << 4, /* Ignore the block device we are currently booted from */ - BLOCKDEV_LIST_IGNORE_EMPTY = 1 << 5, /* Ignore disks of zero size (usually drives without a medium) */ - BLOCKDEV_LIST_METADATA = 1 << 6, /* Fill in model, vendor, subsystem, read_only */ - BLOCKDEV_LIST_IGNORE_READ_ONLY = 1 << 7, /* Ignore read-only block devices */ + BLOCKDEV_LIST_SHOW_SYMLINKS = 1 << 0, /* Pick up symlinks to block devices too */ + BLOCKDEV_LIST_REQUIRE_PARTITION_SCANNING = 1 << 1, /* Only consider block devices with partition scanning */ + BLOCKDEV_LIST_IGNORE_VIRTUAL = 1 << 2, /* Ignore virtual devices (devices under /sys/devices/virtual/) */ + BLOCKDEV_LIST_IGNORE_PARTITIONS = 1 << 3, /* Ignore partitions */ + BLOCKDEV_LIST_IGNORE_ZRAM = 1 << 4, /* Ignore ZRAM */ + BLOCKDEV_LIST_IGNORE_ROOT = 1 << 5, /* Ignore the block device we are currently booted from */ + BLOCKDEV_LIST_IGNORE_EMPTY = 1 << 6, /* Ignore disks of zero size (usually drives without a medium) */ + BLOCKDEV_LIST_IGNORE_READ_ONLY = 1 << 7, /* Ignore read-only block devices */ + BLOCKDEV_LIST_REQUIRE_LUKS = 1 << 8, /* Only consider block devices with LUKS superblocks */ + BLOCKDEV_LIST_METADATA = 1 << 9, /* Fill in model, vendor, subsystem, read_only */ } BlockDevListFlags; /* The "dynamic" filters – ones the kernel can flip at runtime via sysattrs on a live device. From ad997d0630c4bf8216cd4438a095b9e843037017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Aug 2026 09:53:02 +0200 Subject: [PATCH 06/11] shared/blockdev-list: add flag to gather diskseq stats Only one caller ever looks at this field. Let's optimize a bit. --- src/repart/repart-list-candidate-devices.c | 3 ++- src/shared/blockdev-list.c | 8 +++++--- src/shared/blockdev-list.h | 1 + 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/repart/repart-list-candidate-devices.c b/src/repart/repart-list-candidate-devices.c index f4f5a8aa2c..e11e61d900 100644 --- a/src/repart/repart-list-candidate-devices.c +++ b/src/repart/repart-list-candidate-devices.c @@ -161,8 +161,9 @@ int vl_method_list_candidate_devices( BLOCKDEV_LIST_SHOW_SYMLINKS| BLOCKDEV_LIST_REQUIRE_PARTITION_SCANNING| BLOCKDEV_LIST_IGNORE_ZRAM| - BLOCKDEV_LIST_METADATA| BLOCKDEV_LIST_IGNORE_READ_ONLY| + BLOCKDEV_LIST_METADATA| + BLOCKDEV_LIST_DISKSEQ| (p.ignore_empty ? BLOCKDEV_LIST_IGNORE_EMPTY : 0)| (p.ignore_root ? BLOCKDEV_LIST_IGNORE_ROOT : 0); diff --git a/src/shared/blockdev-list.c b/src/shared/blockdev-list.c index f6b70babfc..1a92b0dea7 100644 --- a/src/shared/blockdev-list.c +++ b/src/shared/blockdev-list.c @@ -279,9 +279,11 @@ int blockdev_list_one( } uint64_t diskseq = UINT64_MAX; - r = sd_device_get_diskseq(dev, &diskseq); - if (r < 0) - log_device_debug_errno(dev, r, "Failed to acquire diskseq of device '%s', ignoring: %m", node); + if (FLAGS_SET(flags, BLOCKDEV_LIST_DISKSEQ)) { + r = sd_device_get_diskseq(dev, &diskseq); + if (r < 0) + log_device_debug_errno(dev, r, "Failed to acquire diskseq of device '%s', ignoring: %m", node); + } _cleanup_free_ char *m = strdup(node); if (!m) diff --git a/src/shared/blockdev-list.h b/src/shared/blockdev-list.h index 3d0c17954a..6a2f350a2e 100644 --- a/src/shared/blockdev-list.h +++ b/src/shared/blockdev-list.h @@ -14,6 +14,7 @@ typedef enum BlockDevListFlags { BLOCKDEV_LIST_IGNORE_READ_ONLY = 1 << 7, /* Ignore read-only block devices */ BLOCKDEV_LIST_REQUIRE_LUKS = 1 << 8, /* Only consider block devices with LUKS superblocks */ BLOCKDEV_LIST_METADATA = 1 << 9, /* Fill in model, vendor, subsystem, read_only */ + BLOCKDEV_LIST_DISKSEQ = 1 << 10, /* Fill in the diskseq field */ } BlockDevListFlags; /* The "dynamic" filters – ones the kernel can flip at runtime via sysattrs on a live device. From 47df51ea402e3f8822e0f69628a500dcfca65f9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Aug 2026 10:07:30 +0200 Subject: [PATCH 07/11] shared/blockdev-list: add flag to gather read&write stats --- src/shared/blockdev-list.c | 30 ++++++++++++++++++++++++++++-- src/shared/blockdev-list.h | 13 +++++++++---- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/shared/blockdev-list.c b/src/shared/blockdev-list.c index 1a92b0dea7..32e121bb4b 100644 --- a/src/shared/blockdev-list.c +++ b/src/shared/blockdev-list.c @@ -285,6 +285,30 @@ int blockdev_list_one( log_device_debug_errno(dev, r, "Failed to acquire diskseq of device '%s', ignoring: %m", node); } + uint64_t read_bytes, write_bytes; + bool bytes_good = false; + if (FLAGS_SET(flags, BLOCKDEV_LIST_RW_STATS)) { + const char *v; + + r = sd_device_get_sysattr_value(dev, "stat", &v); + if (r < 0) + log_device_full_errno(dev, ERRNO_IS_NEG_DEVICE_ABSENT(r) ? LOG_DEBUG : LOG_WARNING, + r, "%s: failed to read 'stat' attribute, ignoring: %m", node); + else { + /* Fields 3 and 7: the number of sectors read and written. The "stat" attribute + * always counts in 512 byte sectors, regardless of the device's logical block size. + * The skipped fields are consumed with %*s, since their values may exceed any fixed + * integer width. */ + if (sscanf(v, "%*s %*s %"SCNu64" %*s %*s %*s %"SCNu64, &read_bytes, &write_bytes) < 2) + log_device_warning(dev, "%s: failed to parse 'stat' attribute.", node); + else if (!MUL_SAFE(&read_bytes, read_bytes, UINT64_C(512)) || + !MUL_SAFE(&write_bytes, write_bytes, UINT64_C(512))) + log_device_debug(dev, "%s: sector counters out of range, ignoring.", node); + else + bytes_good = true; + } + } + _cleanup_free_ char *m = strdup(node); if (!m) return log_oom(); @@ -292,11 +316,13 @@ int blockdev_list_one( *ret = (BlockDevice) { .node = TAKE_PTR(m), .symlinks = TAKE_PTR(list), - .diskseq = diskseq, - .size = size, .model = TAKE_PTR(model), .vendor = TAKE_PTR(vendor), .subsystem = TAKE_PTR(subsystem), + .diskseq = diskseq, + .size = size, + .read_bytes = bytes_good ? read_bytes : UINT64_MAX, + .write_bytes = bytes_good ? write_bytes : UINT64_MAX, .read_only = ro, }; diff --git a/src/shared/blockdev-list.h b/src/shared/blockdev-list.h index 6a2f350a2e..e79ad74a76 100644 --- a/src/shared/blockdev-list.h +++ b/src/shared/blockdev-list.h @@ -15,6 +15,7 @@ typedef enum BlockDevListFlags { BLOCKDEV_LIST_REQUIRE_LUKS = 1 << 8, /* Only consider block devices with LUKS superblocks */ BLOCKDEV_LIST_METADATA = 1 << 9, /* Fill in model, vendor, subsystem, read_only */ BLOCKDEV_LIST_DISKSEQ = 1 << 10, /* Fill in the diskseq field */ + BLOCKDEV_LIST_RW_STATS = 1 << 11, /* Fill in read and write measurements */ } BlockDevListFlags; /* The "dynamic" filters – ones the kernel can flip at runtime via sysattrs on a live device. @@ -31,13 +32,17 @@ typedef struct BlockDevice { char *subsystem; uint64_t diskseq; uint64_t size; /* in bytes */ + uint64_t read_bytes; + uint64_t write_bytes; int read_only; } BlockDevice; -#define BLOCK_DEVICE_NULL (BlockDevice) { \ - .diskseq = UINT64_MAX, \ - .size = UINT64_MAX, \ - .read_only = -1, \ +#define BLOCK_DEVICE_NULL (BlockDevice) { \ + .diskseq = UINT64_MAX, \ + .size = UINT64_MAX, \ + .read_bytes = UINT64_MAX, \ + .write_bytes = UINT64_MAX, \ + .read_only = -1, \ } /* Return values of blockdev_list_one(). Not its own type – returned as int. */ From 2e185878726a7605c69e10123ca6224d9a0773a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Aug 2026 10:19:29 +0200 Subject: [PATCH 08/11] shared/blockdev-list: allow uninitalized devices This means we can gather stats without udev running, which can be good in containers and such. --- src/shared/blockdev-list.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/shared/blockdev-list.c b/src/shared/blockdev-list.c index 32e121bb4b..fe71bd4ccc 100644 --- a/src/shared/blockdev-list.c +++ b/src/shared/blockdev-list.c @@ -363,6 +363,13 @@ int blockdev_list(BlockDevListFlags flags, BlockDevice **ret_devices, size_t *re if (r < 0) return log_error_errno(r, "Failed to add subsystem match: %m"); + if (!(flags & (BLOCKDEV_LIST_REQUIRE_LUKS | BLOCKDEV_LIST_SHOW_SYMLINKS | BLOCKDEV_LIST_METADATA))) { + /* If everything we need comes from the device name or sysfs directly, no udev db entry is needed. */ + r = sd_device_enumerator_allow_uninitialized(e); + if (r < 0) + log_debug_errno(r, "Failed to allow uninitialized devices, ignoring: %m"); + } + if (FLAGS_SET(flags, BLOCKDEV_LIST_REQUIRE_LUKS)) { /* blockdev_list_one() enforces this filter authoritatively; the enumerator match is just * an optimization so we don't iterate non-LUKS devices here. */ From b8cc2ef04b2d8527d622171f0d05e99ba6e6f04b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 13 Aug 2026 15:43:35 +0200 Subject: [PATCH 09/11] report-basic: cpu, memory, pressure, and disk metrics Extend the io.systemd.Basic metric families with host-level runtime metrics: cumulative per-state CPU seconds, memory in use (MemTotal minus MemAvailable), swap in use, the kernel's 10s pressure stall averages (cpu/memory/io x some/full). Partitions and stacked devices are excluded from the disk counters, since their traffic is already accounted for in the parent device. Also extend the TEST-74 report subtest with checks for the new metrics. Network metrics are not added, because the service runs with PrivateNetwork=yes, and it doesn't make sense to report stats for 'lo'. After some back-and-forth, I settled on "CPUUsage". We use this scheme of capitalization in other metric family names: "IPv4Public", "IPv6Public", IPv4AddressState", "IPv6AddressState", "OSRelease" "SMBIOS" "BootID", "CPUsOnline", "MachineID", "MachineInfo", "OSRelease", "SMBIOS.SystemManufacturer", "TPM2.Manufacturer", "TPM2.VendorString", "IOReadBytes", "IOReadOperations", "CGroup". The exception is "io.systemd.cgroup.CpuUsage". I think we should treat that one as a mistake and consider changing it to conform. --- src/basic/procfs-util.c | 27 +-- src/basic/procfs-util.h | 1 + src/report/report-basic.c | 263 +++++++++++++++++++++++-- test/units/TEST-74-AUX-UTILS.report.sh | 80 +++++++- 4 files changed, 342 insertions(+), 29 deletions(-) diff --git a/src/basic/procfs-util.c b/src/basic/procfs-util.c index af3f14ae3f..4e80e873a8 100644 --- a/src/basic/procfs-util.c +++ b/src/basic/procfs-util.c @@ -127,18 +127,8 @@ static uint64_t calc_gcd64(uint64_t a, uint64_t b) { return a; } -int procfs_cpu_get_usage(nsec_t *ret) { - ProcfsCpuTicks ticks; - int r; - - assert(ret); - - r = procfs_cpu_get_ticks(&ticks); - if (r < 0) - return r; - +nsec_t procfs_ticks_to_nsec(uint64_t ticks) { uint64_t ticks_per_second = sysconf_clock_ticks_cached(), - sum = ticks.user + ticks.nice + ticks.system + ticks.irq + ticks.softirq, gcd = calc_gcd64(NSEC_PER_SEC, ticks_per_second); /* Reduce NSEC_PER_SEC/ticks_per_second first to avoid overflow of ticks * NSEC_PER_SEC, and split @@ -148,7 +138,20 @@ int procfs_cpu_get_usage(nsec_t *ret) { uint64_t a = NSEC_PER_SEC / gcd, b = ticks_per_second / gcd; - *ret = (nsec_t) (sum / b * a + DIV_ROUND_UP(sum % b * a, b)); + return ticks / b * a + DIV_ROUND_UP(ticks % b * a, b); +} + +int procfs_cpu_get_usage(nsec_t *ret) { + int r; + + assert(ret); + + ProcfsCpuTicks ticks; + r = procfs_cpu_get_ticks(&ticks); + if (r < 0) + return r; + + *ret = procfs_ticks_to_nsec(ticks.user + ticks.nice + ticks.system + ticks.irq + ticks.softirq); return 0; } diff --git a/src/basic/procfs-util.h b/src/basic/procfs-util.h index 15a07871d1..22218ac7c0 100644 --- a/src/basic/procfs-util.h +++ b/src/basic/procfs-util.h @@ -9,6 +9,7 @@ int procfs_get_threads_max(uint64_t *ret); int procfs_tasks_set_limit(uint64_t limit); int procfs_tasks_get_current(uint64_t *ret); +nsec_t procfs_ticks_to_nsec(uint64_t ticks); int procfs_cpu_get_usage(nsec_t *ret); typedef struct ProcfsCpuTicks { diff --git a/src/report/report-basic.c b/src/report/report-basic.c index cc3cdd8b24..7cb36d6885 100644 --- a/src/report/report-basic.c +++ b/src/report/report-basic.c @@ -11,6 +11,7 @@ #include "alloc-util.h" #include "architecture.h" +#include "blockdev-list.h" #include "confidential-virt.h" #include "cpu-set-util.h" #include "env-file.h" @@ -23,8 +24,13 @@ #include "log.h" #include "metrics.h" #include "os-util.h" +#include "path-util.h" +#include "procfs-util.h" +#include "psi-util.h" #include "report-basic.h" #include "string-util.h" +#include "time-util.h" +#include "utf8.h" #include "virt.h" static int architecture_generate(const MetricFamily *mf, sd_varlink *link, void *userdata) { @@ -124,6 +130,94 @@ static int physical_memory_generate(const MetricFamily *mf, sd_varlink *link, vo /* fields= */ NULL); } +static int memory_used_generate(const MetricFamily *mf, sd_varlink *link, void *userdata) { + int r; + + assert(mf && mf->name); + assert(link); + + /* Memory currently in use, i.e. MemTotal − MemAvailable from /proc/meminfo. Note that + * PhysicalMemoryBytes is not necessarily the matching total: it is capped by the root cgroup's + * memory limit, while /proc/meminfo always shows the whole system. */ + + uint64_t used; + r = procfs_memory_get_used(&used); + if (r < 0) { + log_warning_errno(r, "Failed to read memory usage from /proc/meminfo, ignoring: %m"); + return 0; + } + + return metric_build_send_unsigned( + mf, + link, + /* object= */ NULL, + used, + /* fields= */ NULL); +} + +static int pressure_generate(const MetricFamily mf[static 2], sd_varlink *link, void *userdata) { + int r; + + assert(mf && mf[0].name && mf[1].name); + assert(!mf[1].generate); + assert(link); + + /* The kernel's smoothed pressure stall percentages over the last 10s, plus the cumulative stall + * time, as documented in https://docs.kernel.org/accounting/psi.html */ + + for (PressureResource resource = 0; resource < _PRESSURE_RESOURCE_MAX; resource++) { + const char *name = pressure_resource_to_string(resource); + + _cleanup_free_ char *path = path_join("/proc/pressure", name); + if (!path) + return -ENOMEM; + + PressureType type; + FOREACH_ARGUMENT(type, PRESSURE_TYPE_SOME, PRESSURE_TYPE_FULL) { + /* The "full" number for cpu pressure at the system level is always reported as 0, + * so skip it. */ + if (resource == PRESSURE_CPU && type == PRESSURE_TYPE_FULL) + continue; + + ResourcePressure rp; + r = read_resource_pressure(path, type, &rp); + if (r < 0) { + bool absent = IN_SET(r, -ENOENT, -ENODATA) || ERRNO_IS_NEG_NOT_SUPPORTED(r); + log_full_errno(absent ? LOG_DEBUG : LOG_WARNING, r, + "Failed to read %s pressure, ignoring: %m", name); + break; /* If "some" can't be read, "full" won't work either */ + } + + _cleanup_(sd_json_variant_unrefp) sd_json_variant *fields = NULL; + r = sd_json_buildo(&fields, + SD_JSON_BUILD_PAIR_STRING("resource", name), + SD_JSON_BUILD_PAIR_STRING("type", pressure_type_to_string(type))); + if (r < 0) + return r; + + r = metric_build_send_double( + mf, + link, + /* object= */ NULL, + (double) rp.avg10 / LOADAVG_FIXED_POINT_1_0, + fields); + if (r < 0) + return r; + + r = metric_build_send_double( + mf + 1, + link, + /* object= */ NULL, + (double) rp.total / USEC_PER_SEC, + fields); + if (r < 0) + return r; + } + } + + return 0; +} + static int cpus_online_generate(const MetricFamily *mf, sd_varlink *link, void *userdata) { int r; @@ -143,6 +237,52 @@ static int cpus_online_generate(const MetricFamily *mf, sd_varlink *link, void * /* fields= */ NULL); } +static int cpu_usage_generate(const MetricFamily *mf, sd_varlink *link, void *userdata) { + int r; + + assert(mf && mf->name); + assert(link); + + /* The aggregate "cpu" line of /proc/stat, converted from USER_HZ ticks to nanoseconds, matching + * the unit of io.systemd.CGroup.CpuUsage. "nice" time is folded into "user" and "irq"/"softirq" + * into "system", so that the published types together account for all CPU time. */ + + ProcfsCpuTicks ticks; + r = procfs_cpu_get_ticks(&ticks); + if (r < 0) { + log_warning_errno(r, "Failed to read CPU time from /proc/stat, ignoring: %m"); + return 0; + } + + static const char* const types[] = { "user", "system" }; + const uint64_t values[] = { + ticks.user + ticks.nice, + ticks.system + ticks.irq + ticks.softirq, + }; + assert_cc(ELEMENTSOF(types) == ELEMENTSOF(values)); + + for (size_t i = 0; i < ELEMENTSOF(values); i++) { + _cleanup_(sd_json_variant_unrefp) sd_json_variant *fields = NULL; + + r = sd_json_buildo(&fields, SD_JSON_BUILD_PAIR_STRING("type", types[i])); + if (r < 0) + return r; + + uint64_t nsec = procfs_ticks_to_nsec(values[i]); + + r = metric_build_send_unsigned( + mf, + link, + /* object= */ NULL, + nsec, + fields); + if (r < 0) + return r; + } + + return 0; +} + static int load_average_generate(const MetricFamily *mf, sd_varlink *link, void *userdata) { enum { LOAD_AVERAGE_FIELD_1MIN, @@ -181,11 +321,14 @@ static int load_average_generate(const MetricFamily *mf, sd_varlink *link, void return 0; } -static int swap_generate(const MetricFamily *mf, sd_varlink *link, void *userdata) { - assert(mf && mf->name); +static int swap_generate(const MetricFamily mf[static 2], sd_varlink *link, void *userdata) { + int r; + + assert(mf && mf[0].name && mf[1].name); + assert(!mf[1].generate); assert(link); - /* The total amount of configured swap space, in bytes. */ + /* The total amount of configured swap space and the amount currently in use, in bytes. */ struct sysinfo info; if (sysinfo(&info) < 0) @@ -193,15 +336,69 @@ static int swap_generate(const MetricFamily *mf, sd_varlink *link, void *userdat /* Overflow is unrealistic (would need >16 EiB of swap), but use MUL_SAFE to make this obvious to * static analyzers. */ - uint64_t swap; - assert_se(MUL_SAFE(&swap, (uint64_t) info.totalswap, (uint64_t) info.mem_unit)); + uint64_t total, used; + assert_se(MUL_SAFE(&total, (uint64_t) info.totalswap, (uint64_t) info.mem_unit)); + assert_se(MUL_SAFE(&used, (uint64_t) LESS_BY(info.totalswap, info.freeswap), (uint64_t) info.mem_unit)); - return metric_build_send_unsigned( - mf, - link, - /* object= */ NULL, - swap, - /* fields= */ NULL); + const uint64_t values[] = { total, used }; + for (size_t i = 0; i < ELEMENTSOF(values); i++) { + r = metric_build_send_unsigned( + mf + i, + link, + /* object= */ NULL, + values[i], + /* fields= */ NULL); + if (r < 0) + return r; + } + + return 0; +} + +static int disk_io_generate(const MetricFamily mf[static 2], sd_varlink *link, void *userdata) { + int r; + + assert(mf && mf[0].name && mf[1].name); + assert(!mf[1].generate); + assert(link); + + /* Cumulative read/write byte counters of all whole physical block devices. Partitions are skipped, + * their traffic is already accounted for in the parent device. Stacked devices (dm, md, …) are + * skipped for the same reason: their traffic is accounted for again in the devices they are built + * on. */ + + BlockDevice *devices = NULL; + size_t n_devices = 0; + CLEANUP_ARRAY(devices, n_devices, block_device_array_free); + + r = blockdev_list(BLOCKDEV_LIST_IGNORE_VIRTUAL | BLOCKDEV_LIST_IGNORE_PARTITIONS | BLOCKDEV_LIST_RW_STATS, + &devices, &n_devices); + if (r < 0) + return r; + + FOREACH_ARRAY(d, devices, n_devices) { + /* A non-UTF-8 name would make the JSON reply builder fail, taking down the whole method + * call rather than just this device. */ + if (!utf8_is_valid(d->node)) { + log_warning("%s: block device name is not valid UTF-8, ignoring device.", d->node); + continue; + } + + const uint64_t values[] = { d->read_bytes, d->write_bytes }; + for (size_t i = 0; i < ELEMENTSOF(values); i++) + if (values[i] != UINT64_MAX) { + r = metric_build_send_unsigned( + mf + i, + link, + /* object= */ d->node, + values[i], + /* fields= */ NULL); + if (r < 0) + return r; + } + } + + return 0; } enum { @@ -547,6 +744,26 @@ static const MetricFamily metric_family_table[] = { METRIC_FAMILY_TYPE_GAUGE, .generate = cpus_online_generate, }, + { + METRIC_IO_SYSTEMD_BASIC_PREFIX "CPUUsage", + "Aggregate CPU usage across all CPUs in nanoseconds (type=user|system)", + METRIC_FAMILY_TYPE_COUNTER, + .generate = cpu_usage_generate, + }, + { + METRIC_IO_SYSTEMD_BASIC_PREFIX "DiskReadBytes", + "Per block device metric: cumulative number of bytes read " + "(partitions, stacked and virtual devices are excluded)", + METRIC_FAMILY_TYPE_COUNTER, + .generate = disk_io_generate, + }, + { + METRIC_IO_SYSTEMD_BASIC_PREFIX "DiskWriteBytes", + "Per block device metric: cumulative number of bytes written " + "(partitions, stacked and virtual devices are excluded)", + METRIC_FAMILY_TYPE_COUNTER, + }, + /* Keep those ↑ in sync with disk_io_generate(). */ { METRIC_IO_SYSTEMD_BASIC_PREFIX "Hostname", "System hostname", @@ -592,6 +809,12 @@ static const MetricFamily metric_family_table[] = { MACHINE_INFO_STANDARD_FIELD("LOCATION"), MACHINE_INFO_STANDARD_FIELD("TAGS"), /* Keep those ↑ in sync with machine_info_generate(). */ + { + METRIC_IO_SYSTEMD_BASIC_PREFIX "MemoryUsedBytes", + "Memory currently in use (MemTotal minus MemAvailable) in bytes", + METRIC_FAMILY_TYPE_GAUGE, + .generate = memory_used_generate, + }, { METRIC_IO_SYSTEMD_BASIC_PREFIX "OSRelease.NAME", "Operating system human-readable name (PRETTY_NAME= or NAME= field from os-release)", @@ -616,6 +839,18 @@ static const MetricFamily metric_family_table[] = { METRIC_FAMILY_TYPE_GAUGE, .generate = physical_memory_generate, }, + { + METRIC_IO_SYSTEMD_BASIC_PREFIX "PressureAvg10", + "Pressure stall percentage over the last 10s (resource=cpu|memory|io, type=some|full)", + METRIC_FAMILY_TYPE_GAUGE, + .generate = pressure_generate, + }, + { + METRIC_IO_SYSTEMD_BASIC_PREFIX "PressureStallSeconds", + "Cumulative time stalled on a resource, in seconds (resource=cpu|memory|io, type=some|full)", + METRIC_FAMILY_TYPE_COUNTER, + }, + /* Keep those ↑ in sync with pressure_generate(). */ { /* NB: Here we use the naming of the field as per SMBIOS specification, i.e. undo the weird * renaming that Linux did on the fields. When new fields are added here, please make sure to @@ -650,6 +885,12 @@ static const MetricFamily metric_family_table[] = { METRIC_FAMILY_TYPE_GAUGE, .generate = swap_generate, }, + { + METRIC_IO_SYSTEMD_BASIC_PREFIX "SwapUsedBytes", + "Swap space currently in use in bytes", + METRIC_FAMILY_TYPE_GAUGE, + }, + /* Keep those ↑ in sync with swap_generate(). */ { METRIC_IO_SYSTEMD_BASIC_PREFIX "TPM2.Manufacturer", "TPM2 device manufacturer (ID_TPM2_MANUFACTURER property of the tpmrm0 device)", diff --git a/test/units/TEST-74-AUX-UTILS.report.sh b/test/units/TEST-74-AUX-UTILS.report.sh index 627e7fef65..625095c32e 100755 --- a/test/units/TEST-74-AUX-UTILS.report.sh +++ b/test/units/TEST-74-AUX-UTILS.report.sh @@ -2,6 +2,7 @@ # SPDX-License-Identifier: LGPL-2.1-or-later set -eux set -o pipefail +shopt -s nullglob # Unset $PAGER so we don't have to use --no-pager everywhere export PAGER= @@ -77,20 +78,87 @@ id2="$(. /etc/os-release; echo "$ID")" basic_metrics="$(varlinkctl call --more /run/systemd/report/io.systemd.Basic io.systemd.Metrics.List {})" # NB: '| tostring' turns the numeric value into a raw string, so 'jq -r' prints it cleanly # (numbers, unlike strings, are otherwise still emitted with the json-seq record separator). -basic_value() { echo "$basic_metrics" | jq --seq -r "select(.name == \"$1\") | .value | tostring"; } +basic_value() { echo "$basic_metrics" | jq --seq -r "select(.name == \"io.systemd.Basic.$1\") | .value | tostring"; } +# Like basic_value, but the value must be a JSON number (jq's 'numbers' filter emits the value +# only if it is one, so a non-empty result confirms both presence and type), and an optional +# second argument gives a jq condition further restricting the match, e.g. on .object or .fields. +basic_number() { echo "$basic_metrics" | jq --seq -r "select(.name == \"io.systemd.Basic.$1\" and ${2:-true}) | .value | numbers | tostring"; } -# The three classic load average fields must be present and numeric (jq's 'numbers' filter -# emits the value only if it is a JSON number, so a non-empty result confirms both). +# The three classic load average fields must be present and numeric. for field in LoadAverage1Min LoadAverage5Min LoadAverage15Min; do - loadavg="$(echo "$basic_metrics" | jq --seq -r "select(.name == \"io.systemd.Basic.$field\") | .value | numbers | tostring")" - test -n "$loadavg" + test -n "$(basic_number "$field")" done # SwapBytes must match the total the kernel reports in /proc/meminfo (which is in kB). -swap_reported="$(basic_value io.systemd.Basic.SwapBytes)" +swap_reported="$(basic_value SwapBytes)" swap_expected=$(( $(awk '/^SwapTotal:/ { print $2; found=1 } END { if (!found) print 0 }' /proc/meminfo) * 1024 )) [ "$swap_reported" = "$swap_expected" ] +# SwapUsedBytes must be numeric and cannot exceed the configured total. +swap_used="$(basic_number SwapUsedBytes)" +test -n "$swap_used" +[ "$swap_used" -le "$swap_reported" ] + +# Those fields must be reported with a numeric nanoseconds value. +for type in user system; do + test -n "$(basic_number CPUUsage ".fields.type == \"$type\"")" +done + +# MemoryUsedBytes is MemTotal − MemAvailable, so it cannot exceed the MemTotal reported by the +# same file. (PhysicalMemoryBytes is not a valid bound here: it is capped by the root cgroup's +# memory limit, so it can legitimately be smaller than the value computed from /proc/meminfo.) +mem_used="$(basic_number MemoryUsedBytes)" +mem_total=$(( $(awk '/^MemTotal:/ { print $2 }' /proc/meminfo) * 1024 )) +test -n "$mem_used" +[ "$mem_used" -le "$mem_total" ] + +# Pressure metrics are only reported if the kernel supports PSI. +if cat /proc/pressure/cpu >/dev/null 2>&1; then + for family in PressureAvg10 PressureStallSeconds; do + for resource in cpu memory io; do + test -n "$(basic_number "$family" ".fields.resource == \"$resource\" and .fields.type == \"some\"")" + # "full" is deliberately not emitted for cpu, where the kernel always reports 0. + full="$(basic_number "$family" ".fields.resource == \"$resource\" and .fields.type == \"full\"")" + if [ "$resource" = cpu ]; then + test -z "$full" + else + test -n "$full" + fi + done + done + + # The avg10 values are percentages, so they must lie in the 0…100 range. This would catch a + # wrong fixed-point conversion, which would inflate the value 2048x. + echo "$basic_metrics" | jq --seq -r 'select(.name == "io.systemd.Basic.PressureAvg10") | .value | tostring' | + awk '{ if ($1 < 0 || $1 > 100) exit 1 }' +fi + +# Every whole physical block device (not partitions, not virtual or stacked devices) must be +# reported, with byte values derived from the 512-byte sector counters in sysfs. Those counters +# only grow, so the values sampled above cannot exceed what sysfs shows now. +for dev in /sys/block/*; do + devpath="$(udevadm info --query=property --value --property=DEVPATH "$dev" || :)" + devname="$(udevadm info --query=property --value --property=DEVNAME "$dev" || :)" + [[ -n "$devpath" && -n "$devname" ]] || continue + + [[ "$devpath" =~ "/devices/virtual/" ]] && continue + + read_bytes="$(basic_number DiskReadBytes ".object == \"${devname}\"")" + write_bytes="$(basic_number DiskWriteBytes ".object == \"${devname}\"")" + test -n "$read_bytes" + test -n "$write_bytes" + + read -r _ _ read_sectors _ _ _ write_sectors _ <"$dev/stat" + [ "$read_bytes" -le "$(( read_sectors * 512 ))" ] + [ "$write_bytes" -le "$(( write_sectors * 512 ))" ] +done + +# Every new metric family must be described, even those whose values depend on the environment. +basic_describe="$(varlinkctl call --more /run/systemd/report/io.systemd.Basic io.systemd.Metrics.Describe {})" +for name in CPUUsage DiskReadBytes DiskWriteBytes MemoryUsedBytes PressureAvg10 PressureStallSeconds SwapUsedBytes; do + echo "$basic_describe" | grep -F "io.systemd.Basic.$name" >/dev/null +done + # io.systemd.Manager.Version should be non-empty and match what `systemctl --version` reports metrics_version="$(varlinkctl call --more /run/systemd/report/io.systemd.Manager io.systemd.Metrics.List {} | jq --seq -r 'select(.name == "io.systemd.Manager.Version") | .value')" [ -n "$metrics_version" ] From 65ba2ff4a7b126d9f5359ae08d7a1cd98eeff9b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 15 Aug 2026 21:16:34 +0200 Subject: [PATCH 10/11] report-basic: add idle/iowait/steal as CPUWait family As discussed in the PR, those fields can go down, so they cannot be advertised as type COUNTER. But the type is shared for the whole family, so we need to create a new family to keep the other ones as COUNTER and add the new ones as GAUGE. kernel/sched/cputime.c says above get_cpu_idle_time_us() and get_cpu_iowait_time_us(): > Note that this is partially broken due to the counter of iowait tasks that > can be remotely updated without any synchronization. Therefore it is possible > to observe backward values within two consecutive reads. steal is apparently also unreliable because it is reported by the hyperviser. So let's be cautious and report it as gauge too. --- src/report/report-basic.c | 39 ++++++++++++++++++++------ test/units/TEST-74-AUX-UTILS.report.sh | 14 ++++++++- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/report/report-basic.c b/src/report/report-basic.c index 7cb36d6885..4b8d9ce024 100644 --- a/src/report/report-basic.c +++ b/src/report/report-basic.c @@ -237,10 +237,11 @@ static int cpus_online_generate(const MetricFamily *mf, sd_varlink *link, void * /* fields= */ NULL); } -static int cpu_usage_generate(const MetricFamily *mf, sd_varlink *link, void *userdata) { +static int cpu_usage_generate(const MetricFamily mf[static 2], sd_varlink *link, void *userdata) { int r; - assert(mf && mf->name); + assert(mf && mf[0].name && mf[1].name); + assert(!mf[1].generate); assert(link); /* The aggregate "cpu" line of /proc/stat, converted from USER_HZ ticks to nanoseconds, matching @@ -254,24 +255,36 @@ static int cpu_usage_generate(const MetricFamily *mf, sd_varlink *link, void *us return 0; } - static const char* const types[] = { "user", "system" }; - const uint64_t values[] = { + static const struct { + const char *name; + size_t family_offset; + } attributes[] = { + { "user", 0 }, + { "system", 0 }, + { "idle", 1 }, + { "iowait", 1 }, + { "steal", 1 }, + }; + uint64_t values[] = { ticks.user + ticks.nice, ticks.system + ticks.irq + ticks.softirq, + ticks.idle, + ticks.iowait, + ticks.steal, }; - assert_cc(ELEMENTSOF(types) == ELEMENTSOF(values)); + assert_cc(ELEMENTSOF(attributes) == ELEMENTSOF(values)); - for (size_t i = 0; i < ELEMENTSOF(values); i++) { + for (size_t i = 0; i < ELEMENTSOF(attributes); i++) { _cleanup_(sd_json_variant_unrefp) sd_json_variant *fields = NULL; - r = sd_json_buildo(&fields, SD_JSON_BUILD_PAIR_STRING("type", types[i])); + r = sd_json_buildo(&fields, SD_JSON_BUILD_PAIR_STRING("type", attributes[i].name)); if (r < 0) return r; uint64_t nsec = procfs_ticks_to_nsec(values[i]); r = metric_build_send_unsigned( - mf, + &mf[attributes[i].family_offset], link, /* object= */ NULL, nsec, @@ -719,7 +732,7 @@ static int virtualization_generate(const MetricFamily *mf, sd_varlink *link, voi } static const MetricFamily metric_family_table[] = { - /* Keep entries ordered alphabetically */ + /* Keep entries (with .generate) ordered alphabetically */ { METRIC_IO_SYSTEMD_BASIC_PREFIX "Architecture", "CPU architecture", @@ -744,12 +757,20 @@ static const MetricFamily metric_family_table[] = { METRIC_FAMILY_TYPE_GAUGE, .generate = cpus_online_generate, }, + /* METRIC_FAMILY_TYPE_COUNTER must be monotonic, but steal and iowait can go down, + * so CPUUsage is split into two families of different types. */ { METRIC_IO_SYSTEMD_BASIC_PREFIX "CPUUsage", "Aggregate CPU usage across all CPUs in nanoseconds (type=user|system)", METRIC_FAMILY_TYPE_COUNTER, .generate = cpu_usage_generate, }, + { + METRIC_IO_SYSTEMD_BASIC_PREFIX "CPUWait", + "Aggregate CPU wait across all CPUs in nanoseconds (type=idle|iowait|steal)", + METRIC_FAMILY_TYPE_GAUGE, + }, + /* Keep those ↑ in sync with cpu_usage_generate(). */ { METRIC_IO_SYSTEMD_BASIC_PREFIX "DiskReadBytes", "Per block device metric: cumulative number of bytes read " diff --git a/test/units/TEST-74-AUX-UTILS.report.sh b/test/units/TEST-74-AUX-UTILS.report.sh index 625095c32e..5cf8cb7ac8 100755 --- a/test/units/TEST-74-AUX-UTILS.report.sh +++ b/test/units/TEST-74-AUX-UTILS.report.sh @@ -104,6 +104,18 @@ for type in user system; do test -n "$(basic_number CPUUsage ".fields.type == \"$type\"")" done +# The values are USER_HZ ticks converted to nanoseconds. The metrics were sampled before the +# current /proc/stat state, and the idle counter mostly grows, so the reported idle time +# should be positive and no larger than what /proc/stat shows now with some margin. +cpu_idle_reported="$(basic_number CPUWait '.fields.type == "idle"')" +awk -v hz="$(getconf CLK_TCK)" -v sampled="$cpu_idle_reported" \ + '/^cpu / { exit !(sampled > 0 && sampled <= ($5 + 1) / hz * 1e9 ) }' /proc/stat + +# iowait/idle/steal must be valid nanoseconds values, but can go up and down. +for type in idle iowait steal; do + test -n "$(basic_number CPUWait ".fields.type == \"$type\"")" +done + # MemoryUsedBytes is MemTotal − MemAvailable, so it cannot exceed the MemTotal reported by the # same file. (PhysicalMemoryBytes is not a valid bound here: it is capped by the root cgroup's # memory limit, so it can legitimately be smaller than the value computed from /proc/meminfo.) @@ -155,7 +167,7 @@ done # Every new metric family must be described, even those whose values depend on the environment. basic_describe="$(varlinkctl call --more /run/systemd/report/io.systemd.Basic io.systemd.Metrics.Describe {})" -for name in CPUUsage DiskReadBytes DiskWriteBytes MemoryUsedBytes PressureAvg10 PressureStallSeconds SwapUsedBytes; do +for name in CPUUsage CPUWait DiskReadBytes DiskWriteBytes MemoryUsedBytes PressureAvg10 PressureStallSeconds SwapUsedBytes; do echo "$basic_describe" | grep -F "io.systemd.Basic.$name" >/dev/null done From 27f63ef988f359825ca9608f63120d7da4c34b19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 19 Aug 2026 08:54:48 +0200 Subject: [PATCH 11/11] basic/procfs-util: make procfs_cpu_get_ticks lenient again In review it was raised that the file is often synthesized and that this synthetization might not include all the fields, let's allow the more recently added fields to be missing to avoid a regression in compatibility. --- src/basic/procfs-util.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/basic/procfs-util.c b/src/basic/procfs-util.c index 4e80e873a8..a255153422 100644 --- a/src/basic/procfs-util.c +++ b/src/basic/procfs-util.c @@ -6,6 +6,7 @@ #include "extract-word.h" #include "fd-util.h" #include "fileio.h" +#include "log.h" #include "parse-util.h" #include "process-util.h" #include "procfs-util.h" @@ -170,10 +171,11 @@ int procfs_cpu_get_ticks(ProcfsCpuTicks *ret) { return -EINVAL; /* All eight fields exist on any kernel we support: iowait was added in 2.5.41, steal in 2.6.11. - * "guest" and "guest_nice" are not returned, the kernel already includes them in "user" and - * "nice". */ - ProcfsCpuTicks t; - if (sscanf(p, "%"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64, + * "guest" and "guest_nice" are not returned, the kernel already includes them in "user" and "nice". + * Nevertheless, this file might be synthesized slopilly, so let's not require the last three fields. + * We don't care about them in most cases anyway. */ + ProcfsCpuTicks t = {}; + r = sscanf(p, "%"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64" %"SCNu64, &t.user, &t.nice, &t.system, @@ -181,8 +183,11 @@ int procfs_cpu_get_ticks(ProcfsCpuTicks *ret) { &t.iowait, &t.irq, &t.softirq, - &t.steal) < 8) + &t.steal); + if (r < 5) return -EINVAL; + if (r < 8) + log_once(LOG_INFO, "Short read (%d fields) from /proc/stat, continuing.", r); *ret = t; return 0;