Re: [LTP] [PATCH v8 1/2] kill01: New case cgroup kill

Li Wang
2023-04-21T06:35:58Z

On Fri, Apr 21, 2023 at 9:27 AM Wei Gao via ltp <ltp@lists.linux.it> wrote:

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
> lib/tst_cgroup.c | 1 +
> runtest/controllers | 1 +
> .../kernel/controllers/cgroup/kill/.gitignore | 1 +
> .../kernel/controllers/cgroup/kill/Makefile | 6 +
> .../kernel/controllers/cgroup/kill/kill01.c | 144 ++++++++++++++++++
> 5 files changed, 153 insertions(+)
> create mode 100644 testcases/kernel/controllers/cgroup/kill/.gitignore
> create mode 100644 testcases/kernel/controllers/cgroup/kill/Makefile
> create mode 100644 testcases/kernel/controllers/cgroup/kill/kill01.c
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 50699bc63..77575431d 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -166,6 +166,7 @@ static const struct cgroup_file cgroup_ctrl_files[] = {
> { "cgroup.controllers", NULL, 0 },
> { "cgroup.subtree_control", NULL, 0 },
> { "cgroup.clone_children", "cgroup.clone_children", 0 },
> + { "cgroup.kill", NULL, 0 },
> { }
> };
>
> diff --git a/runtest/controllers b/runtest/controllers
> index 8d1b936bf..2f69a8ec2 100644
> --- a/runtest/controllers
> +++ b/runtest/controllers
> @@ -23,6 +23,7 @@ memcontrol01 memcontrol01
> memcontrol02 memcontrol02
> memcontrol03 memcontrol03
> memcontrol04 memcontrol04
> +kill01 kill01
>
> cgroup_fj_function_debug cgroup_fj_function.sh debug
> cgroup_fj_function_cpuset cgroup_fj_function.sh cpuset
> diff --git a/testcases/kernel/controllers/cgroup/kill/.gitignore
> b/testcases/kernel/controllers/cgroup/kill/.gitignore
> new file mode 100644
> index 000000000..4f9649e27
> --- /dev/null
> +++ b/testcases/kernel/controllers/cgroup/kill/.gitignore
> @@ -0,0 +1 @@
> +/kill01
> diff --git a/testcases/kernel/controllers/cgroup/kill/Makefile
> b/testcases/kernel/controllers/cgroup/kill/Makefile
> new file mode 100644
> index 000000000..bf5aea9e7
> --- /dev/null
> +++ b/testcases/kernel/controllers/cgroup/kill/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +top_srcdir ?= ../../../../../
>

The depth of top_srcdir is incorrect, also the upper dir Makefile
should be changed to include generic_trunk_target.mk.

i.e.

# git diff
diff --git a/testcases/kernel/controllers/cgroup/Makefile
b/testcases/kernel/controllers/cgroup/Makefile
index 0db0a7d..1592d8e 100644
--- a/testcases/kernel/controllers/cgroup/Makefile
+++ b/testcases/kernel/controllers/cgroup/Makefile
@@ -9,4 +9,4 @@ include $(abs_srcdir)/../Makefile.inc

INSTALL_TARGETS := *.sh

-include $(top_srcdir)/include/mk/generic_leaf_target.mk
+include $(top_srcdir)/include/mk/generic_trunk_target.mk
diff --git a/testcases/kernel/controllers/cgroup/kill/Makefile
b/testcases/kernel/controllers/cgroup/kill/Makefile
index bf5aea9..2f198e6 100644
--- a/testcases/kernel/controllers/cgroup/kill/Makefile
+++ b/testcases/kernel/controllers/cgroup/kill/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-or-later

-top_srcdir ?= ../../../../../
+top_srcdir ?= ../../../../..

include $(top_srcdir)/include/mk/testcases.mk
include $(top_srcdir)/include/mk/generic_leaf_target.mk

But I guess the suggestion of Richard is probably to rename
kill01.c to cgroup_core03.c/cgroup_kill01.c and put it into
".../controllers/cgroup/" directly.

Then we don't need to create a subdir or Makefile for kill/ anymore.

> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/controllers/cgroup/kill/kill01.c
> b/testcases/kernel/controllers/cgroup/kill/kill01.c
> new file mode 100644
> index 000000000..6a328a770
> --- /dev/null
> +++ b/testcases/kernel/controllers/cgroup/kill/kill01.c
> @@ -0,0 +1,144 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2012 Christian Brauner <brauner-AT-kernel.org>
> + * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test is copied from kselftest
> + * tools/testing/selftests/cgroup/test_kill.c
> + * Only simple test implemented within current case, the other cases such
> + * as test_cgkill_tree and test_cgkill_forkbomb can be created later.
> + *
> + */
> +
> +#include <sys/wait.h>
> +
> +#include "lapi/syscalls.h"
> +#include "tst_test.h"
> +
> +#define MAX_PID_NUM 100
> +#define PID_NUM MIN(MAX_PID_NUM, (tst_ncpus_available() + 1))
> +#define BUF_LEN (20 * PID_NUM)
> +
> +static int *data_ptr;
> +static char *buf;
> +static struct tst_cg_group *cg_child_test_simple;
> +
> +static int wait_for_pid(pid_t pid)
> +{
> + int status, ret;
> +
> +again:
> + ret = waitpid(pid, &status, 0);
> + if (ret == -1) {
> + if (errno == EINTR)
> + goto again;
> +
> + return -1;
> + }
> +
> + if (WIFSIGNALED(status))
> + return 0;
> +
> + return -1;
> +}
> +
> +/*
> + * A simple process running in a sleep loop until being
> + * re-parented.
> + */
> +static void child_fn(void)
> +{
> + int ppid = getppid();
> +
> + while (getppid() == ppid)
> + usleep(1000);
> +
> +}
> +
> +static int cg_run_nowait(const struct tst_cg_group *const cg,
> + void (*fn)(void))
> +{
> + int pid;
> +
> + pid = SAFE_FORK();
> + if (pid == 0) {
> + SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
> + tst_atomic_inc(data_ptr);
> + if (tst_atomic_load(data_ptr) == PID_NUM)
> + TST_CHECKPOINT_WAKE(0);
> + fn();
> + }
> +
> + return pid;
> +}
> +
> +static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int
> count)
> +{
> + char *ptr;
> +
> + int nr = 0;
> +
> + SAFE_CG_READ(cg, "cgroup.procs", buf, BUF_LEN);
> +
> + for (ptr = buf; *ptr; ptr++)
> + if (*ptr == '\n')
> + nr++;
> +
> + if (nr >= count)
> + return 0;
> +
> + tst_res(TINFO, "Expect process num is %d but get %d", count, nr);
> +
> + return -1;
> +}
> +
> +static void run(void)
> +{
> + pid_t pids[MAX_PID_NUM];
> + int i;
> + *data_ptr = 0;
> +
> + cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
> +
> + memset(buf, 0, BUF_LEN);
> +
> + for (i = 0; i < PID_NUM; i++)
> + pids[i] = cg_run_nowait(cg_child_test_simple, child_fn);
> +
> + TST_CHECKPOINT_WAIT(0);
> + TST_EXP_PASS(cg_wait_for_proc_count(cg_child_test_simple,
> PID_NUM));
> + SAFE_CG_PRINTF(cg_child_test_simple, "cgroup.kill", "%d", 1);
> +
> + for (i = 0; i < PID_NUM; i++)
> + TST_EXP_PASS_SILENT(wait_for_pid(pids[i]));
> +
> + cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
> +}
> +
> +static void setup(void)
> +{
> + buf = tst_alloc(BUF_LEN);
> + data_ptr = SAFE_MMAP(NULL, sizeof(uintptr_t), PROT_READ |
> PROT_WRITE,
> + MAP_SHARED |
> MAP_ANONYMOUS, -1, 0);
> +}
> +
> +static void cleanup(void)
> +{
> + if (data_ptr)
> + SAFE_MUNMAP(data_ptr, sizeof(uintptr_t));
> +}
> +
> +static struct tst_test test = {
> + .test_all = run,
> + .setup = setup,
> + .cleanup = cleanup,
> + .forks_child = 1,
> + .max_runtime = 20,
> + .needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
>

Obviously, we shouldn't request "memory" controller anymore.

.needs_cgroup_ctrls = (const char *const []){ "pseudo", NULL },

> + .needs_cgroup_ver = TST_CG_V2,
> + .needs_checkpoints = 1,
> +};
> --
> 2.35.3
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

--
Regards,
Li Wang