From 0887fa711cc3ed63114ec53b4052a626f6929915 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 17:25:11 +0200 Subject: [PATCH 1/8] cgroup-util: debug log if /proc/self/ns/cgroup is not available for unexpected reasons --- src/basic/cgroup-util.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 19723d3084..65ca6f2f50 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -129,10 +129,12 @@ bool cg_ns_supported(void) { if (enabled >= 0) return enabled; - if (access("/proc/self/ns/cgroup", F_OK) == 0) - enabled = 1; - else - enabled = 0; + if (access("/proc/self/ns/cgroup", F_OK) < 0) { + if (errno != ENOENT) + log_debug_errno(errno, "Failed to check whether /proc/self/ns/cgroup is available, assuming not: %m"); + enabled = false; + } else + enabled = true; return enabled; } From 38a90d45ad517fbfdb6fdb8ab04c71991727b482 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 17:25:51 +0200 Subject: [PATCH 2/8] cgroup-util: don't expect cg_mask_from_string()'s return value to be initialized Also, when we fail, don't clobber the return value. This brings the call more in-line with our usual coding style, and removes surprises. None of the callers seemed to care about this behaviour. --- src/basic/cgroup-util.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 65ca6f2f50..d2e8ef0c07 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -2303,8 +2303,10 @@ int cg_mask_to_string(CGroupMask mask, char **ret) { return 0; } -int cg_mask_from_string(const char *value, CGroupMask *mask) { - assert(mask); +int cg_mask_from_string(const char *value, CGroupMask *ret) { + CGroupMask m = 0; + + assert(ret); assert(value); for (;;) { @@ -2322,13 +2324,15 @@ int cg_mask_from_string(const char *value, CGroupMask *mask) { if (v < 0) continue; - *mask |= CGROUP_CONTROLLER_TO_MASK(v); + m |= CGROUP_CONTROLLER_TO_MASK(v); } + + *ret = m; return 0; } int cg_mask_supported(CGroupMask *ret) { - CGroupMask mask = 0; + CGroupMask mask; int r; /* Determines the mask of supported cgroup controllers. Only @@ -2372,6 +2376,7 @@ int cg_mask_supported(CGroupMask *ret) { /* In the legacy hierarchy, we check whether which * hierarchies are mounted. */ + mask = 0; for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { const char *n; From 604028de6021e3bba62b1065e3655b071e342bf5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 17:28:17 +0200 Subject: [PATCH 3/8] cgroup-util: disable buffering for cg_enable_everywhere() when writing to cgroup attributes Let's better be safe than sorry. --- src/basic/cgroup-util.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index d2e8ef0c07..a501049619 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -819,7 +819,7 @@ int cg_attach(const char *controller, const char *path, pid_t pid) { xsprintf(c, PID_FMT "\n", pid); - r = write_string_file(fs, c, 0); + r = write_string_file(fs, c, WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) return r; @@ -1101,7 +1101,7 @@ int cg_install_release_agent(const char *controller, const char *agent) { sc = strstrip(contents); if (isempty(sc)) { - r = write_string_file(fs, agent, 0); + r = write_string_file(fs, agent, WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) return r; } else if (!path_equal(sc, agent)) @@ -1119,7 +1119,7 @@ int cg_install_release_agent(const char *controller, const char *agent) { sc = strstrip(contents); if (streq(sc, "0")) { - r = write_string_file(fs, "1", 0); + r = write_string_file(fs, "1", WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) return r; @@ -1146,7 +1146,7 @@ int cg_uninstall_release_agent(const char *controller) { if (r < 0) return r; - r = write_string_file(fs, "0", 0); + r = write_string_file(fs, "0", WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) return r; @@ -1156,7 +1156,7 @@ int cg_uninstall_release_agent(const char *controller) { if (r < 0) return r; - r = write_string_file(fs, "", 0); + r = write_string_file(fs, "", WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) return r; @@ -2015,7 +2015,7 @@ int cg_set_attribute(const char *controller, const char *path, const char *attri if (r < 0) return r; - return write_string_file(p, value, 0); + return write_string_file(p, value, WRITE_STRING_FILE_DISABLE_BUFFER); } int cg_get_attribute(const char *controller, const char *path, const char *attribute, char **ret) { @@ -2604,7 +2604,7 @@ int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p) { } } - r = write_string_stream(f, s, 0); + r = write_string_stream(f, s, WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) { log_debug_errno(r, "Failed to enable controller %s for %s (%s): %m", n, p, fs); clearerr(f); From 4edd65e4cff40158dded65c010b8c3e0b5ff5519 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 17:30:46 +0200 Subject: [PATCH 4/8] cgroup-util: add mask definitions for sets of controllers supported by cgroupsv1 vs. cgroupsv2 --- src/basic/cgroup-util.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index 73b431a27a..546b86eb7c 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -49,6 +49,16 @@ typedef enum CGroupMask { CGROUP_MASK_PIDS = CGROUP_CONTROLLER_TO_MASK(CGROUP_CONTROLLER_PIDS), CGROUP_MASK_BPF_FIREWALL = CGROUP_CONTROLLER_TO_MASK(CGROUP_CONTROLLER_BPF_FIREWALL), CGROUP_MASK_BPF_DEVICES = CGROUP_CONTROLLER_TO_MASK(CGROUP_CONTROLLER_BPF_DEVICES), + + /* All real cgroupv1 controllers */ + CGROUP_MASK_V1 = CGROUP_MASK_CPU|CGROUP_MASK_CPUACCT|CGROUP_MASK_BLKIO|CGROUP_MASK_MEMORY|CGROUP_MASK_DEVICES|CGROUP_MASK_PIDS, + + /* All real cgroupv2 controllers */ + CGROUP_MASK_V2 = CGROUP_MASK_CPU|CGROUP_MASK_IO|CGROUP_MASK_MEMORY|CGROUP_MASK_PIDS, + + /* All cgroupv2 BPF pseudo-controllers */ + CGROUP_MASK_BPF = CGROUP_MASK_BPF_FIREWALL|CGROUP_MASK_BPF_DEVICES, + _CGROUP_MASK_ALL = CGROUP_CONTROLLER_TO_MASK(_CGROUP_CONTROLLER_MAX) - 1 } CGroupMask; From ab275f23867311e136887755728e6b368324831d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 17:31:51 +0200 Subject: [PATCH 5/8] cgroup-util: before operating on a mounted cgroup controller check if it actually can be mounted We now have the "BPF" pseudo-controllers. These should never be assumed to be accessible as /sys/fs/cgroup/ and not through "cgroup.subtree_control" either, hence always check explicitly before we go to the file system. We do this through our new CGROUP_MASK_V1 and CGROUP_MASK_V2 definitions. --- src/basic/cgroup-util.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index a501049619..520e18e1b0 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -2139,6 +2139,9 @@ int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); const char *n; + if (!FLAGS_SET(CGROUP_MASK_V1, bit)) + continue; + n = cgroup_controller_to_string(c); if (mask & bit) @@ -2168,6 +2171,9 @@ int cg_attach_everywhere(CGroupMask supported, const char *path, pid_t pid, cg_m CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); const char *p = NULL; + if (!FLAGS_SET(CGROUP_MASK_V1, bit)) + continue; + if (!(supported & bit)) continue; @@ -2220,6 +2226,9 @@ int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); const char *p = NULL; + if (!FLAGS_SET(CGROUP_MASK_V1, bit)) + continue; + if (!(supported & bit)) continue; @@ -2252,6 +2261,9 @@ int cg_trim_everywhere(CGroupMask supported, const char *path, bool delete_root) for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); + if (!FLAGS_SET(CGROUP_MASK_V1, bit)) + continue; + if (!(supported & bit)) continue; @@ -2586,6 +2598,9 @@ int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p) { CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); const char *n; + if (!FLAGS_SET(CGROUP_MASK_V2, bit)) + continue; + if (!(supported & bit)) continue; From 03afd780298897f948dab9345c0c4f1bc408bf8d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Oct 2018 17:35:19 +0200 Subject: [PATCH 6/8] cgroup: when discovering which controllers the kernel supports mask with what we support Let's use our new CGROUP_MASK_V1 and CGROUP_MASK_V2 definitions for this. --- src/basic/cgroup-util.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 520e18e1b0..1e159571c2 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -2377,24 +2377,26 @@ int cg_mask_supported(CGroupMask *ret) { if (r < 0) return r; - /* Currently, we support the cpu, memory, io and pids - * controller in the unified hierarchy, mask + /* Currently, we support the cpu, memory, io and pids controller in the unified hierarchy, mask * everything else off. */ - mask &= CGROUP_MASK_CPU | CGROUP_MASK_MEMORY | CGROUP_MASK_IO | CGROUP_MASK_PIDS; + mask &= CGROUP_MASK_V2; } else { CGroupController c; - /* In the legacy hierarchy, we check whether which - * hierarchies are mounted. */ + /* In the legacy hierarchy, we check which hierarchies are mounted. */ mask = 0; for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { + CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); const char *n; + if (!FLAGS_SET(CGROUP_MASK_V1, bit)) + continue; + n = cgroup_controller_to_string(c); if (controller_is_accessible(n) >= 0) - mask |= CGROUP_CONTROLLER_TO_MASK(c); + mask |= bit; } } From f99850a0d4fb6d00f43c050f33a1863f704358ad Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Oct 2018 15:25:21 +0200 Subject: [PATCH 7/8] cgroup-util: FLAGS_SET()ify all things --- src/basic/cgroup-util.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 1e159571c2..5c02f0cb35 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -2144,9 +2144,9 @@ int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path n = cgroup_controller_to_string(c); - if (mask & bit) + if (FLAGS_SET(mask, bit)) (void) cg_create(n, path); - else if (supported & bit) + else if (FLAGS_SET(supported, bit)) (void) cg_trim(n, path, true); } @@ -2174,7 +2174,7 @@ int cg_attach_everywhere(CGroupMask supported, const char *path, pid_t pid, cg_m if (!FLAGS_SET(CGROUP_MASK_V1, bit)) continue; - if (!(supported & bit)) + if (!FLAGS_SET(supported, bit)) continue; if (path_callback) @@ -2229,7 +2229,7 @@ int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to if (!FLAGS_SET(CGROUP_MASK_V1, bit)) continue; - if (!(supported & bit)) + if (!FLAGS_SET(supported, bit)) continue; if (to_callback) @@ -2264,7 +2264,7 @@ int cg_trim_everywhere(CGroupMask supported, const char *path, bool delete_root) if (!FLAGS_SET(CGROUP_MASK_V1, bit)) continue; - if (!(supported & bit)) + if (!FLAGS_SET(supported, bit)) continue; (void) cg_trim(cgroup_controller_to_string(c), path, delete_root); @@ -2290,7 +2290,7 @@ int cg_mask_to_string(CGroupMask mask, char **ret) { const char *k; size_t l; - if (!(mask & CGROUP_CONTROLLER_TO_MASK(c))) + if (!FLAGS_SET(mask, CGROUP_CONTROLLER_TO_MASK(c))) continue; k = cgroup_controller_to_string(c); @@ -2603,14 +2603,14 @@ int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p) { if (!FLAGS_SET(CGROUP_MASK_V2, bit)) continue; - if (!(supported & bit)) + if (!FLAGS_SET(supported, bit)) continue; n = cgroup_controller_to_string(c); { char s[1 + strlen(n) + 1]; - s[0] = mask & bit ? '+' : '-'; + s[0] = FLAGS_SET(mask, bit) ? '+' : '-'; strcpy(s + 1, n); if (!f) { From 46f84f955fa718fe674bc56e7ea40cc286043142 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 26 Oct 2018 15:31:30 +0200 Subject: [PATCH 8/8] cgroup-util: make definition of CGROUP_CONTROLLER_TO_MASK() unsigned Otherwise doing comparing a CGroupMask (which is unsigned in effect) with the result of CGROUP_CONTROLLER_TO_MASK() will result in warnings about signedness differences. --- src/basic/cgroup-util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index 546b86eb7c..c2bf487ab3 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -36,7 +36,7 @@ typedef enum CGroupController { _CGROUP_CONTROLLER_INVALID = -1, } CGroupController; -#define CGROUP_CONTROLLER_TO_MASK(c) (1 << (c)) +#define CGROUP_CONTROLLER_TO_MASK(c) (1U << (c)) /* A bit mask of well known cgroup controllers */ typedef enum CGroupMask {