[LTP] [PATCH] cgroup/cgroup_core03: convert to new LTP API with parameterised subtests

[LTP] [PATCH] cgroup/cgroup_core03: convert to new LTP API with parameterised subtests

[Yeswanth Krishna Tellakula]

The test used the old .test_all single-function style. Convert it to
the new parameterised .test/.tcnt API, splitting the three inline
checks into named subtest functions with explicit tst_res() reporting.

Changes:
- Replace .test_all with .test/tcnt=3 (new LTP API)
- Split run() into test_cg_kill_procs_count(), test_cg_kill_trigger(),
  test_cg_kill_empty() for per-subtest PASS/FAIL granularity
- Move cg_child lifecycle and SAFE_CG_HAS() check into setup()/cleanup()
  to ensure cgroup is cleaned up on any abort path
- Promote pids[] and num_pids to file scope so they survive across
  parameterised subtest calls
- Fix BUF_LEN to use compile-time MAX_PID_NUM instead of runtime PID_NUM
- Replace TST_EXP_VAL/TST_EXP_PASS_SILENT with tst_res(TPASS/TFAIL)
  with descriptive diagnostic messages
- Fix sizeof(*data_ptr) for type safety over sizeof(uintptr_t)
- Add .needs_root, .taint_check and increase .timeout to 30

No functional behaviour change.

Signed-off-by: Yeswanth Krishna Tellakula <yeswanth@linux.ibm.com>
---
 .../kernel/controllers/cgroup/cgroup_core03.c | 202 +++++++++++++-----
 1 file changed, 152 insertions(+), 50 deletions(-)

diff --git a/testcases/kernel/controllers/cgroup/cgroup_core03.c b/testcases/kernel/controllers/cgroup/cgroup_core03.c
index 846c00f29..44e1ac44a 100644
--- a/testcases/kernel/controllers/cgroup/cgroup_core03.c
+++ b/testcases/kernel/controllers/cgroup/cgroup_core03.c
@@ -2,15 +2,28 @@
 /*
  * Copyright (c) 2012 Christian Brauner <brauner-AT-kernel.org>
  * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
+ * Copyright (c) 2024 Linux Test Project
  */
 
 /*\
- * This test is copied from kselftest
- * tools/testing/selftests/cgroup/test_kill.c.
+ * [Description]
  *
- * Only simple test implemented within current case, the other cases such
- * as test_cgkill_tree and test_cgkill_forkbomb can be created later.
+ * Functional test for cgroup.kill interface (cgroupv2).
  *
+ * Derived from kselftest tools/testing/selftests/cgroup/test_kill.c
+ * (test_cgkill_simple subcase).
+ *
+ * The test verifies the following sequence:
+ *   1. Fork PID_NUM child processes, each joining a dedicated child cgroup.
+ *   2. Confirm all PID_NUM processes are visible in cgroup.procs.
+ *   3. Write 1 to cgroup.kill; confirm every process exits with a signal.
+ *   4. Confirm cgroup.procs is empty after the kill.
+ *
+ * [Algorithm]
+ *
+ * - Subtest 1: all forked pids appear in cgroup.procs before kill.
+ * - Subtest 2: writing 1 to cgroup.kill terminates every member process.
+ * - Subtest 3: cgroup.procs count reaches 0 after kill completes.
  */
 
 #include <sys/wait.h>
@@ -18,14 +31,24 @@
 #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)
+#define MAX_PID_NUM	100
+#define PID_NUM		MIN(MAX_PID_NUM, (tst_ncpus_available() + 1))
+#define BUF_LEN		(20 * MAX_PID_NUM)
 
 static tst_atomic_t *data_ptr;
 static char *buf;
-static struct tst_cg_group *cg_child_test_simple;
+static struct tst_cg_group *cg_child;
+static pid_t pids[MAX_PID_NUM];
+static int num_pids;
+
+/* ------------------------------------------------------------------ */
+/* Helpers                                                              */
+/* ------------------------------------------------------------------ */
 
