On Thu, Apr 29, 2021 at 09:05:35PM -0700, Roman Gushchin wrote:
> On Thu, Apr 29, 2021 at 02:01:13PM +0200, Christian Brauner wrote:
> > From: Christian Brauner <christian.brauner@xxxxxxxxxx>
> >
> > Test that the new cgroup.kill feature works as intended.
> >
> > Cc: Roman Gushchin <guro@xxxxxx>
> > Cc: Tejun Heo <tj@xxxxxxxxxx>
> > Cc: cgroups@xxxxxxxxxxxxxxx
> > Signed-off-by: Christian Brauner <christian.brauner@xxxxxxxxxx>
> > ---
> > tools/testing/selftests/cgroup/.gitignore | 3 +-
> > tools/testing/selftests/cgroup/Makefile | 2 +
> > tools/testing/selftests/cgroup/test_kill.c | 293 +++++++++++++++++++++
> > 3 files changed, 297 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..87b7b0d90773 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
> > diff --git a/tools/testing/selftests/cgroup/test_kill.c b/tools/testing/selftests/cgroup/test_kill.c
> > new file mode 100644
> > index 000000000000..c4e7b2e87395
> > --- /dev/null
> > +++ b/tools/testing/selftests/cgroup/test_kill.c
> > @@ -0,0 +1,293 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#include <stdbool.h>
> > +#include <linux/limits.h>
> > +#include <sys/ptrace.h>
> > +#include <sys/types.h>
> > +#include <sys/mman.h>
> > +#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>
> > +
> > +#include "../kselftest.h"
> > +#include "cgroup_util.h"
> > +
> > +#define DEBUG
> > +#ifdef DEBUG
> > +#define debug(args...) fprintf(stderr, args)
> > +#else
> > +#define debug(args...)
> > +#endif
> > +
> > +/*
> > + * 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) {
> > + debug("Error: cg_write() failed\n");
> > + goto out;
> > + }
> > +
> > + ret = cg_wait_for(fd);
> > + if (ret)
> > + goto out;
> > +
> > + ret = cg_read_strcmp(cgroup, "cgroup.events", "populated 0\n");
> > +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)
> > +{
> > + 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++)
> > + cg_run_nowait(cgroup, child_fn, NULL);
> > +
> > + if (cg_wait_for_proc_count(cgroup, 100))
> > + goto cleanup;
> > +
> > + if (cg_write(cgroup, "cgroup.kill", "1"))
> > + goto cleanup;
> > +
> > + if (cg_read_strcmp(cgroup, "cgroup.events", "populated 1\n"))
> > + goto cleanup;
> > +
> > + if (cg_kill_wait(cgroup))
> > + goto cleanup;
> > +
> > + if (cg_read_strcmp(cgroup, "cgroup.events", "populated 0\n"))
> > + goto cleanup;
> > +
> > + ret = KSFT_PASS;
> > +
> > +cleanup:
> > + 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)
> > +{
> > + 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;
> > +
> > + cg_run_nowait(cgroup[2], child_fn, NULL);
> > + cg_run_nowait(cgroup[7], child_fn, NULL);
> > + cg_run_nowait(cgroup[9], child_fn, NULL);
> > + cg_run_nowait(cgroup[9], child_fn, NULL);
> > + 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;
> > +
> > + if (cg_read_strcmp(cgroup[0], "cgroup.events", "populated 0\n"))
> > + goto cleanup;
> > +
> > + ret = KSFT_PASS;
> > +
> > +cleanup:
> > + 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;
> > +
> > + cgroup = cg_name(root, "cg_forkbomb_test");
> > + if (!cgroup)
> > + goto cleanup;
> > +
> > + if (cg_create(cgroup))
> > + goto cleanup;
> > +
> > + cg_run_nowait(cgroup, forkbomb_fn, NULL);
> > +
> > + usleep(100000);
> > +
> > + if (cg_kill_wait(cgroup))
> > + goto cleanup;
> > +
> > + if (cg_wait_for_proc_count(cgroup, 0))
> > + goto cleanup;
> > +
> > + ret = KSFT_PASS;
> > +
> > +cleanup:
> > + 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),
>
> I'm a little bit worried about the forkbomb test: how reliable it is and won't
> it kill the system, but Idk how make it better. Maybe instead of a true fork
> bomb we need to spawn children by a single process? I'm not exactly sure.
I had the tests run in tight a loop and it didn't take down the system.
The cgroup was nicely killed everytime which is why I kept it. Freezer
performs the same test. So what we could do is freeze first and then
kill under the assumption that the freezer tests don't suffer from the
same problem.
>
> Other than that the patch looks good to me.
>
> And thank you for writing tests!
>
> Roman
