From 1b42022388f7c87929dfa37717a14b7c25fac965 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Feb 2022 13:18:58 +0100 Subject: [PATCH 1/3] cgroup: downgrade warning if we can't get ID off cgroup The cgroupid feature was not available in old cgroupvs2 kernels, hence try to get it but if we can't because it's not supported, then only debug log about it and proceed. (We only needs this for cgroup bpf stuff, but that isn't available on such old kernels anyway) Fixes: #22483 --- src/core/cgroup.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 316e8f571d..f2044c2ffc 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -2134,7 +2134,6 @@ static int unit_update_cgroup( bool created, is_root_slice; CGroupMask migrate_mask = 0; _cleanup_free_ char *cgroup_full_path = NULL; - uint64_t cgroup_id = 0; int r; assert(u); @@ -2154,11 +2153,14 @@ static int unit_update_cgroup( created = r; if (cg_unified_controller(SYSTEMD_CGROUP_CONTROLLER) > 0) { + uint64_t cgroup_id = 0; + r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, NULL, &cgroup_full_path); if (r == 0) { r = cg_path_get_cgroupid(cgroup_full_path, &cgroup_id); if (r < 0) - log_unit_warning_errno(u, r, "Failed to get cgroup ID on cgroup %s, ignoring: %m", cgroup_full_path); + log_unit_full_errno(u, ERRNO_IS_NOT_SUPPORTED(r) ? LOG_DEBUG : LOG_WARNING, r, + "Failed to get cgroup ID of cgroup %s, ignoring: %m", cgroup_full_path); } else log_unit_warning_errno(u, r, "Failed to get full cgroup path on cgroup %s, ignoring: %m", empty_to_root(u->cgroup_path)); @@ -2169,7 +2171,6 @@ static int unit_update_cgroup( (void) unit_watch_cgroup(u); (void) unit_watch_cgroup_memory(u); - /* For v2 we preserve enabled controllers in delegated units, adjust others, * for v1 we figure out which controller hierarchies need migration. */ if (created || !u->cgroup_realized || !unit_cgroup_delegate(u)) { From a561253f0bea9dc584bf369662f6107af0a28c96 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Feb 2022 13:23:32 +0100 Subject: [PATCH 2/3] cgroup-util: minor modernizations Rename return parameters to "ret", use ternary op without second argument, rebreak comments, use isempty() more. --- src/basic/cgroup-util.c | 47 ++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index a626ecf2e2..1890cb09ff 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -462,14 +462,11 @@ int cg_kill_recursive( } static const char *controller_to_dirname(const char *controller) { - const char *e; - assert(controller); - /* Converts a controller name to the directory name below - * /sys/fs/cgroup/ we want to mount it to. Effectively, this - * just cuts off the name= prefixed used for named - * hierarchies, if it is specified. */ + /* Converts a controller name to the directory name below /sys/fs/cgroup/ we want to mount it + * to. Effectively, this just cuts off the name= prefixed used for named hierarchies, if it is + * specified. */ if (streq(controller, SYSTEMD_CGROUP_CONTROLLER)) { if (cg_hybrid_unified() > 0) @@ -478,18 +475,14 @@ static const char *controller_to_dirname(const char *controller) { controller = SYSTEMD_CGROUP_CONTROLLER_LEGACY; } - e = startswith(controller, "name="); - if (e) - return e; - - return controller; + return startswith(controller, "name=") ?: controller; } -static int join_path_legacy(const char *controller, const char *path, const char *suffix, char **fs) { +static int join_path_legacy(const char *controller, const char *path, const char *suffix, char **ret) { const char *dn; char *t = NULL; - assert(fs); + assert(ret); assert(controller); dn = controller_to_dirname(controller); @@ -505,14 +498,14 @@ static int join_path_legacy(const char *controller, const char *path, const char if (!t) return -ENOMEM; - *fs = t; + *ret = t; return 0; } -static int join_path_unified(const char *path, const char *suffix, char **fs) { +static int join_path_unified(const char *path, const char *suffix, char **ret) { char *t; - assert(fs); + assert(ret); if (isempty(path) && isempty(suffix)) t = strdup("/sys/fs/cgroup"); @@ -525,34 +518,34 @@ static int join_path_unified(const char *path, const char *suffix, char **fs) { if (!t) return -ENOMEM; - *fs = t; + *ret = t; return 0; } -int cg_get_path(const char *controller, const char *path, const char *suffix, char **fs) { +int cg_get_path(const char *controller, const char *path, const char *suffix, char **ret) { int r; - assert(fs); + assert(ret); if (!controller) { char *t; - /* If no controller is specified, we return the path - * *below* the controllers, without any prefix. */ + /* If no controller is specified, we return the path *below* the controllers, without any + * prefix. */ if (!path && !suffix) return -EINVAL; - if (!suffix) + if (isempty(suffix)) t = strdup(path); - else if (!path) + else if (isempty(path)) t = strdup(suffix); else t = path_join(path, suffix); if (!t) return -ENOMEM; - *fs = path_simplify(t); + *ret = path_simplify(t); return 0; } @@ -563,13 +556,13 @@ int cg_get_path(const char *controller, const char *path, const char *suffix, ch if (r < 0) return r; if (r > 0) - r = join_path_unified(path, suffix, fs); + r = join_path_unified(path, suffix, ret); else - r = join_path_legacy(controller, path, suffix, fs); + r = join_path_legacy(controller, path, suffix, ret); if (r < 0) return r; - path_simplify(*fs); + path_simplify(*ret); return 0; } From 5483fca07ac66726bc0f7c60890e5f8796ce49a8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Feb 2022 13:24:35 +0100 Subject: [PATCH 3/3] pid1: export cgroup ID among per-unit cgroup information It's really interesting for debugging purposes and we have it already, hence expose it as dbus property. --- man/org.freedesktop.systemd1.xml | 36 ++++++++++++++++++++++++++++++++ src/core/dbus-unit.c | 1 + 2 files changed, 37 insertions(+) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 97a8c98b39..8171241e4e 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -2452,6 +2452,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ControlGroup = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") + readonly t ControlGroupId = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly t MemoryCurrent = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly t MemoryAvailable = ...; @@ -3023,6 +3025,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + @@ -3599,6 +3603,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + @@ -4334,6 +4340,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2esocket { @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ControlGroup = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") + readonly t ControlGroupId = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly t MemoryCurrent = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly t MemoryAvailable = ...; @@ -4929,6 +4937,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2esocket { + + @@ -5499,6 +5509,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2esocket { + + @@ -6123,6 +6135,8 @@ node /org/freedesktop/systemd1/unit/home_2emount { @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ControlGroup = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") + readonly t ControlGroupId = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly t MemoryCurrent = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly t MemoryAvailable = ...; @@ -6646,6 +6660,8 @@ node /org/freedesktop/systemd1/unit/home_2emount { + + @@ -7134,6 +7150,8 @@ node /org/freedesktop/systemd1/unit/home_2emount { + + @@ -7885,6 +7903,8 @@ node /org/freedesktop/systemd1/unit/dev_2dsda3_2eswap { @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ControlGroup = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") + readonly t ControlGroupId = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly t MemoryCurrent = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly t MemoryAvailable = ...; @@ -8394,6 +8414,8 @@ node /org/freedesktop/systemd1/unit/dev_2dsda3_2eswap { + + @@ -8868,6 +8890,8 @@ node /org/freedesktop/systemd1/unit/dev_2dsda3_2eswap { + + @@ -9478,6 +9502,8 @@ node /org/freedesktop/systemd1/unit/system_2eslice { @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ControlGroup = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") + readonly t ControlGroupId = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly t MemoryCurrent = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly t MemoryAvailable = ...; @@ -9629,6 +9655,8 @@ node /org/freedesktop/systemd1/unit/system_2eslice { + + @@ -9783,6 +9811,8 @@ node /org/freedesktop/systemd1/unit/system_2eslice { + + @@ -9961,6 +9991,8 @@ node /org/freedesktop/systemd1/unit/session_2d1_2escope { @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ControlGroup = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") + readonly t ControlGroupId = ...; + @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly t MemoryCurrent = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly t MemoryAvailable = ...; @@ -10130,6 +10162,8 @@ node /org/freedesktop/systemd1/unit/session_2d1_2escope { + + @@ -10312,6 +10346,8 @@ node /org/freedesktop/systemd1/unit/session_2d1_2escope { + + diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 1128c42ad9..1e01241676 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1588,6 +1588,7 @@ const sd_bus_vtable bus_unit_cgroup_vtable[] = { SD_BUS_VTABLE_START(0), SD_BUS_PROPERTY("Slice", "s", property_get_slice, 0, 0), SD_BUS_PROPERTY("ControlGroup", "s", property_get_cgroup, 0, 0), + SD_BUS_PROPERTY("ControlGroupId", "t", NULL, offsetof(Unit, cgroup_id), 0), SD_BUS_PROPERTY("MemoryCurrent", "t", property_get_current_memory, 0, 0), SD_BUS_PROPERTY("MemoryAvailable", "t", property_get_available_memory, 0, 0), SD_BUS_PROPERTY("CPUUsageNSec", "t", property_get_cpu_usage, 0, 0),