+/*
+ * wait_for_pid() - wait for a single pid and verify it was killed by signal.
+ * Returns 0 on success (process was signalled), -1 otherwise.
+ */
 static int wait_for_pid(pid_t pid)
 {
 	int status, ret;
@@ -35,95 +58,174 @@ again:
 	if (ret == -1) {
 		if (errno == EINTR)
 			goto again;
-
 		return -1;
 	}
 
-	if (WIFSIGNALED(status))
-		return 0;
+	return WIFSIGNALED(status) ? 0 : -1;
+}
+
+/*
+ * cg_count_procs() - count newline-separated PIDs in cgroup.procs.
+ */
+static int cg_count_procs(const struct tst_cg_group *cg)
+{
+	char *ptr;
+	int nr = 0;
+
+	SAFE_CG_READ(cg, "cgroup.procs", buf, BUF_LEN);
+	for (ptr = buf; *ptr; ptr++)
+		if (*ptr == '\n')
+			nr++;
 
-	return -1;
+	return nr;
 }
 
-static int cg_run_nowait(const struct tst_cg_group *const cg)
+/*
+ * cg_run_nowait() - fork a child, move it into cg, then pause.
+ * Returns the child pid to the parent.
+ */
+static pid_t cg_run_nowait(const struct tst_cg_group *cg)
 {
-	int pid;
+	pid_t pid;
 
 	pid = SAFE_FORK();
 	if (pid == 0) {
 		SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
-		if (tst_atomic_inc(data_ptr) == PID_NUM)
+		if (tst_atomic_inc(data_ptr) == num_pids)
 			TST_CHECKPOINT_WAKE(0);
 		pause();
+		exit(0);
 	}
 
 	return pid;
 }
 
-static int cg_count_procs(const struct tst_cg_group *cg)
+/* ------------------------------------------------------------------ */
+/* Sub-tests                                                            */
+/* ------------------------------------------------------------------ */
+
+/*
+ * test_cg_kill_procs_count - verify PID_NUM processes are inside the cgroup
+ * before cgroup.kill is triggered.
+ */
+static void test_cg_kill_procs_count(void)
 {
-	char *ptr;
+	int actual;
+
+	actual = cg_count_procs(cg_child);
+	if (actual == num_pids)
+		tst_res(TPASS, "cgroup.procs has %d entries as expected", actual);
+	else
+		tst_res(TFAIL, "cgroup.procs has %d entries, expected %d",
+			actual, num_pids);
+}
 
-	int nr = 0;
+/*
+ * test_cg_kill_trigger - write 1 to cgroup.kill and confirm every forked
+ * process exits via a signal.
+ */
+static void test_cg_kill_trigger(void)
+{
+	int i, failed = 0;
 
-	SAFE_CG_READ(cg, "cgroup.procs", buf, BUF_LEN);
+	SAFE_CG_PRINTF(cg_child, "cgroup.kill", "%d", 1);
 
-	for (ptr = buf; *ptr; ptr++)
-		if (*ptr == '\n')
-			nr++;
+	for (i = 0; i < num_pids; i++) {
+		if (wait_for_pid(pids[i]) != 0) {
+			tst_res(TFAIL, "pid[%d]=%d did not exit via signal",
+				i, pids[i]);
+			failed++;
+		}
+	}
 
-	return nr;
+	if (!failed)
+		tst_res(TPASS, "all %d processes were killed by cgroup.kill",
+			num_pids);
 }
 
-static void run(void)
+/*
+ * test_cg_kill_empty - verify cgroup.procs is empty after cgroup.kill.
+ */
+static void test_cg_kill_empty(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");
-
-	if (!SAFE_CG_HAS(cg_child_test_simple, "cgroup.kill")) {
-		cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
-		tst_brk(TCONF, "cgroup.kill is not supported on your distribution");
-	}
+	int actual;
+
+	actual = cg_count_procs(cg_child);
+	if (actual == 0)
+		tst_res(TPASS, "cgroup.procs is empty after cgroup.kill");
+	else
+		tst_res(TFAIL, "cgroup.procs still has %d entries after kill",
+			actual);
+}
 
