From 879952a85311ca375c673480d9718a9e0cd93c1d Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 26 May 2025 22:46:17 +0200 Subject: [PATCH 1/3] core/cgroup: CGRuntime.cgroup_path indicates whether the cg is still alive so drop redundant checks in attr getters. Memory and IO accounting functions already follow this pattern. --- src/core/cgroup.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index ed5d0f975d..61ac5b6f44 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -3626,8 +3626,6 @@ static int unit_get_cpu_usage_raw(const Unit *u, const CGroupRuntime *crt, nsec_ uint64_t us; r = cg_get_keyed_attribute("cpu", crt->cgroup_path, "cpu.stat", STRV_MAKE("usage_usec"), &val); - if (IN_SET(r, -ENOENT, -ENXIO)) - return -ENODATA; if (r < 0) return r; @@ -4082,8 +4080,6 @@ static int unit_cgroup_freezer_kernel_state(Unit *u, FreezerState *ret) { "cgroup.events", STRV_MAKE("frozen"), &val); - if (IN_SET(r, -ENOENT, -ENXIO)) - return -ENODATA; if (r < 0) return r; From c3f900770d0cff0cdf89751b614397b5e226194a Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 26 May 2025 22:27:26 +0200 Subject: [PATCH 2/3] cgroup-util: drop handcrafted cg_is_empty(), always check cgroup.events populated field This effectively renames cg_is_empty_recursive() to cg_is_empty(). Note that all existing code calls the former and not the latter, hence with cgv1 support being dropped it's trivial to consult cgroup.events directly for populated state everywhere. Additionally, use more generic cg_get_keyed_attribute() helper rather than cg_read_event(). --- src/basic/cgroup-util.c | 76 ++++++---------------------------------- src/basic/cgroup-util.h | 1 - src/core/cgroup.c | 2 +- src/core/service.c | 2 +- src/login/loginctl.c | 2 +- src/machine/machinectl.c | 2 +- src/shared/cgroup-show.c | 2 +- src/test/test-cgroup.c | 4 +-- 8 files changed, 16 insertions(+), 75 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 1c75242892..02398d074a 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -792,81 +792,25 @@ int cg_pidref_get_path(const char *controller, const PidRef *pidref, char **ret_ } int cg_is_empty(const char *controller, const char *path) { - _cleanup_fclose_ FILE *f = NULL; - pid_t pid; + _cleanup_free_ char *t = NULL; int r; + /* Check if the cgroup hierarchy under 'path' is empty. On cgroup v2 it's exposed via the "populated" + * attribute of "cgroup.events". */ + assert(path); - r = cg_enumerate_processes(controller, path, &f); + /* The root cgroup is always populated */ + if (empty_or_root(path)) + return false; + + r = cg_get_keyed_attribute(SYSTEMD_CGROUP_CONTROLLER, path, "cgroup.events", STRV_MAKE("populated"), &t); if (r == -ENOENT) return true; if (r < 0) return r; - r = cg_read_pid(f, &pid, CGROUP_DONT_SKIP_UNMAPPED); - if (r < 0) - return r; - - return r == 0; -} - -int cg_is_empty_recursive(const char *controller, const char *path) { - int r; - - assert(path); - - /* The root cgroup is always populated */ - if (controller && empty_or_root(path)) - return false; - - r = cg_unified_controller(controller); - if (r < 0) - return r; - if (r > 0) { - _cleanup_free_ char *t = NULL; - - /* On the unified hierarchy we can check empty state - * via the "populated" attribute of "cgroup.events". */ - - r = cg_read_event(controller, path, "populated", &t); - if (r == -ENOENT) - return true; - if (r < 0) - return r; - - return streq(t, "0"); - } else { - _cleanup_closedir_ DIR *d = NULL; - char *fn; - - r = cg_is_empty(controller, path); - if (r <= 0) - return r; - - r = cg_enumerate_subgroups(controller, path, &d); - if (r == -ENOENT) - return true; - if (r < 0) - return r; - - while ((r = cg_read_subgroup(d, &fn)) > 0) { - _cleanup_free_ char *p = NULL; - - p = path_join(path, fn); - free(fn); - if (!p) - return -ENOMEM; - - r = cg_is_empty_recursive(controller, p); - if (r <= 0) - return r; - } - if (r < 0) - return r; - - return true; - } + return streq(t, "0"); } int cg_split_spec(const char *spec, char **ret_controller, char **ret_path) { diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index acb3427875..9f02e65410 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -233,7 +233,6 @@ int cg_get_xattr_bool(const char *path, const char *name); int cg_remove_xattr(const char *path, const char *name); int cg_is_empty(const char *controller, const char *path); -int cg_is_empty_recursive(const char *controller, const char *path); int cg_get_root_path(char **path); diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 61ac5b6f44..abaffb22cd 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -2760,7 +2760,7 @@ int unit_cgroup_is_empty(Unit *u) { if (!crt->cgroup_path) return -EOWNERDEAD; - r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, crt->cgroup_path); + r = cg_is_empty(SYSTEMD_CGROUP_CONTROLLER, crt->cgroup_path); if (r < 0) log_unit_debug_errno(u, r, "Failed to determine whether cgroup %s is empty: %m", empty_to_root(crt->cgroup_path)); return r; diff --git a/src/core/service.c b/src/core/service.c index d5874ce393..c976ee63e4 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1976,7 +1976,7 @@ static int cgroup_good(Service *s) { if (!s->cgroup_runtime || !s->cgroup_runtime->cgroup_path) return 0; - r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, s->cgroup_runtime->cgroup_path); + r = cg_is_empty(SYSTEMD_CGROUP_CONTROLLER, s->cgroup_runtime->cgroup_path); if (r < 0) return r; diff --git a/src/login/loginctl.c b/src/login/loginctl.c index 0f66c9dee6..1cbbd4ecf7 100644 --- a/src/login/loginctl.c +++ b/src/login/loginctl.c @@ -457,7 +457,7 @@ static int show_unit_cgroup( /* Fallback for older systemd versions where the GetUnitProcesses() call is not yet available */ - if (cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, cgroup) != 0 && leader <= 0) + if (cg_is_empty(SYSTEMD_CGROUP_CONTROLLER, cgroup) != 0 && leader <= 0) return 0; show_cgroup_and_extra(SYSTEMD_CGROUP_CONTROLLER, cgroup, prefix, c, &leader, leader > 0, get_output_flags()); diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index d8dbaa09c5..32849529c9 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -436,7 +436,7 @@ static int show_unit_cgroup(sd_bus *bus, const char *unit, pid_t leader) { /* Fallback for older systemd versions where the GetUnitProcesses() call is not yet available */ - if (cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, cgroup) != 0 && leader <= 0) + if (cg_is_empty(SYSTEMD_CGROUP_CONTROLLER, cgroup) != 0 && leader <= 0) return 0; show_cgroup_and_extra(SYSTEMD_CGROUP_CONTROLLER, cgroup, "\t\t ", c, &leader, leader > 0, get_output_flags()); diff --git a/src/shared/cgroup-show.c b/src/shared/cgroup-show.c index 258229daf3..b95cecb3e6 100644 --- a/src/shared/cgroup-show.c +++ b/src/shared/cgroup-show.c @@ -251,7 +251,7 @@ int show_cgroup_by_path( if (!k) return -ENOMEM; - if (!(flags & OUTPUT_SHOW_ALL) && cg_is_empty_recursive(NULL, k) > 0) + if (!(flags & OUTPUT_SHOW_ALL) && cg_is_empty(NULL, k) > 0) continue; if (!shown_pids) { diff --git a/src/test/test-cgroup.c b/src/test/test-cgroup.c index 4d1bbd4b34..31f2d87800 100644 --- a/src/test/test-cgroup.c +++ b/src/test/test-cgroup.c @@ -106,9 +106,7 @@ TEST(cg_create) { free(path); ASSERT_OK_POSITIVE(cg_is_empty(SYSTEMD_CGROUP_CONTROLLER, test_a)); - ASSERT_OK_POSITIVE(cg_is_empty(SYSTEMD_CGROUP_CONTROLLER, test_b)); - ASSERT_OK_POSITIVE(cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, test_a)); - ASSERT_OK_ZERO(cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, test_b)); + ASSERT_OK_ZERO(cg_is_empty(SYSTEMD_CGROUP_CONTROLLER, test_b)); ASSERT_OK_ZERO(cg_kill_recursive(test_a, 0, 0, NULL, NULL, NULL)); ASSERT_OK_POSITIVE(cg_kill_recursive(test_b, 0, 0, NULL, NULL, NULL)); From b823f7bb6a2295f3634e60fade3fd36ae0fa7cdb Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 26 May 2025 22:32:12 +0200 Subject: [PATCH 3/3] cgroup-util: remove now unused cg_read_event() cg_get_keyed_attribute() is a more generic version of this, and cg_is_empty_recursive() was the only user of this function, which got converted in the previous commit. --- src/basic/cgroup-util.c | 41 ----------------------------------------- src/basic/cgroup-util.h | 1 - 2 files changed, 42 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 02398d074a..4ceec690ab 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -198,47 +198,6 @@ int cg_read_pidref(FILE *f, PidRef *ret, CGroupFlags flags) { } } -int cg_read_event( - const char *controller, - const char *path, - const char *event, - char **ret) { - - _cleanup_free_ char *events = NULL, *content = NULL; - int r; - - r = cg_get_path(controller, path, "cgroup.events", &events); - if (r < 0) - return r; - - r = read_full_virtual_file(events, &content, NULL); - if (r < 0) - return r; - - for (const char *p = content;;) { - _cleanup_free_ char *line = NULL, *key = NULL; - const char *q; - - r = extract_first_word(&p, &line, "\n", 0); - if (r < 0) - return r; - if (r == 0) - return -ENOENT; - - q = line; - r = extract_first_word(&q, &key, " ", 0); - if (r < 0) - return r; - if (r == 0) - return -EINVAL; - - if (!streq(key, event)) - continue; - - return strdup_to(ret, q); - } -} - bool cg_kill_supported(void) { static thread_local int supported = -1; diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index 9f02e65410..27778dabed 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -188,7 +188,6 @@ typedef enum CGroupFlags { int cg_enumerate_processes(const char *controller, const char *path, FILE **ret); int cg_read_pid(FILE *f, pid_t *ret, CGroupFlags flags); int cg_read_pidref(FILE *f, PidRef *ret, CGroupFlags flags); -int cg_read_event(const char *controller, const char *path, const char *event, char **ret); int cg_enumerate_subgroups(const char *controller, const char *path, DIR **ret); int cg_read_subgroup(DIR *d, char **ret);