From: Mike Yuan Date: Tue, 30 Jul 2024 20:43:49 +0000 (+0200) Subject: cgroup-util: clean up cg_kill() and friends, completely split out cg_kill_kernel_sigk... X-Git-Tag: v257-rc1~753^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2176841b9ec2163f24f0ccdbeb4b9d1981be7c24;p=thirdparty%2Fsystemd.git cgroup-util: clean up cg_kill() and friends, completely split out cg_kill_kernel_sigkill() cg_kill_kernel_sigkill() has a narrow use case, and currently no code really reaches that branch. Let's detach it from cg_kill_recursive() hence, and call it explicitly later where appropriate. --- diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 01af942cab8..55a36c4f1b6 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -293,27 +293,29 @@ int cg_read_subgroup(DIR *d, char **ret) { static int cg_kill_items( const char *path, + const char *item, int sig, CGroupFlags flags, Set *s, cg_kill_log_func_t log_kill, - void *userdata, - const char *item) { + void *userdata) { _cleanup_set_free_ Set *allocated_set = NULL; - bool done = false; - int r, ret = 0, ret_log_kill = 0; + int r, ret = 0; + assert(path); + assert(item); assert(sig >= 0); - /* Don't send SIGCONT twice. Also, SIGKILL always works even when process is suspended, hence don't send - * SIGCONT on SIGKILL. */ + /* Don't send SIGCONT twice. Also, SIGKILL always works even when process is suspended, hence + * don't send SIGCONT on SIGKILL. */ if (IN_SET(sig, SIGCONT, SIGKILL)) flags &= ~CGROUP_SIGCONT; - /* This goes through the tasks list and kills them all. This - * is repeated until no further processes are added to the - * tasks list, to properly handle forking processes */ + /* This goes through the tasks list and kills them all. This is repeated until no further processes + * are added to the tasks list, to properly handle forking processes. + * + * When sending SIGKILL, prefer cg_kill_kernel_sigkill(), which is fully atomic. */ if (!s) { s = allocated_set = set_new(NULL); @@ -321,8 +323,11 @@ static int cg_kill_items( return -ENOMEM; } + bool done; do { _cleanup_fclose_ FILE *f = NULL; + int ret_log_kill; + done = true; r = cg_enumerate_items(SYSTEMD_CGROUP_CONTROLLER, path, &f, item); @@ -343,7 +348,7 @@ static int cg_kill_items( if ((flags & CGROUP_IGNORE_SELF) && pidref_is_self(&pidref)) continue; - if (set_get(s, PID_TO_PTR(pidref.pid)) == PID_TO_PTR(pidref.pid)) + if (set_contains(s, PID_TO_PTR(pidref.pid))) continue; /* Ignore kernel threads to mimick the behavior of cgroup.kill. */ @@ -396,13 +401,13 @@ int cg_kill( int r, ret; - r = cg_kill_items(path, sig, flags, s, log_kill, userdata, "cgroup.procs"); - if (r < 0) - log_debug_errno(r, "Failed to kill processes in cgroup '%s' item cgroup.procs: %m", path); - if (r < 0 || sig != SIGKILL) - return r; + assert(path); - ret = r; + ret = cg_kill_items(path, "cgroup.procs", sig, flags, s, log_kill, userdata); + if (ret < 0) + return log_debug_errno(ret, "Failed to kill processes in cgroup '%s' item cgroup.procs: %m", path); + if (sig != SIGKILL) + return ret; /* Only in case of killing with SIGKILL and when using cgroupsv2, kill remaining threads manually as a workaround for kernel bug. It was fixed in 5.2-rc5 (c03cd7738a83), backported to 4.19.66 @@ -417,36 +422,13 @@ int cg_kill( * older kernels or without PIDFD_THREAD pidfd_open() fails with EINVAL. Since we might read non * thread group leader IDs from cgroup.threads, we set CGROUP_NO_PIDFD to avoid trying open pidfd's * for them and instead use the regular pid. */ - r = cg_kill_items(path, sig, flags|CGROUP_NO_PIDFD, s, log_kill, userdata, "cgroup.threads"); + r = cg_kill_items(path, "cgroup.threads", sig, flags|CGROUP_NO_PIDFD, s, log_kill, userdata); if (r < 0) return log_debug_errno(r, "Failed to kill processes in cgroup '%s' item cgroup.threads: %m", path); return r > 0 || ret > 0; } -int cg_kill_kernel_sigkill(const char *path) { - /* Kills the cgroup at `path` directly by writing to its cgroup.kill file. This sends SIGKILL to all - * processes in the cgroup and has the advantage of being completely atomic, unlike cg_kill_items(). */ - - _cleanup_free_ char *killfile = NULL; - int r; - - assert(path); - - if (!cg_kill_supported()) - return -EOPNOTSUPP; - - r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, path, "cgroup.kill", &killfile); - if (r < 0) - return r; - - r = write_string_file(killfile, "1", WRITE_STRING_FILE_DISABLE_BUFFER); - if (r < 0) - return log_debug_errno(r, "Failed to write to cgroup.kill for cgroup '%s': %m", path); - - return 0; -} - int cg_kill_recursive( const char *path, int sig, @@ -455,61 +437,77 @@ int cg_kill_recursive( cg_kill_log_func_t log_kill, void *userdata) { + _cleanup_set_free_ Set *allocated_set = NULL; + _cleanup_closedir_ DIR *d = NULL; int r, ret; assert(path); assert(sig >= 0); - if (sig == SIGKILL && cg_kill_supported() && - !FLAGS_SET(flags, CGROUP_IGNORE_SELF) && !s && !log_kill) - /* ignore CGROUP_SIGCONT, since this is a no-op alongside SIGKILL */ - ret = cg_kill_kernel_sigkill(path); - else { - _cleanup_set_free_ Set *allocated_set = NULL; - _cleanup_closedir_ DIR *d = NULL; - - if (!s) { - s = allocated_set = set_new(NULL); - if (!s) - return -ENOMEM; - } + if (!s) { + s = allocated_set = set_new(NULL); + if (!s) + return -ENOMEM; + } - ret = cg_kill(path, sig, flags, s, log_kill, userdata); + ret = cg_kill(path, sig, flags, s, log_kill, userdata); - r = cg_enumerate_subgroups(SYSTEMD_CGROUP_CONTROLLER, path, &d); - if (r < 0) { - if (r != -ENOENT) - RET_GATHER(ret, log_debug_errno(r, "Failed to enumerate cgroup '%s' subgroups: %m", path)); + r = cg_enumerate_subgroups(SYSTEMD_CGROUP_CONTROLLER, path, &d); + if (r < 0) { + if (r != -ENOENT) + RET_GATHER(ret, log_debug_errno(r, "Failed to enumerate cgroup '%s' subgroups: %m", path)); - return ret; - } + return ret; + } - for (;;) { - _cleanup_free_ char *fn = NULL, *p = NULL; + for (;;) { + _cleanup_free_ char *fn = NULL, *p = NULL; - r = cg_read_subgroup(d, &fn); - if (r < 0) { - RET_GATHER(ret, log_debug_errno(r, "Failed to read subgroup from cgroup '%s': %m", path)); - break; - } - if (r == 0) - break; + r = cg_read_subgroup(d, &fn); + if (r < 0) { + RET_GATHER(ret, log_debug_errno(r, "Failed to read subgroup from cgroup '%s': %m", path)); + break; + } + if (r == 0) + break; - p = path_join(empty_to_root(path), fn); - if (!p) - return -ENOMEM; + p = path_join(empty_to_root(path), fn); + if (!p) + return -ENOMEM; - r = cg_kill_recursive(p, sig, flags, s, log_kill, userdata); - if (r < 0) - log_debug_errno(r, "Failed to recursively kill processes in cgroup '%s': %m", p); - if (r != 0 && ret >= 0) - ret = r; - } + r = cg_kill_recursive(p, sig, flags, s, log_kill, userdata); + if (r < 0) + log_debug_errno(r, "Failed to recursively kill processes in cgroup '%s': %m", p); + if (r != 0 && ret >= 0) + ret = r; } return ret; } +int cg_kill_kernel_sigkill(const char *path) { + _cleanup_free_ char *killfile = NULL; + int r; + + /* Kills the cgroup at `path` directly by writing to its cgroup.kill file. This sends SIGKILL to all + * processes in the cgroup and has the advantage of being completely atomic, unlike cg_kill_items(). */ + + assert(path); + + if (!cg_kill_supported()) + return -EOPNOTSUPP; + + r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, path, "cgroup.kill", &killfile); + if (r < 0) + return r; + + r = write_string_file(killfile, "1", WRITE_STRING_FILE_DISABLE_BUFFER); + if (r < 0) + return log_debug_errno(r, "Failed to write to cgroup.kill for cgroup '%s': %m", path); + + return 0; +} + static const char *controller_to_dirname(const char *controller) { assert(controller);