-	memset(buf, 0, BUF_LEN);
+/*
+ * run() - dispatch sub-tests; fork wave happens once before test 1.
+ */
+static void run(unsigned int n)
+{
+	int i;
 
-	for (i = 0; i < PID_NUM; i++)
-		pids[i] = cg_run_nowait(cg_child_test_simple);
+	/* Fork wave: only needed before the first subtest. */
+	if (n == 0) {
+		*data_ptr = 0;
+		num_pids = PID_NUM;
 
-	TST_CHECKPOINT_WAIT(0);
-	TST_EXP_VAL(cg_count_procs(cg_child_test_simple), PID_NUM);
-	SAFE_CG_PRINTF(cg_child_test_simple, "cgroup.kill", "%d", 1);
+		for (i = 0; i < num_pids; i++)
+			pids[i] = cg_run_nowait(cg_child);
 
-	for (i = 0; i < PID_NUM; i++)
-		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]));
+		TST_CHECKPOINT_WAIT(0);
+	}
 
-	TST_EXP_VAL(cg_count_procs(cg_child_test_simple), 0);
-	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+	switch (n) {
+	case 0:
+		test_cg_kill_procs_count();
+		break;
+	case 1:
+		test_cg_kill_trigger();
+		break;
+	case 2:
+		test_cg_kill_empty();
+		break;
+	}
 }
 
+/* ------------------------------------------------------------------ */
+/* Setup / Cleanup                                                      */
+/* ------------------------------------------------------------------ */
+
 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);
+	data_ptr = SAFE_MMAP(NULL, sizeof(*data_ptr), PROT_READ | PROT_WRITE,
+			     MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+
+	cg_child = tst_cg_group_mk(tst_cg, "cg_test_simple");
+
+	if (!SAFE_CG_HAS(cg_child, "cgroup.kill")) {
+		cg_child = tst_cg_group_rm(cg_child);
+		tst_brk(TCONF, "cgroup.kill is not supported on this kernel");
+	}
 }
 
 static void cleanup(void)
 {
+	if (cg_child)
+		cg_child = tst_cg_group_rm(cg_child);
+
 	if (data_ptr)
-		SAFE_MUNMAP(data_ptr, sizeof(uintptr_t));
+		SAFE_MUNMAP(data_ptr, sizeof(*data_ptr));
 }
 
 static struct tst_test test = {
-	.test_all = run,
+	.test = run,
+	.tcnt = 3,
 	.setup = setup,
 	.cleanup = cleanup,
 	.forks_child = 1,
-	.timeout = 20,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+	.timeout = 30,
+	.taint_check = TST_TAINT_W | TST_TAINT_D,
 	.needs_cgroup_ctrls = (const char *const []){ "base", NULL },
 	.needs_cgroup_ver = TST_CG_V2,
-	.needs_checkpoints = 1,
 };
-- 
2.50.1 (Apple Git-155)


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH] cgroup/cgroup_core03: convert to new LTP API with parameterised subtests

[Yeswanth Krishna Tellakula]

The test used the old .test_all single-function style. Convert it to
the new parameterised .test/.tcnt API, splitting the three inline
checks into named subtest functions with explicit tst_res() reporting.

Changes:
- Replace .test_all with .test/tcnt=3 (new LTP API)
- Split run() into test_cg_kill_procs_count(), test_cg_kill_trigger(),
  test_cg_kill_empty() for per-subtest PASS/FAIL granularity
- Move cg_child lifecycle and SAFE_CG_HAS() check into setup()/cleanup()
  to ensure cgroup is cleaned up on any abort path
- Promote pids[] and num_pids to file scope so they survive across
  parameterised subtest calls
- Fix BUF_LEN to use compile-time MAX_PID_NUM instead of runtime PID_NUM
- Replace TST_EXP_VAL/TST_EXP_PASS_SILENT with tst_res(TPASS/TFAIL)
  with descriptive diagnostic messages
- Fix sizeof(*data_ptr) for type safety over sizeof(uintptr_t)
- Add .needs_root, .taint_check and increase .timeout to 30

No functional behaviour change.

Signed-off-by: Yeswanth Krishna Tellakula <yeswanth@linux.ibm.com>
---
 .../kernel/controllers/cgroup/cgroup_core03.c | 202 +++++++++++++-----
 1 file changed, 152 insertions(+), 50 deletions(-)

