From bd9f2fc2d080e0ab3de8d159cc98aa5948007d37 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 17:13:53 +0200 Subject: set: return NULL on destructors Like we do it pretty much everywhere else. --- src/basic/set.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/basic') diff --git a/src/basic/set.h b/src/basic/set.h index 51e40d3a6c..4554ef2d49 100644 --- a/src/basic/set.h +++ b/src/basic/set.h @@ -28,12 +28,14 @@ Set *internal_set_new(const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS); #define set_new(ops) internal_set_new(ops HASHMAP_DEBUG_SRC_ARGS) -static inline void set_free(Set *s) { +static inline Set *set_free(Set *s) { internal_hashmap_free(HASHMAP_BASE(s)); + return NULL; } -static inline void set_free_free(Set *s) { +static inline Set *set_free_free(Set *s) { internal_hashmap_free_free(HASHMAP_BASE(s)); + return NULL; } /* no set_free_free_free */ -- cgit v1.2.3-54-g00ecf From 9b84c7f959dca7b14dde522a443a3fcf2f5c5c2d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 17:53:14 +0200 Subject: cgroup: never migrate kernel threads out of the root cgroup It won't work anyway. --- src/basic/cgroup-util.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'src/basic') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 6b3162a35f..2a6dc2769b 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -321,6 +321,14 @@ int cg_migrate(const char *cfrom, const char *pfrom, const char *cto, const char if (set_get(s, LONG_TO_PTR(pid)) == LONG_TO_PTR(pid)) continue; + /* Ignore kernel threads. Since they can only + * exist in the root cgroup, we only check for + * them there. */ + if (cfrom && + (isempty(pfrom) || path_equal(pfrom, "/")) && + is_kernel_thread(pid) > 0) + continue; + r = cg_attach(cto, pto, pid); if (r < 0) { if (ret >= 0 && r != -ESRCH) -- cgit v1.2.3-54-g00ecf From f01327adb7e953ef7d2d0e2add92d6a1d484e2fc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 17:54:17 +0200 Subject: cgroup: don't allow hidden cgroups We really should care for all cgroups, and not allow hidden ones. --- src/basic/cgroup-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/basic') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 2a6dc2769b..1eed26d7bd 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -113,7 +113,7 @@ int cg_read_subgroup(DIR *d, char **fn) { assert(d); assert(fn); - FOREACH_DIRENT(de, d, return -errno) { + FOREACH_DIRENT_ALL(de, d, return -errno) { char *b; if (de->d_type != DT_DIR) -- cgit v1.2.3-54-g00ecf From e155a0aa04e899a535fc3b6a98ef6141181d710f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 18:02:43 +0200 Subject: cgroup: small cleanups and coding style fixes A number of simplications and adjustments to brings things closer to our coding style. --- src/basic/cgroup-util.c | 60 +++++++++++++++++++++++++++++-------------------- src/core/main.c | 2 +- 2 files changed, 37 insertions(+), 25 deletions(-) (limited to 'src/basic') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 1eed26d7bd..b9b27862ff 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -29,7 +29,6 @@ #include #include -#include "cgroup-util.h" #include "set.h" #include "macro.h" #include "util.h" @@ -41,6 +40,7 @@ #include "special.h" #include "mkdir.h" #include "login-util.h" +#include "cgroup-util.h" int cg_enumerate_processes(const char *controller, const char *path, FILE **_f) { _cleanup_free_ char *fs = NULL; @@ -197,7 +197,7 @@ int cg_kill(const char *controller, const char *path, int sig, bool sigcont, boo ret = -errno; } else { if (sigcont && sig != SIGKILL) - kill(pid, SIGCONT); + (void) kill(pid, SIGCONT); if (ret == 0) ret = 1; @@ -233,7 +233,7 @@ int cg_kill(const char *controller, const char *path, int sig, bool sigcont, boo int cg_kill_recursive(const char *controller, const char *path, int sig, bool sigcont, bool ignore_self, bool rem, Set *s) { _cleanup_set_free_ Set *allocated_set = NULL; _cleanup_closedir_ DIR *d = NULL; - int r, ret = 0; + int r, ret; char *fn; assert(path); @@ -264,7 +264,7 @@ int cg_kill_recursive(const char *controller, const char *path, int sig, bool si return -ENOMEM; r = cg_kill_recursive(controller, p, sig, sigcont, ignore_self, rem, s); - if (ret >= 0 && r != 0) + if (r != 0 && ret >= 0) ret = r; } @@ -390,12 +390,8 @@ int cg_migrate_recursive( p = strjoin(pfrom, "/", fn, NULL); free(fn); - if (!p) { - if (ret >= 0) - return -ENOMEM; - - return ret; - } + if (!p) + return -ENOMEM; r = cg_migrate_recursive(cfrom, p, cto, pto, ignore_self, rem); if (r != 0 && ret >= 0) @@ -436,13 +432,15 @@ int cg_migrate_recursive_fallback( /* This didn't work? Then let's try all prefixes of the destination */ PATH_FOREACH_PREFIX(prefix, pto) { - r = cg_migrate_recursive(cfrom, pfrom, cto, prefix, ignore_self, rem); - if (r >= 0) - break; + int q; + + q = cg_migrate_recursive(cfrom, pfrom, cto, prefix, ignore_self, rem); + if (q >= 0) + return q; } } - return 0; + return r; } static const char *normalize_controller(const char *controller) { @@ -557,7 +555,7 @@ static int trim_cb(const char *path, const struct stat *sb, int typeflag, struct if (ftwbuf->level < 1) return 0; - rmdir(path); + (void) rmdir(path); return 0; } @@ -572,8 +570,14 @@ int cg_trim(const char *controller, const char *path, bool delete_root) { return r; errno = 0; - if (nftw(fs, trim_cb, 64, FTW_DEPTH|FTW_MOUNT|FTW_PHYS) != 0) - r = errno ? -errno : -EIO; + if (nftw(fs, trim_cb, 64, FTW_DEPTH|FTW_MOUNT|FTW_PHYS) != 0) { + if (errno == ENOENT) + r = 0; + else if (errno != 0) + r = -errno; + else + r = -EIO; + } if (delete_root) { if (rmdir(fs) < 0 && errno != ENOENT) @@ -672,13 +676,15 @@ int cg_attach_fallback(const char *controller, const char *path, pid_t pid) { * the destination */ PATH_FOREACH_PREFIX(prefix, path) { - r = cg_attach(controller, prefix, pid); - if (r >= 0) - break; + int q; + + q = cg_attach(controller, prefix, pid); + if (q >= 0) + return q; } } - return 0; + return r; } int cg_set_group_access( @@ -691,7 +697,8 @@ int cg_set_group_access( _cleanup_free_ char *fs = NULL; int r; - assert(path); + if (mode == MODE_INVALID && uid == UID_INVALID && gid == GID_INVALID) + return 0; if (mode != MODE_INVALID) mode &= 0777; @@ -827,7 +834,7 @@ int cg_install_release_agent(const char *controller, const char *agent) { return r; sc = strstrip(contents); - if (sc[0] == 0) { + if (isempty(sc)) { r = write_string_file(fs, agent, 0); if (r < 0) return r; @@ -1877,6 +1884,11 @@ int cg_kernel_controllers(Set *controllers) { assert(controllers); + /* Determines the full list of kernel-known controllers. Might + * include controllers we don't actually support, arbitrary + * named hierarchies and controllers that aren't currently + * accessible (because not mounted). */ + f = fopen("/proc/cgroups", "re"); if (!f) { if (errno == ENOENT) @@ -1897,7 +1909,7 @@ int cg_kernel_controllers(Set *controllers) { if (feof(f)) break; - if (ferror(f) && errno) + if (ferror(f) && errno != 0) return -errno; return -EBADMSG; diff --git a/src/core/main.c b/src/core/main.c index e232be88c0..f29a595097 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2043,7 +2043,7 @@ finish: * kernel; at this point, we will not listen to the * signals anyway */ if (detect_container(NULL) <= 0) - cg_uninstall_release_agent(SYSTEMD_CGROUP_CONTROLLER); + (void) cg_uninstall_release_agent(SYSTEMD_CGROUP_CONTROLLER); execve(SYSTEMD_SHUTDOWN_BINARY_PATH, (char **) command_line, env_block); log_error_errno(errno, "Failed to execute shutdown binary, %s: %m", -- cgit v1.2.3-54-g00ecf From 6f883237f1b8a96ec0ea354866e033b6fcea9506 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 18:32:07 +0200 Subject: cgroup: drop "ignore_self" argument from cg_is_empty() In all cases where the function (or cg_is_empty_recursive()) ignoring the calling process is actually wrong, as a process keeps a cgroup busy regardless if its the current one or another. Hence, let's simplify things and drop the "ignore_self" parameter. --- src/basic/cgroup-util.c | 33 +++++++++++++-------------------- src/basic/cgroup-util.h | 4 ++-- src/core/cgroup.c | 2 +- src/core/scope.c | 2 +- src/core/service.c | 2 +- src/login/loginctl.c | 2 +- src/machine/machinectl.c | 2 +- src/shared/cgroup-show.c | 2 +- src/systemctl/systemctl.c | 2 +- src/test/test-cgroup.c | 12 ++++++------ 10 files changed, 28 insertions(+), 35 deletions(-) (limited to 'src/basic') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index b9b27862ff..61681ada8d 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -891,49 +891,42 @@ int cg_uninstall_release_agent(const char *controller) { return 0; } -int cg_is_empty(const char *controller, const char *path, bool ignore_self) { +int cg_is_empty(const char *controller, const char *path) { _cleanup_fclose_ FILE *f = NULL; - pid_t pid = 0, self_pid; - bool found = false; + pid_t pid = 0; int r; assert(path); r = cg_enumerate_processes(controller, path, &f); + if (r == -ENOENT) + return 1; if (r < 0) - return r == -ENOENT ? 1 : r; - - self_pid = getpid(); - - while ((r = cg_read_pid(f, &pid)) > 0) { - - if (ignore_self && pid == self_pid) - continue; - - found = true; - break; - } + return r; + r = cg_read_pid(f, &pid); if (r < 0) return r; - return !found; + return r == 0; } -int cg_is_empty_recursive(const char *controller, const char *path, bool ignore_self) { +int cg_is_empty_recursive(const char *controller, const char *path) { _cleanup_closedir_ DIR *d = NULL; char *fn; int r; assert(path); - r = cg_is_empty(controller, path, ignore_self); + r = cg_is_empty(controller, path); if (r <= 0) return r; r = cg_enumerate_subgroups(controller, path, &d); + if (r == -ENOENT) + return 1; if (r < 0) - return r == -ENOENT ? 1 : r; + return r; while ((r = cg_read_subgroup(d, &fn)) > 0) { _cleanup_free_ char *p = NULL; @@ -943,7 +936,7 @@ int cg_is_empty_recursive(const char *controller, const char *path, bool ignore_ if (!p) return -ENOMEM; - r = cg_is_empty_recursive(controller, p, ignore_self); + r = cg_is_empty_recursive(controller, p); if (r <= 0) return r; } diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index fd72e9e5c5..1c86581eb5 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -93,8 +93,8 @@ int cg_set_task_access(const char *controller, const char *path, mode_t mode, ui int cg_install_release_agent(const char *controller, const char *agent); int cg_uninstall_release_agent(const char *controller); -int cg_is_empty(const char *controller, const char *path, bool ignore_self); -int cg_is_empty_recursive(const char *controller, const char *path, bool ignore_self); +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 da6de68637..aafd75f424 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1030,7 +1030,7 @@ int manager_notify_cgroup_empty(Manager *m, const char *cgroup) { if (!u) return 0; - r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, true); + r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path); if (r <= 0) return r; diff --git a/src/core/scope.c b/src/core/scope.c index c594ab5294..1e94d63561 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -396,7 +396,7 @@ static bool scope_check_gc(Unit *u) { if (u->cgroup_path) { int r; - r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, true); + r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path); if (r <= 0) return true; } diff --git a/src/core/service.c b/src/core/service.c index a05ae95072..5a0a3aa867 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1269,7 +1269,7 @@ static int cgroup_good(Service *s) { if (!UNIT(s)->cgroup_path) return 0; - r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, UNIT(s)->cgroup_path, true); + r = cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, UNIT(s)->cgroup_path); if (r < 0) return r; diff --git a/src/login/loginctl.c b/src/login/loginctl.c index 5fa98e069f..a7e64071cf 100644 --- a/src/login/loginctl.c +++ b/src/login/loginctl.c @@ -263,7 +263,7 @@ static int show_unit_cgroup(sd_bus *bus, const char *interface, const char *unit if (isempty(cgroup)) return 0; - if (cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, cgroup, false) != 0 && leader <= 0) + if (cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, cgroup) != 0 && leader <= 0) return 0; c = columns(); diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 8bd0ed756b..bb8c5ac64b 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -375,7 +375,7 @@ static int show_unit_cgroup(sd_bus *bus, const char *unit, pid_t leader) { if (r < 0) return bus_log_parse_error(r); - if (cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, cgroup, false) != 0 && leader <= 0) + if (cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, cgroup) != 0 && leader <= 0) return 0; c = columns(); diff --git a/src/shared/cgroup-show.c b/src/shared/cgroup-show.c index 3abccdb49a..31b4f6c684 100644 --- a/src/shared/cgroup-show.c +++ b/src/shared/cgroup-show.c @@ -152,7 +152,7 @@ int show_cgroup_by_path(const char *path, const char *prefix, unsigned n_columns if (!k) return -ENOMEM; - if (!(flags & OUTPUT_SHOW_ALL) && cg_is_empty_recursive(NULL, k, false) > 0) + if (!(flags & OUTPUT_SHOW_ALL) && cg_is_empty_recursive(NULL, k) > 0) continue; if (!shown_pids) { diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 3cb5f61868..8d80aae182 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3557,7 +3557,7 @@ static void print_status_info( if (i->control_group && (i->main_pid > 0 || i->control_pid > 0 || - ((arg_transport != BUS_TRANSPORT_LOCAL && arg_transport != BUS_TRANSPORT_MACHINE) || cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, i->control_group, false) == 0))) { + ((arg_transport != BUS_TRANSPORT_LOCAL && arg_transport != BUS_TRANSPORT_MACHINE) || cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, i->control_group) == 0))) { unsigned c; printf(" CGroup: %s\n", i->control_group); diff --git a/src/test/test-cgroup.c b/src/test/test-cgroup.c index 4be69a408d..8b0302cfe6 100644 --- a/src/test/test-cgroup.c +++ b/src/test/test-cgroup.c @@ -56,18 +56,18 @@ int main(int argc, char*argv[]) { assert_se(path_equal(path, "/sys/fs/cgroup/systemd/test-b/test-d")); free(path); - assert_se(cg_is_empty(SYSTEMD_CGROUP_CONTROLLER, "/test-a", false) > 0); - assert_se(cg_is_empty(SYSTEMD_CGROUP_CONTROLLER, "/test-b", false) > 0); - assert_se(cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-a", false) > 0); - assert_se(cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-b", false) == 0); + assert_se(cg_is_empty(SYSTEMD_CGROUP_CONTROLLER, "/test-a") > 0); + assert_se(cg_is_empty(SYSTEMD_CGROUP_CONTROLLER, "/test-b") > 0); + assert_se(cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-a") > 0); + assert_se(cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-b") == 0); assert_se(cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-a", 0, false, false, false, NULL) == 0); assert_se(cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-b", 0, false, false, false, NULL) > 0); assert_se(cg_migrate_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-b", SYSTEMD_CGROUP_CONTROLLER, "/test-a", false, false) > 0); - assert_se(cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-a", false) == 0); - assert_se(cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-b", false) > 0); + assert_se(cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-a") == 0); + assert_se(cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-b") > 0); assert_se(cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-a", 0, false, false, false, NULL) > 0); assert_se(cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, "/test-b", 0, false, false, false, NULL) == 0); -- cgit v1.2.3-54-g00ecf From 6fd66507378bd8706cba38f82167a54d9d116e43 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 18:36:28 +0200 Subject: cgroup: the root cgroup is always populated --- src/basic/cgroup-util.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/basic') diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 61681ada8d..98adace55a 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -918,6 +918,10 @@ int cg_is_empty_recursive(const char *controller, const char *path) { assert(path); + /* The root cgroup is always populated */ + if (controller && (isempty(path) || path_equal(path, "/"))) + return 0; + r = cg_is_empty(controller, path); if (r <= 0) return r; -- cgit v1.2.3-54-g00ecf