From ae2a2c53dd059f0f20fa8080f6b67389be3d3e89 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 02:34:19 +0200 Subject: manager: don't write first-boot flag file all the time Instead, remember that we have already written it. --- src/core/manager.c | 13 ++++++++----- src/core/manager.h | 3 ++- 2 files changed, 10 insertions(+), 6 deletions(-) (limited to 'src/core') diff --git a/src/core/manager.c b/src/core/manager.c index ede2a9910d..14f069ba97 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -573,6 +573,7 @@ int manager_new(ManagerRunningAs running_as, bool test_run, Manager **_m) { m->ask_password_inotify_fd = -1; m->have_ask_password = -EINVAL; /* we don't know */ + m->first_boot = -1; m->test_run = test_run; @@ -2998,12 +2999,14 @@ void manager_set_first_boot(Manager *m, bool b) { if (m->running_as != MANAGER_SYSTEM) return; - m->first_boot = b; + if (m->first_boot != (int) b) { + if (b) + (void) touch("/run/systemd/first-boot"); + else + (void) unlink("/run/systemd/first-boot"); + } - if (m->first_boot) - touch("/run/systemd/first-boot"); - else - unlink("/run/systemd/first-boot"); + m->first_boot = b; } void manager_status_printf(Manager *m, StatusType type, const char *status, const char *format, ...) { diff --git a/src/core/manager.h b/src/core/manager.h index 1e01f2bdef..3f7fa24e58 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -233,7 +233,6 @@ struct Manager { bool dispatching_dbus_queue:1; bool taint_usr:1; - bool first_boot:1; bool test_run:1; @@ -295,6 +294,8 @@ struct Manager { const char *unit_log_field; const char *unit_log_format_string; + + int first_boot; }; int manager_new(ManagerRunningAs running_as, bool test_run, Manager **m); -- cgit v1.2.3-54-g00ecf From 957c3cf97cd0063a3b78aa068d5351ae3b1b0c0c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 16:45:34 +0200 Subject: unit: suppress unnecessary cgroup empty check Rework the "service is good" check, to only check the cgroup state if we really need to instead of always. This allows us to suppress going to the cgroupfs for an empty check for the majority of services. No functional change. --- src/core/service.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) (limited to 'src/core') diff --git a/src/core/service.c b/src/core/service.c index 3c4232417d..a05ae95072 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1520,18 +1520,33 @@ fail: service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_FAILURE_RESOURCES); } +static bool service_good(Service *s) { + int main_pid_ok; + assert(s); + + if (s->type == SERVICE_DBUS && !s->bus_name_good) + return false; + + main_pid_ok = main_pid_good(s); + if (main_pid_ok > 0) /* It's alive */ + return true; + if (main_pid_ok == 0) /* It's dead */ + return false; + + /* OK, we don't know anything about the main PID, maybe + * because there is none. Let's check the control group + * instead. */ + + return cgroup_good(s) != 0; +} + static void service_enter_running(Service *s, ServiceResult f) { - int main_pid_ok, cgroup_ok; assert(s); if (f != SERVICE_SUCCESS) s->result = f; - main_pid_ok = main_pid_good(s); - cgroup_ok = cgroup_good(s); - - if ((main_pid_ok > 0 || (main_pid_ok < 0 && cgroup_ok != 0)) && - (s->bus_name_good || s->type != SERVICE_DBUS)) { + if (service_good(s)) { /* If there are any queued up sd_notify() * notifications, process them now */ -- cgit v1.2.3-54-g00ecf From e9db43d5910717a1084924c512bf85e2b8265375 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 17:25:59 +0200 Subject: units: enable waiting for unit termination in certain cases The legacy cgroup hierarchy does not support reliable empty notifications in containers and if there are left-over subgroups in a cgroup. This makes it hard to correctly wait for them running empty, and thus we previously disabled this logic entirely. With this change we explicitly check for the container case, and whether the unit is a "delegation" unit (i.e. one where programs may create their own subgroups). If we are neither in a container, nor operating on a delegation unit cgroup empty notifications become reliable and thus we start waiting for the empty notifications again. This doesn't really fix the general problem around cgroup notifications but reduces the effect around it. (This also reorders #include lines by their focus, as suggsted in CODING_STYLE. We have to add "virt.h", so let's do that at the right place.) Also see #317. --- src/core/cgroup.c | 12 ++++++++++++ src/core/cgroup.h | 2 ++ src/core/unit.c | 41 +++++++++++++++++++++++------------------ 3 files changed, 37 insertions(+), 18 deletions(-) (limited to 'src/core') diff --git a/src/core/cgroup.c b/src/core/cgroup.c index c26807ba2b..da6de68637 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1124,6 +1124,18 @@ int unit_reset_cpu_usage(Unit *u) { return 0; } +bool unit_cgroup_delegate(Unit *u) { + CGroupContext *c; + + assert(u); + + c = unit_get_cgroup_context(u); + if (!c) + return false; + + return c->delegate; +} + static const char* const cgroup_device_policy_table[_CGROUP_DEVICE_POLICY_MAX] = { [CGROUP_AUTO] = "auto", [CGROUP_CLOSED] = "closed", diff --git a/src/core/cgroup.h b/src/core/cgroup.h index 869ddae8c4..7b38d210fb 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -130,5 +130,7 @@ int unit_get_memory_current(Unit *u, uint64_t *ret); int unit_get_cpu_usage(Unit *u, nsec_t *ret); int unit_reset_cpu_usage(Unit *u); +bool unit_cgroup_delegate(Unit *u); + const char* cgroup_device_policy_to_string(CGroupDevicePolicy i) _const_; CGroupDevicePolicy cgroup_device_policy_from_string(const char *s) _pure_; diff --git a/src/core/unit.c b/src/core/unit.c index 5f602bdf5f..3fec8c4c36 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -28,27 +28,28 @@ #include "sd-id128.h" #include "sd-messages.h" #include "set.h" -#include "unit.h" #include "macro.h" #include "strv.h" #include "path-util.h" -#include "load-fragment.h" -#include "load-dropin.h" #include "log.h" -#include "unit-name.h" -#include "dbus-unit.h" -#include "special.h" #include "cgroup-util.h" #include "missing.h" #include "mkdir.h" #include "fileio-label.h" -#include "bus-common-errors.h" -#include "dbus.h" -#include "execute.h" -#include "dropin.h" #include "formats-util.h" #include "process-util.h" +#include "virt.h" +#include "bus-common-errors.h" #include "bus-util.h" +#include "dropin.h" +#include "unit-name.h" +#include "special.h" +#include "unit.h" +#include "load-fragment.h" +#include "load-dropin.h" +#include "dbus.h" +#include "dbus-unit.h" +#include "execute.h" const UnitVTable * const unit_vtable[_UNIT_TYPE_MAX] = { [UNIT_SERVICE] = &service_vtable, @@ -3594,14 +3595,18 @@ int unit_kill_context( } else if (r > 0) { /* FIXME: For now, we will not wait for the - * cgroup members to die, simply because - * cgroup notification is unreliable. It - * doesn't work at all in containers, and - * outside of containers it can be confused - * easily by leaving directories in the - * cgroup. */ - - /* wait_for_exit = true; */ + * cgroup members to die if we are running in + * a container or if this is a delegation + * unit, simply because cgroup notification is + * unreliable in these cases. It doesn't work + * at all in containers, and outside of + * containers it can be confused easily by + * left-over directories in the cgroup -- + * which however should not exist in + * non-delegated units. */ + + if (detect_container(NULL) == 0 && !unit_cgroup_delegate(u)) + wait_for_exit = true; if (c->send_sighup && k != KILL_KILL) { set_free(pid_set); -- 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/core') 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/core') 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 5fe8876b320e9f6355425df9991ac38363684117 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 18:47:46 +0200 Subject: core: when looking for the unit for a process, look at the PID hashmaps first It's cheaper that going to cgroupfs, and also usually the better choice since it's not racy and can map PIDs even if they were moved to a different unit. --- src/core/cgroup.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'src/core') diff --git a/src/core/cgroup.c b/src/core/cgroup.c index aafd75f424..e92d2cc850 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1005,6 +1005,7 @@ Unit* manager_get_unit_by_cgroup(Manager *m, const char *cgroup) { Unit *manager_get_unit_by_pid(Manager *m, pid_t pid) { _cleanup_free_ char *cgroup = NULL; + Unit *u; int r; assert(m); @@ -1012,6 +1013,14 @@ Unit *manager_get_unit_by_pid(Manager *m, pid_t pid) { if (pid <= 1) return NULL; + u = hashmap_get(m->watch_pids1, LONG_TO_PTR(pid)); + if (u) + return u; + + u = hashmap_get(m->watch_pids2, LONG_TO_PTR(pid)); + if (u) + return u; + r = cg_pid_get_path(SYSTEMD_CGROUP_CONTROLLER, pid, &cgroup); if (r < 0) return NULL; -- cgit v1.2.3-54-g00ecf From b821a397c0fd382266a6018c1b6738ced69f8041 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 18:51:44 +0200 Subject: unit: small clean-ups Always say when we ignore errors. Cast calls whose return value we knowingly ingore to (void). Use "bool" where we actually mean a boolean, even if we return it as an int later on. --- src/core/unit.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'src/core') diff --git a/src/core/unit.c b/src/core/unit.c index 3fec8c4c36..64ab49dc4a 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2017,9 +2017,9 @@ void unit_unwatch_pid(Unit *u, pid_t pid) { assert(u); assert(pid >= 1); - hashmap_remove_value(u->manager->watch_pids1, LONG_TO_PTR(pid), u); - hashmap_remove_value(u->manager->watch_pids2, LONG_TO_PTR(pid), u); - set_remove(u->pids, LONG_TO_PTR(pid)); + (void) hashmap_remove_value(u->manager->watch_pids1, LONG_TO_PTR(pid), u); + (void) hashmap_remove_value(u->manager->watch_pids2, LONG_TO_PTR(pid), u); + (void) set_remove(u->pids, LONG_TO_PTR(pid)); } void unit_unwatch_all_pids(Unit *u) { @@ -3525,7 +3525,8 @@ int unit_kill_context( pid_t control_pid, bool main_pid_alien) { - int sig, wait_for_exit = false, r; + bool wait_for_exit = false; + int sig, r; assert(u); assert(c); @@ -3554,7 +3555,7 @@ int unit_kill_context( _cleanup_free_ char *comm = NULL; get_process_comm(main_pid, &comm); - log_unit_warning_errno(u, r, "Failed to kill main process " PID_FMT " (%s): %m", main_pid, strna(comm)); + log_unit_warning_errno(u, r, "Failed to kill main process " PID_FMT " (%s), ignoring: %m", main_pid, strna(comm)); } else { if (!main_pid_alien) wait_for_exit = true; @@ -3571,7 +3572,7 @@ int unit_kill_context( _cleanup_free_ char *comm = NULL; get_process_comm(control_pid, &comm); - log_unit_warning_errno(u, r, "Failed to kill control process " PID_FMT " (%s): %m", control_pid, strna(comm)); + log_unit_warning_errno(u, r, "Failed to kill control process " PID_FMT " (%s), ignoring: %m", control_pid, strna(comm)); } else { wait_for_exit = true; @@ -3580,7 +3581,8 @@ int unit_kill_context( } } - if ((c->kill_mode == KILL_CONTROL_GROUP || (c->kill_mode == KILL_MIXED && k == KILL_KILL)) && u->cgroup_path) { + if (u->cgroup_path && + (c->kill_mode == KILL_CONTROL_GROUP || (c->kill_mode == KILL_MIXED && k == KILL_KILL))) { _cleanup_set_free_ Set *pid_set = NULL; /* Exclude the main/control pids from being killed via the cgroup */ @@ -3591,7 +3593,8 @@ int unit_kill_context( r = cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, sig, true, true, false, pid_set); if (r < 0) { if (r != -EAGAIN && r != -ESRCH && r != -ENOENT) - log_unit_warning_errno(u, r, "Failed to kill control group: %m"); + log_unit_warning_errno(u, r, "Failed to kill control group %s, ignoring: %m", u->cgroup_path); + } else if (r > 0) { /* FIXME: For now, we will not wait for the -- cgit v1.2.3-54-g00ecf From 102ef9829ed72d3991cb73bc893880d715e667c5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 18:53:29 +0200 Subject: core: don't allow changing the slice of a unit while it is active --- src/core/unit.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/core') diff --git a/src/core/unit.c b/src/core/unit.c index 64ab49dc4a..a43f1d7785 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2441,6 +2441,9 @@ int unit_set_slice(Unit *u, Unit *slice) { if (u->type == UNIT_SLICE) return -EINVAL; + if (unit_active_state(u) != UNIT_INACTIVE) + return -EBUSY; + if (slice->type != UNIT_SLICE) return -EINVAL; -- cgit v1.2.3-54-g00ecf From d06673212e7672da44d5147d1d393278d9b1b478 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 18:54:08 +0200 Subject: core: rework when we kill with which signal When the user wants to explicitly send our own PID a signal, then do so. Don't follow up SIGABRT with a SIGHUP if send_sighup is enabled. At that point the process should have segfaulted, hence there's no point in following up with a SIGHUP. Send only termination signals to ourselves, never KILL or ABRT signals. --- src/core/unit.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src/core') diff --git a/src/core/unit.c b/src/core/unit.c index a43f1d7785..34d3adcd3b 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3172,7 +3172,7 @@ int unit_kill_common( if (!pid_set) return -ENOMEM; - q = cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, signo, false, true, false, pid_set); + q = cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, signo, false, false, false, pid_set); if (q < 0 && q != -EAGAIN && q != -ESRCH && q != -ENOENT) r = q; } @@ -3563,8 +3563,8 @@ int unit_kill_context( if (!main_pid_alien) wait_for_exit = true; - if (c->send_sighup && k != KILL_KILL) - kill(main_pid, SIGHUP); + if (c->send_sighup && k == KILL_TERMINATE) + (void) kill(main_pid, SIGHUP); } } @@ -3579,8 +3579,8 @@ int unit_kill_context( } else { wait_for_exit = true; - if (c->send_sighup && k != KILL_KILL) - kill(control_pid, SIGHUP); + if (c->send_sighup && k == KILL_TERMINATE) + (void) kill(control_pid, SIGHUP); } } @@ -3593,7 +3593,7 @@ int unit_kill_context( if (!pid_set) return -ENOMEM; - r = cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, sig, true, true, false, pid_set); + r = cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, sig, true, k != KILL_TERMINATE, false, pid_set); if (r < 0) { if (r != -EAGAIN && r != -ESRCH && r != -ENOENT) log_unit_warning_errno(u, r, "Failed to kill control group %s, ignoring: %m", u->cgroup_path); -- cgit v1.2.3-54-g00ecf