diff --git a/testcases/kernel/controllers/cgroup/cgroup_core03.c b/testcases/kernel/controllers/cgroup/cgroup_core03.c
index 846c00f29..44e1ac44a 100644
--- a/testcases/kernel/controllers/cgroup/cgroup_core03.c
+++ b/testcases/kernel/controllers/cgroup/cgroup_core03.c
@@ -2,15 +2,28 @@
 /*
  * Copyright (c) 2012 Christian Brauner <brauner-AT-kernel.org>
  * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
+ * Copyright (c) 2024 Linux Test Project
  */
 
 /*\
- * This test is copied from kselftest
- * tools/testing/selftests/cgroup/test_kill.c.
+ * [Description]
  *
- * Only simple test implemented within current case, the other cases such
- * as test_cgkill_tree and test_cgkill_forkbomb can be created later.
+ * Functional test for cgroup.kill interface (cgroupv2).
  *
+ * Derived from kselftest tools/testing/selftests/cgroup/test_kill.c
+ * (test_cgkill_simple subcase).
+ *
+ * The test verifies the following sequence:
+ *   1. Fork PID_NUM child processes, each joining a dedicated child cgroup.
+ *   2. Confirm all PID_NUM processes are visible in cgroup.procs.
+ *   3. Write 1 to cgroup.kill; confirm every process exits with a signal.
+ *   4. Confirm cgroup.procs is empty after the kill.
+ *
+ * [Algorithm]
+ *
+ * - Subtest 1: all forked pids appear in cgroup.procs before kill.
+ * - Subtest 2: writing 1 to cgroup.kill terminates every member process.
+ * - Subtest 3: cgroup.procs count reaches 0 after kill completes.
  */
 
 #include <sys/wait.h>
@@ -18,14 +31,24 @@
 #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)
+#define MAX_PID_NUM	100
+#define PID_NUM		MIN(MAX_PID_NUM, (tst_ncpus_available() + 1))
+#define BUF_LEN		(20 * MAX_PID_NUM)
 
 static tst_atomic_t *data_ptr;
 static char *buf;
-static struct tst_cg_group *cg_child_test_simple;
+static struct tst_cg_group *cg_child;
+static pid_t pids[MAX_PID_NUM];
+static int num_pids;
+
+/* ------------------------------------------------------------------ */
+/* Helpers                                                              */
+/* ------------------------------------------------------------------ */
 
+/*
+ * wait_for_pid() - wait for a single pid and verify it was killed by signal.
+ * Returns 0 on success (process was signalled), -1 otherwise.
+ */
 static int wait_for_pid(pid_t pid)
 {
 	int status, ret;
@@ -35,95 +58,174 @@ again:
 	if (ret == -1) {
 		if (errno == EINTR)
 			goto again;
-
 		return -1;
 	}
 
-	if (WIFSIGNALED(status))
-		return 0;
+	return WIFSIGNALED(status) ? 0 : -1;
+}
+
+/*
+ * cg_count_procs() - count newline-separated PIDs in cgroup.procs.
+ */
+static int cg_count_procs(const struct tst_cg_group *cg)
+{
+	char *ptr;
+	int nr = 0;
+
+	SAFE_CG_READ(cg, "cgroup.procs", buf, BUF_LEN);
+	for (ptr = buf; *ptr; ptr++)
+		if (*ptr == '\n')
+			nr++;
 
-	return -1;
+	return nr;
 }
 
-static int cg_run_nowait(const struct tst_cg_group *const cg)
+/*
+ * cg_run_nowait() - fork a child, move it into cg, then pause.
+ * Returns the child pid to the parent.
+ */
+static pid_t cg_run_nowait(const struct tst_cg_group *cg)
 {
-	int pid;
+	pid_t pid;
 
 	pid = SAFE_FORK();
 	if (pid == 0) {
 		SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
-		if (tst_atomic_inc(data_ptr) == PID_NUM)
+		if (tst_atomic_inc(data_ptr) == num_pids)
 			TST_CHECKPOINT_WAKE(0);
 		pause();
+		exit(0);
 	}
 
 	return pid;
 }
 
-static int cg_count_procs(const struct tst_cg_group *cg)
+/* ------------------------------------------------------------------ */
+/* Sub-tests                                                            */
+/* ------------------------------------------------------------------ */
+
+/*
+ * test_cg_kill_procs_count - verify PID_NUM processes are inside the cgroup
+ * before cgroup.kill is triggered.
+ */
+static void test_cg_kill_procs_count(void)
 {
-	char *ptr;
+	int actual;
+
+	actual = cg_count_procs(cg_child);
+	if (actual == num_pids)
+		tst_res(TPASS, "cgroup.procs has %d entries as expected", actual);
+	else
+		tst_res(TFAIL, "cgroup.procs has %d entries, expected %d",
+			actual, num_pids);
+}
 
