* [PATCH v3 2/5] docs/cgroup: add entry for cgroup.kill
2021-05-08 12:15 [PATCH v3 1/5] cgroup: introduce cgroup.kill Christian Brauner
@ 2021-05-08 12:15 ` Christian Brauner
2021-05-08 12:15 ` [PATCH v3 3/5] tests/cgroup: use cgroup.kill in cg_killall() Christian Brauner
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Christian Brauner @ 2021-05-08 12:15 UTC (permalink / raw)
To: Tejun Heo, Roman Gushchin
Cc: Shakeel Butt, Zefan Li, Johannes Weiner, cgroups, containers,
Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
Give a brief overview of the cgroup.kill functionality.
Link: https://lore.kernel.org/r/20210503143922.3093755-2-brauner@kernel.org
Cc: Roman Gushchin <guro@fb.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: cgroups@vger.kernel.org
Reviewed-by: Shakeel Butt <shakeelb@google.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Roman Gushchin <guro@fb.com>:
- Drop sentence that mentions combined useage of cgroup.kill and
cgroup.freezer.
/* v3 */
unchanged
---
Documentation/admin-guide/cgroup-v2.rst | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 64c62b979f2f..6adc749d863e 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -949,6 +949,21 @@ All cgroup core files are prefixed with "cgroup."
it's possible to delete a frozen (and empty) cgroup, as well as
create new sub-cgroups.
+ cgroup.kill
+ A write-only single value file which exists in non-root cgroups.
+ The only allowed value is "1".
+
+ Writing "1" to the file causes the cgroup and all descendant cgroups to
+ be killed. This means that all processes located in the affected cgroup
+ tree will be killed via SIGKILL.
+
+ Killing a cgroup tree will deal with concurrent forks appropriately and
+ is protected against migrations.
+
+ In a threaded cgroup, writing this file fails with EOPNOTSUPP as
+ killing cgroups is a process directed operation, i.e. it affects
+ the whole thread-group.
+
Controllers
===========
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 3/5] tests/cgroup: use cgroup.kill in cg_killall()
2021-05-08 12:15 [PATCH v3 1/5] cgroup: introduce cgroup.kill Christian Brauner
2021-05-08 12:15 ` [PATCH v3 2/5] docs/cgroup: add entry for cgroup.kill Christian Brauner
@ 2021-05-08 12:15 ` Christian Brauner
2021-05-08 12:15 ` [PATCH v3 4/5] tests/cgroup: move cg_wait_for(), cg_prepare_for_wait() Christian Brauner
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Christian Brauner @ 2021-05-08 12:15 UTC (permalink / raw)
To: Tejun Heo, Roman Gushchin
Cc: Shakeel Butt, Zefan Li, Johannes Weiner, cgroups, containers,
Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
If cgroup.kill file is supported make use of it.
Link: https://lore.kernel.org/r/20210503143922.3093755-3-brauner@kernel.org
Cc: Tejun Heo <tj@kernel.org>
Cc: cgroups@vger.kernel.org
Reviewed-by: Shakeel Butt <shakeelb@google.com>
Acked-by: Roman Gushchin <guro@fb.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Roman Gushchin <guro@fb.com>:
- Fix whitespace.
/* v3 */
unchanged
---
tools/testing/selftests/cgroup/cgroup_util.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/cgroup/cgroup_util.c b/tools/testing/selftests/cgroup/cgroup_util.c
index 027014662fb2..f60f7d764690 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/cgroup_util.c
@@ -252,6 +252,10 @@ int cg_killall(const char *cgroup)
char buf[PAGE_SIZE];
char *ptr = buf;
+ /* If cgroup.kill exists use it. */
+ if (!cg_write(cgroup, "cgroup.kill", "1"))
+ return 0;
+
if (cg_read(cgroup, "cgroup.procs", buf, sizeof(buf)))
return -1;
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 4/5] tests/cgroup: move cg_wait_for(), cg_prepare_for_wait()
2021-05-08 12:15 [PATCH v3 1/5] cgroup: introduce cgroup.kill Christian Brauner
2021-05-08 12:15 ` [PATCH v3 2/5] docs/cgroup: add entry for cgroup.kill Christian Brauner
2021-05-08 12:15 ` [PATCH v3 3/5] tests/cgroup: use cgroup.kill in cg_killall() Christian Brauner
@ 2021-05-08 12:15 ` Christian Brauner
2021-05-08 12:15 ` [PATCH v3 5/5] tests/cgroup: test cgroup.kill Christian Brauner
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Christian Brauner @ 2021-05-08 12:15 UTC (permalink / raw)
To: Tejun Heo, Roman Gushchin
Cc: Shakeel Butt, Zefan Li, Johannes Weiner, cgroups, containers,
Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
as they will be used by the tests for cgroup killing.
Link: https://lore.kernel.org/r/20210503143922.3093755-4-brauner@kernel.org
Cc: Tejun Heo <tj@kernel.org>
Cc: cgroups@vger.kernel.org
Reviewed-by: Shakeel Butt <shakeelb@google.com>
Acked-by: Roman Gushchin <guro@fb.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
unchanged
/* v3 */
unchanged
---
tools/testing/selftests/cgroup/cgroup_util.c | 47 +++++++++++++++
tools/testing/selftests/cgroup/cgroup_util.h | 2 +
tools/testing/selftests/cgroup/test_freezer.c | 57 -------------------
3 files changed, 49 insertions(+), 57 deletions(-)
diff --git a/tools/testing/selftests/cgroup/cgroup_util.c b/tools/testing/selftests/cgroup/cgroup_util.c
index f60f7d764690..623cec04ad42 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/cgroup_util.c
@@ -5,10 +5,12 @@
#include <errno.h>
#include <fcntl.h>
#include <linux/limits.h>
+#include <poll.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <sys/inotify.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/wait.h>
@@ -580,3 +582,48 @@ int clone_into_cgroup_run_wait(const char *cgroup)
(void)clone_reap(pid, WEXITED);
return 0;
}
+
+int cg_prepare_for_wait(const char *cgroup)
+{
+ int fd, ret = -1;
+
+ fd = inotify_init1(0);
+ if (fd == -1)
+ return fd;
+
+ ret = inotify_add_watch(fd, cg_control(cgroup, "cgroup.events"),
+ IN_MODIFY);
+ if (ret == -1) {
+ close(fd);
+ fd = -1;
+ }
+
+ return fd;
+}
+
+int cg_wait_for(int fd)
+{
+ int ret = -1;
+ struct pollfd fds = {
+ .fd = fd,
+ .events = POLLIN,
+ };
+
+ while (true) {
+ ret = poll(&fds, 1, 10000);
+
+ if (ret == -1) {
+ if (errno == EINTR)
+ continue;
+
+ break;
+ }
+
+ if (ret > 0 && fds.revents & POLLIN) {
+ ret = 0;
+ break;
+ }
+ }
+
+ return ret;
+}
diff --git a/tools/testing/selftests/cgroup/cgroup_util.h b/tools/testing/selftests/cgroup/cgroup_util.h
index 5a1305dd1f0b..82e59cdf16e7 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.h
+++ b/tools/testing/selftests/cgroup/cgroup_util.h
@@ -54,3 +54,5 @@ extern pid_t clone_into_cgroup(int cgroup_fd);
extern int clone_reap(pid_t pid, int options);
extern int clone_into_cgroup_run_wait(const char *cgroup);
extern int dirfd_open_opath(const char *dir);
+extern int cg_prepare_for_wait(const char *cgroup);
+extern int cg_wait_for(int fd);
diff --git a/tools/testing/selftests/cgroup/test_freezer.c b/tools/testing/selftests/cgroup/test_freezer.c
index 23d8fa4a3e4e..ff519029f6f4 100644
--- a/tools/testing/selftests/cgroup/test_freezer.c
+++ b/tools/testing/selftests/cgroup/test_freezer.c
@@ -7,9 +7,7 @@
#include <unistd.h>
#include <stdio.h>
#include <errno.h>
-#include <poll.h>
#include <stdlib.h>
-#include <sys/inotify.h>
#include <string.h>
#include <sys/wait.h>
@@ -54,61 +52,6 @@ static int cg_freeze_nowait(const char *cgroup, bool freeze)
return cg_write(cgroup, "cgroup.freeze", freeze ? "1" : "0");
}
-/*
- * Prepare for waiting on cgroup.events file.
- */
-static int cg_prepare_for_wait(const char *cgroup)
-{
- int fd, ret = -1;
-
- fd = inotify_init1(0);
- if (fd == -1) {
- debug("Error: inotify_init1() failed\n");
- return fd;
- }
-
- ret = inotify_add_watch(fd, cg_control(cgroup, "cgroup.events"),
- IN_MODIFY);
- if (ret == -1) {
- debug("Error: inotify_add_watch() failed\n");
- close(fd);
- fd = -1;
- }
-
- return fd;
-}
-
-/*
- * Wait for an event. If there are no events for 10 seconds,
- * treat this an error.
- */
-static int cg_wait_for(int fd)
-{
- int ret = -1;
- struct pollfd fds = {
- .fd = fd,
- .events = POLLIN,
- };
-
- while (true) {
- ret = poll(&fds, 1, 10000);
-
- if (ret == -1) {
- if (errno == EINTR)
- continue;
- debug("Error: poll() failed\n");
- break;
- }
-
- if (ret > 0 && fds.revents & POLLIN) {
- ret = 0;
- break;
- }
- }
-
- return ret;
-}
-
/*
* Attach a task to the given cgroup and wait for a cgroup frozen event.
* All transient events (e.g. populated) are ignored.
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 5/5] tests/cgroup: test cgroup.kill
2021-05-08 12:15 [PATCH v3 1/5] cgroup: introduce cgroup.kill Christian Brauner
` (2 preceding siblings ...)
2021-05-08 12:15 ` [PATCH v3 4/5] tests/cgroup: move cg_wait_for(), cg_prepare_for_wait() Christian Brauner
@ 2021-05-08 12:15 ` Christian Brauner
2021-05-10 22:02 ` Shakeel Butt
2021-05-10 14:42 ` [PATCH v3 1/5] cgroup: introduce cgroup.kill Tejun Heo
2021-05-10 15:39 ` Jonathan Corbet
5 siblings, 1 reply; 10+ messages in thread
From: Christian Brauner @ 2021-05-08 12:15 UTC (permalink / raw)
To: Tejun Heo, Roman Gushchin
Cc: Shakeel Butt, Zefan Li, Johannes Weiner, cgroups, containers,
Christian Brauner
From: Christian Brauner <christian.brauner@ubuntu.com>
Test that the new cgroup.kill feature works as intended.
Link: https://lore.kernel.org/r/20210503143922.3093755-5-brauner@kernel.org
Cc: Shakeel Butt <shakeelb@google.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: cgroups@vger.kernel.org
Acked-by: Roman Gushchin <guro@fb.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
- Christian Brauner <christian.brauner@ubuntu.com>:
- Remove debug logging (It wasn't very helpful in the first place.).
/* v3 */
- Shakeel Butt <shakeelb@google.com>:
- Remove misplaced second cgroup.kill since it is immediately followed
by a cg_kill_wait() call.
- Christian Brauner <christian.brauner@ubuntu.com>:
- Move check for unpopulated == 0 after all killed processes have been
reaped to avoid spurious failures when running test_kill in loops
(e.g. during stress test).
---
tools/testing/selftests/cgroup/.gitignore | 3 +-
tools/testing/selftests/cgroup/Makefile | 2 +
tools/testing/selftests/cgroup/test_kill.c | 297 +++++++++++++++++++++
3 files changed, 301 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/cgroup/test_kill.c
diff --git a/tools/testing/selftests/cgroup/.gitignore b/tools/testing/selftests/cgroup/.gitignore
index 84cfcabea838..be9643ef6285 100644
--- a/tools/testing/selftests/cgroup/.gitignore
+++ b/tools/testing/selftests/cgroup/.gitignore
@@ -2,4 +2,5 @@
test_memcontrol
test_core
test_freezer
-test_kmem
\ No newline at end of file
+test_kmem
+test_kill
diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
index f027d933595b..59e222460581 100644
--- a/tools/testing/selftests/cgroup/Makefile
+++ b/tools/testing/selftests/cgroup/Makefile
@@ -9,6 +9,7 @@ TEST_GEN_PROGS = test_memcontrol
TEST_GEN_PROGS += test_kmem
TEST_GEN_PROGS += test_core
TEST_GEN_PROGS += test_freezer
+TEST_GEN_PROGS += test_kill
include ../lib.mk
@@ -16,3 +17,4 @@ $(OUTPUT)/test_memcontrol: cgroup_util.c ../clone3/clone3_selftests.h
$(OUTPUT)/test_kmem: cgroup_util.c ../clone3/clone3_selftests.h
$(OUTPUT)/test_core: cgroup_util.c ../clone3/clone3_selftests.h
$(OUTPUT)/test_freezer: cgroup_util.c ../clone3/clone3_selftests.h
+$(OUTPUT)/test_kill: cgroup_util.c ../clone3/clone3_selftests.h ../pidfd/pidfd.h
diff --git a/tools/testing/selftests/cgroup/test_kill.c b/tools/testing/selftests/cgroup/test_kill.c
new file mode 100644
index 000000000000..6153690319c9
--- /dev/null
+++ b/tools/testing/selftests/cgroup/test_kill.c
@@ -0,0 +1,297 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <errno.h>
+#include <linux/limits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "../kselftest.h"
+#include "../pidfd/pidfd.h"
+#include "cgroup_util.h"
+
+/*
+ * Kill the given cgroup and wait for the inotify signal.
+ * If there are no events in 10 seconds, treat this as an error.
+ * Then check that the cgroup is in the desired state.
+ */
+static int cg_kill_wait(const char *cgroup)
+{
+ int fd, ret = -1;
+
+ fd = cg_prepare_for_wait(cgroup);
+ if (fd < 0)
+ return fd;
+
+ ret = cg_write(cgroup, "cgroup.kill", "1");
+ if (ret)
+ goto out;
+
+ ret = cg_wait_for(fd);
+ if (ret)
+ goto out;
+
+out:
+ close(fd);
+ return ret;
+}
+
+/*
+ * A simple process running in a sleep loop until being
+ * re-parented.
+ */
+static int child_fn(const char *cgroup, void *arg)
+{
+ int ppid = getppid();
+
+ while (getppid() == ppid)
+ usleep(1000);
+
+ return getppid() == ppid;
+}
+
+static int test_cgkill_simple(const char *root)
+{
+ pid_t pids[100];
+ int ret = KSFT_FAIL;
+ char *cgroup = NULL;
+ int i;
+
+ cgroup = cg_name(root, "cg_test_simple");
+ if (!cgroup)
+ goto cleanup;
+
+ if (cg_create(cgroup))
+ goto cleanup;
+
+ for (i = 0; i < 100; i++)
+ pids[i] = cg_run_nowait(cgroup, child_fn, NULL);
+
+ if (cg_wait_for_proc_count(cgroup, 100))
+ goto cleanup;
+
+ if (cg_read_strcmp(cgroup, "cgroup.events", "populated 1\n"))
+ goto cleanup;
+
+ if (cg_kill_wait(cgroup))
+ goto cleanup;
+
+ ret = KSFT_PASS;
+
+cleanup:
+ for (i = 0; i < 100; i++)
+ wait_for_pid(pids[i]);
+
+ if (ret == KSFT_PASS &&
+ cg_read_strcmp(cgroup, "cgroup.events", "populated 0\n"))
+ ret = KSFT_FAIL;
+
+ if (cgroup)
+ cg_destroy(cgroup);
+ free(cgroup);
+ return ret;
+}
+
+/*
+ * The test creates the following hierarchy:
+ * A
+ * / / \ \
+ * B E I K
+ * /\ |
+ * C D F
+ * |
+ * G
+ * |
+ * H
+ *
+ * with a process in C, H and 3 processes in K.
+ * Then it tries to kill the whole tree.
+ */
+static int test_cgkill_tree(const char *root)
+{
+ pid_t pids[5];
+ char *cgroup[10] = {0};
+ int ret = KSFT_FAIL;
+ int i;
+
+ cgroup[0] = cg_name(root, "cg_test_tree_A");
+ if (!cgroup[0])
+ goto cleanup;
+
+ cgroup[1] = cg_name(cgroup[0], "B");
+ if (!cgroup[1])
+ goto cleanup;
+
+ cgroup[2] = cg_name(cgroup[1], "C");
+ if (!cgroup[2])
+ goto cleanup;
+
+ cgroup[3] = cg_name(cgroup[1], "D");
+ if (!cgroup[3])
+ goto cleanup;
+
+ cgroup[4] = cg_name(cgroup[0], "E");
+ if (!cgroup[4])
+ goto cleanup;
+
+ cgroup[5] = cg_name(cgroup[4], "F");
+ if (!cgroup[5])
+ goto cleanup;
+
+ cgroup[6] = cg_name(cgroup[5], "G");
+ if (!cgroup[6])
+ goto cleanup;
+
+ cgroup[7] = cg_name(cgroup[6], "H");
+ if (!cgroup[7])
+ goto cleanup;
+
+ cgroup[8] = cg_name(cgroup[0], "I");
+ if (!cgroup[8])
+ goto cleanup;
+
+ cgroup[9] = cg_name(cgroup[0], "K");
+ if (!cgroup[9])
+ goto cleanup;
+
+ for (i = 0; i < 10; i++)
+ if (cg_create(cgroup[i]))
+ goto cleanup;
+
+ pids[0] = cg_run_nowait(cgroup[2], child_fn, NULL);
+ pids[1] = cg_run_nowait(cgroup[7], child_fn, NULL);
+ pids[2] = cg_run_nowait(cgroup[9], child_fn, NULL);
+ pids[3] = cg_run_nowait(cgroup[9], child_fn, NULL);
+ pids[4] = cg_run_nowait(cgroup[9], child_fn, NULL);
+
+ /*
+ * Wait until all child processes will enter
+ * corresponding cgroups.
+ */
+
+ if (cg_wait_for_proc_count(cgroup[2], 1) ||
+ cg_wait_for_proc_count(cgroup[7], 1) ||
+ cg_wait_for_proc_count(cgroup[9], 3))
+ goto cleanup;
+
+ /*
+ * Kill A and check that we get an empty notification.
+ */
+ if (cg_kill_wait(cgroup[0]))
+ goto cleanup;
+
+ ret = KSFT_PASS;
+
+cleanup:
+ for (i = 0; i < 5; i++)
+ wait_for_pid(pids[i]);
+
+ if (ret == KSFT_PASS &&
+ cg_read_strcmp(cgroup[0], "cgroup.events", "populated 0\n"))
+ ret = KSFT_FAIL;
+
+ for (i = 9; i >= 0 && cgroup[i]; i--) {
+ cg_destroy(cgroup[i]);
+ free(cgroup[i]);
+ }
+
+ return ret;
+}
+
+static int forkbomb_fn(const char *cgroup, void *arg)
+{
+ int ppid;
+
+ fork();
+ fork();
+
+ ppid = getppid();
+
+ while (getppid() == ppid)
+ usleep(1000);
+
+ return getppid() == ppid;
+}
+
+/*
+ * The test runs a fork bomb in a cgroup and tries to kill it.
+ */
+static int test_cgkill_forkbomb(const char *root)
+{
+ int ret = KSFT_FAIL;
+ char *cgroup = NULL;
+ pid_t pid = -ESRCH;
+
+ cgroup = cg_name(root, "cg_forkbomb_test");
+ if (!cgroup)
+ goto cleanup;
+
+ if (cg_create(cgroup))
+ goto cleanup;
+
+ pid = cg_run_nowait(cgroup, forkbomb_fn, NULL);
+ if (pid < 0)
+ goto cleanup;
+
+ usleep(100000);
+
+ if (cg_kill_wait(cgroup))
+ goto cleanup;
+
+ if (cg_wait_for_proc_count(cgroup, 0))
+ goto cleanup;
+
+ ret = KSFT_PASS;
+
+cleanup:
+ if (pid > 0)
+ wait_for_pid(pid);
+
+ if (ret == KSFT_PASS &&
+ cg_read_strcmp(cgroup, "cgroup.events", "populated 0\n"))
+ ret = KSFT_FAIL;
+
+ if (cgroup)
+ cg_destroy(cgroup);
+ free(cgroup);
+ return ret;
+}
+
+#define T(x) { x, #x }
+struct cgkill_test {
+ int (*fn)(const char *root);
+ const char *name;
+} tests[] = {
+ T(test_cgkill_simple),
+ T(test_cgkill_tree),
+ T(test_cgkill_forkbomb),
+};
+#undef T
+
+int main(int argc, char *argv[])
+{
+ char root[PATH_MAX];
+ int i, ret = EXIT_SUCCESS;
+
+ if (cg_find_unified_root(root, sizeof(root)))
+ ksft_exit_skip("cgroup v2 isn't mounted\n");
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
+ switch (tests[i].fn(root)) {
+ case KSFT_PASS:
+ ksft_test_result_pass("%s\n", tests[i].name);
+ break;
+ case KSFT_SKIP:
+ ksft_test_result_skip("%s\n", tests[i].name);
+ break;
+ default:
+ ret = EXIT_FAILURE;
+ ksft_test_result_fail("%s\n", tests[i].name);
+ break;
+ }
+ }
+
+ return ret;
+}
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 1/5] cgroup: introduce cgroup.kill
2021-05-08 12:15 [PATCH v3 1/5] cgroup: introduce cgroup.kill Christian Brauner
` (3 preceding siblings ...)
2021-05-08 12:15 ` [PATCH v3 5/5] tests/cgroup: test cgroup.kill Christian Brauner
@ 2021-05-10 14:42 ` Tejun Heo
2021-05-10 14:49 ` Christian Brauner
2021-05-10 15:39 ` Jonathan Corbet
5 siblings, 1 reply; 10+ messages in thread
From: Tejun Heo @ 2021-05-10 14:42 UTC (permalink / raw)
To: Christian Brauner
Cc: Roman Gushchin, Shakeel Butt, Zefan Li, Johannes Weiner, cgroups,
containers, Christian Brauner, Serge Hallyn
Applied to cgroup/for-5.14.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3 1/5] cgroup: introduce cgroup.kill
2021-05-10 14:42 ` [PATCH v3 1/5] cgroup: introduce cgroup.kill Tejun Heo
@ 2021-05-10 14:49 ` Christian Brauner
0 siblings, 0 replies; 10+ messages in thread
From: Christian Brauner @ 2021-05-10 14:49 UTC (permalink / raw)
To: Tejun Heo
Cc: Christian Brauner, Roman Gushchin, Shakeel Butt, Zefan Li,
Johannes Weiner, cgroups, containers, Serge Hallyn
On Mon, May 10, 2021 at 10:42:17AM -0400, Tejun Heo wrote:
> Applied to cgroup/for-5.14.
Thanks!
Christian
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/5] cgroup: introduce cgroup.kill
2021-05-08 12:15 [PATCH v3 1/5] cgroup: introduce cgroup.kill Christian Brauner
` (4 preceding siblings ...)
2021-05-10 14:42 ` [PATCH v3 1/5] cgroup: introduce cgroup.kill Tejun Heo
@ 2021-05-10 15:39 ` Jonathan Corbet
2021-05-10 16:43 ` Roman Gushchin
5 siblings, 1 reply; 10+ messages in thread
From: Jonathan Corbet @ 2021-05-10 15:39 UTC (permalink / raw)
To: Christian Brauner, Tejun Heo, Roman Gushchin
Cc: Shakeel Butt, Zefan Li, Johannes Weiner, cgroups, containers,
Christian Brauner, Serge Hallyn
Christian Brauner <brauner@kernel.org> writes:
> Introduce the cgroup.kill file. It does what it says on the tin and
> allows a caller to kill a cgroup by writing "1" into cgroup.kill.
> The file is available in non-root cgroups.
So I feel like I'm missing something fundamental here...perhaps somebody
can supply a suitable cluebat. It seems inevitable to me that, sooner
or later, somebody will surely wish that this mechanism could send a
signal other than SIGKILL, but this interface won't allow that. Even if
you won't want to implement an arbitrary signal now, it seems like
defining the interface to require writing "9" rather than "1" would
avoid closing that option off in the future.
I assume there's some reason why you don't want to do that, but I'm to
slow to figure out what it is...?
Thanks,
jon
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3 1/5] cgroup: introduce cgroup.kill
2021-05-10 15:39 ` Jonathan Corbet
@ 2021-05-10 16:43 ` Roman Gushchin
0 siblings, 0 replies; 10+ messages in thread
From: Roman Gushchin @ 2021-05-10 16:43 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Christian Brauner, Tejun Heo, Shakeel Butt, Zefan Li,
Johannes Weiner, cgroups, containers, Christian Brauner,
Serge Hallyn
On Mon, May 10, 2021 at 09:39:02AM -0600, Jonathan Corbet wrote:
> Christian Brauner <brauner@kernel.org> writes:
>
> > Introduce the cgroup.kill file. It does what it says on the tin and
> > allows a caller to kill a cgroup by writing "1" into cgroup.kill.
> > The file is available in non-root cgroups.
>
> So I feel like I'm missing something fundamental here...perhaps somebody
> can supply a suitable cluebat. It seems inevitable to me that, sooner
> or later, somebody will surely wish that this mechanism could send a
> signal other than SIGKILL, but this interface won't allow that. Even if
> you won't want to implement an arbitrary signal now, it seems like
> defining the interface to require writing "9" rather than "1" would
> avoid closing that option off in the future.
>
> I assume there's some reason why you don't want to do that, but I'm to
> slow to figure out what it is...?
I think the consensus was that most signals are a process/thread-level thing
and are rarely sent to more than one process, so sending them to all processes
in a cgroup is never required. So far nobody came with any real-life use case
behind SIGKILL. SIGSTOP/SIGCONT could be that case, but the cgroup v2 freezer
is doing virtually the same and eliminates this need.
Even SIGTERM which is often used before SIGKILL as a grateful termination
attempt has to be sent to a proper process in the cgroup (e.g. container-level
init or the main process in the workload).
If a new case will arise, it will be possible to introduce a new interface
file, but it looks very unlikely that there will be many such cases to justify
a more generic interface. On the other side keeping it boolean makes it more
consistent with the rest of the cgroup v2 api.
Thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread