From fea72cc0336f4d90875cdddc1aa9739dcbb174f5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 3 Sep 2015 13:22:51 +0200 Subject: macro: introduce new PID_TO_PTR macros and make use of them This adds a new PID_TO_PTR() macro, plus PTR_TO_PID() and makes use of it wherever we maintain processes in a hash table. Previously we sometimes used LONG_TO_PTR() and other times ULONG_TO_PTR() for that, hence let's make this more explicit and clean up things. --- src/basic/cgroup-util.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src/basic/cgroup-util.c') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 0ebe570bb8..74e1668a17 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -187,7 +187,7 @@ int cg_kill(const char *controller, const char *path, int sig, bool sigcont, boo if (ignore_self && pid == my_pid) continue; - if (set_get(s, LONG_TO_PTR(pid)) == LONG_TO_PTR(pid)) + if (set_get(s, PID_TO_PTR(pid)) == PID_TO_PTR(pid)) continue; /* If we haven't killed this process yet, kill @@ -205,7 +205,7 @@ int cg_kill(const char *controller, const char *path, int sig, bool sigcont, boo done = false; - r = set_put(s, LONG_TO_PTR(pid)); + r = set_put(s, PID_TO_PTR(pid)); if (r < 0) { if (ret >= 0) return r; @@ -318,7 +318,7 @@ int cg_migrate(const char *cfrom, const char *pfrom, const char *cto, const char if (ignore_self && pid == my_pid) continue; - if (set_get(s, LONG_TO_PTR(pid)) == LONG_TO_PTR(pid)) + if (set_get(s, PID_TO_PTR(pid)) == PID_TO_PTR(pid)) continue; /* Ignore kernel threads. Since they can only @@ -338,7 +338,7 @@ int cg_migrate(const char *cfrom, const char *pfrom, const char *cto, const char done = false; - r = set_put(s, LONG_TO_PTR(pid)); + r = set_put(s, PID_TO_PTR(pid)); if (r < 0) { if (ret >= 0) return r; @@ -1898,7 +1898,7 @@ int cg_attach_many_everywhere(CGroupMask supported, const char *path, Set* pids, int r = 0; SET_FOREACH(pidp, pids, i) { - pid_t pid = PTR_TO_LONG(pidp); + pid_t pid = PTR_TO_PID(pidp); int q; q = cg_attach_everywhere(supported, path, pid, path_callback, userdata); -- cgit v1.2.3-54-g00ecf From 569b19d8fedc4525a6c15f1a3ab9c15dcb7c694a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 3 Sep 2015 14:56:26 +0200 Subject: cgroup: move controller to dirname translation into join_path_legacy() Let's simplify things a bit. --- src/basic/cgroup-util.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) (limited to 'src/basic/cgroup-util.c') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 74e1668a17..8299f6ffd5 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -460,20 +460,23 @@ static const char *controller_to_dirname(const char *controller) { return controller; } -static int join_path_legacy(const char *controller_dn, const char *path, const char *suffix, char **fs) { +static int join_path_legacy(const char *controller, const char *path, const char *suffix, char **fs) { + const char *dn; char *t = NULL; assert(fs); - assert(controller_dn); + assert(controller); + + dn = controller_to_dirname(controller); if (isempty(path) && isempty(suffix)) - t = strappend("/sys/fs/cgroup/", controller_dn); + t = strappend("/sys/fs/cgroup/", dn); else if (isempty(path)) - t = strjoin("/sys/fs/cgroup/", controller_dn, "/", suffix, NULL); + t = strjoin("/sys/fs/cgroup/", dn, "/", suffix, NULL); else if (isempty(suffix)) - t = strjoin("/sys/fs/cgroup/", controller_dn, "/", path, NULL); + t = strjoin("/sys/fs/cgroup/", dn, "/", path, NULL); else - t = strjoin("/sys/fs/cgroup/", controller_dn, "/", path, "/", suffix, NULL); + t = strjoin("/sys/fs/cgroup/", dn, "/", path, "/", suffix, NULL); if (!t) return -ENOMEM; @@ -509,8 +512,8 @@ int cg_get_path(const char *controller, const char *path, const char *suffix, ch if (!controller) { char *t; - /* If no controller is specified, we assume only the - * path below the controller matters */ + /* If no controller is specified, we return the path + * *below* the controllers, without any prefix. */ if (!path && !suffix) return -EINVAL; @@ -537,14 +540,8 @@ int cg_get_path(const char *controller, const char *path, const char *suffix, ch if (unified > 0) r = join_path_unified(path, suffix, fs); - else { - const char *dn; - - dn = controller_to_dirname(controller); - - r = join_path_legacy(dn, path, suffix, fs); - } - + else + r = join_path_legacy(controller, path, suffix, fs); if (r < 0) return r; -- cgit v1.2.3-54-g00ecf From b8725df8b338b13405e430d037f0aac7872ce3d5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 3 Sep 2015 18:27:19 +0200 Subject: cgroup: when comparing agent paths, use path_equal() After all a path is a path is a path and we should use path_equal() to comapre those. --- src/basic/cgroup-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/basic/cgroup-util.c') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 8299f6ffd5..3ef96b238e 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -899,7 +899,7 @@ int cg_install_release_agent(const char *controller, const char *agent) { r = write_string_file(fs, agent, 0); if (r < 0) return r; - } else if (!streq(sc, agent)) + } else if (!path_equal(sc, agent)) return -EEXIST; fs = mfree(fs); -- cgit v1.2.3-54-g00ecf From 9a66c87a2386468fd3adc250cd9003644a7a5e6b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 3 Sep 2015 18:28:21 +0200 Subject: cgroup: make sure cg_is_empty_recursive() returns 1 for non-existing cgroups Previously, on the legacy hierarchy a non-existing cgroup was considered identical to an empty one, but the unified hierarchy the check for a non-existing one returned ENOENT. --- src/basic/cgroup-util.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/basic/cgroup-util.c') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 3ef96b238e..bf897c9b2d 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -1002,6 +1002,8 @@ int cg_is_empty_recursive(const char *controller, const char *path) { return r; r = read_one_line_file(populated, &t); + if (r == -ENOENT) + return 1; if (r < 0) return r; -- cgit v1.2.3-54-g00ecf From 989189eabf420690ca64d4713cea62e79c945d86 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 3 Sep 2015 19:43:15 +0200 Subject: cgroup: fix potential bad memory access --- src/basic/cgroup-util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/basic/cgroup-util.c') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index bf897c9b2d..77375f3669 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -518,9 +518,9 @@ int cg_get_path(const char *controller, const char *path, const char *suffix, ch if (!path && !suffix) return -EINVAL; - if (isempty(suffix)) + if (!suffix) t = strdup(path); - else if (isempty(path)) + else if (!path) t = strdup(suffix); else t = strjoin(path, "/", suffix, NULL); -- cgit v1.2.3-54-g00ecf From 1c80e425124457146ab03279e44ba5155d3e1716 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 3 Sep 2015 19:44:02 +0200 Subject: cgroup-util: make cg_pid_get_path() return -ENODATA when controller can't be found If the controller managed by systemd cannot found in /proc/$PID/cgroup, return ENODATA, the usual error for cases where the data being looked for does not exist, even if the process does. --- src/basic/cgroup-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/basic/cgroup-util.c') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 77375f3669..812308e243 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -870,7 +870,7 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **path) { return 0; } - return -ENOENT; + return -ENODATA; } int cg_install_release_agent(const char *controller, const char *agent) { -- cgit v1.2.3-54-g00ecf From ba09d9c687986305e5b60f923e85e2ec1e78d979 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 3 Sep 2015 19:46:23 +0200 Subject: cgroup: fix potential access of uninitialized variable --- src/basic/cgroup-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/basic/cgroup-util.c') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 812308e243..fe8750d0f3 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -1910,7 +1910,7 @@ int cg_attach_many_everywhere(CGroupMask supported, const char *path, Set* pids, int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to, cg_migrate_callback_t to_callback, void *userdata) { CGroupController c; - int r, unified; + int r = 0, unified; if (!path_equal(from, to)) { r = cg_migrate_recursive(SYSTEMD_CGROUP_CONTROLLER, from, SYSTEMD_CGROUP_CONTROLLER, to, false, true); -- cgit v1.2.3-54-g00ecf From 5f4c5fef66581383ee852b301db67f687663004c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 3 Sep 2015 19:50:37 +0200 Subject: cgroup: always read the supported controllers from the root cgroup of the local container Otherwise we might end up thinking that we support more controllers than actually enabled for the container we are running in. --- src/basic/cgroup-util.c | 12 ++++++++++-- src/test/test-cgroup-util.c | 12 ++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) (limited to 'src/basic/cgroup-util.c') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index fe8750d0f3..388bd629ee 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -1981,14 +1981,22 @@ int cg_mask_supported(CGroupMask *ret) { if (unified < 0) return unified; if (unified > 0) { - _cleanup_free_ char *controllers = NULL; + _cleanup_free_ char *root = NULL, *controllers = NULL, *path = NULL; const char *c; /* In the unified hierarchy we can read the supported * and accessible controllers from a the top-level * cgroup attribute */ - r = read_one_line_file("/sys/fs/cgroup/cgroup.controllers", &controllers); + r = cg_get_root_path(&root); + if (r < 0) + return r; + + r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, root, "cgroup.controllers", &path); + if (r < 0) + return r; + + r = read_one_line_file(path, &controllers); if (r < 0) return r; diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c index ecc9d70bf4..ff7e45901c 100644 --- a/src/test/test-cgroup-util.c +++ b/src/test/test-cgroup-util.c @@ -295,6 +295,17 @@ static void test_shift_path(void) { test_shift_path_one("/foobar/waldo", "/fuckfuck", "/foobar/waldo"); } +static void test_mask_supported(void) { + + CGroupMask m; + CGroupController c; + + assert_se(cg_mask_supported(&m) >= 0); + + for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) + printf("'%s' is supported: %s\n", cgroup_controller_to_string(c), yes_no(m & CGROUP_CONTROLLER_TO_MASK(c))); +} + int main(void) { test_path_decode_unit(); test_path_get_unit(); @@ -309,6 +320,7 @@ int main(void) { test_controller_is_valid(); test_slice_to_path(); test_shift_path(); + test_mask_supported(); return 0; } -- cgit v1.2.3-54-g00ecf From 98e4d8d7635496cbf62c8127ce6a8e8f7604a031 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 3 Sep 2015 20:10:00 +0200 Subject: nspawn: enable all controllers we can for the "payload" subcgroup we create In the unified hierarchy delegating controller access is safe, hence make sure to enable all controllers for the "payload" subcgroup if we create it, so that the container will have all controllers enabled the nspawn service itself has. --- src/basic/cgroup-util.c | 2 +- src/nspawn/nspawn.c | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'src/basic/cgroup-util.c') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 388bd629ee..a298b29382 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -2163,7 +2163,7 @@ int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p) { r = write_string_file(fs, s, 0); if (r < 0) - log_warning_errno(r, "Failed to enable controller %s for %s (%s): %m", n, p, fs); + log_debug_errno(r, "Failed to enable controller %s for %s (%s): %m", n, p, fs); } } diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index a56960506c..1c64c3e771 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4737,6 +4737,7 @@ static int create_subcgroup(pid_t pid) { _cleanup_free_ char *cgroup = NULL; const char *child; int unified, r; + CGroupMask supported; /* In the unified hierarchy inner nodes may only only contain * subgroups, but not processes. Hence, if we running in the @@ -4756,6 +4757,10 @@ static int create_subcgroup(pid_t pid) { if (unified == 0) return 0; + r = cg_mask_supported(&supported); + if (r < 0) + return log_error_errno(r, "Failed to determine supported controllers: %m"); + r = cg_pid_get_path(SYSTEMD_CGROUP_CONTROLLER, 0, &cgroup); if (r < 0) return log_error_errno(r, "Failed to get our control group: %m"); @@ -4770,6 +4775,8 @@ static int create_subcgroup(pid_t pid) { if (r < 0) return log_error_errno(r, "Failed to create %s subcgroup: %m", child); + /* Try to enable as many controllers as possible for the new payload. */ + (void) cg_enable_everywhere(supported, supported, cgroup); return 0; } -- cgit v1.2.3-54-g00ecf