-	int nr = 0;
+/*
+ * test_cg_kill_trigger - write 1 to cgroup.kill and confirm every forked
+ * process exits via a signal.
+ */
+static void test_cg_kill_trigger(void)
+{
+	int i, failed = 0;
 
-	SAFE_CG_READ(cg, "cgroup.procs", buf, BUF_LEN);
+	SAFE_CG_PRINTF(cg_child, "cgroup.kill", "%d", 1);
 
-	for (ptr = buf; *ptr; ptr++)
-		if (*ptr == '\n')
-			nr++;
+	for (i = 0; i < num_pids; i++) {
+		if (wait_for_pid(pids[i]) != 0) {
+			tst_res(TFAIL, "pid[%d]=%d did not exit via signal",
+				i, pids[i]);
+			failed++;
+		}
+	}
 
-	return nr;
+	if (!failed)
+		tst_res(TPASS, "all %d processes were killed by cgroup.kill",
+			num_pids);
 }
 
-static void run(void)
+/*
+ * test_cg_kill_empty - verify cgroup.procs is empty after cgroup.kill.
+ */
+static void test_cg_kill_empty(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");
-
-	if (!SAFE_CG_HAS(cg_child_test_simple, "cgroup.kill")) {
-		cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
-		tst_brk(TCONF, "cgroup.kill is not supported on your distribution");
-	}
+	int actual;
+
+	actual = cg_count_procs(cg_child);
+	if (actual == 0)
+		tst_res(TPASS, "cgroup.procs is empty after cgroup.kill");
+	else
+		tst_res(TFAIL, "cgroup.procs still has %d entries after kill",
+			actual);
+}
 
-	memset(buf, 0, BUF_LEN);
+/*
+ * run() - dispatch sub-tests; fork wave happens once before test 1.
+ */
+static void run(unsigned int n)
+{
+	int i;
 
-	for (i = 0; i < PID_NUM; i++)
-		pids[i] = cg_run_nowait(cg_child_test_simple);
+	/* Fork wave: only needed before the first subtest. */
+	if (n == 0) {
+		*data_ptr = 0;
+		num_pids = PID_NUM;
 
-	TST_CHECKPOINT_WAIT(0);
-	TST_EXP_VAL(cg_count_procs(cg_child_test_simple), PID_NUM);
-	SAFE_CG_PRINTF(cg_child_test_simple, "cgroup.kill", "%d", 1);
+		for (i = 0; i < num_pids; i++)
+			pids[i] = cg_run_nowait(cg_child);
 
-	for (i = 0; i < PID_NUM; i++)
-		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]));
+		TST_CHECKPOINT_WAIT(0);
+	}
 
-	TST_EXP_VAL(cg_count_procs(cg_child_test_simple), 0);
-	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+	switch (n) {
+	case 0:
+		test_cg_kill_procs_count();
+		break;
+	case 1:
+		test_cg_kill_trigger();
+		break;
+	case 2:
+		test_cg_kill_empty();
+		break;
+	}
 }
 
+/* ------------------------------------------------------------------ */
+/* Setup / Cleanup                                                      */
+/* ------------------------------------------------------------------ */
+
 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);
+	data_ptr = SAFE_MMAP(NULL, sizeof(*data_ptr), PROT_READ | PROT_WRITE,
+			     MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+
+	cg_child = tst_cg_group_mk(tst_cg, "cg_test_simple");
+
+	if (!SAFE_CG_HAS(cg_child, "cgroup.kill")) {
+		cg_child = tst_cg_group_rm(cg_child);
+		tst_brk(TCONF, "cgroup.kill is not supported on this kernel");
+	}
 }
 
 static void cleanup(void)
 {
+	if (cg_child)
+		cg_child = tst_cg_group_rm(cg_child);
+
 	if (data_ptr)
-		SAFE_MUNMAP(data_ptr, sizeof(uintptr_t));
+		SAFE_MUNMAP(data_ptr, sizeof(*data_ptr));
 }
 
 static struct tst_test test = {
-	.test_all = run,
+	.test = run,
+	.tcnt = 3,
 	.setup = setup,
 	.cleanup = cleanup,
 	.forks_child = 1,
-	.timeout = 20,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+	.timeout = 30,
+	.taint_check = TST_TAINT_W | TST_TAINT_D,
 	.needs_cgroup_ctrls = (const char *const []){ "base", NULL },
 	.needs_cgroup_ver = TST_CG_V2,
-	.needs_checkpoints = 1,
 };
-- 
2.50.1 (Apple Git-155)


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH] cgroup/cgroup_core03: convert to new LTP API with parameterised subtests

[Yeswanth Krishna Tellakula]

The test used the old .test_all single-function style. Convert it to
the new parameterised .test/.tcnt API, splitting the three inline
checks into named subtest functions with explicit tst_res() reporting.

Changes:
- Replace .test_all with .test/tcnt=3 (new LTP API)
- Split run() into test_cg_kill_procs_count(), test_cg_kill_trigger(),
  test_cg_kill_empty() for per-subtest PASS/FAIL granularity
- Move cg_child lifecycle and SAFE_CG_HAS() check into setup()/cleanup()
  to ensure cgroup is cleaned up on any abort path
- Promote pids[] and num_pids to file scope so they survive across
  parameterised subtest calls
- Fix BUF_LEN to use compile-time MAX_PID_NUM instead of runtime PID_NUM
- Replace TST_EXP_VAL/TST_EXP_PASS_SILENT with tst_res(TPASS/TFAIL)
  with descriptive diagnostic messages
- Fix sizeof(*data_ptr) for type safety over sizeof(uintptr_t)
- Add .needs_root, .taint_check and increase .timeout to 30

No functional behaviour change.

Signed-off-by: Yeswanth Krishna Tellakula <yeswanth@linux.ibm.com>
---
 .../kernel/controllers/cgroup/cgroup_core03.c | 202 +++++++++++++-----
 1 file changed, 152 insertions(+), 50 deletions(-)

diff --git a/testcases/kernel/controllers/cgroup/cgroup_core03.c b/testcases/kernel/controllers/cgroup/cgroup_core03.c
index 846c00f29..44e1ac44a 100644
--- a/testcases/kernel/controllers/cgroup/cgroup_core03.c
+++ b/testcases/kernel/controllers/cgroup/cgroup_core03.c
@@ -2,15 +2,28 @@
 /*
  * Copyright (c) 2012 Christian Brauner <brauner-AT-kernel.org>
  * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
+ * Copyright (c) 2024 Linux Test Project
  */
 
 /*\
- * This test is copied from kselftest
- * tools/testing/selftests/cgroup/test_kill.c.
+ * [Description]
  *
- * Only simple test implemented within current case, the other cases such
- * as test_cgkill_tree and test_cgkill_forkbomb can be created later.
+ * Functional test for cgroup.kill interface (cgroupv2).
  *
+ * Derived from kselftest tools/testing/selftests/cgroup/test_kill.c
+ * (test_cgkill_simple subcase).
+ *
+ * The test verifies the following sequence:
+ *   1. Fork PID_NUM child processes, each joining a dedicated child cgroup.
+ *   2. Confirm all PID_NUM processes are visible in cgroup.procs.
+ *   3. Write 1 to cgroup.kill; confirm every process exits with a signal.
+ *   4. Confirm cgroup.procs is empty after the kill.
+ *
+ * [Algorithm]
+ *
+ * - Subtest 1: all forked pids appear in cgroup.procs before kill.
+ * - Subtest 2: writing 1 to cgroup.kill terminates every member process.
+ * - Subtest 3: cgroup.procs count reaches 0 after kill completes.
  */
 
 #include <sys/wait.h>
@@ -18,14 +31,24 @@
 #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)
+#define MAX_PID_NUM	100
+#define PID_NUM		MIN(MAX_PID_NUM, (tst_ncpus_available() + 1))
+#define BUF_LEN		(20 * MAX_PID_NUM)
 
 static tst_atomic_t *data_ptr;
 static char *buf;
-static struct tst_cg_group *cg_child_test_simple;
+static struct tst_cg_group *cg_child;
+static pid_t pids[MAX_PID_NUM];
+static int num_pids;
+
+/* ------------------------------------------------------------------ */
+/* Helpers                                                              */
+/* ------------------------------------------------------------------ */
 
+/*
+ * wait_for_pid() - wait for a single pid and verify it was killed by signal.
+ * Returns 0 on success (process was signalled), -1 otherwise.
+ */
 static int wait_for_pid(pid_t pid)
 {
 	int status, ret;
@@ -35,95 +58,174 @@ again:
 	if (ret == -1) {
 		if (errno == EINTR)
 			goto again;
-
 		return -1;
 	}
 
-	if (WIFSIGNALED(status))
-		return 0;
+	return WIFSIGNALED(status) ? 0 : -1;
+}
+
+/*
+ * cg_count_procs() - count newline-separated PIDs in cgroup.procs.
+ */
+static int cg_count_procs(const struct tst_cg_group *cg)
+{
+	char *ptr;
+	int nr = 0;
+
+	SAFE_CG_READ(cg, "cgroup.procs", buf, BUF_LEN);
+	for (ptr = buf; *ptr; ptr++)
+		if (*ptr == '\n')
+			nr++;
 
-	return -1;
+	return nr;
 }
 
-static int cg_run_nowait(const struct tst_cg_group *const cg)
+/*
+ * cg_run_nowait() - fork a child, move it into cg, then pause.
+ * Returns the child pid to the parent.
+ */
+static pid_t cg_run_nowait(const struct tst_cg_group *cg)
 {
-	int pid;
+	pid_t pid;
 
 	pid = SAFE_FORK();
 	if (pid == 0) {
 		SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
-		if (tst_atomic_inc(data_ptr) == PID_NUM)
+		if (tst_atomic_inc(data_ptr) == num_pids)
 			TST_CHECKPOINT_WAKE(0);
 		pause();
+		exit(0);
 	}
 
 	return pid;
 }
 
-static int cg_count_procs(const struct tst_cg_group *cg)
+/* ------------------------------------------------------------------ */
+/* Sub-tests                                                            */
+/* ------------------------------------------------------------------ */
+
+/*
+ * test_cg_kill_procs_count - verify PID_NUM processes are inside the cgroup
+ * before cgroup.kill is triggered.
+ */
+static void test_cg_kill_procs_count(void)
 {
-	char *ptr;
+	int actual;
+
+	actual = cg_count_procs(cg_child);
+	if (actual == num_pids)
+		tst_res(TPASS, "cgroup.procs has %d entries as expected", actual);
+	else
+		tst_res(TFAIL, "cgroup.procs has %d entries, expected %d",
+			actual, num_pids);
+}
 
-	int nr = 0;
+/*
+ * test_cg_kill_trigger - write 1 to cgroup.kill and confirm every forked
+ * process exits via a signal.
+ */
+static void test_cg_kill_trigger(void)
+{
+	int i, failed = 0;
 
-	SAFE_CG_READ(cg, "cgroup.procs", buf, BUF_LEN);
+	SAFE_CG_PRINTF(cg_child, "cgroup.kill", "%d", 1);
 
-	for (ptr = buf; *ptr; ptr++)
-		if (*ptr == '\n')
-			nr++;
+	for (i = 0; i < num_pids; i++) {
+		if (wait_for_pid(pids[i]) != 0) {
+			tst_res(TFAIL, "pid[%d]=%d did not exit via signal",
+				i, pids[i]);
+			failed++;
+		}
+	}
 
-	return nr;
+	if (!failed)
+		tst_res(TPASS, "all %d processes were killed by cgroup.kill",
+			num_pids);
 }
 
-static void run(void)
+/*
+ * test_cg_kill_empty - verify cgroup.procs is empty after cgroup.kill.
+ */
+static void test_cg_kill_empty(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");
-
-	if (!SAFE_CG_HAS(cg_child_test_simple, "cgroup.kill")) {
-		cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
-		tst_brk(TCONF, "cgroup.kill is not supported on your distribution");
-	}
+	int actual;
+
+	actual = cg_count_procs(cg_child);
+	if (actual == 0)
+		tst_res(TPASS, "cgroup.procs is empty after cgroup.kill");
+	else
+		tst_res(TFAIL, "cgroup.procs still has %d entries after kill",
+			actual);
+}
 
-	memset(buf, 0, BUF_LEN);
+/*
+ * run() - dispatch sub-tests; fork wave happens once before test 1.
+ */
+static void run(unsigned int n)
+{
+	int i;
 
-	for (i = 0; i < PID_NUM; i++)
-		pids[i] = cg_run_nowait(cg_child_test_simple);
+	/* Fork wave: only needed before the first subtest. */
+	if (n == 0) {
+		*data_ptr = 0;
+		num_pids = PID_NUM;
 
-	TST_CHECKPOINT_WAIT(0);
-	TST_EXP_VAL(cg_count_procs(cg_child_test_simple), PID_NUM);
-	SAFE_CG_PRINTF(cg_child_test_simple, "cgroup.kill", "%d", 1);
+		for (i = 0; i < num_pids; i++)
+			pids[i] = cg_run_nowait(cg_child);
 
-	for (i = 0; i < PID_NUM; i++)
-		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]));
+		TST_CHECKPOINT_WAIT(0);
+	}
 
-	TST_EXP_VAL(cg_count_procs(cg_child_test_simple), 0);
-	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+	switch (n) {
+	case 0:
+		test_cg_kill_procs_count();
+		break;
+	case 1:
+		test_cg_kill_trigger();
+		break;
+	case 2:
+		test_cg_kill_empty();
+		break;
+	}
 }
 
+/* ------------------------------------------------------------------ */
+/* Setup / Cleanup                                                      */
+/* ------------------------------------------------------------------ */
+
 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);
+	data_ptr = SAFE_MMAP(NULL, sizeof(*data_ptr), PROT_READ | PROT_WRITE,
+			     MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+
+	cg_child = tst_cg_group_mk(tst_cg, "cg_test_simple");
+
+	if (!SAFE_CG_HAS(cg_child, "cgroup.kill")) {
+		cg_child = tst_cg_group_rm(cg_child);
+		tst_brk(TCONF, "cgroup.kill is not supported on this kernel");
+	}
 }
 
 static void cleanup(void)
 {
+	if (cg_child)
+		cg_child = tst_cg_group_rm(cg_child);
+
 	if (data_ptr)
-		SAFE_MUNMAP(data_ptr, sizeof(uintptr_t));
+		SAFE_MUNMAP(data_ptr, sizeof(*data_ptr));
 }
 
 static struct tst_test test = {
-	.test_all = run,
+	.test = run,
+	.tcnt = 3,
 	.setup = setup,
 	.cleanup = cleanup,
 	.forks_child = 1,
-	.timeout = 20,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+	.timeout = 30,
+	.taint_check = TST_TAINT_W | TST_TAINT_D,
 	.needs_cgroup_ctrls = (const char *const []){ "base", NULL },
 	.needs_cgroup_ver = TST_CG_V2,
-	.needs_checkpoints = 1,
 };
-- 
2.50.1 (Apple Git-155)


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] cgroup/cgroup_core03: convert to new LTP API with parameterised subtests

[linuxtestproject.agent]

Hi Yeswanth,

On Fri, 31 Jul 2026, Yeswanth Krishna Tellakula wrote:
> cgroup/cgroup_core03: convert to new LTP API with parameterised subtests

> +	if (n == 0) {
> +		*data_ptr = 0;
> +		num_pids = PID_NUM;
> +
> +		for (i = 0; i < num_pids; i++)
> +			pids[i] = cg_run_nowait(cg_child);
> +
> +		TST_CHECKPOINT_WAIT(0);
> +	}
> +
> +	switch (n) {
> +	case 0:
> +		test_cg_kill_procs_count();
> +		break;
> +	case 1:
> +		test_cg_kill_trigger();

Could this sequence remain in one .test_all callback? After run(0) returns,
the LTP runner calls tst_reap_children(), which blocks waiting for the paused
children. The callback that writes to cgroup.kill is therefore never reached,
and the test times out.

Helper functions can still report the individual checks, but no live child
can remain when a .test callback returns.

> + * [Description]
> + *
> + * Functional test for cgroup.kill interface (cgroupv2).

Could the deprecated [Description] marker be removed? Test catalog comments
begin directly with the description; only [Algorithm] is a supported section
marker.

> cgroup/cgroup_core03: convert to new LTP API with parameterised subtests
>
> The test used the old .test_all single-function style.
> [...]
> No functional behaviour change.

Could the commit message describe this as a callback-model refactoring rather
than a new API conversion? The existing test already uses tst_test.h and
struct tst_test, and the callback split currently changes behavior by causing
the timeout above.

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp