[LTP] [PATCH v1] kill01: New case cgroup kill

[LTP] [PATCH v1] kill01: New case cgroup kill

[Wei Gao via ltp]

Move basic test from kernel for cgroup kill button.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 runtest/controllers                           |   1 +
 testcases/kernel/controllers/kill/.gitignore  |   3 +
 testcases/kernel/controllers/kill/Makefile    |  13 +
 .../kernel/controllers/kill/cgroup_util.c     | 668 ++++++++++++++++++
 .../kernel/controllers/kill/cgroup_util.h     |  59 ++
 testcases/kernel/controllers/kill/kill01.c    | 336 +++++++++
 6 files changed, 1080 insertions(+)
 create mode 100644 testcases/kernel/controllers/kill/.gitignore
 create mode 100644 testcases/kernel/controllers/kill/Makefile
 create mode 100644 testcases/kernel/controllers/kill/cgroup_util.c
 create mode 100644 testcases/kernel/controllers/kill/cgroup_util.h
 create mode 100644 testcases/kernel/controllers/kill/kill01.c

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/kill/.gitignore b/testcases/kernel/controllers/kill/.gitignore
new file mode 100644
index 000000000..a79fc6c6b
--- /dev/null
+++ b/testcases/kernel/controllers/kill/.gitignore
@@ -0,0 +1,3 @@
+/kill01
+/kill01.o
+/cgroup_util.o
diff --git a/testcases/kernel/controllers/kill/Makefile b/testcases/kernel/controllers/kill/Makefile
new file mode 100644
index 000000000..89f70903e
--- /dev/null
+++ b/testcases/kernel/controllers/kill/Makefile
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+MAKE_TARGETS            := kill01
+kill01: cgroup_util.o kill01.o
+	$(LINK.o) $^ $(LOADLIBES) $(LDLIBS) $(OUTPUT_OPTION)
+
+kill01.o cgroup_util.o : %.o: %.c
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/controllers/kill/cgroup_util.c b/testcases/kernel/controllers/kill/cgroup_util.c
new file mode 100644
index 000000000..662395146
--- /dev/null
+++ b/testcases/kernel/controllers/kill/cgroup_util.c
@@ -0,0 +1,668 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2012 Christian Brauner <brauner-AT-kernel.org>
+ */
+
+
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/limits.h>
+#include <linux/sched.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>
+#include <unistd.h>
+
+#include "lapi/syscalls.h"
+#include "cgroup_util.h"
+
+struct __clone_args {
+	__aligned_u64 flags;
+	__aligned_u64 pidfd;
+	__aligned_u64 child_tid;
+	__aligned_u64 parent_tid;
+	__aligned_u64 exit_signal;
+	__aligned_u64 stack;
+	__aligned_u64 stack_size;
+	__aligned_u64 tls;
+#ifndef CLONE_ARGS_SIZE_VER0
+#define CLONE_ARGS_SIZE_VER0 64 /* sizeof first published struct */
+#endif
+	__aligned_u64 set_tid;
+	__aligned_u64 set_tid_size;
+#ifndef CLONE_ARGS_SIZE_VER1
+#define CLONE_ARGS_SIZE_VER1 80 /* sizeof second published struct */
+#endif
+	__aligned_u64 cgroup;
+#ifndef CLONE_ARGS_SIZE_VER2
+#define CLONE_ARGS_SIZE_VER2 88 /* sizeof third published struct */
+#endif
+};
+
+static pid_t sys_clone3(struct __clone_args *args, size_t size)
+{
+	fflush(stdout);
+	fflush(stderr);
+	return syscall(__NR_clone3, args, size);
+}
+
+/* Returns read len on success, or -errno on failure. */
+static ssize_t read_text(const char *path, char *buf, size_t max_len)
+{
+	ssize_t len;
+	int fd;
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return -errno;
+
+	len = read(fd, buf, max_len - 1);
+
+	if (len >= 0)
+		buf[len] = 0;
+
+	close(fd);
+	return len < 0 ? -errno : len;
+}
+
+/* Returns written len on success, or -errno on failure. */
+static ssize_t write_text(const char *path, char *buf, ssize_t len)
+{
+	int fd;
+
+	fd = open(path, O_WRONLY | O_APPEND);
+	if (fd < 0)
+		return -errno;
+
+	len = write(fd, buf, len);
+	close(fd);
+	return len < 0 ? -errno : len;
+}
+
+char *cg_name(const char *root, const char *name)
+{
+	size_t len = strlen(root) + strlen(name) + 2;
+	char *ret = malloc(len);
+
+	snprintf(ret, len, "%s/%s", root, name);
+
+	return ret;
+}
+
+char *cg_name_indexed(const char *root, const char *name, int index)
+{
+	size_t len = strlen(root) + strlen(name) + 10;
+	char *ret = malloc(len);
+
+	snprintf(ret, len, "%s/%s_%d", root, name, index);
+
+	return ret;
+}
+
+char *cg_control(const char *cgroup, const char *control)
+{
+	size_t len = strlen(cgroup) + strlen(control) + 2;
+	char *ret = malloc(len);
+
+	snprintf(ret, len, "%s/%s", cgroup, control);
+
+	return ret;
+}
+
+/* Returns 0 on success, or -errno on failure. */
+int cg_read(const char *cgroup, const char *control, char *buf, size_t len)
+{
+	char path[PATH_MAX];
+	ssize_t ret;
+
+	snprintf(path, sizeof(path), "%s/%s", cgroup, control);
+
+	ret = read_text(path, buf, len);
+	return ret >= 0 ? 0 : ret;
+}
+
+int cg_read_strcmp(const char *cgroup, const char *control,
+		   const char *expected)
+{
+	size_t size;
+	char *buf;
+	int ret;
+
+	/* Handle the case of comparing against empty string */
+	if (!expected)
+		return -1;
+
+	size = strlen(expected) + 1;
+
+	buf = malloc(size);
+	if (!buf)
+		return -1;
+
+	if (cg_read(cgroup, control, buf, size)) {
+		free(buf);
+		return -1;
+	}
+
+	ret = strcmp(expected, buf);
+	free(buf);
+	return ret;
+}
+
+int cg_read_strstr(const char *cgroup, const char *control, const char *needle)
+{
+	char buf[PAGE_SIZE];
+
+	if (cg_read(cgroup, control, buf, sizeof(buf)))
+		return -1;
+
+	return strstr(buf, needle) ? 0 : -1;
+}
+
+long cg_read_long(const char *cgroup, const char *control)
+{
+	char buf[128];
+
+	if (cg_read(cgroup, control, buf, sizeof(buf)))
+		return -1;
+
+	return atol(buf);
+}
+
+long cg_read_key_long(const char *cgroup, const char *control, const char *key)
+{
+	char buf[PAGE_SIZE];
+	char *ptr;
+
+	if (cg_read(cgroup, control, buf, sizeof(buf)))
+		return -1;
+
+	ptr = strstr(buf, key);
+	if (!ptr)
+		return -1;
+
+	return atol(ptr + strlen(key));
+}
+
+long cg_read_lc(const char *cgroup, const char *control)
+{
+	char buf[PAGE_SIZE];
+	static const char delim[] = "\n";
+	char *line;
+	long cnt = 0;
+
+	if (cg_read(cgroup, control, buf, sizeof(buf)))
+		return -1;
+
+	for (line = strtok(buf, delim); line; line = strtok(NULL, delim))
+		cnt++;
+
+	return cnt;
+}
+
+/* Returns 0 on success, or -errno on failure. */
+int cg_write(const char *cgroup, const char *control, char *buf)
+{
+	char path[PATH_MAX];
+	ssize_t len = strlen(buf), ret;
+
+	snprintf(path, sizeof(path), "%s/%s", cgroup, control);
+	ret = write_text(path, buf, len);
+	return ret == len ? 0 : ret;
+}
+
+int cg_write_numeric(const char *cgroup, const char *control, long value)
+{
+	char buf[64];
+	int ret;
+
+	ret = sprintf(buf, "%lu", value);
+	if (ret < 0)
+		return ret;
+
+	return cg_write(cgroup, control, buf);
+}
+
+int cg_find_unified_root(char *root, size_t len)
+{
+	char buf[10 * PAGE_SIZE];
+	char *fs, *mount, *type;
+	static const char delim[] = "\n\t ";
+
+	if (read_text("/proc/self/mounts", buf, sizeof(buf)) <= 0)
+		return -1;
+
+	/*
+	 * Example:
+	 * cgroup /sys/fs/cgroup cgroup2 rw,seclabel,noexec,relatime 0 0
+	 */
+	for (fs = strtok(buf, delim); fs; fs = strtok(NULL, delim)) {
+		mount = strtok(NULL, delim);
+		type = strtok(NULL, delim);
+		strtok(NULL, delim);
+		strtok(NULL, delim);
+		strtok(NULL, delim);
+
+		if (strcmp(type, "cgroup2") == 0) {
+			strncpy(root, mount, len);
+			return 0;
+		}
+	}
+
+	return -1;
+}
+
+int cg_create(const char *cgroup)
+{
+	return mkdir(cgroup, 0755);
+}
+
+int cg_wait_for_proc_count(const char *cgroup, int count)
+{
+	char buf[10 * PAGE_SIZE] = {0};
+	int attempts;
+	char *ptr;
+
+	for (attempts = 10; attempts >= 0; attempts--) {
+		int nr = 0;
+
+		if (cg_read(cgroup, "cgroup.procs", buf, sizeof(buf)))
+			break;
+
+		for (ptr = buf; *ptr; ptr++)
+			if (*ptr == '\n')
+				nr++;
+
+		if (nr >= count)
+			return 0;
+
+		usleep(100000);
+	}
+
+	return -1;
+}
+
+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;
+
+	while (ptr < buf + sizeof(buf)) {
+		int pid = strtol(ptr, &ptr, 10);
+
+		if (pid == 0)
+			break;
+		if (*ptr)
+			ptr++;
+		else
+			break;
+		if (kill(pid, SIGKILL))
+			return -1;
+	}
+
+	return 0;
+}
+
+int cg_destroy(const char *cgroup)
+{
+	int ret;
+
+retry:
+	ret = rmdir(cgroup);
+	if (ret && errno == EBUSY) {
+		cg_killall(cgroup);
+		usleep(100);
+		goto retry;
+	}
+
+	if (ret && errno == ENOENT)
+		ret = 0;
+
+	return ret;
+}
+
+int cg_enter(const char *cgroup, int pid)
+{
+	char pidbuf[64];
+
+	snprintf(pidbuf, sizeof(pidbuf), "%d", pid);
+	return cg_write(cgroup, "cgroup.procs", pidbuf);
+}
+
+int cg_enter_current(const char *cgroup)
+{
+	return cg_write(cgroup, "cgroup.procs", "0");
+}
+
+int cg_enter_current_thread(const char *cgroup)
+{
+	return cg_write(cgroup, "cgroup.threads", "0");
+}
+
+int cg_run(const char *cgroup,
+	   int (*fn)(const char *cgroup, void *arg),
+	   void *arg)
+{
+	int pid, retcode;
+
+	pid = fork();
+	if (pid < 0) {
+		return pid;
+	} else if (pid == 0) {
+		char buf[64];
+
+		snprintf(buf, sizeof(buf), "%d", getpid());
+		if (cg_write(cgroup, "cgroup.procs", buf))
+			exit(EXIT_FAILURE);
+		exit(fn(cgroup, arg));
+	} else {
+		waitpid(pid, &retcode, 0);
+		if (WIFEXITED(retcode))
+			return WEXITSTATUS(retcode);
+		else
+			return -1;
+	}
+}
+
+pid_t clone_into_cgroup(int cgroup_fd)
+{
+#ifdef CLONE_ARGS_SIZE_VER2
+	pid_t pid;
+
+	struct __clone_args args = {
+		.flags = CLONE_INTO_CGROUP,
+		.exit_signal = SIGCHLD,
+		.cgroup = cgroup_fd,
+	};
+
+	pid = sys_clone3(&args, sizeof(struct __clone_args));
+	/*
+	 * Verify that this is a genuine test failure:
+	 * ENOSYS -> clone3() not available
+	 * E2BIG  -> CLONE_INTO_CGROUP not available
+	 */
+	if (pid < 0 && (errno == ENOSYS || errno == E2BIG))
+		goto pretend_enosys;
+
+	return pid;
+
+pretend_enosys:
+#endif
+	errno = ENOSYS;
+	return -ENOSYS;
+}
+
+int clone_reap(pid_t pid, int options)
+{
+	int ret;
+	siginfo_t info = {
+		.si_signo = 0,
+	};
+
+again:
+	ret = waitid(P_PID, pid, &info, options | __WALL | __WNOTHREAD);
+	if (ret < 0) {
+		if (errno == EINTR)
+			goto again;
+		return -1;
+	}
+
+	if (options & WEXITED) {
+		if (WIFEXITED(info.si_status))
+			return WEXITSTATUS(info.si_status);
+	}
+
+	if (options & WSTOPPED) {
+		if (WIFSTOPPED(info.si_status))
+			return WSTOPSIG(info.si_status);
+	}
+
+	if (options & WCONTINUED) {
+		if (WIFCONTINUED(info.si_status))
+			return 0;
+	}
+
+	return -1;
+}
+
+int dirfd_open_opath(const char *dir)
+{
+	return open(dir, O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW | O_PATH);
+}
+
+#define close_prot_errno(fd)                                                           \
+	do {                                                                           \
+		if (fd >= 0) {                                                         \
+			int _e_ = errno;                                               \
+			close(fd);                                                     \
+			errno = _e_;                                                   \
+		} \
+	} while (0)
+static int clone_into_cgroup_run_nowait(const char *cgroup,
+					int (*fn)(void))
+{
+	int cgroup_fd;
+	pid_t pid;
+
+	cgroup_fd =  dirfd_open_opath(cgroup);
+	if (cgroup_fd < 0)
+		return -1;
+
+	pid = clone_into_cgroup(cgroup_fd);
+	close_prot_errno(cgroup_fd);
+	if (pid == 0)
+		exit(fn());
+
+	return pid;
+}
+
+int cg_run_nowait(const char *cgroup,
+		  int (*fn)(void))
+{
+	int pid;
+
+	pid = clone_into_cgroup_run_nowait(cgroup, fn);
+	if (pid > 0)
+		return pid;
+
+	/* Genuine test failure. */
+	if (pid < 0 && errno != ENOSYS)
+		return -1;
+
+	pid = fork();
+	if (pid == 0) {
+		char buf[64];
+
+		snprintf(buf, sizeof(buf), "%d", getpid());
+		if (cg_write(cgroup, "cgroup.procs", buf))
+			exit(EXIT_FAILURE);
+		exit(fn());
+	}
+
+	return pid;
+}
+
+int get_temp_fd(void)
+{
+	return open(".", O_TMPFILE | O_RDWR | O_EXCL, 0600);
+}
+
+int alloc_anon(void *arg)
+{
+	size_t size = (unsigned long)arg;
+	char *buf, *ptr;
+
+	buf = malloc(size);
+	for (ptr = buf; ptr < buf + size; ptr += PAGE_SIZE)
+		*ptr = 0;
+
+	free(buf);
+	return 0;
+}
+
+int is_swap_enabled(void)
+{
+	char buf[PAGE_SIZE];
+	static const char delim[] = "\n";
+	int cnt = 0;
+	char *line;
+
+	if (read_text("/proc/swaps", buf, sizeof(buf)) <= 0)
+		return -1;
+
+	for (line = strtok(buf, delim); line; line = strtok(NULL, delim))
+		cnt++;
+
+	return cnt > 1;
+}
+
+int set_oom_adj_score(int pid, int score)
+{
+	char path[PATH_MAX];
+	int fd, len;
+
+	sprintf(path, "/proc/%d/oom_score_adj", pid);
+
+	fd = open(path, O_WRONLY | O_APPEND);
+	if (fd < 0)
+		return fd;
+
+	len = dprintf(fd, "%d", score);
+	if (len < 0) {
+		close(fd);
+		return len;
+	}
+
+	close(fd);
+	return 0;
+}
+
+int proc_mount_contains(const char *option)
+{
+	char buf[4 * PAGE_SIZE];
+	ssize_t read;
+
+	read = read_text("/proc/mounts", buf, sizeof(buf));
+	if (read < 0)
+		return read;
+
+	return strstr(buf, option) != NULL;
+}
+
+ssize_t proc_read_text(int pid, bool thread, const char *item, char *buf, size_t size)
+{
+	char path[PATH_MAX];
+	ssize_t ret;
+
+	if (!pid)
+		snprintf(path, sizeof(path), "/proc/%s/%s",
+			 thread ? "thread-self" : "self", item);
+	else
+		snprintf(path, sizeof(path), "/proc/%d/%s", pid, item);
+
+	ret = read_text(path, buf, size);
+	return ret < 0 ? -1 : ret;
+}
+
+int proc_read_strstr(int pid, bool thread, const char *item, const char *needle)
+{
+	char buf[PAGE_SIZE];
+
+	if (proc_read_text(pid, thread, item, buf, sizeof(buf)) < 0)
+		return -1;
+
+	return strstr(buf, needle) ? 0 : -1;
+}
+
+int clone_into_cgroup_run_wait(const char *cgroup)
+{
+	int cgroup_fd;
+	pid_t pid;
+
+	cgroup_fd =  dirfd_open_opath(cgroup);
+	if (cgroup_fd < 0)
+		return -1;
+
+	pid = clone_into_cgroup(cgroup_fd);
+	close_prot_errno(cgroup_fd);
+	if (pid < 0)
+		return -1;
+
+	if (pid == 0)
+		exit(EXIT_SUCCESS);
+
+	/*
+	 * We don't care whether this fails. We only care whether the initial
+	 * clone succeeded.
+	 */
+	(void)clone_reap(pid, WEXITED);
+	return 0;
+}
+
+static int __prepare_for_wait(const char *cgroup, const char *filename)
+{
+	int fd, ret = -1;
+
+	fd = inotify_init1(0);
+	if (fd == -1)
+		return fd;
+
+	ret = inotify_add_watch(fd, cg_control(cgroup, filename), IN_MODIFY);
+	if (ret == -1) {
+		close(fd);
+		fd = -1;
+	}
+
+	return fd;
+}
+
+int cg_prepare_for_wait(const char *cgroup)
+{
+	return __prepare_for_wait(cgroup, "cgroup.events");
+}
+
+int memcg_prepare_for_wait(const char *cgroup)
+{
+	return __prepare_for_wait(cgroup, "memory.events");
+}
+
+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/testcases/kernel/controllers/kill/cgroup_util.h b/testcases/kernel/controllers/kill/cgroup_util.h
new file mode 100644
index 000000000..bc068eead
--- /dev/null
+++ b/testcases/kernel/controllers/kill/cgroup_util.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <stdbool.h>
+#include <stdlib.h>
+
+#define PAGE_SIZE 4096
+#define MB(x) (x << 20)
+#define USEC_PER_SEC	1000000L
+#define NSEC_PER_SEC	1000000000L
+
+/*
+ * Checks if two given values differ by less than err% of their sum.
+ */
+static inline int values_close(long a, long b, int err)
+{
+	return abs(a - b) <= (a + b) / 100 * err;
+}
+
+extern int cg_find_unified_root(char *root, size_t len);
+extern char *cg_name(const char *root, const char *name);
+extern char *cg_name_indexed(const char *root, const char *name, int index);
+extern char *cg_control(const char *cgroup, const char *control);
+extern int cg_create(const char *cgroup);
+extern int cg_destroy(const char *cgroup);
+extern int cg_read(const char *cgroup, const char *control,
+		   char *buf, size_t len);
+extern int cg_read_strcmp(const char *cgroup, const char *control,
+			  const char *expected);
+extern int cg_read_strstr(const char *cgroup, const char *control,
+			  const char *needle);
+extern long cg_read_long(const char *cgroup, const char *control);
+long cg_read_key_long(const char *cgroup, const char *control, const char *key);
+extern long cg_read_lc(const char *cgroup, const char *control);
+extern int cg_write(const char *cgroup, const char *control, char *buf);
+int cg_write_numeric(const char *cgroup, const char *control, long value);
+extern int cg_run(const char *cgroup,
+		  int (*fn)(const char *cgroup, void *arg),
+		  void *arg);
+extern int cg_enter(const char *cgroup, int pid);
+extern int cg_enter_current(const char *cgroup);
+extern int cg_enter_current_thread(const char *cgroup);
+extern int cg_run_nowait(const char *cgroup,
+			 int (*fn)(void));
+extern int get_temp_fd(void);
+extern int alloc_pagecache(int fd, size_t size);
+extern int alloc_anon(void *arg);
+extern int is_swap_enabled(void);
+extern int set_oom_adj_score(int pid, int score);
+extern int cg_wait_for_proc_count(const char *cgroup, int count);
+extern int cg_killall(const char *cgroup);
+int proc_mount_contains(const char *option);
+extern ssize_t proc_read_text(int pid, bool thread, const char *item, char *buf, size_t size);
+extern int proc_read_strstr(int pid, bool thread, const char *item, const char *needle);
+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 memcg_prepare_for_wait(const char *cgroup);
+extern int cg_wait_for(int fd);
diff --git a/testcases/kernel/controllers/kill/kill01.c b/testcases/kernel/controllers/kill/kill01.c
new file mode 100644
index 000000000..e589b6c16
--- /dev/null
+++ b/testcases/kernel/controllers/kill/kill01.c
@@ -0,0 +1,336 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2012 Christian Brauner <brauner-AT-kernel.org>
+ */
+
+/*\
+ * [Description]
+ *
+ * Basic cgroup kill test.
+ *
+ */
+
+#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 <sys/wait.h>
+
+#include "lapi/syscalls.h"
+#include "cgroup_util.h"
+#include "tst_test.h"
+
+/* define kselftest exit codes */
+#define KSFT_PASS  0
+#define KSFT_FAIL  1
+#define KSFT_XFAIL 2
+#define KSFT_XPASS 3
+#define KSFT_SKIP  4
+
+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 (!WIFEXITED(status))
+		return -1;
+
+	return WEXITSTATUS(status);
+}
+
+
+/*
+ * 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(void)
+{
+	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);
+
+	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);
+	pids[1] = cg_run_nowait(cgroup[7], child_fn);
+	pids[2] = cg_run_nowait(cgroup[9], child_fn);
+	pids[3] = cg_run_nowait(cgroup[9], child_fn);
+	pids[4] = cg_run_nowait(cgroup[9], child_fn);
+
+	/*
+	 * 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(void)
+{
+	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);
+	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 }
+static struct tcase {
+	int (*fn)(const char *root);
+	const char *tname;
+} tcases[] = {
+	T(test_cgkill_simple),
+	T(test_cgkill_tree),
+	T(test_cgkill_forkbomb),
+};
+#undef T
+
+static void run_test(unsigned int n)
+{
+	char root[PATH_MAX];
+	struct tcase *tc = &tcases[n];
+
+	if (cg_find_unified_root(root, sizeof(root)))
+		tst_brk(TBROK, "cgroup v2 isn't mounted");
+	switch (tc->fn(root)) {
+	case KSFT_PASS:
+		tst_res(TPASS, "Successfully tested %s", tc->tname);
+		break;
+	case KSFT_SKIP:
+		tst_res(TCONF, "Skip %s since not support", tc->tname);
+		break;
+	default:
+		tst_res(TFAIL, "%s failed", tc->tname);
+		break;
+	}
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run_test,
+	.forks_child = 1,
+};
-- 
2.35.3


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

Re: [LTP] [PATCH v1] kill01: New case cgroup kill

[Cyril Hrubis]

Hi!
We already have a library for cgroup manipulations see
include/tst_cgroup.h in the LTP git tree.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v1] kill01: New case cgroup kill

[Wei Gao via ltp]

On Fri, Feb 24, 2023 at 11:12:16AM +0100, Cyril Hrubis wrote:
> Hi!
> We already have a library for cgroup manipulations see
> include/tst_cgroup.h in the LTP git tree.
Thanks a lot, will check it.
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

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

[LTP] [PATCH v2] kill01: New case cgroup kill

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c                             |   1 +
 runtest/controllers                          |   1 +
 testcases/kernel/controllers/kill/.gitignore |   1 +
 testcases/kernel/controllers/kill/Makefile   |   6 +
 testcases/kernel/controllers/kill/kill01.c   | 130 +++++++++++++++++++
 5 files changed, 139 insertions(+)
 create mode 100644 testcases/kernel/controllers/kill/.gitignore
 create mode 100644 testcases/kernel/controllers/kill/Makefile
 create mode 100644 testcases/kernel/controllers/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/kill/.gitignore b/testcases/kernel/controllers/kill/.gitignore
new file mode 100644
index 000000000..4f9649e27
--- /dev/null
+++ b/testcases/kernel/controllers/kill/.gitignore
@@ -0,0 +1 @@
+/kill01
diff --git a/testcases/kernel/controllers/kill/Makefile b/testcases/kernel/controllers/kill/Makefile
new file mode 100644
index 000000000..5ea7d67db
--- /dev/null
+++ b/testcases/kernel/controllers/kill/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/controllers/kill/kill01.c b/testcases/kernel/controllers/kill/kill01.c
new file mode 100644
index 000000000..aafc7ba5f
--- /dev/null
+++ b/testcases/kernel/controllers/kill/kill01.c
@@ -0,0 +1,130 @@
+// 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]
+ *
+ * Basic cgroup kill test.
+ *
+ */
+
+#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 <sys/wait.h>
+
+#include "lapi/syscalls.h"
+#include "tst_test.h"
+
+#define pid_num 100
+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 (!WIFEXITED(status))
+		return -1;
+
+	return WEXITSTATUS(status);
+}
+
+/*
+ * A simple process running in a sleep loop until being
+ * re-parented.
+ */
+static int child_fn(void)
+{
+	int ppid = getppid();
+
+	while (getppid() == ppid)
+		usleep(1000);
+
+	return getppid() == ppid;
+}
+
+static int cg_run_nowait(const struct tst_cg_group *const cg,
+		  int (*fn)(void))
+{
+	int pid;
+
+	pid = fork();
+	if (pid == 0) {
+		SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
+		exit(fn());
+	}
+
+	return pid;
+}
+
+static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int count)
+{
+	char buf[20 * pid_num] = {0};
+	int attempts;
+	char *ptr;
+
+	for (attempts = 10; attempts >= 0; attempts--) {
+		int nr = 0;
+
+		SAFE_CG_READ(cg, "cgroup.procs", buf, sizeof(buf));
+
+		for (ptr = buf; *ptr; ptr++)
+			if (*ptr == '\n')
+				nr++;
+
+		if (nr >= count)
+			return 0;
+
+		usleep(100000);
+	}
+
+	return -1;
+}
+
+static void run(void)
+{
+
+	pid_t pids[100];
+	int i;
+
+	cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
+
+	for (i = 0; i < pid_num; i++)
+		pids[i] = cg_run_nowait(cg_child_test_simple, child_fn);
+
+	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++) {
+		/* wait_for_pid(pids[i]); */
+		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]) == SIGKILL);
+	}
+
+	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.forks_child = 1,
+	.max_runtime = 5,
+	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+};
-- 
2.35.3


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

Re: [LTP] [PATCH v2] kill01: New case cgroup kill

[Li Wang]

On Sun, Mar 5, 2023 at 5:14 PM 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 +
>  testcases/kernel/controllers/kill/.gitignore |   1 +
>  testcases/kernel/controllers/kill/Makefile   |   6 +
>  testcases/kernel/controllers/kill/kill01.c   | 130 +++++++++++++++++++
>  5 files changed, 139 insertions(+)
>  create mode 100644 testcases/kernel/controllers/kill/.gitignore
>  create mode 100644 testcases/kernel/controllers/kill/Makefile
>  create mode 100644 testcases/kernel/controllers/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/kill/.gitignore
> b/testcases/kernel/controllers/kill/.gitignore
> new file mode 100644
> index 000000000..4f9649e27
> --- /dev/null
> +++ b/testcases/kernel/controllers/kill/.gitignore
> @@ -0,0 +1 @@
> +/kill01
> diff --git a/testcases/kernel/controllers/kill/Makefile
> b/testcases/kernel/controllers/kill/Makefile
> new file mode 100644
> index 000000000..5ea7d67db
> --- /dev/null
> +++ b/testcases/kernel/controllers/kill/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +top_srcdir             ?= ../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/controllers/kill/kill01.c
> b/testcases/kernel/controllers/kill/kill01.c
> new file mode 100644
> index 000000000..aafc7ba5f
> --- /dev/null
> +++ b/testcases/kernel/controllers/kill/kill01.c
> @@ -0,0 +1,130 @@
> +// 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]
> + *
> + * Basic cgroup kill test.
> + *
> + */
> +
> +#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 <sys/wait.h>
> +
> +#include "lapi/syscalls.h"
> +#include "tst_test.h"
> +
> +#define pid_num 100
>

My concern about defining pid_num as a fixed variable is that
the test may spend a long time on a single_cpu or slower system.
A sanity way is probably to use a dynamical number according
to the test bed available CPUs (e.g. tst_ncpus_available() + 1).



> +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 (!WIFEXITED(status))
> +               return -1;
> +
> +       return WEXITSTATUS(status);
> +}
> +
> +/*
> + * A simple process running in a sleep loop until being
> + * re-parented.
> + */
> +static int child_fn(void)
> +{
> +       int ppid = getppid();
> +
> +       while (getppid() == ppid)
> +               usleep(1000);
> +
> +       return getppid() == ppid;
>

why do we need to return the value of this comparison?
I suppose most time the child does _not_ have a chance
to get here.



> +}
> +
> +static int cg_run_nowait(const struct tst_cg_group *const cg,
> +                 int (*fn)(void))
> +{
> +       int pid;
> +
> +       pid = fork();
>

use SAFE_FORK() maybe better.



> +       if (pid == 0) {
> +               SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
> +               exit(fn());
> +       }
> +
> +       return pid;
> +}
> +
> +static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int
> count)
> +{
> +       char buf[20 * pid_num] = {0};
> +       int attempts;
> +       char *ptr;
> +
> +       for (attempts = 10; attempts >= 0; attempts--) {
> +               int nr = 0;
> +
> +               SAFE_CG_READ(cg, "cgroup.procs", buf, sizeof(buf));
> +
> +               for (ptr = buf; *ptr; ptr++)
> +                       if (*ptr == '\n')
> +                               nr++;
> +
> +               if (nr >= count)
> +                       return 0;
> +
> +               usleep(100000);
>

In this loop, there is only 1 second for waiting for children ready.
So, if test on a slower/overload machine that is a bit longer than this
time,
what will happen? shouldn't we handle this as a corner case failure?



> +       }
> +
> +       return -1;
> +}
> +
> +static void run(void)
> +{
> +
> +       pid_t pids[100];
> +       int i;
> +
> +       cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
> +
> +       for (i = 0; i < pid_num; i++)
> +               pids[i] = cg_run_nowait(cg_child_test_simple, child_fn);
> +
> +       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++) {
> +               /* wait_for_pid(pids[i]); */
> +               TST_EXP_PASS_SILENT(wait_for_pid(pids[i]) == SIGKILL);
> +       }
> +
> +       cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
> +}
> +
> +static struct tst_test test = {
> +       .test_all = run,
> +       .forks_child = 1,
> +       .max_runtime = 5,
> +       .needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
> +       .needs_cgroup_ver = TST_CG_V2,
> +};
> --
> 2.35.3
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v2] kill01: New case cgroup kill

[Wei Gao via ltp]

On Mon, Mar 06, 2023 at 06:16:26PM +0800, Li Wang wrote:
> On Sun, Mar 5, 2023 at 5:14 PM Wei Gao via ltp <ltp@lists.linux.it> wrote:
> 
> > Signed-off-by: Wei Gao <wegao@suse.com>
> > ---
> > +#define pid_num 100
> >
> 
> My concern about defining pid_num as a fixed variable is that
> the test may spend a long time on a single_cpu or slower system.
> A sanity way is probably to use a dynamical number according
> to the test bed available CPUs (e.g. tst_ncpus_available() + 1).
good idea!
> 
> 
> 
> > +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 (!WIFEXITED(status))
> > +               return -1;
> > +
> > +       return WEXITSTATUS(status);
> > +}
> > +
> > +/*
> > + * A simple process running in a sleep loop until being
> > + * re-parented.
> > + */
> > +static int child_fn(void)
> > +{
> > +       int ppid = getppid();
> > +
> > +       while (getppid() == ppid)
> > +               usleep(1000);
> > +
> > +       return getppid() == ppid;
> >
> 
> why do we need to return the value of this comparison?
> I suppose most time the child does _not_ have a chance
> to get here.
yes, chance to reach here is small in our scenario, remove
the logic here.
> 
> 
> 
> > +}
> > +
> > +static int cg_run_nowait(const struct tst_cg_group *const cg,
> > +                 int (*fn)(void))
> > +{
> > +       int pid;
> > +
> > +       pid = fork();
> >
> 
> use SAFE_FORK() maybe better.
good catch!
> 
> 
> 
> > +       if (pid == 0) {
> > +               SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
> > +               exit(fn());
> > +       }
> > +
> > +       return pid;
> > +}
> > +
> > +static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int
> > count)
> > +{
> > +       char buf[20 * pid_num] = {0};
> > +       int attempts;
> > +       char *ptr;
> > +
> > +       for (attempts = 10; attempts >= 0; attempts--) {
> > +               int nr = 0;
> > +
> > +               SAFE_CG_READ(cg, "cgroup.procs", buf, sizeof(buf));
> > +
> > +               for (ptr = buf; *ptr; ptr++)
> > +                       if (*ptr == '\n')
> > +                               nr++;
> > +
> > +               if (nr >= count)
> > +                       return 0;
> > +
> > +               usleep(100000);
> >
> 
> In this loop, there is only 1 second for waiting for children ready.
> So, if test on a slower/overload machine that is a bit longer than this
> time,
> what will happen? shouldn't we handle this as a corner case failure?
I will increase to 10 second, then if all the children processes can not ready in correct 
cgroup we will take this as a failure case.
> 
> 
> 
> > +       }
> > +
> > +       return -1;
> > +}
> > +
> > +static void run(void)
> > +{
> > +
> > +       pid_t pids[100];
> > +       int i;
> > +
> > +       cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
> > +
> > +       for (i = 0; i < pid_num; i++)
> > +               pids[i] = cg_run_nowait(cg_child_test_simple, child_fn);
> > +
> > +       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++) {
> > +               /* wait_for_pid(pids[i]); */
> > +               TST_EXP_PASS_SILENT(wait_for_pid(pids[i]) == SIGKILL);
> > +       }
> > +
> > +       cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
> > +}
> > +
> > +static struct tst_test test = {
> > +       .test_all = run,
> > +       .forks_child = 1,
> > +       .max_runtime = 5,
> > +       .needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
> > +       .needs_cgroup_ver = TST_CG_V2,
> > +};
> > --
> > 2.35.3
> >
> >
> > --
> > Mailing list info: https://lists.linux.it/listinfo/ltp
> >
> >
> 
> -- 
> Regards,
> Li Wang

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

[LTP] [PATCH v3] kill01: New case cgroup kill

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
Reviewed-by: Li Wang <liwang@redhat.com>
---
 lib/tst_cgroup.c                             |   1 +
 runtest/controllers                          |   1 +
 testcases/kernel/controllers/kill/.gitignore |   1 +
 testcases/kernel/controllers/kill/Makefile   |   6 +
 testcases/kernel/controllers/kill/kill01.c   | 143 +++++++++++++++++++
 5 files changed, 152 insertions(+)
 create mode 100644 testcases/kernel/controllers/kill/.gitignore
 create mode 100644 testcases/kernel/controllers/kill/Makefile
 create mode 100644 testcases/kernel/controllers/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/kill/.gitignore b/testcases/kernel/controllers/kill/.gitignore
new file mode 100644
index 000000000..4f9649e27
--- /dev/null
+++ b/testcases/kernel/controllers/kill/.gitignore
@@ -0,0 +1 @@
+/kill01
diff --git a/testcases/kernel/controllers/kill/Makefile b/testcases/kernel/controllers/kill/Makefile
new file mode 100644
index 000000000..5ea7d67db
--- /dev/null
+++ b/testcases/kernel/controllers/kill/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/controllers/kill/kill01.c b/testcases/kernel/controllers/kill/kill01.c
new file mode 100644
index 000000000..4024049e0
--- /dev/null
+++ b/testcases/kernel/controllers/kill/kill01.c
@@ -0,0 +1,143 @@
+// 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]
+ *
+ * Basic cgroup kill test.
+ *
+ */
+
+#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 <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 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 (!WIFEXITED(status))
+		return -1;
+
+	return WEXITSTATUS(status);
+}
+
+/*
+ * 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());
+		fn();
+	}
+
+	return pid;
+}
+
+static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int count)
+{
+	int attempts;
+	char *ptr;
+
+	for (attempts = 100; attempts >= 0; attempts--) {
+		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;
+
+		usleep(100000);
+	}
+
+	return -1;
+}
+
+static void run(void)
+{
+	pid_t pids[MAX_PID_NUM];
+	int i;
+
+	cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
+
+	for (i = 0; i < pid_num; i++)
+		pids[i] = cg_run_nowait(cg_child_test_simple, child_fn);
+
+	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++) {
+		/* wait_for_pid(pids[i]); */
+		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]) == SIGKILL);
+	}
+
+	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+}
+
+static void setup(void)
+{
+	buf = SAFE_MALLOC(buf_len);
+	memset(buf, 0, buf_len);
+}
+
+static void cleanup(void)
+{
+	free(buf);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+	.max_runtime = 15,
+	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+};
-- 
2.35.3


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

[LTP] [PATCH v4] kill01: New case cgroup kill

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c                             |   1 +
 runtest/controllers                          |   1 +
 testcases/kernel/controllers/kill/.gitignore |   1 +
 testcases/kernel/controllers/kill/Makefile   |   6 +
 testcases/kernel/controllers/kill/kill01.c   | 143 +++++++++++++++++++
 5 files changed, 152 insertions(+)
 create mode 100644 testcases/kernel/controllers/kill/.gitignore
 create mode 100644 testcases/kernel/controllers/kill/Makefile
 create mode 100644 testcases/kernel/controllers/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/kill/.gitignore b/testcases/kernel/controllers/kill/.gitignore
new file mode 100644
index 000000000..4f9649e27
--- /dev/null
+++ b/testcases/kernel/controllers/kill/.gitignore
@@ -0,0 +1 @@
+/kill01
diff --git a/testcases/kernel/controllers/kill/Makefile b/testcases/kernel/controllers/kill/Makefile
new file mode 100644
index 000000000..5ea7d67db
--- /dev/null
+++ b/testcases/kernel/controllers/kill/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/controllers/kill/kill01.c b/testcases/kernel/controllers/kill/kill01.c
new file mode 100644
index 000000000..4024049e0
--- /dev/null
+++ b/testcases/kernel/controllers/kill/kill01.c
@@ -0,0 +1,143 @@
+// 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]
+ *
+ * Basic cgroup kill test.
+ *
+ */
+
+#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 <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 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 (!WIFEXITED(status))
+		return -1;
+
+	return WEXITSTATUS(status);
+}
+
+/*
+ * 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());
+		fn();
+	}
+
+	return pid;
+}
+
+static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int count)
+{
+	int attempts;
+	char *ptr;
+
+	for (attempts = 100; attempts >= 0; attempts--) {
+		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;
+
+		usleep(100000);
+	}
+
+	return -1;
+}
+
+static void run(void)
+{
+	pid_t pids[MAX_PID_NUM];
+	int i;
+
+	cg_child_test_simple = tst_cg_group_mk(tst_cg, "cg_test_simple");
+
+	for (i = 0; i < pid_num; i++)
+		pids[i] = cg_run_nowait(cg_child_test_simple, child_fn);
+
+	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++) {
+		/* wait_for_pid(pids[i]); */
+		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]) == SIGKILL);
+	}
+
+	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+}
+
+static void setup(void)
+{
+	buf = SAFE_MALLOC(buf_len);
+	memset(buf, 0, buf_len);
+}
+
+static void cleanup(void)
+{
+	free(buf);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+	.max_runtime = 15,
+	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+};
-- 
2.35.3


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

Re: [LTP] [PATCH v4] kill01: New case cgroup kill

[Li Wang]

Hi Wei,

The new test generally looks good to me.

I'm thinking maybe we make use of guarded buffers which could
remove the setup/cleanup function then. And doing memset(buf,...)
before reading into buf[] is quite necessary, to avoid someone
running this test with parameter '-i N' that the buf[] is polluted
by the first time.

https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L1799

<https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L1799>
e.g.

--- a/testcases/kernel/controllers/kill/kill01.c
+++ b/testcases/kernel/controllers/kill/kill01.c
@@ -26,7 +26,7 @@

 #define MAX_PID_NUM 100
 #define pid_num MIN(MAX_PID_NUM, (tst_ncpus_available() + 1))
-#define buf_len (20 * pid_num)
+#define buf_len (10 * MAX_PID_NUM)

 static char *buf;
 static struct tst_cg_group *cg_child_test_simple;
@@ -60,7 +60,6 @@ static void child_fn(void)

        while (getppid() == ppid)
                usleep(1000);
-
 }

 static int cg_run_nowait(const struct tst_cg_group *const cg,
@@ -82,6 +81,8 @@ static int cg_wait_for_proc_count(const struct
tst_cg_group *cg, int count)
        int attempts;
        char *ptr;

+       memset(buf, 0, buf_len);
+
        for (attempts = 100; attempts >= 0; attempts--) {
                int nr = 0;

@@ -121,23 +122,14 @@ static void run(void)
        cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
 }

-static void setup(void)
-{
-       buf = SAFE_MALLOC(buf_len);
-       memset(buf, 0, buf_len);
-}
-
-static void cleanup(void)
-{
-       free(buf);
-}
-
 static struct tst_test test = {
        .test_all = run,
-       .setup = setup,
-       .cleanup = cleanup,
        .forks_child = 1,
        .max_runtime = 15,
        .needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
        .needs_cgroup_ver = TST_CG_V2,
+       .bufs = (struct tst_buffers []) {
+               {&buf, .size = buf_len},
+               {}
+       }
 };

Anyway this version you could add my reviewed-by:

Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v4] kill01: New case cgroup kill

[Wei Gao via ltp]

On Tue, Mar 07, 2023 at 03:13:56PM +0800, Li Wang wrote:
> Hi Wei,
> 
> The new test generally looks good to me.
> 
> I'm thinking maybe we make use of guarded buffers which could
> remove the setup/cleanup function then. And doing memset(buf,...)
> before reading into buf[] is quite necessary, to avoid someone
> running this test with parameter '-i N' that the buf[] is polluted
> by the first time.
> 
> https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L1799
> 
> <https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L1799>
> e.g.

I have tried use guarded buffers but compile can not pass. 
Following error will trigger since buf_len is caculate instead of FIX constant. So 
I suppose we can not use guarded buffers?

kill01.c:131:34: error: initializer element is not constant
  .bufs = (struct tst_buffers []) {

For memset before reading buf issue, i will update in next patch.

> 
> --- a/testcases/kernel/controllers/kill/kill01.c
> +++ b/testcases/kernel/controllers/kill/kill01.c
> @@ -26,7 +26,7 @@
> 
>  #define MAX_PID_NUM 100
>  #define pid_num MIN(MAX_PID_NUM, (tst_ncpus_available() + 1))
> -#define buf_len (20 * pid_num)
> +#define buf_len (10 * MAX_PID_NUM)
> 
>  static char *buf;
>  static struct tst_cg_group *cg_child_test_simple;
> @@ -60,7 +60,6 @@ static void child_fn(void)
> 
>         while (getppid() == ppid)
>                 usleep(1000);
> -
>  }
> 
>  static int cg_run_nowait(const struct tst_cg_group *const cg,
> @@ -82,6 +81,8 @@ static int cg_wait_for_proc_count(const struct
> tst_cg_group *cg, int count)
>         int attempts;
>         char *ptr;
> 
> +       memset(buf, 0, buf_len);
> +
>         for (attempts = 100; attempts >= 0; attempts--) {
>                 int nr = 0;
> 
> @@ -121,23 +122,14 @@ static void run(void)
>         cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
>  }
> 
> -static void setup(void)
> -{
> -       buf = SAFE_MALLOC(buf_len);
> -       memset(buf, 0, buf_len);
> -}
> -
> -static void cleanup(void)
> -{
> -       free(buf);
> -}
> -
>  static struct tst_test test = {
>         .test_all = run,
> -       .setup = setup,
> -       .cleanup = cleanup,
>         .forks_child = 1,
>         .max_runtime = 15,
>         .needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
>         .needs_cgroup_ver = TST_CG_V2,
> +       .bufs = (struct tst_buffers []) {
> +               {&buf, .size = buf_len},
> +               {}
> +       }
>  };
> 
> Anyway this version you could add my reviewed-by:
> 
> Reviewed-by: Li Wang <liwang@redhat.com>
> 
> 
> -- 
> Regards,
> Li Wang

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

[LTP] [PATCH v5] kill01: New case cgroup kill

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c                             |   1 +
 runtest/controllers                          |   1 +
 testcases/kernel/controllers/kill/.gitignore |   1 +
 testcases/kernel/controllers/kill/Makefile   |   6 +
 testcases/kernel/controllers/kill/kill01.c   | 138 +++++++++++++++++++
 5 files changed, 147 insertions(+)
 create mode 100644 testcases/kernel/controllers/kill/.gitignore
 create mode 100644 testcases/kernel/controllers/kill/Makefile
 create mode 100644 testcases/kernel/controllers/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/kill/.gitignore b/testcases/kernel/controllers/kill/.gitignore
new file mode 100644
index 000000000..4f9649e27
--- /dev/null
+++ b/testcases/kernel/controllers/kill/.gitignore
@@ -0,0 +1 @@
+/kill01
diff --git a/testcases/kernel/controllers/kill/Makefile b/testcases/kernel/controllers/kill/Makefile
new file mode 100644
index 000000000..5ea7d67db
--- /dev/null
+++ b/testcases/kernel/controllers/kill/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/controllers/kill/kill01.c b/testcases/kernel/controllers/kill/kill01.c
new file mode 100644
index 000000000..432747e16
--- /dev/null
+++ b/testcases/kernel/controllers/kill/kill01.c
@@ -0,0 +1,138 @@
+// 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]
+ *
+ * Basic cgroup kill test.
+ *
+ */
+
+#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 <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 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 (!WIFEXITED(status))
+		return -1;
+
+	return WEXITSTATUS(status);
+}
+
+/*
+ * 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());
+		fn();
+	}
+
+	return pid;
+}
+
+static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int count)
+{
+	int attempts;
+	char *ptr;
+
+	for (attempts = 100; attempts >= 0; attempts--) {
+		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;
+
+		usleep(100000);
+	}
+
+	return -1;
+}
+
+static void run(void)
+{
+	pid_t pids[MAX_PID_NUM];
+	int i;
+
+	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_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++) {
+		/* wait_for_pid(pids[i]); */
+		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]) == SIGKILL);
+	}
+
+	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
+}
+
+static void setup(void)
+{
+	buf = tst_alloc(buf_len);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.forks_child = 1,
+	.max_runtime = 15,
+	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+};
-- 
2.35.3


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

Re: [LTP] [PATCH v4] kill01: New case cgroup kill

[Li Wang]

On Tue, Mar 7, 2023 at 4:28 PM Wei Gao <wegao@suse.com> wrote:

> On Tue, Mar 07, 2023 at 03:13:56PM +0800, Li Wang wrote:
> > Hi Wei,
> >
> > The new test generally looks good to me.
> >
> > I'm thinking maybe we make use of guarded buffers which could
> > remove the setup/cleanup function then. And doing memset(buf,...)
> > before reading into buf[] is quite necessary, to avoid someone
> > running this test with parameter '-i N' that the buf[] is polluted
> > by the first time.
> >
> >
> https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L1799
> >
> > <
> https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L1799
> >
> > e.g.
>
> I have tried use guarded buffers but compile can not pass.
> Following error will trigger since buf_len is caculate instead of FIX
> constant. So
> I suppose we can not use guarded buffers?
>

Sure, it doesn't matter.
You used an uncertain variable in .size of tst_buffer, that's why it throws
out the compiling error.



>
> kill01.c:131:34: error: initializer element is not constant
>   .bufs = (struct tst_buffers []) {
>
> For memset before reading buf issue, i will update in next patch.
>
> >
> > --- a/testcases/kernel/controllers/kill/kill01.c
> > +++ b/testcases/kernel/controllers/kill/kill01.c
> > @@ -26,7 +26,7 @@
> >
> >  #define MAX_PID_NUM 100
> >  #define pid_num MIN(MAX_PID_NUM, (tst_ncpus_available() + 1))
> > -#define buf_len (20 * pid_num)
> > +#define buf_len (10 * MAX_PID_NUM)
> >
> >  static char *buf;
> >  static struct tst_cg_group *cg_child_test_simple;
> > @@ -60,7 +60,6 @@ static void child_fn(void)
> >
> >         while (getppid() == ppid)
> >                 usleep(1000);
> > -
> >  }
> >
> >  static int cg_run_nowait(const struct tst_cg_group *const cg,
> > @@ -82,6 +81,8 @@ static int cg_wait_for_proc_count(const struct
> > tst_cg_group *cg, int count)
> >         int attempts;
> >         char *ptr;
> >
> > +       memset(buf, 0, buf_len);
> > +
> >         for (attempts = 100; attempts >= 0; attempts--) {
> >                 int nr = 0;
> >
> > @@ -121,23 +122,14 @@ static void run(void)
> >         cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
> >  }
> >
> > -static void setup(void)
> > -{
> > -       buf = SAFE_MALLOC(buf_len);
> > -       memset(buf, 0, buf_len);
> > -}
> > -
> > -static void cleanup(void)
> > -{
> > -       free(buf);
> > -}
> > -
> >  static struct tst_test test = {
> >         .test_all = run,
> > -       .setup = setup,
> > -       .cleanup = cleanup,
> >         .forks_child = 1,
> >         .max_runtime = 15,
> >         .needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
> >         .needs_cgroup_ver = TST_CG_V2,
> > +       .bufs = (struct tst_buffers []) {
> > +               {&buf, .size = buf_len},
> > +               {}
> > +       }
> >  };
> >
> > Anyway this version you could add my reviewed-by:
> >
> > Reviewed-by: Li Wang <liwang@redhat.com>
> >
> >
> > --
> > Regards,
> > Li Wang
>
>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v5] kill01: New case cgroup kill

[Li Wang]

On Tue, Mar 7, 2023 at 4:52 PM Wei Gao <wegao@suse.com> wrote:

> Signed-off-by: Wei Gao <wegao@suse.com>
>

Reviewed-by: Li Wang <liwang@redhat.com>



> ---
>  lib/tst_cgroup.c                             |   1 +
>  runtest/controllers                          |   1 +
>  testcases/kernel/controllers/kill/.gitignore |   1 +
>  testcases/kernel/controllers/kill/Makefile   |   6 +
>  testcases/kernel/controllers/kill/kill01.c   | 138 +++++++++++++++++++
>  5 files changed, 147 insertions(+)
>  create mode 100644 testcases/kernel/controllers/kill/.gitignore
>  create mode 100644 testcases/kernel/controllers/kill/Makefile
>  create mode 100644 testcases/kernel/controllers/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/kill/.gitignore
> b/testcases/kernel/controllers/kill/.gitignore
> new file mode 100644
> index 000000000..4f9649e27
> --- /dev/null
> +++ b/testcases/kernel/controllers/kill/.gitignore
> @@ -0,0 +1 @@
> +/kill01
> diff --git a/testcases/kernel/controllers/kill/Makefile
> b/testcases/kernel/controllers/kill/Makefile
> new file mode 100644
> index 000000000..5ea7d67db
> --- /dev/null
> +++ b/testcases/kernel/controllers/kill/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +top_srcdir             ?= ../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/controllers/kill/kill01.c
> b/testcases/kernel/controllers/kill/kill01.c
> new file mode 100644
> index 000000000..432747e16
> --- /dev/null
> +++ b/testcases/kernel/controllers/kill/kill01.c
> @@ -0,0 +1,138 @@
> +// 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]
> + *
> + * Basic cgroup kill test.
> + *
> + */
> +
> +#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 <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 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 (!WIFEXITED(status))
> +               return -1;
> +
> +       return WEXITSTATUS(status);
> +}
> +
> +/*
> + * 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());
> +               fn();
> +       }
> +
> +       return pid;
> +}
> +
> +static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int
> count)
> +{
> +       int attempts;
> +       char *ptr;
> +
> +       for (attempts = 100; attempts >= 0; attempts--) {
> +               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;
> +
> +               usleep(100000);
> +       }
> +
> +       return -1;
> +}
> +
> +static void run(void)
> +{
> +       pid_t pids[MAX_PID_NUM];
> +       int i;
> +
> +       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_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++) {
> +               /* wait_for_pid(pids[i]); */
> +               TST_EXP_PASS_SILENT(wait_for_pid(pids[i]) == SIGKILL);
> +       }
> +
> +       cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
> +}
> +
> +static void setup(void)
> +{
> +       buf = tst_alloc(buf_len);
> +}
> +
> +static struct tst_test test = {
> +       .test_all = run,
> +       .setup = setup,
> +       .forks_child = 1,
> +       .max_runtime = 15,
> +       .needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
> +       .needs_cgroup_ver = TST_CG_V2,
> +};
> --
> 2.35.3
>
>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v5] kill01: New case cgroup kill

[Petr Vorel]

Hi Wei,

Closes: https://github.com/linux-test-project/ltp/issues/846

Test LGTM.

...
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2012 Christian Brauner <brauner-AT-kernel.org>
Is it based on some Christian's reproducer?
I haven't noticed any in https://lwn.net/Articles/855049/ (linked in #846)
nor in the patch
https://lwn.net/ml/cgroups/20210503143922.3093755-1-brauner@kernel.org/.
But maybe I just overlooked it.

> + * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Basic cgroup kill test.
> + *
> + */
> +
> +#include <errno.h>
> +#include <linux/limits.h>
> +#include <stdbool.h>
nit: stdbool.h is not needed (can be removed before merge).

...
> +static void run(void)
> +{
> +	pid_t pids[MAX_PID_NUM];
> +	int i;
> +
> +	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_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++) {
> +		/* wait_for_pid(pids[i]); */
I guess this comment should be removed before merge.
> +		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]) == SIGKILL);
> +	}
> +
> +	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
> +}

Kind regards,
Petr

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

Re: [LTP] [PATCH v5] kill01: New case cgroup kill

[Richard Palethorpe]

Hello,

Wei Gao via ltp <ltp@lists.linux.it> writes:

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  lib/tst_cgroup.c                             |   1 +
>  runtest/controllers                          |   1 +
>  testcases/kernel/controllers/kill/.gitignore |   1 +
>  testcases/kernel/controllers/kill/Makefile   |   6 +
>  testcases/kernel/controllers/kill/kill01.c   | 138 +++++++++++++++++++

nit: kill is not a controller, it is a core function. This should go in
controllers/cgroup.

>  5 files changed, 147 insertions(+)
>  create mode 100644 testcases/kernel/controllers/kill/.gitignore
>  create mode 100644 testcases/kernel/controllers/kill/Makefile
>  create mode 100644 testcases/kernel/controllers/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/kill/.gitignore b/testcases/kernel/controllers/kill/.gitignore
> new file mode 100644
> index 000000000..4f9649e27
> --- /dev/null
> +++ b/testcases/kernel/controllers/kill/.gitignore
> @@ -0,0 +1 @@
> +/kill01
> diff --git a/testcases/kernel/controllers/kill/Makefile b/testcases/kernel/controllers/kill/Makefile
> new file mode 100644
> index 000000000..5ea7d67db
> --- /dev/null
> +++ b/testcases/kernel/controllers/kill/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +top_srcdir		?= ../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/controllers/kill/kill01.c b/testcases/kernel/controllers/kill/kill01.c
> new file mode 100644
> index 000000000..432747e16
> --- /dev/null
> +++ b/testcases/kernel/controllers/kill/kill01.c
> @@ -0,0 +1,138 @@
> +// 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]
> + *
> + * Basic cgroup kill test.
> + *
> + */

I guess this is copied from the self tests and you can mention that in
the description and link to the original.

> +
> +#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 <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 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 (!WIFEXITED(status))
> +		return -1;
> +
> +	return WEXITSTATUS(status);
> +}

We have tst_reap_children for this, but this just appears to be wrong
for this test.

> +
> +/*
> + * 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))

Why keep this function?

If you want to convert tests to LTP, then don't do the minimum possible
to use the LTP API. Use as much of it as possible otherwise we are just
importing brittle self tests.

> +{
> +	int pid;
> +
> +	pid = SAFE_FORK();
> +	if (pid == 0) {
> +		SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
> +		fn();
> +	}
> +
> +	return pid;
> +}
> +
> +static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int count)
> +{
> +	int attempts;
> +	char *ptr;
> +
> +	for (attempts = 100; attempts >= 0; attempts--) {
> +		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;
> +
> +		usleep(100000);

It's best to avoid arbitrary sleep values and attempts. You could use
TST_CHECKPOINT* or increment a counter in some shared memory with
SAFE_MMAP and tst_atomic_inc.

> +	}
> +
> +	return -1;
> +}
> +
> +static void run(void)
> +{
> +	pid_t pids[MAX_PID_NUM];
> +	int i;
> +
> +	cg_child_test_simple = tst_cg_group_mk(tst_cg,
> "cg_test_simple");
> +
> +	memset(buf, 0, buf_len);

IIRC guarded buffers are zeroed already.

> +
> +	for (i = 0; i < pid_num; i++)
> +		pids[i] = cg_run_nowait(cg_child_test_simple, child_fn);

If the parent is killed and the children are moved then they will return
and cause a fork bomb.

This is not obvious because of the unecessary indirection (function
pointer and functions).

> +
> +	TST_EXP_PASS(cg_wait_for_proc_count(cg_child_test_simple,
> pid_num));

If this fails then there will be little information to debug it. This is
a common issue with the self tests which we will be importing into the LTP.

> +	SAFE_CG_PRINTF(cg_child_test_simple, "cgroup.kill", "%d", 1);
> +
> +	for (i = 0; i < pid_num; i++) {
> +		/* wait_for_pid(pids[i]); */
> +		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]) == SIGKILL);

It seems wait_for_pid will never == SIGKILL. The function does not
inspect the signal a process was killed with at all.

The test passes becaues this is not the correct use of TST_EXP_PASS*.

> +	}
> +
> +	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
> +}
> +
> +static void setup(void)
> +{
> +	buf = tst_alloc(buf_len);

Simple allocations like this can be done in the test struct.

> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.forks_child = 1,
> +	.max_runtime = 15,
> +	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },

Why do we need the memory controller?

If it is just to make the LTP library happy, then you can change the
library instead (e.g. add a "cgroup" pseudo controller if we didn't do
that already).

> +	.needs_cgroup_ver = TST_CG_V2,
> +};
> -- 
> 2.35.3

-- 
Thank you,
Richard.

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

Re: [LTP] [PATCH v5] kill01: New case cgroup kill

[Li Wang]

Richard Palethorpe <rpalethorpe@suse.de> wrote:


> > +static void run(void)
> > +{
> > +     pid_t pids[MAX_PID_NUM];
> > +     int i;
> > +
> > +     cg_child_test_simple = tst_cg_group_mk(tst_cg,
> > "cg_test_simple");
> > +
> > +     memset(buf, 0, buf_len);
>
> IIRC guarded buffers are zeroed already.
>

The memset() is necessary since there is probably run test with '-i N'
which lead to buf[] being impacted by last time.



> > +static void setup(void)
> > +{
> > +     buf = tst_alloc(buf_len);
>
> Simple allocations like this can be done in the test struct.
>

I pointed out this before but he seems don't want to use constant
for the buf_len definition (to bypass compile error in tst_test).


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v5] kill01: New case cgroup kill

[Wei Gao via ltp]

On Thu, Mar 09, 2023 at 10:40:46PM +0100, Petr Vorel wrote:
> Hi Wei,
> 
> Closes: https://github.com/linux-test-project/ltp/issues/846
> 
> Test LGTM.
> 
> ...
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2012 Christian Brauner <brauner-AT-kernel.org>
> Is it based on some Christian's reproducer?
> I haven't noticed any in https://lwn.net/Articles/855049/ (linked in #846)
> nor in the patch
> https://lwn.net/ml/cgroups/20210503143922.3093755-1-brauner@kernel.org/.
> But maybe I just overlooked it.
This case base kernel self test, will add more description here.
> 
> > + * Copyright (c) 2023 SUSE LLC <wegao@suse.com>
> > + */
> > +
> > +/*\
> > + * [Description]
> > + *
> > + * Basic cgroup kill test.
> > + *
> > + */
> > +
> > +#include <errno.h>
> > +#include <linux/limits.h>
> > +#include <stdbool.h>
> nit: stdbool.h is not needed (can be removed before merge).
ack
> 
> ...
> > +static void run(void)
> > +{
> > +	pid_t pids[MAX_PID_NUM];
> > +	int i;
> > +
> > +	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_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++) {
> > +		/* wait_for_pid(pids[i]); */
> I guess this comment should be removed before merge.
ack
> > +		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]) == SIGKILL);
> > +	}
> > +
> > +	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
> > +}
> 
> Kind regards,
> Petr

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

Re: [LTP] [PATCH v5] kill01: New case cgroup kill

[Wei Gao via ltp]

On Mon, Mar 13, 2023 at 10:45:12AM +0000, Richard Palethorpe wrote:
> Hello,
> 
> > +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 (!WIFEXITED(status))
> > +		return -1;
> > +
> > +	return WEXITSTATUS(status);
> > +}
> 
> We have tst_reap_children for this, but this just appears to be wrong
> for this test.
tst_reap_children can not return reason of status, such as i need call 
WIFSIGNALED(wstatus) in next patch to make sure children is killed by 
signal.

> > + * 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))
> 
> Why keep this function?
> 
> If you want to convert tests to LTP, then don't do the minimum possible
> to use the LTP API. Use as much of it as possible otherwise we are just
> importing brittle self tests.
> 
function is useful and wrap the fork action & put pid into cgroup.procs,
is there any LTP API can replace this function? Could you help give example.
> > +{
> > +	int pid;
> > +
> > +	pid = SAFE_FORK();
> > +	if (pid == 0) {
> > +		SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
> > +		fn();
> > +	}
> > +
> > +	return pid;
> > +}
> > +
> > +static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int count)
> > +{
> > +	int attempts;
> > +	char *ptr;
> > +
> > +	for (attempts = 100; attempts >= 0; attempts--) {
> > +		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;
> > +
> > +		usleep(100000);
> 
> It's best to avoid arbitrary sleep values and attempts. You could use
> TST_CHECKPOINT* or increment a counter in some shared memory with
> SAFE_MMAP and tst_atomic_inc.
> 
I will try to use TST_CHECKPOINT* to sync before call this function
> > +	}
> > +
> > +	return -1;
> > +}
> > +
> > +static void run(void)
> > +{
> > +	pid_t pids[MAX_PID_NUM];
> > +	int i;
> > +
> > +	cg_child_test_simple = tst_cg_group_mk(tst_cg,
> > "cg_test_simple");
> > +
> > +	memset(buf, 0, buf_len);
> 
> IIRC guarded buffers are zeroed already.
Already explained by Li Wang in other email.
> 
> > +
> > +	for (i = 0; i < pid_num; i++)
> > +		pids[i] = cg_run_nowait(cg_child_test_simple, child_fn);
> 
> If the parent is killed and the children are moved then they will return
> and cause a fork bomb.
There is no extra fork action in child_fn so all child_fn will reparent and exit.
So i do not think fork bomb will happen.
> 
> This is not obvious because of the unecessary indirection (function
> pointer and functions).
> 
> > +
> > +	TST_EXP_PASS(cg_wait_for_proc_count(cg_child_test_simple,
> > pid_num));
> 
> If this fails then there will be little information to debug it. This is
> a common issue with the self tests which we will be importing into the LTP.
> 
Add extra log info into this function maybe help, what's your suggestion?
> > +	SAFE_CG_PRINTF(cg_child_test_simple, "cgroup.kill", "%d", 1);
> > +
> > +	for (i = 0; i < pid_num; i++) {
> > +		/* wait_for_pid(pids[i]); */
> > +		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]) == SIGKILL);
> 
> It seems wait_for_pid will never == SIGKILL. The function does not
> inspect the signal a process was killed with at all.
> 
> The test passes becaues this is not the correct use of TST_EXP_PASS*.
Good catch! Thanks a lot for finding this, i should use WIFSIGNALED 
to translate status and check children killed by SIGKILL, will fix this
in next patch.
> 
> > +	}
> > +
> > +	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
> > +}
> > +
> > +static void setup(void)
> > +{
> > +	buf = tst_alloc(buf_len);
> 
> Simple allocations like this can be done in the test struct.
This already discussed with Wang Li, compile error will happen since buf_len 
not fixed in my case.
> 
> > +}
> > +
> > +static struct tst_test test = {
> > +	.test_all = run,
> > +	.setup = setup,
> > +	.forks_child = 1,
> > +	.max_runtime = 15,
> > +	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
> 
> Why do we need the memory controller?
> 
> If it is just to make the LTP library happy, then you can change the
> library instead (e.g. add a "cgroup" pseudo controller if we didn't do
> that already).
You guess right, i just go quick way to let LTP happy xD
I will check library and try to implement this.

Thanks again for your valuable feedback!
> 
> > +	.needs_cgroup_ver = TST_CG_V2,
> > +};
> > -- 
> > 2.35.3
> 
> -- 
> Thank you,
> Richard.

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

Re: [LTP] [PATCH v5] kill01: New case cgroup kill

[Petr Vorel]

Hi all,

> >  lib/tst_cgroup.c                             |   1 +
> >  runtest/controllers                          |   1 +
> >  testcases/kernel/controllers/kill/.gitignore |   1 +
> >  testcases/kernel/controllers/kill/Makefile   |   6 +
> >  testcases/kernel/controllers/kill/kill01.c   | 138 +++++++++++++++++++

> nit: kill is not a controller, it is a core function. This should go in
> controllers/cgroup.

Very good point!

Kind regards,
Petr

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

Re: [LTP] [PATCH v5] kill01: New case cgroup kill

[Richard Palethorpe]

Wei Gao <wegao@suse.com> writes:

> On Mon, Mar 13, 2023 at 10:45:12AM +0000, Richard Palethorpe wrote:
>> Hello,
>> 
>> > +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 (!WIFEXITED(status))
>> > +		return -1;
>> > +
>> > +	return WEXITSTATUS(status);
>> > +}
>> 
>> We have tst_reap_children for this, but this just appears to be wrong
>> for this test.
> tst_reap_children can not return reason of status, such as i need call 
> WIFSIGNALED(wstatus) in next patch to make sure children is killed by 
> signal.
>
>> > + * 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))
>> 
>> Why keep this function?
>> 
>> If you want to convert tests to LTP, then don't do the minimum possible
>> to use the LTP API. Use as much of it as possible otherwise we are just
>> importing brittle self tests.
>> 
> function is useful and wrap the fork action & put pid into cgroup.procs,
> is there any LTP API can replace this function? Could you help give
> example.

IMO you don't need to wrap fork or SAFE_CG_PRINTF, just write it inline.

>> > +{
>> > +	int pid;
>> > +
>> > +	pid = SAFE_FORK();
>> > +	if (pid == 0) {
>> > +		SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
>> > +		fn();
>> > +	}
>> > +
>> > +	return pid;
>> > +}
>> > +
>> > +static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int count)
>> > +{
>> > +	int attempts;
>> > +	char *ptr;
>> > +
>> > +	for (attempts = 100; attempts >= 0; attempts--) {
>> > +		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;
>> > +
>> > +		usleep(100000);
>> 
>> It's best to avoid arbitrary sleep values and attempts. You could use
>> TST_CHECKPOINT* or increment a counter in some shared memory with
>> SAFE_MMAP and tst_atomic_inc.
>> 
> I will try to use TST_CHECKPOINT* to sync before call this function
>> > +	}
>> > +
>> > +	return -1;
>> > +}
>> > +
>> > +static void run(void)
>> > +{
>> > +	pid_t pids[MAX_PID_NUM];
>> > +	int i;
>> > +
>> > +	cg_child_test_simple = tst_cg_group_mk(tst_cg,
>> > "cg_test_simple");
>> > +
>> > +	memset(buf, 0, buf_len);
>> 
>> IIRC guarded buffers are zeroed already.
> Already explained by Li Wang in other email.
>> 
>> > +
>> > +	for (i = 0; i < pid_num; i++)
>> > +		pids[i] = cg_run_nowait(cg_child_test_simple, child_fn);
>> 
>> If the parent is killed and the children are moved then they will return
>> and cause a fork bomb.
> There is no extra fork action in child_fn so all child_fn will
> reparent and exit.

I don't see how it will exit? It seems like it will just return.

Possibly I just missed it, but that's why I have an issue with the
function pointer as well.

> So i do not think fork bomb will happen.
>> 
>> This is not obvious because of the unecessary indirection (function
>> pointer and functions).
>> 
>> > +
>> > +	TST_EXP_PASS(cg_wait_for_proc_count(cg_child_test_simple,
>> > pid_num));
>> 
>> If this fails then there will be little information to debug it. This is
>> a common issue with the self tests which we will be importing into the LTP.
>> 
> Add extra log info into this function maybe help, what's your
> suggestion?

Yes, something like that.

>> > +	SAFE_CG_PRINTF(cg_child_test_simple, "cgroup.kill", "%d", 1);
>> > +
>> > +	for (i = 0; i < pid_num; i++) {
>> > +		/* wait_for_pid(pids[i]); */
>> > +		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]) == SIGKILL);
>> 
>> It seems wait_for_pid will never == SIGKILL. The function does not
>> inspect the signal a process was killed with at all.
>> 
>> The test passes becaues this is not the correct use of TST_EXP_PASS*.
> Good catch! Thanks a lot for finding this, i should use WIFSIGNALED 
> to translate status and check children killed by SIGKILL, will fix this
> in next patch.
>> 
>> > +	}
>> > +
>> > +	cg_child_test_simple = tst_cg_group_rm(cg_child_test_simple);
>> > +}
>> > +
>> > +static void setup(void)
>> > +{
>> > +	buf = tst_alloc(buf_len);
>> 
>> Simple allocations like this can be done in the test struct.
> This already discussed with Wang Li, compile error will happen since buf_len 
> not fixed in my case.
>> 
>> > +}
>> > +
>> > +static struct tst_test test = {
>> > +	.test_all = run,
>> > +	.setup = setup,
>> > +	.forks_child = 1,
>> > +	.max_runtime = 15,
>> > +	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
>> 
>> Why do we need the memory controller?
>> 
>> If it is just to make the LTP library happy, then you can change the
>> library instead (e.g. add a "cgroup" pseudo controller if we didn't do
>> that already).
> You guess right, i just go quick way to let LTP happy xD
> I will check library and try to implement this.
>
> Thanks again for your valuable feedback!

Thanks!

>> 
>> > +	.needs_cgroup_ver = TST_CG_V2,
>> > +};
>> > -- 
>> > 2.35.3
>> 
>> -- 
>> Thank you,
>> Richard.


-- 
Thank you,
Richard.

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

[LTP] [PATCH v6] kill01: New case cgroup kill

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c                              |  55 ++++---
 runtest/controllers                           |   1 +
 .../kernel/controllers/cgroup/kill/.gitignore |   1 +
 .../kernel/controllers/cgroup/kill/Makefile   |   6 +
 .../kernel/controllers/cgroup/kill/kill01.c   | 149 ++++++++++++++++++
 5 files changed, 193 insertions(+), 19 deletions(-)
 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..ff1c734d1 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
 	CTRL_MISC,
 	CTRL_PERFEVENT,
 	CTRL_DEBUG,
-	CTRL_RDMA
+	CTRL_RDMA,
+	CTRL_PSEUDO
 };
-#define CTRLS_MAX CTRL_RDMA
+#define CTRLS_MAX CTRL_PSEUDO
 
 /* At most we can have one cgroup V1 tree for each controller and one
  * (empty) v2 tree.
@@ -166,6 +167,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 },
 	{ }
 };
 
@@ -258,6 +260,10 @@ static const struct cgroup_file rdma_ctrl_files[] = {
 	{ }
 };
 
+static const struct cgroup_file cgroup_pseudo_ctrl_files[] = {
+	{ }
+};
+
 #define CTRL_NAME_MAX 31
 #define CGROUP_CTRL_MEMBER(x, y)[y] = { .ctrl_name = #x, .files = \
 	x ## _ctrl_files, .ctrl_indx = y, NULL, 0 }
@@ -281,6 +287,7 @@ static struct cgroup_ctrl controllers[] = {
 	CGROUP_CTRL_MEMBER(perf_event, CTRL_PERFEVENT),
 	CGROUP_CTRL_MEMBER(debug, CTRL_DEBUG),
 	CGROUP_CTRL_MEMBER(rdma, CTRL_RDMA),
+	CGROUP_CTRL_MEMBER(cgroup_pseudo, CTRL_PSEUDO),
 	{ }
 };
 
@@ -826,6 +833,10 @@ void tst_cg_require(const char *const ctrl_name,
 	if (options->needs_ver != TST_CG_V2)
 		cgroup_mount_v1(ctrl);
 
+	if (!strcmp(ctrl->ctrl_name, "cgroup_pseudo")) {
+		ctrl->ctrl_root = roots;
+	}
+
 	if (!ctrl->ctrl_root) {
 		tst_brk(TCONF,
 			"'%s' controller required, but not available",
@@ -848,13 +859,15 @@ mkdirs:
 			ctrl->ctrl_name);
 	}
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
-		if (root->we_mounted_it) {
-			SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
-					   cgsc, "+%s", ctrl->ctrl_name);
-		} else {
-			tst_file_printfat(root->mnt_dir.dir_fd,
-					  cgsc, "+%s", ctrl->ctrl_name);
+	if (strcmp(ctrl->ctrl_name,"cgroup_pseudo")) {
+		if (cgroup_ctrl_on_v2(ctrl)) {
+			if (root->we_mounted_it) {
+				SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
+						cgsc, "+%s", ctrl->ctrl_name);
+			} else {
+				tst_file_printfat(root->mnt_dir.dir_fd,
+						cgsc, "+%s", ctrl->ctrl_name);
+			}
 		}
 	}
 
@@ -863,15 +876,17 @@ mkdirs:
 	else
 		root->ltp_dir.ctrl_field |= root->mnt_dir.ctrl_field;
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   cgsc, "+%s", ctrl->ctrl_name);
-	} else {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   "cgroup.clone_children", "%d", 1);
+	if (strcmp(ctrl->ctrl_name,"cgroup_pseudo")) {
+		if (cgroup_ctrl_on_v2(ctrl)) {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					cgsc, "+%s", ctrl->ctrl_name);
+		} else {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					"cgroup.clone_children", "%d", 1);
 
-		if (ctrl->ctrl_indx == CTRL_CPUSET)
-			cgroup_copy_cpuset(root);
+			if (ctrl->ctrl_indx == CTRL_CPUSET)
+				cgroup_copy_cpuset(root);
+		}
 	}
 
 	cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);
@@ -1049,8 +1064,10 @@ static void cgroup_group_add_dir(const struct tst_cg_group *const parent,
 		if (!parent || dir->dir_root->ver == TST_CG_V1)
 			continue;
 
-		SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
-				   "+%s", ctrl->ctrl_name);
+		if (strcmp(ctrl->ctrl_name, "cgroup_pseudo")) {
+			SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
+					"+%s", ctrl->ctrl_name);
+		}
 	}
 
 	for (i = 0; cg->dirs[i]; i++)
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		?= ../../../../../
+
+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..0da6de350
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/kill/kill01.c
@@ -0,0 +1,149 @@
+// 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 kernel self test case
+ * tools/testing/selftests/cgroup/test_kill.c
+ *
+ */
+
+#include <errno.h>
+#include <linux/limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/types.h>
+#include <unistd.h>
+#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 []){ "cgroup_pseudo", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+};
-- 
2.35.3


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

Re: [LTP] [PATCH v5] kill01: New case cgroup kill

[Wei Gao via ltp]

On Thu, Mar 16, 2023 at 11:10:07AM +0000, Richard Palethorpe wrote:
> 
> Wei Gao <wegao@suse.com> writes:
> 
> >> > +static int cg_run_nowait(const struct tst_cg_group *const cg,
> >> > +		  void (*fn)(void))
> >> 
> >> Why keep this function?
> >> 
> >> If you want to convert tests to LTP, then don't do the minimum possible
> >> to use the LTP API. Use as much of it as possible otherwise we are just
> >> importing brittle self tests.
> >> 
> > function is useful and wrap the fork action & put pid into cgroup.procs,
> > is there any LTP API can replace this function? Could you help give
> > example.
> 
> IMO you don't need to wrap fork or SAFE_CG_PRINTF, just write it inline.
More logic added into this function in latest patch v6, i prefer keep it currently.
> 
> >> > +
> >> > +	for (i = 0; i < pid_num; i++)
> >> > +		pids[i] = cg_run_nowait(cg_child_test_simple, child_fn);
> >> 
> >> If the parent is killed and the children are moved then they will return
> >> and cause a fork bomb.
> > There is no extra fork action in child_fn so all child_fn will
> > reparent and exit.
> 
> I don't see how it will exit? It seems like it will just return.

The child_fun will exit if parent id is changed like below

+static void child_fn(void)
+{
+	int ppid = getppid();
+
+	while (getppid() == ppid)
+		usleep(1000);
+
+}


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

Re: [LTP] [PATCH v6] kill01: New case cgroup kill

[Petr Vorel]

Hi Wei,

NOTE: it's slightly easier to review if you write sum of changes from previous version.

...
> +++ b/lib/tst_cgroup.c
> @@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
>  	CTRL_MISC,
>  	CTRL_PERFEVENT,
>  	CTRL_DEBUG,
> -	CTRL_RDMA
> +	CTRL_RDMA,
> +	CTRL_PSEUDO
>  };
> -#define CTRLS_MAX CTRL_RDMA
> +#define CTRLS_MAX CTRL_PSEUDO

Maybe CTRL_PSEUDO should be added as a separate commit with a proper description
why it's needed.

...
> +static const struct cgroup_file cgroup_pseudo_ctrl_files[] = {
> +	{ }
> +};
> +
>  #define CTRL_NAME_MAX 31
>  #define CGROUP_CTRL_MEMBER(x, y)[y] = { .ctrl_name = #x, .files = \
>  	x ## _ctrl_files, .ctrl_indx = y, NULL, 0 }
> @@ -281,6 +287,7 @@ static struct cgroup_ctrl controllers[] = {
>  	CGROUP_CTRL_MEMBER(perf_event, CTRL_PERFEVENT),
>  	CGROUP_CTRL_MEMBER(debug, CTRL_DEBUG),
>  	CGROUP_CTRL_MEMBER(rdma, CTRL_RDMA),
> +	CGROUP_CTRL_MEMBER(cgroup_pseudo, CTRL_PSEUDO),
>  	{ }
>  };

> @@ -826,6 +833,10 @@ void tst_cg_require(const char *const ctrl_name,
>  	if (options->needs_ver != TST_CG_V2)
>  		cgroup_mount_v1(ctrl);

> +	if (!strcmp(ctrl->ctrl_name, "cgroup_pseudo")) {
> +		ctrl->ctrl_root = roots;
> +	}

tst_cgroup.c:836: WARNING: braces {} are not necessary for single statement blocks
=> please remove { }

Also strcmp for "cgroup_pseudo" is checked 3x in tst_cg_require(), why not creating
a variable and use it in if ()?
int cgroup_pseudo = !strcmp(ctrl->ctrl_name, "cgroup_pseudo");

> +
>  	if (!ctrl->ctrl_root) {
>  		tst_brk(TCONF,
>  			"'%s' controller required, but not available",
> @@ -848,13 +859,15 @@ mkdirs:
>  			ctrl->ctrl_name);
>  	}

> -	if (cgroup_ctrl_on_v2(ctrl)) {
> -		if (root->we_mounted_it) {
> -			SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
> -					   cgsc, "+%s", ctrl->ctrl_name);
> -		} else {
> -			tst_file_printfat(root->mnt_dir.dir_fd,
> -					  cgsc, "+%s", ctrl->ctrl_name);
> +	if (strcmp(ctrl->ctrl_name,"cgroup_pseudo")) {
nit: missing space after comma. I actually found by searching for previous
strcmp line (with comma) and it found just 2 places instead of 3, but make check
would report it for you:

make check-tst_cgroup
tst_cgroup.c:836: WARNING: braces {} are not necessary for single statement blocks
tst_cgroup.c:862: ERROR: space required after that ',' (ctx:VxV)
tst_cgroup.c:879: ERROR: space required after that ',' (ctx:VxV)

> +		if (cgroup_ctrl_on_v2(ctrl)) {
> +			if (root->we_mounted_it) {
> +				SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
> +						cgsc, "+%s", ctrl->ctrl_name);
> +			} else {
> +				tst_file_printfat(root->mnt_dir.dir_fd,
> +						cgsc, "+%s", ctrl->ctrl_name);
> +			}
>  		}
>  	}

> @@ -863,15 +876,17 @@ mkdirs:
>  	else
>  		root->ltp_dir.ctrl_field |= root->mnt_dir.ctrl_field;

> -	if (cgroup_ctrl_on_v2(ctrl)) {
> -		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> -				   cgsc, "+%s", ctrl->ctrl_name);
> -	} else {
> -		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> -				   "cgroup.clone_children", "%d", 1);
> +	if (strcmp(ctrl->ctrl_name,"cgroup_pseudo")) {
> +		if (cgroup_ctrl_on_v2(ctrl)) {
> +			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> +					cgsc, "+%s", ctrl->ctrl_name);
> +		} else {
> +			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> +					"cgroup.clone_children", "%d", 1);

> -		if (ctrl->ctrl_indx == CTRL_CPUSET)
> -			cgroup_copy_cpuset(root);
> +			if (ctrl->ctrl_indx == CTRL_CPUSET)
> +				cgroup_copy_cpuset(root);
> +		}
>  	}

>  	cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);
> @@ -1049,8 +1064,10 @@ static void cgroup_group_add_dir(const struct tst_cg_group *const parent,
>  		if (!parent || dir->dir_root->ver == TST_CG_V1)
>  			continue;

> -		SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
> -				   "+%s", ctrl->ctrl_name);
> +		if (strcmp(ctrl->ctrl_name, "cgroup_pseudo")) {
> +			SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
> +					"+%s", ctrl->ctrl_name);
> +		}
>  	}

>  	for (i = 0; cg->dirs[i]; i++)
> 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		?= ../../../../../
> +
> +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..0da6de350
> --- /dev/null
> +++ b/testcases/kernel/controllers/cgroup/kill/kill01.c
> @@ -0,0 +1,149 @@
> +// 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 kernel self test case
nit: I'd just wrote kselftest instead of "kernel self test case".

> + * tools/testing/selftests/cgroup/test_kill.c

BTW am I correct that you converted only test_cgkill_simple() ?
There are 2 other: test_cgkill_tree() and test_cgkill_forkbomb(),
I have no idea if they are worth to be implemented or should be implemented now.
But maybe it'd be worth to mention that only simple test case was backported.


> + *
> + */
> +
> +#include <errno.h>
> +#include <linux/limits.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <sys/wait.h>
nit: only <sys/wait.h> is needed (others either imported by tst_test.h or needed
in the original kselftest but not in LTP port).

> +
> +#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)
constants should be upper case (PID_NUM and BUF_LEN).

Kind regards,
Petr

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

[LTP] [PATCH v7 0/2] kill01: New case cgroup kill

[Wei Gao via ltp]

Wei Gao (2):
  kill01: New case cgroup kill
  tst_cgroup.c: Add a cgroup pseudo controller

 lib/tst_cgroup.c                              |  55 ++++---
 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, 188 insertions(+), 19 deletions(-)
 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


base-commit: 1eda2532d8a7409f591d259e440f4e06f55615f2
-- 
2.35.3


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

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

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 runtest/controllers                           |   1 +
 .../kernel/controllers/cgroup/kill/.gitignore |   1 +
 .../kernel/controllers/cgroup/kill/Makefile   |   6 +
 .../kernel/controllers/cgroup/kill/kill01.c   | 144 ++++++++++++++++++
 4 files changed, 152 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/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		?= ../../../../../
+
+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 },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+};
-- 
2.35.3


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

[LTP] [PATCH v7 2/2] tst_cgroup.c: Add a cgroup pseudo controller

[Wei Gao via ltp]

For new test case such as kill01.c no need specific controller, it just
need LTP cgroup library start work, so we need add a "cgroup" pseudo
controller.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c | 55 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 50699bc63..6a70bf4b4 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
 	CTRL_MISC,
 	CTRL_PERFEVENT,
 	CTRL_DEBUG,
-	CTRL_RDMA
+	CTRL_RDMA,
+	CTRL_PSEUDO
 };
-#define CTRLS_MAX CTRL_RDMA
+#define CTRLS_MAX CTRL_PSEUDO
 
 /* At most we can have one cgroup V1 tree for each controller and one
  * (empty) v2 tree.
@@ -166,6 +167,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 },
 	{ }
 };
 
@@ -258,6 +260,10 @@ static const struct cgroup_file rdma_ctrl_files[] = {
 	{ }
 };
 
+static const struct cgroup_file cgroup_pseudo_ctrl_files[] = {
+	{ }
+};
+
 #define CTRL_NAME_MAX 31
 #define CGROUP_CTRL_MEMBER(x, y)[y] = { .ctrl_name = #x, .files = \
 	x ## _ctrl_files, .ctrl_indx = y, NULL, 0 }
@@ -281,6 +287,7 @@ static struct cgroup_ctrl controllers[] = {
 	CGROUP_CTRL_MEMBER(perf_event, CTRL_PERFEVENT),
 	CGROUP_CTRL_MEMBER(debug, CTRL_DEBUG),
 	CGROUP_CTRL_MEMBER(rdma, CTRL_RDMA),
+	CGROUP_CTRL_MEMBER(cgroup_pseudo, CTRL_PSEUDO),
 	{ }
 };
 
@@ -797,6 +804,7 @@ void tst_cg_require(const char *const ctrl_name,
 	const char *const cgsc = "cgroup.subtree_control";
 	struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
 	struct cgroup_root *root;
+	int cgroup_pseudo = !strcmp(ctrl->ctrl_name, "cgroup_pseudo");
 
 	if (!ctrl) {
 		tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
@@ -826,6 +834,9 @@ void tst_cg_require(const char *const ctrl_name,
 	if (options->needs_ver != TST_CG_V2)
 		cgroup_mount_v1(ctrl);
 
+	if (cgroup_pseudo)
+		ctrl->ctrl_root = roots;
+
 	if (!ctrl->ctrl_root) {
 		tst_brk(TCONF,
 			"'%s' controller required, but not available",
@@ -848,13 +859,15 @@ mkdirs:
 			ctrl->ctrl_name);
 	}
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
-		if (root->we_mounted_it) {
-			SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
-					   cgsc, "+%s", ctrl->ctrl_name);
-		} else {
-			tst_file_printfat(root->mnt_dir.dir_fd,
-					  cgsc, "+%s", ctrl->ctrl_name);
+	if (!cgroup_pseudo) {
+		if (cgroup_ctrl_on_v2(ctrl)) {
+			if (root->we_mounted_it) {
+				SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
+						cgsc, "+%s", ctrl->ctrl_name);
+			} else {
+				tst_file_printfat(root->mnt_dir.dir_fd,
+						cgsc, "+%s", ctrl->ctrl_name);
+			}
 		}
 	}
 
@@ -863,15 +876,17 @@ mkdirs:
 	else
 		root->ltp_dir.ctrl_field |= root->mnt_dir.ctrl_field;
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   cgsc, "+%s", ctrl->ctrl_name);
-	} else {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   "cgroup.clone_children", "%d", 1);
+	if (!cgroup_pseudo) {
+		if (cgroup_ctrl_on_v2(ctrl)) {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					cgsc, "+%s", ctrl->ctrl_name);
+		} else {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					"cgroup.clone_children", "%d", 1);
 
-		if (ctrl->ctrl_indx == CTRL_CPUSET)
-			cgroup_copy_cpuset(root);
+			if (ctrl->ctrl_indx == CTRL_CPUSET)
+				cgroup_copy_cpuset(root);
+		}
 	}
 
 	cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);
@@ -1049,8 +1064,10 @@ static void cgroup_group_add_dir(const struct tst_cg_group *const parent,
 		if (!parent || dir->dir_root->ver == TST_CG_V1)
 			continue;
 
-		SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
-				   "+%s", ctrl->ctrl_name);
+		if (strcmp(ctrl->ctrl_name, "cgroup_pseudo")) {
+			SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
+					"+%s", ctrl->ctrl_name);
+		}
 	}
 
 	for (i = 0; cg->dirs[i]; i++)
-- 
2.35.3


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

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

[Wei Gao via ltp]

Wei Gao (2):
  kill01: New case cgroup kill
  tst_cgroup.c: Add a cgroup pseudo controller

 lib/tst_cgroup.c                              |  55 ++++---
 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, 188 insertions(+), 19 deletions(-)
 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


base-commit: b67facdd9c3f0ef471d74dd59ac276139e63f4c4
-- 
2.35.3


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

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

[Wei Gao via ltp]

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		?= ../../../../../
+
+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 },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+};
-- 
2.35.3


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

[LTP] [PATCH v8 2/2] tst_cgroup.c: Add a cgroup pseudo controller

[Wei Gao via ltp]

For new test case such as kill01.c no need specific controller, it just
need LTP cgroup library start work, so we need add a "cgroup" pseudo
controller.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c | 54 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 77575431d..6a70bf4b4 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
 	CTRL_MISC,
 	CTRL_PERFEVENT,
 	CTRL_DEBUG,
-	CTRL_RDMA
+	CTRL_RDMA,
+	CTRL_PSEUDO
 };
-#define CTRLS_MAX CTRL_RDMA
+#define CTRLS_MAX CTRL_PSEUDO
 
 /* At most we can have one cgroup V1 tree for each controller and one
  * (empty) v2 tree.
@@ -259,6 +260,10 @@ static const struct cgroup_file rdma_ctrl_files[] = {
 	{ }
 };
 
+static const struct cgroup_file cgroup_pseudo_ctrl_files[] = {
+	{ }
+};
+
 #define CTRL_NAME_MAX 31
 #define CGROUP_CTRL_MEMBER(x, y)[y] = { .ctrl_name = #x, .files = \
 	x ## _ctrl_files, .ctrl_indx = y, NULL, 0 }
@@ -282,6 +287,7 @@ static struct cgroup_ctrl controllers[] = {
 	CGROUP_CTRL_MEMBER(perf_event, CTRL_PERFEVENT),
 	CGROUP_CTRL_MEMBER(debug, CTRL_DEBUG),
 	CGROUP_CTRL_MEMBER(rdma, CTRL_RDMA),
+	CGROUP_CTRL_MEMBER(cgroup_pseudo, CTRL_PSEUDO),
 	{ }
 };
 
@@ -798,6 +804,7 @@ void tst_cg_require(const char *const ctrl_name,
 	const char *const cgsc = "cgroup.subtree_control";
 	struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
 	struct cgroup_root *root;
+	int cgroup_pseudo = !strcmp(ctrl->ctrl_name, "cgroup_pseudo");
 
 	if (!ctrl) {
 		tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
@@ -827,6 +834,9 @@ void tst_cg_require(const char *const ctrl_name,
 	if (options->needs_ver != TST_CG_V2)
 		cgroup_mount_v1(ctrl);
 
+	if (cgroup_pseudo)
+		ctrl->ctrl_root = roots;
+
 	if (!ctrl->ctrl_root) {
 		tst_brk(TCONF,
 			"'%s' controller required, but not available",
@@ -849,13 +859,15 @@ mkdirs:
 			ctrl->ctrl_name);
 	}
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
-		if (root->we_mounted_it) {
-			SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
-					   cgsc, "+%s", ctrl->ctrl_name);
-		} else {
-			tst_file_printfat(root->mnt_dir.dir_fd,
-					  cgsc, "+%s", ctrl->ctrl_name);
+	if (!cgroup_pseudo) {
+		if (cgroup_ctrl_on_v2(ctrl)) {
+			if (root->we_mounted_it) {
+				SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
+						cgsc, "+%s", ctrl->ctrl_name);
+			} else {
+				tst_file_printfat(root->mnt_dir.dir_fd,
+						cgsc, "+%s", ctrl->ctrl_name);
+			}
 		}
 	}
 
@@ -864,15 +876,17 @@ mkdirs:
 	else
 		root->ltp_dir.ctrl_field |= root->mnt_dir.ctrl_field;
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   cgsc, "+%s", ctrl->ctrl_name);
-	} else {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   "cgroup.clone_children", "%d", 1);
+	if (!cgroup_pseudo) {
+		if (cgroup_ctrl_on_v2(ctrl)) {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					cgsc, "+%s", ctrl->ctrl_name);
+		} else {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					"cgroup.clone_children", "%d", 1);
 
-		if (ctrl->ctrl_indx == CTRL_CPUSET)
-			cgroup_copy_cpuset(root);
+			if (ctrl->ctrl_indx == CTRL_CPUSET)
+				cgroup_copy_cpuset(root);
+		}
 	}
 
 	cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);
@@ -1050,8 +1064,10 @@ static void cgroup_group_add_dir(const struct tst_cg_group *const parent,
 		if (!parent || dir->dir_root->ver == TST_CG_V1)
 			continue;
 
-		SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
-				   "+%s", ctrl->ctrl_name);
+		if (strcmp(ctrl->ctrl_name, "cgroup_pseudo")) {
+			SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
+					"+%s", ctrl->ctrl_name);
+		}
 	}
 
 	for (i = 0; cg->dirs[i]; i++)
-- 
2.35.3


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

Re: [LTP] [PATCH v8 2/2] tst_cgroup.c: Add a cgroup pseudo controller

[Li Wang]

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

> For new test case such as kill01.c no need specific controller, it just
> need LTP cgroup library start work, so we need add a "cgroup" pseudo
> controller.
>
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  lib/tst_cgroup.c | 54 +++++++++++++++++++++++++++++++-----------------
>  1 file changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 77575431d..6a70bf4b4 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
>         CTRL_MISC,
>         CTRL_PERFEVENT,
>         CTRL_DEBUG,
> -       CTRL_RDMA
> +       CTRL_RDMA,
> +       CTRL_PSEUDO
>  };
> -#define CTRLS_MAX CTRL_RDMA
> +#define CTRLS_MAX CTRL_PSEUDO
>
>  /* At most we can have one cgroup V1 tree for each controller and one
>   * (empty) v2 tree.
> @@ -259,6 +260,10 @@ static const struct cgroup_file rdma_ctrl_files[] = {
>         { }
>  };
>
> +static const struct cgroup_file cgroup_pseudo_ctrl_files[] = {
>

As the controller defined CTRL_PSEUDO here may be better to
use pseudo_ctrl_files without "cgroup_" prefix.



> +       { }
> +};
> +
>  #define CTRL_NAME_MAX 31
>  #define CGROUP_CTRL_MEMBER(x, y)[y] = { .ctrl_name = #x, .files = \
>         x ## _ctrl_files, .ctrl_indx = y, NULL, 0 }
> @@ -282,6 +287,7 @@ static struct cgroup_ctrl controllers[] = {
>         CGROUP_CTRL_MEMBER(perf_event, CTRL_PERFEVENT),
>         CGROUP_CTRL_MEMBER(debug, CTRL_DEBUG),
>         CGROUP_CTRL_MEMBER(rdma, CTRL_RDMA),
> +       CGROUP_CTRL_MEMBER(cgroup_pseudo, CTRL_PSEUDO),
>

use pseudo directly without "cgroup_" prefix.


>         { }
>  };
>
> @@ -798,6 +804,7 @@ void tst_cg_require(const char *const ctrl_name,
>         const char *const cgsc = "cgroup.subtree_control";
>         struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
>         struct cgroup_root *root;
> +       int cgroup_pseudo = !strcmp(ctrl->ctrl_name, "cgroup_pseudo");
>

int cgroup_pseudo = !strcmp(ctrl->ctrl_name, "pseudo");



>
>         if (!ctrl) {
>                 tst_brk(TBROK, "'%s' controller is unknown to LTP",
> ctrl_name);
> @@ -827,6 +834,9 @@ void tst_cg_require(const char *const ctrl_name,
>         if (options->needs_ver != TST_CG_V2)
>                 cgroup_mount_v1(ctrl);
>
> +       if (cgroup_pseudo)
> +               ctrl->ctrl_root = roots;
> +
>         if (!ctrl->ctrl_root) {
>                 tst_brk(TCONF,
>                         "'%s' controller required, but not available",
> @@ -849,13 +859,15 @@ mkdirs:
>                         ctrl->ctrl_name);
>         }
>
> -       if (cgroup_ctrl_on_v2(ctrl)) {
> -               if (root->we_mounted_it) {
> -                       SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
> -                                          cgsc, "+%s", ctrl->ctrl_name);
> -               } else {
> -                       tst_file_printfat(root->mnt_dir.dir_fd,
> -                                         cgsc, "+%s", ctrl->ctrl_name);
> +       if (!cgroup_pseudo) {
> +               if (cgroup_ctrl_on_v2(ctrl)) {
>

This makes things more complicated, what about:
    if (cgroup_ctrl_on_v2(ctrl) && !cgroup_pseudo) {
to reduce one nested.



> +                       if (root->we_mounted_it) {
> +                               SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
> +                                               cgsc, "+%s",
> ctrl->ctrl_name);
> +                       } else {
> +                               tst_file_printfat(root->mnt_dir.dir_fd,
> +                                               cgsc, "+%s",
> ctrl->ctrl_name);
> +                       }
>                 }
>         }
>
> @@ -864,15 +876,17 @@ mkdirs:
>         else
>                 root->ltp_dir.ctrl_field |= root->mnt_dir.ctrl_field;
>
> -       if (cgroup_ctrl_on_v2(ctrl)) {
> -               SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> -                                  cgsc, "+%s", ctrl->ctrl_name);
> -       } else {
> -               SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> -                                  "cgroup.clone_children", "%d", 1);
> +       if (!cgroup_pseudo) {
> +               if (cgroup_ctrl_on_v2(ctrl)) {
>

if (cgroup_ctrl_on_v2(ctrl) && !cgroup_pseudo) {



> +                       SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> +                                       cgsc, "+%s", ctrl->ctrl_name);
> +               } else {
> +                       SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> +                                       "cgroup.clone_children", "%d", 1);
>
> -               if (ctrl->ctrl_indx == CTRL_CPUSET)
> -                       cgroup_copy_cpuset(root);
> +                       if (ctrl->ctrl_indx == CTRL_CPUSET)
> +                               cgroup_copy_cpuset(root);
> +               }
>         }
>
>         cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir,
> &root->drain_dir);
> @@ -1050,8 +1064,10 @@ static void cgroup_group_add_dir(const struct
> tst_cg_group *const parent,
>                 if (!parent || dir->dir_root->ver == TST_CG_V1)
>                         continue;
>
> -               SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
> -                                  "+%s", ctrl->ctrl_name);
> +               if (strcmp(ctrl->ctrl_name, "cgroup_pseudo")) {
>

Remove "cgroup_" prefix.



> +                       SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
> +                                       "+%s", ctrl->ctrl_name);
> +               }
>         }
>
>         for (i = 0; cg->dirs[i]; i++)
> --
> 2.35.3
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
> Other two issues we have to resolve:

1. We'd better guarantee the "pseudo" controller is safely ignored by
Cgroup_V1 in case of '.needs_cgroup_ver = TST_CG_V1'.
Because cgroup_mount_v1(ctrl) will try to mount a 'pseudo' controller
once a system without any available (V1 and V2) ctrls, which ultimately
caused the test to break.

2. Assume the test running on a CgroupV1 supported system, which will
try to mount CgroupV2 because pseudo is not detected exist in V1
controllers, so it by default thinks we need to mount CgroupV2 and
test there.

But the fact is that CgroupV1-only system is not fully supported CgroupV2,
even if we mount the V2 successfully the function is limited and this
ultimately
uses a lack of cgroup.kill file and failed again.


-- 
Regards,
Li Wang

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

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

[Li Wang]

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

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

Re: [LTP] [PATCH v8 2/2] tst_cgroup.c: Add a cgroup pseudo controller

[Cyril Hrubis]

Hi!
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 77575431d..6a70bf4b4 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
>  	CTRL_MISC,
>  	CTRL_PERFEVENT,
>  	CTRL_DEBUG,
> -	CTRL_RDMA
> +	CTRL_RDMA,
> +	CTRL_PSEUDO

I wonder if PSEUDO is the right name here. Maybe CTRL_BASE as we do not
mount any controller to access the control files.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH v9 0/2] kill01: New case cgroup kill

[Wei Gao via ltp]

Wei Gao (2):
  kill01: New case cgroup kill
  tst_cgroup.c: Add a cgroup pseudo controller

 lib/tst_cgroup.c                              |  48 ++++--
 runtest/controllers                           |   1 +
 .../kernel/controllers/cgroup/.gitignore      |   1 +
 .../kernel/controllers/cgroup/cgroup_core03.c | 144 ++++++++++++++++++
 4 files changed, 179 insertions(+), 15 deletions(-)
 create mode 100644 testcases/kernel/controllers/cgroup/cgroup_core03.c


base-commit: b67facdd9c3f0ef471d74dd59ac276139e63f4c4
-- 
2.35.3


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

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

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c                              |   1 +
 runtest/controllers                           |   1 +
 .../kernel/controllers/cgroup/.gitignore      |   1 +
 .../kernel/controllers/cgroup/cgroup_core03.c | 144 ++++++++++++++++++
 4 files changed, 147 insertions(+)
 create mode 100644 testcases/kernel/controllers/cgroup/cgroup_core03.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..93c52c439 100644
--- a/runtest/controllers
+++ b/runtest/controllers
@@ -1,6 +1,7 @@
 #DESCRIPTION:Resource Management testing
 cgroup_core01	cgroup_core01
 cgroup_core02	cgroup_core02
+cgroup_core03	cgroup_core03
 cgroup		cgroup_regression_test.sh
 memcg_regression	memcg_regression_test.sh
 memcg_test_3	memcg_test_3
diff --git a/testcases/kernel/controllers/cgroup/.gitignore b/testcases/kernel/controllers/cgroup/.gitignore
index 8deae77da..9f1d1ada9 100644
--- a/testcases/kernel/controllers/cgroup/.gitignore
+++ b/testcases/kernel/controllers/cgroup/.gitignore
@@ -2,3 +2,4 @@
 /cgroup_regression_getdelays
 /cgroup_core01
 /cgroup_core02
+/cgroup_core03
diff --git a/testcases/kernel/controllers/cgroup/cgroup_core03.c b/testcases/kernel/controllers/cgroup/cgroup_core03.c
new file mode 100644
index 000000000..2a6941c44
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/cgroup_core03.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 []){ "pseudo", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+};
-- 
2.35.3


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

[LTP] [PATCH v9 2/2] tst_cgroup.c: Add a cgroup pseudo controller

[Wei Gao via ltp]

For new test case such as kill01.c no need specific controller, it just
need LTP cgroup library start work, so we need add a "cgroup" pseudo
controller.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c | 47 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 77575431d..ed3e0758f 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
 	CTRL_MISC,
 	CTRL_PERFEVENT,
 	CTRL_DEBUG,
-	CTRL_RDMA
+	CTRL_RDMA,
+	CTRL_PSEUDO
 };
-#define CTRLS_MAX CTRL_RDMA
+#define CTRLS_MAX CTRL_PSEUDO
 
 /* At most we can have one cgroup V1 tree for each controller and one
  * (empty) v2 tree.
@@ -259,6 +260,10 @@ static const struct cgroup_file rdma_ctrl_files[] = {
 	{ }
 };
 
+static const struct cgroup_file pseudo_ctrl_files[] = {
+	{ }
+};
+
 #define CTRL_NAME_MAX 31
 #define CGROUP_CTRL_MEMBER(x, y)[y] = { .ctrl_name = #x, .files = \
 	x ## _ctrl_files, .ctrl_indx = y, NULL, 0 }
@@ -282,6 +287,7 @@ static struct cgroup_ctrl controllers[] = {
 	CGROUP_CTRL_MEMBER(perf_event, CTRL_PERFEVENT),
 	CGROUP_CTRL_MEMBER(debug, CTRL_DEBUG),
 	CGROUP_CTRL_MEMBER(rdma, CTRL_RDMA),
+	CGROUP_CTRL_MEMBER(pseudo, CTRL_PSEUDO),
 	{ }
 };
 
@@ -798,6 +804,10 @@ void tst_cg_require(const char *const ctrl_name,
 	const char *const cgsc = "cgroup.subtree_control";
 	struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
 	struct cgroup_root *root;
+	int pseudo = !strcmp(ctrl->ctrl_name, "pseudo");
+
+	if (pseudo && options->needs_ver != TST_CG_V2)
+		tst_brk(TCONF, "pseudo control only support needs_ver TST_CG_V2!");
 
 	if (!ctrl) {
 		tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
@@ -827,6 +837,9 @@ void tst_cg_require(const char *const ctrl_name,
 	if (options->needs_ver != TST_CG_V2)
 		cgroup_mount_v1(ctrl);
 
+	if (pseudo)
+		ctrl->ctrl_root = roots;
+
 	if (!ctrl->ctrl_root) {
 		tst_brk(TCONF,
 			"'%s' controller required, but not available",
@@ -849,13 +862,13 @@ mkdirs:
 			ctrl->ctrl_name);
 	}
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
+	if (cgroup_ctrl_on_v2(ctrl) && !pseudo) {
 		if (root->we_mounted_it) {
 			SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
-					   cgsc, "+%s", ctrl->ctrl_name);
+					cgsc, "+%s", ctrl->ctrl_name);
 		} else {
 			tst_file_printfat(root->mnt_dir.dir_fd,
-					  cgsc, "+%s", ctrl->ctrl_name);
+					cgsc, "+%s", ctrl->ctrl_name);
 		}
 	}
 
@@ -864,15 +877,17 @@ mkdirs:
 	else
 		root->ltp_dir.ctrl_field |= root->mnt_dir.ctrl_field;
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   cgsc, "+%s", ctrl->ctrl_name);
-	} else {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   "cgroup.clone_children", "%d", 1);
+	if (!pseudo) {
+		if (cgroup_ctrl_on_v2(ctrl)) {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					cgsc, "+%s", ctrl->ctrl_name);
+		} else {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					"cgroup.clone_children", "%d", 1);
 
-		if (ctrl->ctrl_indx == CTRL_CPUSET)
-			cgroup_copy_cpuset(root);
+			if (ctrl->ctrl_indx == CTRL_CPUSET)
+				cgroup_copy_cpuset(root);
+		}
 	}
 
 	cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);
@@ -1050,8 +1065,10 @@ static void cgroup_group_add_dir(const struct tst_cg_group *const parent,
 		if (!parent || dir->dir_root->ver == TST_CG_V1)
 			continue;
 
-		SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
-				   "+%s", ctrl->ctrl_name);
+		if (strcmp(ctrl->ctrl_name, "pseudo")) {
+			SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
+					"+%s", ctrl->ctrl_name);
+		}
 	}
 
 	for (i = 0; cg->dirs[i]; i++)
-- 
2.35.3


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

Re: [LTP] [PATCH v9 2/2] tst_cgroup.c: Add a cgroup pseudo controller

[Li Wang]

Hi Wei,

On Sat, Apr 22, 2023 at 9:54 PM Wei Gao via ltp <ltp@lists.linux.it> wrote:

> For new test case such as kill01.c no need specific controller, it just
> need LTP cgroup library start work, so we need add a "cgroup" pseudo
> controller.
>
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  lib/tst_cgroup.c | 47 ++++++++++++++++++++++++++++++++---------------
>  1 file changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 77575431d..ed3e0758f 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
>         CTRL_MISC,
>         CTRL_PERFEVENT,
>         CTRL_DEBUG,
> -       CTRL_RDMA
> +       CTRL_RDMA,
> +       CTRL_PSEUDO
>  };
> -#define CTRLS_MAX CTRL_RDMA
> +#define CTRLS_MAX CTRL_PSEUDO
>

I vote for considering use of 'CTRL_BASE' (suggested by Cryil),
because here we indeed use the basic functionality of CgroupV
 but not a pseudo controller.

Otherwise, the rest looks good to me.

=======================

@Cryil, @Richard

Apart from that, there is another problem with the test logic
of this library, that it potentially mixed Cgroup V1 and V2
together to be mounted at the same time. The scenario
happens once someone just requests CTRL_BASE
(or controllers not mounted) on a V1-mounted system.

Upstream always objected to enabling Cgroup like that,
which brings many unexpected issue during the test.

So I would strongly suggest avoiding LTP mount V1&V2
even if there is no overlap in controllers.

Which should be achieved in a separate patch:
(document should be updated as well)

--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -828,13 +828,14 @@ void tst_cg_require(const char *const ctrl_name,
        if (ctrl->ctrl_root)
                goto mkdirs;

-       if (!cgroup_v2_mounted() && options->needs_ver != TST_CG_V1)
+       if (!cgroup_v2_mounted() && options->needs_ver != TST_CG_V1
+                       && !cgroup_v1_mounted())
                cgroup_mount_v2();

        if (ctrl->ctrl_root)
                goto mkdirs;

-       if (options->needs_ver != TST_CG_V2)
+       if (options->needs_ver != TST_CG_V2 && !cgroup_v2_mounted())
                cgroup_mount_v1(ctrl);

        if (pseudo)



-- 
Regards,
Li Wang

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

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

[Cyril Hrubis]

Hi!
> ---
>  lib/tst_cgroup.c                              |   1 +
>  runtest/controllers                           |   1 +
>  .../kernel/controllers/cgroup/.gitignore      |   1 +
>  .../kernel/controllers/cgroup/cgroup_core03.c | 144 ++++++++++++++++++
>  4 files changed, 147 insertions(+)
>  create mode 100644 testcases/kernel/controllers/cgroup/cgroup_core03.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..93c52c439 100644
> --- a/runtest/controllers
> +++ b/runtest/controllers
> @@ -1,6 +1,7 @@
>  #DESCRIPTION:Resource Management testing
>  cgroup_core01	cgroup_core01
>  cgroup_core02	cgroup_core02
> +cgroup_core03	cgroup_core03
>  cgroup		cgroup_regression_test.sh
>  memcg_regression	memcg_regression_test.sh
>  memcg_test_3	memcg_test_3
> diff --git a/testcases/kernel/controllers/cgroup/.gitignore b/testcases/kernel/controllers/cgroup/.gitignore
> index 8deae77da..9f1d1ada9 100644
> --- a/testcases/kernel/controllers/cgroup/.gitignore
> +++ b/testcases/kernel/controllers/cgroup/.gitignore
> @@ -2,3 +2,4 @@
>  /cgroup_regression_getdelays
>  /cgroup_core01
>  /cgroup_core02
> +/cgroup_core03
> diff --git a/testcases/kernel/controllers/cgroup/cgroup_core03.c b/testcases/kernel/controllers/cgroup/cgroup_core03.c
> new file mode 100644
> index 000000000..2a6941c44
> --- /dev/null
> +++ b/testcases/kernel/controllers/cgroup/cgroup_core03.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);
> +

What's the reason we are waiting for reparenting here? Aren't these
processes killed anyways? I suppose that we can just call pause() here
instead.

> +}
> +
> +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);

I do not think that this is 100% correct, as it is we may end up calling
the TST_CHECKOPOINT_WAKE(0) more than once here, since there is a race
between increment and load. An if that happens only one of the calls to
TST_CHECKPOINT_WAKE(0) will succeeds, while the rest would fail and
break the test.

First of all the tst_atomic_inc() returns the value after the increment,
so that we can actually get the counter value without a race.

Secondly I think that we can get rid of the atomic operations
completely, we just have to loop N times over the TST_CHECKPOINT_WAIT()
in the run() function and call TST_CHECKPOINT_WAKE() in each child
instance. We can also add TST_CHECKPOINT_WAIT_NR() to the test library
that would also take a number of wait calls passed as a parameter.

> +		fn();
> +	}
> +
> +	return pid;
> +}
> +
> +static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int count)

The function does not wait for anything, so it should be called
cg_count_procs() or similar.

> +{
> +	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));

This is very minor, but I would make the function return the number of
pids found in the cgroup and called it as:

	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 < PID_NUM; i++)
> +		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]));

Then we can do here:

	TST_EXP_VAL(cg_count_procs(cg_child_test_simple), 0);

> +	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 []){ "pseudo", NULL },
> +	.needs_cgroup_ver = TST_CG_V2,
> +	.needs_checkpoints = 1,
> +};
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v9 2/2] tst_cgroup.c: Add a cgroup pseudo controller

[Cyril Hrubis]

Hi!
> For new test case such as kill01.c no need specific controller, it just
> need LTP cgroup library start work, so we need add a "cgroup" pseudo
> controller.

Can we please call it "base" controller? Or something better fitting?

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  lib/tst_cgroup.c | 47 ++++++++++++++++++++++++++++++++---------------
>  1 file changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 77575431d..ed3e0758f 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
>  	CTRL_MISC,
>  	CTRL_PERFEVENT,
>  	CTRL_DEBUG,
> -	CTRL_RDMA
> +	CTRL_RDMA,
> +	CTRL_PSEUDO
>  };
> -#define CTRLS_MAX CTRL_RDMA
> +#define CTRLS_MAX CTRL_PSEUDO
>  
>  /* At most we can have one cgroup V1 tree for each controller and one
>   * (empty) v2 tree.
> @@ -259,6 +260,10 @@ static const struct cgroup_file rdma_ctrl_files[] = {
>  	{ }
>  };
>  
> +static const struct cgroup_file pseudo_ctrl_files[] = {
> +	{ }
> +};
> +
>  #define CTRL_NAME_MAX 31
>  #define CGROUP_CTRL_MEMBER(x, y)[y] = { .ctrl_name = #x, .files = \
>  	x ## _ctrl_files, .ctrl_indx = y, NULL, 0 }
> @@ -282,6 +287,7 @@ static struct cgroup_ctrl controllers[] = {
>  	CGROUP_CTRL_MEMBER(perf_event, CTRL_PERFEVENT),
>  	CGROUP_CTRL_MEMBER(debug, CTRL_DEBUG),
>  	CGROUP_CTRL_MEMBER(rdma, CTRL_RDMA),
> +	CGROUP_CTRL_MEMBER(pseudo, CTRL_PSEUDO),
>  	{ }
>  };
>  
> @@ -798,6 +804,10 @@ void tst_cg_require(const char *const ctrl_name,
>  	const char *const cgsc = "cgroup.subtree_control";
>  	struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
>  	struct cgroup_root *root;
> +	int pseudo = !strcmp(ctrl->ctrl_name, "pseudo");
> +
> +	if (pseudo && options->needs_ver != TST_CG_V2)
> +		tst_brk(TCONF, "pseudo control only support needs_ver TST_CG_V2!");
>  
>  	if (!ctrl) {
>  		tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
> @@ -827,6 +837,9 @@ void tst_cg_require(const char *const ctrl_name,
>  	if (options->needs_ver != TST_CG_V2)
>  		cgroup_mount_v1(ctrl);
>  
> +	if (pseudo)
> +		ctrl->ctrl_root = roots;
> +
>  	if (!ctrl->ctrl_root) {
>  		tst_brk(TCONF,
>  			"'%s' controller required, but not available",
> @@ -849,13 +862,13 @@ mkdirs:
>  			ctrl->ctrl_name);
>  	}
>  
> -	if (cgroup_ctrl_on_v2(ctrl)) {
> +	if (cgroup_ctrl_on_v2(ctrl) && !pseudo) {
>  		if (root->we_mounted_it) {
>  			SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
> -					   cgsc, "+%s", ctrl->ctrl_name);
> +					cgsc, "+%s", ctrl->ctrl_name);
>  		} else {
>  			tst_file_printfat(root->mnt_dir.dir_fd,
> -					  cgsc, "+%s", ctrl->ctrl_name);
> +					cgsc, "+%s", ctrl->ctrl_name);
>  		}
>  	}
>  
> @@ -864,15 +877,17 @@ mkdirs:
>  	else
>  		root->ltp_dir.ctrl_field |= root->mnt_dir.ctrl_field;
>  
> -	if (cgroup_ctrl_on_v2(ctrl)) {
> -		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> -				   cgsc, "+%s", ctrl->ctrl_name);
> -	} else {
> -		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> -				   "cgroup.clone_children", "%d", 1);
> +	if (!pseudo) {
> +		if (cgroup_ctrl_on_v2(ctrl)) {
> +			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> +					cgsc, "+%s", ctrl->ctrl_name);
> +		} else {
> +			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
> +					"cgroup.clone_children", "%d", 1);
>  
> -		if (ctrl->ctrl_indx == CTRL_CPUSET)
> -			cgroup_copy_cpuset(root);
> +			if (ctrl->ctrl_indx == CTRL_CPUSET)
> +				cgroup_copy_cpuset(root);
> +		}
>  	}
>  
>  	cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);
> @@ -1050,8 +1065,10 @@ static void cgroup_group_add_dir(const struct tst_cg_group *const parent,
>  		if (!parent || dir->dir_root->ver == TST_CG_V1)
>  			continue;
>  
> -		SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
> -				   "+%s", ctrl->ctrl_name);
> +		if (strcmp(ctrl->ctrl_name, "pseudo")) {
> +			SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
> +					"+%s", ctrl->ctrl_name);
> +		}
>  	}
>  
>  	for (i = 0; cg->dirs[i]; i++)
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Shivani Samala]

Please remove me from your mailing list. I’m not from your team. Why are
you sending all this to me. Just remove me from your list.

On Wed, 26 Apr 2023 at 6:40 PM, Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > ---
> >  lib/tst_cgroup.c                              |   1 +
> >  runtest/controllers                           |   1 +
> >  .../kernel/controllers/cgroup/.gitignore      |   1 +
> >  .../kernel/controllers/cgroup/cgroup_core03.c | 144 ++++++++++++++++++
> >  4 files changed, 147 insertions(+)
> >  create mode 100644 testcases/kernel/controllers/cgroup/cgroup_core03.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..93c52c439 100644
> > --- a/runtest/controllers
> > +++ b/runtest/controllers
> > @@ -1,6 +1,7 @@
> >  #DESCRIPTION:Resource Management testing
> >  cgroup_core01        cgroup_core01
> >  cgroup_core02        cgroup_core02
> > +cgroup_core03        cgroup_core03
> >  cgroup               cgroup_regression_test.sh
> >  memcg_regression     memcg_regression_test.sh
> >  memcg_test_3 memcg_test_3
> > diff --git a/testcases/kernel/controllers/cgroup/.gitignore
> b/testcases/kernel/controllers/cgroup/.gitignore
> > index 8deae77da..9f1d1ada9 100644
> > --- a/testcases/kernel/controllers/cgroup/.gitignore
> > +++ b/testcases/kernel/controllers/cgroup/.gitignore
> > @@ -2,3 +2,4 @@
> >  /cgroup_regression_getdelays
> >  /cgroup_core01
> >  /cgroup_core02
> > +/cgroup_core03
> > diff --git a/testcases/kernel/controllers/cgroup/cgroup_core03.c
> b/testcases/kernel/controllers/cgroup/cgroup_core03.c
> > new file mode 100644
> > index 000000000..2a6941c44
> > --- /dev/null
> > +++ b/testcases/kernel/controllers/cgroup/cgroup_core03.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);
> > +
>
> What's the reason we are waiting for reparenting here? Aren't these
> processes killed anyways? I suppose that we can just call pause() here
> instead.
>
> > +}
> > +
> > +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);
>
> I do not think that this is 100% correct, as it is we may end up calling
> the TST_CHECKOPOINT_WAKE(0) more than once here, since there is a race
> between increment and load. An if that happens only one of the calls to
> TST_CHECKPOINT_WAKE(0) will succeeds, while the rest would fail and
> break the test.
>
> First of all the tst_atomic_inc() returns the value after the increment,
> so that we can actually get the counter value without a race.
>
> Secondly I think that we can get rid of the atomic operations
> completely, we just have to loop N times over the TST_CHECKPOINT_WAIT()
> in the run() function and call TST_CHECKPOINT_WAKE() in each child
> instance. We can also add TST_CHECKPOINT_WAIT_NR() to the test library
> that would also take a number of wait calls passed as a parameter.
>
> > +             fn();
> > +     }
> > +
> > +     return pid;
> > +}
> > +
> > +static int cg_wait_for_proc_count(const struct tst_cg_group *cg, int
> count)
>
> The function does not wait for anything, so it should be called
> cg_count_procs() or similar.
>
> > +{
> > +     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));
>
> This is very minor, but I would make the function return the number of
> pids found in the cgroup and called it as:
>
>         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 < PID_NUM; i++)
> > +             TST_EXP_PASS_SILENT(wait_for_pid(pids[i]));
>
> Then we can do here:
>
>         TST_EXP_VAL(cg_count_procs(cg_child_test_simple), 0);
>
> > +     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 []){ "pseudo", NULL },
> > +     .needs_cgroup_ver = TST_CG_V2,
> > +     .needs_checkpoints = 1,
> > +};
> > --
> > 2.35.3
> >
> >
> > --
> > Mailing list info: https://lists.linux.it/listinfo/ltp
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>

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

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

[Cyril Hrubis]

Hi!
> Please remove me from your mailing list. I’m not from your team. Why are
> you sending all this to me. Just remove me from your list.

I'm a bit confused here, you apparenlty subscribed yourself to this
mailing list and then you are complaining that you are getting emails?

Moreover if you have subscribed yourself you should be able to
unsubscribe as well. No need to spam the mailing list with such
requests.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH v10 0/2] kill01: New case cgroup kill

[Wei Gao via ltp]

Wei Gao (2):
  kill01: New case cgroup kill
  tst_cgroup.c: Add a cgroup base controller

 lib/tst_cgroup.c                              |  48 ++++---
 runtest/controllers                           |   1 +
 .../kernel/controllers/cgroup/.gitignore      |   1 +
 .../kernel/controllers/cgroup/cgroup_core03.c | 125 ++++++++++++++++++
 4 files changed, 160 insertions(+), 15 deletions(-)
 create mode 100644 testcases/kernel/controllers/cgroup/cgroup_core03.c


base-commit: b67facdd9c3f0ef471d74dd59ac276139e63f4c4
-- 
2.35.3


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

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

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c                              |   1 +
 runtest/controllers                           |   1 +
 .../kernel/controllers/cgroup/.gitignore      |   1 +
 .../kernel/controllers/cgroup/cgroup_core03.c | 125 ++++++++++++++++++
 4 files changed, 128 insertions(+)
 create mode 100644 testcases/kernel/controllers/cgroup/cgroup_core03.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..93c52c439 100644
--- a/runtest/controllers
+++ b/runtest/controllers
@@ -1,6 +1,7 @@
 #DESCRIPTION:Resource Management testing
 cgroup_core01	cgroup_core01
 cgroup_core02	cgroup_core02
+cgroup_core03	cgroup_core03
 cgroup		cgroup_regression_test.sh
 memcg_regression	memcg_regression_test.sh
 memcg_test_3	memcg_test_3
diff --git a/testcases/kernel/controllers/cgroup/.gitignore b/testcases/kernel/controllers/cgroup/.gitignore
index 8deae77da..9f1d1ada9 100644
--- a/testcases/kernel/controllers/cgroup/.gitignore
+++ b/testcases/kernel/controllers/cgroup/.gitignore
@@ -2,3 +2,4 @@
 /cgroup_regression_getdelays
 /cgroup_core01
 /cgroup_core02
+/cgroup_core03
diff --git a/testcases/kernel/controllers/cgroup/cgroup_core03.c b/testcases/kernel/controllers/cgroup/cgroup_core03.c
new file mode 100644
index 000000000..ed876fff3
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/cgroup_core03.c
@@ -0,0 +1,125 @@
+// 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;
+}
+
+static int cg_run_nowait(const struct tst_cg_group *const cg)
+{
+	int pid;
+
+	pid = SAFE_FORK();
+	if (pid == 0) {
+		SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
+		if (tst_atomic_inc(data_ptr) == PID_NUM)
+			TST_CHECKPOINT_WAKE(0);
+		pause();
+	}
+
+	return pid;
+}
+
+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 nr;
+}
+
+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);
+
+	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 < PID_NUM; i++)
+		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]));
+
+	TST_EXP_VAL(cg_count_procs(cg_child_test_simple), 0);
+	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 []){ "base", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+};
-- 
2.35.3


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

[LTP] [PATCH v10 2/2] tst_cgroup.c: Add a cgroup base controller

[Wei Gao via ltp]

For new test case such as kill01.c no need specific controller, it just
need LTP cgroup library start work, so we need add a "cgroup" base
controller.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c | 47 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 77575431d..d08a31651 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
 	CTRL_MISC,
 	CTRL_PERFEVENT,
 	CTRL_DEBUG,
-	CTRL_RDMA
+	CTRL_RDMA,
+	CTRL_BASE
 };
-#define CTRLS_MAX CTRL_RDMA
+#define CTRLS_MAX CTRL_BASE
 
 /* At most we can have one cgroup V1 tree for each controller and one
  * (empty) v2 tree.
@@ -259,6 +260,10 @@ static const struct cgroup_file rdma_ctrl_files[] = {
 	{ }
 };
 
+static const struct cgroup_file base_ctrl_files[] = {
+	{ }
+};
+
 #define CTRL_NAME_MAX 31
 #define CGROUP_CTRL_MEMBER(x, y)[y] = { .ctrl_name = #x, .files = \
 	x ## _ctrl_files, .ctrl_indx = y, NULL, 0 }
@@ -282,6 +287,7 @@ static struct cgroup_ctrl controllers[] = {
 	CGROUP_CTRL_MEMBER(perf_event, CTRL_PERFEVENT),
 	CGROUP_CTRL_MEMBER(debug, CTRL_DEBUG),
 	CGROUP_CTRL_MEMBER(rdma, CTRL_RDMA),
+	CGROUP_CTRL_MEMBER(base, CTRL_BASE),
 	{ }
 };
 
@@ -798,6 +804,10 @@ void tst_cg_require(const char *const ctrl_name,
 	const char *const cgsc = "cgroup.subtree_control";
 	struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
 	struct cgroup_root *root;
+	int base = !strcmp(ctrl->ctrl_name, "base");
+
+	if (base && options->needs_ver != TST_CG_V2)
+		tst_brk(TCONF, "Base control only support needs_ver TST_CG_V2!");
 
 	if (!ctrl) {
 		tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
@@ -827,6 +837,9 @@ void tst_cg_require(const char *const ctrl_name,
 	if (options->needs_ver != TST_CG_V2)
 		cgroup_mount_v1(ctrl);
 
+	if (base)
+		ctrl->ctrl_root = roots;
+
 	if (!ctrl->ctrl_root) {
 		tst_brk(TCONF,
 			"'%s' controller required, but not available",
@@ -849,13 +862,13 @@ mkdirs:
 			ctrl->ctrl_name);
 	}
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
+	if (cgroup_ctrl_on_v2(ctrl) && !base) {
 		if (root->we_mounted_it) {
 			SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
-					   cgsc, "+%s", ctrl->ctrl_name);
+					cgsc, "+%s", ctrl->ctrl_name);
 		} else {
 			tst_file_printfat(root->mnt_dir.dir_fd,
-					  cgsc, "+%s", ctrl->ctrl_name);
+					cgsc, "+%s", ctrl->ctrl_name);
 		}
 	}
 
@@ -864,15 +877,17 @@ mkdirs:
 	else
 		root->ltp_dir.ctrl_field |= root->mnt_dir.ctrl_field;
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   cgsc, "+%s", ctrl->ctrl_name);
-	} else {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   "cgroup.clone_children", "%d", 1);
+	if (!base) {
+		if (cgroup_ctrl_on_v2(ctrl)) {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					cgsc, "+%s", ctrl->ctrl_name);
+		} else {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					"cgroup.clone_children", "%d", 1);
 
-		if (ctrl->ctrl_indx == CTRL_CPUSET)
-			cgroup_copy_cpuset(root);
+			if (ctrl->ctrl_indx == CTRL_CPUSET)
+				cgroup_copy_cpuset(root);
+		}
 	}
 
 	cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);
@@ -1050,8 +1065,10 @@ static void cgroup_group_add_dir(const struct tst_cg_group *const parent,
 		if (!parent || dir->dir_root->ver == TST_CG_V1)
 			continue;
 
-		SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
-				   "+%s", ctrl->ctrl_name);
+		if (strcmp(ctrl->ctrl_name, "base")) {
+			SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
+					"+%s", ctrl->ctrl_name);
+		}
 	}
 
 	for (i = 0; cg->dirs[i]; i++)
-- 
2.35.3


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

Re: [LTP] [PATCH v10 2/2] tst_cgroup.c: Add a cgroup base controller

[Petr Vorel]

Hi Wei,

>  lib/tst_cgroup.c | 47 ++++++++++++++++++++++++++++++++---------------
...
> @@ -798,6 +804,10 @@ void tst_cg_require(const char *const ctrl_name,
>  	const char *const cgsc = "cgroup.subtree_control";
>  	struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
>  	struct cgroup_root *root;
> +	int base = !strcmp(ctrl->ctrl_name, "base");
> +
> +	if (base && options->needs_ver != TST_CG_V2)
> +		tst_brk(TCONF, "Base control only support needs_ver TST_CG_V2!");

>  	if (!ctrl) {
>  		tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
> @@ -827,6 +837,9 @@ void tst_cg_require(const char *const ctrl_name,
>  	if (options->needs_ver != TST_CG_V2)
>  		cgroup_mount_v1(ctrl);

> +	if (base)
> +		ctrl->ctrl_root = roots;
> +
>  	if (!ctrl->ctrl_root) {
>  		tst_brk(TCONF,
>  			"'%s' controller required, but not available",
> @@ -849,13 +862,13 @@ mkdirs:
>  			ctrl->ctrl_name);
>  	}

> -	if (cgroup_ctrl_on_v2(ctrl)) {
> +	if (cgroup_ctrl_on_v2(ctrl) && !base) {
>  		if (root->we_mounted_it) {
>  			SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
> -					   cgsc, "+%s", ctrl->ctrl_name);
> +					cgsc, "+%s", ctrl->ctrl_name);
This is just unneeded whitespace change, it should be reverted.
I guess you changed it just by accident, right?
>  		} else {
>  			tst_file_printfat(root->mnt_dir.dir_fd,
> -					  cgsc, "+%s", ctrl->ctrl_name);
> +					cgsc, "+%s", ctrl->ctrl_name);
Here as well.

Kind regards,
Petr
>  		}
>  	}
...


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

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

[Petr Vorel]

Hi Wei,

this commit should be second, after "tst_cgroup.c: Add a cgroup base
controller". Because it depends on it. With the current (wrong) order,
it compiles, but it fails in runtime:

$ ./cgroup_core03
tst_cgroup.c:803: TBROK: 'base' controller is unknown to LTP

We try to have each commit "working": 1) possible to compile
2) test actually working. This is needed when we try to search for error with
git bisect.

You can mention in "tst_cgroup.c: Add a cgroup base controller",
that it will be needed in the test added in following commit.

Kind regards,
Petr

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

[LTP] [PATCH v11 0/2] New case test cgroup kill feature

[Wei Gao via ltp]

Wei Gao (2):
  tst_cgroup.c: Add a cgroup base controller
  cgroup_core03.c: New case test cgroup kill feature

 lib/tst_cgroup.c                              |  44 ++++--
 runtest/controllers                           |   1 +
 .../kernel/controllers/cgroup/.gitignore      |   1 +
 .../kernel/controllers/cgroup/cgroup_core03.c | 125 ++++++++++++++++++
 4 files changed, 158 insertions(+), 13 deletions(-)
 create mode 100644 testcases/kernel/controllers/cgroup/cgroup_core03.c


base-commit: b67facdd9c3f0ef471d74dd59ac276139e63f4c4
-- 
2.35.3


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

[LTP] [PATCH v11 1/2] tst_cgroup.c: Add a cgroup base controller

[Wei Gao via ltp]

For new test case such as kill01.c no need specific controller, it just
need LTP cgroup library start work, so we need add a "cgroup" base
controller.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 50699bc63..249f25aa4 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
 	CTRL_MISC,
 	CTRL_PERFEVENT,
 	CTRL_DEBUG,
-	CTRL_RDMA
+	CTRL_RDMA,
+	CTRL_BASE
 };
-#define CTRLS_MAX CTRL_RDMA
+#define CTRLS_MAX CTRL_BASE
 
 /* At most we can have one cgroup V1 tree for each controller and one
  * (empty) v2 tree.
@@ -258,6 +259,10 @@ static const struct cgroup_file rdma_ctrl_files[] = {
 	{ }
 };
 
+static const struct cgroup_file base_ctrl_files[] = {
+	{ }
+};
+
 #define CTRL_NAME_MAX 31
 #define CGROUP_CTRL_MEMBER(x, y)[y] = { .ctrl_name = #x, .files = \
 	x ## _ctrl_files, .ctrl_indx = y, NULL, 0 }
@@ -281,6 +286,7 @@ static struct cgroup_ctrl controllers[] = {
 	CGROUP_CTRL_MEMBER(perf_event, CTRL_PERFEVENT),
 	CGROUP_CTRL_MEMBER(debug, CTRL_DEBUG),
 	CGROUP_CTRL_MEMBER(rdma, CTRL_RDMA),
+	CGROUP_CTRL_MEMBER(base, CTRL_BASE),
 	{ }
 };
 
@@ -797,6 +803,10 @@ void tst_cg_require(const char *const ctrl_name,
 	const char *const cgsc = "cgroup.subtree_control";
 	struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
 	struct cgroup_root *root;
+	int base = !strcmp(ctrl->ctrl_name, "base");
+
+	if (base && options->needs_ver != TST_CG_V2)
+		tst_brk(TCONF, "Base control only support needs_ver TST_CG_V2!");
 
 	if (!ctrl) {
 		tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
@@ -826,6 +836,9 @@ void tst_cg_require(const char *const ctrl_name,
 	if (options->needs_ver != TST_CG_V2)
 		cgroup_mount_v1(ctrl);
 
+	if (base)
+		ctrl->ctrl_root = roots;
+
 	if (!ctrl->ctrl_root) {
 		tst_brk(TCONF,
 			"'%s' controller required, but not available",
@@ -848,7 +861,7 @@ mkdirs:
 			ctrl->ctrl_name);
 	}
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
+	if (cgroup_ctrl_on_v2(ctrl) && !base) {
 		if (root->we_mounted_it) {
 			SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
 					   cgsc, "+%s", ctrl->ctrl_name);
@@ -863,15 +876,17 @@ mkdirs:
 	else
 		root->ltp_dir.ctrl_field |= root->mnt_dir.ctrl_field;
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   cgsc, "+%s", ctrl->ctrl_name);
-	} else {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   "cgroup.clone_children", "%d", 1);
+	if (!base) {
+		if (cgroup_ctrl_on_v2(ctrl)) {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					cgsc, "+%s", ctrl->ctrl_name);
+		} else {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					"cgroup.clone_children", "%d", 1);
 
-		if (ctrl->ctrl_indx == CTRL_CPUSET)
-			cgroup_copy_cpuset(root);
+			if (ctrl->ctrl_indx == CTRL_CPUSET)
+				cgroup_copy_cpuset(root);
+		}
 	}
 
 	cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);
@@ -1049,8 +1064,10 @@ static void cgroup_group_add_dir(const struct tst_cg_group *const parent,
 		if (!parent || dir->dir_root->ver == TST_CG_V1)
 			continue;
 
-		SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
-				   "+%s", ctrl->ctrl_name);
+		if (strcmp(ctrl->ctrl_name, "base")) {
+			SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
+					"+%s", ctrl->ctrl_name);
+		}
 	}
 
 	for (i = 0; cg->dirs[i]; i++)
-- 
2.35.3


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

[LTP] [PATCH v11 2/2] cgroup_core03.c: New case test cgroup kill feature

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c                              |   1 +
 runtest/controllers                           |   1 +
 .../kernel/controllers/cgroup/.gitignore      |   1 +
 .../kernel/controllers/cgroup/cgroup_core03.c | 125 ++++++++++++++++++
 4 files changed, 128 insertions(+)
 create mode 100644 testcases/kernel/controllers/cgroup/cgroup_core03.c

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 249f25aa4..8f7204340 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -167,6 +167,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..93c52c439 100644
--- a/runtest/controllers
+++ b/runtest/controllers
@@ -1,6 +1,7 @@
 #DESCRIPTION:Resource Management testing
 cgroup_core01	cgroup_core01
 cgroup_core02	cgroup_core02
+cgroup_core03	cgroup_core03
 cgroup		cgroup_regression_test.sh
 memcg_regression	memcg_regression_test.sh
 memcg_test_3	memcg_test_3
diff --git a/testcases/kernel/controllers/cgroup/.gitignore b/testcases/kernel/controllers/cgroup/.gitignore
index 8deae77da..9f1d1ada9 100644
--- a/testcases/kernel/controllers/cgroup/.gitignore
+++ b/testcases/kernel/controllers/cgroup/.gitignore
@@ -2,3 +2,4 @@
 /cgroup_regression_getdelays
 /cgroup_core01
 /cgroup_core02
+/cgroup_core03
diff --git a/testcases/kernel/controllers/cgroup/cgroup_core03.c b/testcases/kernel/controllers/cgroup/cgroup_core03.c
new file mode 100644
index 000000000..ed876fff3
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/cgroup_core03.c
@@ -0,0 +1,125 @@
+// 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;
+}
+
+static int cg_run_nowait(const struct tst_cg_group *const cg)
+{
+	int pid;
+
+	pid = SAFE_FORK();
+	if (pid == 0) {
+		SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
+		if (tst_atomic_inc(data_ptr) == PID_NUM)
+			TST_CHECKPOINT_WAKE(0);
+		pause();
+	}
+
+	return pid;
+}
+
+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 nr;
+}
+
+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);
+
+	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 < PID_NUM; i++)
+		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]));
+
+	TST_EXP_VAL(cg_count_procs(cg_child_test_simple), 0);
+	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 []){ "base", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+};
-- 
2.35.3


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

[LTP] [PATCH v12 0/2] cgroup_core03.c: New case test cgroup kill feature

[Wei Gao via ltp]

Wei Gao (2):
  tst_cgroup.c: Add a cgroup base controller
  cgroup_core03.c: New case test cgroup kill feature

 lib/tst_cgroup.c                              |  44 ++++--
 runtest/controllers                           |   1 +
 .../kernel/controllers/cgroup/.gitignore      |   1 +
 .../kernel/controllers/cgroup/cgroup_core03.c | 130 ++++++++++++++++++
 4 files changed, 163 insertions(+), 13 deletions(-)
 create mode 100644 testcases/kernel/controllers/cgroup/cgroup_core03.c


base-commit: b67facdd9c3f0ef471d74dd59ac276139e63f4c4
-- 
2.35.3


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

[LTP] [PATCH v12 1/2] tst_cgroup.c: Add a cgroup base controller

[Wei Gao via ltp]

For new test case such as kill01.c no need specific controller, it just
need LTP cgroup library start work, so we need add a "cgroup" base
controller.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 lib/tst_cgroup.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 50699bc63..249f25aa4 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -94,9 +94,10 @@ enum cgroup_ctrl_indx {
 	CTRL_MISC,
 	CTRL_PERFEVENT,
 	CTRL_DEBUG,
-	CTRL_RDMA
+	CTRL_RDMA,
+	CTRL_BASE
 };
-#define CTRLS_MAX CTRL_RDMA
+#define CTRLS_MAX CTRL_BASE
 
 /* At most we can have one cgroup V1 tree for each controller and one
  * (empty) v2 tree.
@@ -258,6 +259,10 @@ static const struct cgroup_file rdma_ctrl_files[] = {
 	{ }
 };
 
+static const struct cgroup_file base_ctrl_files[] = {
+	{ }
+};
+
 #define CTRL_NAME_MAX 31
 #define CGROUP_CTRL_MEMBER(x, y)[y] = { .ctrl_name = #x, .files = \
 	x ## _ctrl_files, .ctrl_indx = y, NULL, 0 }
@@ -281,6 +286,7 @@ static struct cgroup_ctrl controllers[] = {
 	CGROUP_CTRL_MEMBER(perf_event, CTRL_PERFEVENT),
 	CGROUP_CTRL_MEMBER(debug, CTRL_DEBUG),
 	CGROUP_CTRL_MEMBER(rdma, CTRL_RDMA),
+	CGROUP_CTRL_MEMBER(base, CTRL_BASE),
 	{ }
 };
 
@@ -797,6 +803,10 @@ void tst_cg_require(const char *const ctrl_name,
 	const char *const cgsc = "cgroup.subtree_control";
 	struct cgroup_ctrl *const ctrl = cgroup_find_ctrl(ctrl_name);
 	struct cgroup_root *root;
+	int base = !strcmp(ctrl->ctrl_name, "base");
+
+	if (base && options->needs_ver != TST_CG_V2)
+		tst_brk(TCONF, "Base control only support needs_ver TST_CG_V2!");
 
 	if (!ctrl) {
 		tst_brk(TBROK, "'%s' controller is unknown to LTP", ctrl_name);
@@ -826,6 +836,9 @@ void tst_cg_require(const char *const ctrl_name,
 	if (options->needs_ver != TST_CG_V2)
 		cgroup_mount_v1(ctrl);
 
+	if (base)
+		ctrl->ctrl_root = roots;
+
 	if (!ctrl->ctrl_root) {
 		tst_brk(TCONF,
 			"'%s' controller required, but not available",
@@ -848,7 +861,7 @@ mkdirs:
 			ctrl->ctrl_name);
 	}
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
+	if (cgroup_ctrl_on_v2(ctrl) && !base) {
 		if (root->we_mounted_it) {
 			SAFE_FILE_PRINTFAT(root->mnt_dir.dir_fd,
 					   cgsc, "+%s", ctrl->ctrl_name);
@@ -863,15 +876,17 @@ mkdirs:
 	else
 		root->ltp_dir.ctrl_field |= root->mnt_dir.ctrl_field;
 
-	if (cgroup_ctrl_on_v2(ctrl)) {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   cgsc, "+%s", ctrl->ctrl_name);
-	} else {
-		SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
-				   "cgroup.clone_children", "%d", 1);
+	if (!base) {
+		if (cgroup_ctrl_on_v2(ctrl)) {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					cgsc, "+%s", ctrl->ctrl_name);
+		} else {
+			SAFE_FILE_PRINTFAT(root->ltp_dir.dir_fd,
+					"cgroup.clone_children", "%d", 1);
 
-		if (ctrl->ctrl_indx == CTRL_CPUSET)
-			cgroup_copy_cpuset(root);
+			if (ctrl->ctrl_indx == CTRL_CPUSET)
+				cgroup_copy_cpuset(root);
+		}
 	}
 
 	cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);
@@ -1049,8 +1064,10 @@ static void cgroup_group_add_dir(const struct tst_cg_group *const parent,
 		if (!parent || dir->dir_root->ver == TST_CG_V1)
 			continue;
 
-		SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
-				   "+%s", ctrl->ctrl_name);
+		if (strcmp(ctrl->ctrl_name, "base")) {
+			SAFE_CG_PRINTF(parent, "cgroup.subtree_control",
+					"+%s", ctrl->ctrl_name);
+		}
 	}
 
 	for (i = 0; cg->dirs[i]; i++)
-- 
2.35.3


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

[LTP] [PATCH v12 2/2] cgroup_core03.c: New case test cgroup kill feature

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
v11->v12: skip the test gracefully if there is no 'cgroup.kill' interface.

 lib/tst_cgroup.c                              |   1 +
 runtest/controllers                           |   1 +
 .../kernel/controllers/cgroup/.gitignore      |   1 +
 .../kernel/controllers/cgroup/cgroup_core03.c | 130 ++++++++++++++++++
 4 files changed, 133 insertions(+)
 create mode 100644 testcases/kernel/controllers/cgroup/cgroup_core03.c

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 249f25aa4..8f7204340 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -167,6 +167,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..93c52c439 100644
--- a/runtest/controllers
+++ b/runtest/controllers
@@ -1,6 +1,7 @@
 #DESCRIPTION:Resource Management testing
 cgroup_core01	cgroup_core01
 cgroup_core02	cgroup_core02
+cgroup_core03	cgroup_core03
 cgroup		cgroup_regression_test.sh
 memcg_regression	memcg_regression_test.sh
 memcg_test_3	memcg_test_3
diff --git a/testcases/kernel/controllers/cgroup/.gitignore b/testcases/kernel/controllers/cgroup/.gitignore
index 8deae77da..9f1d1ada9 100644
--- a/testcases/kernel/controllers/cgroup/.gitignore
+++ b/testcases/kernel/controllers/cgroup/.gitignore
@@ -2,3 +2,4 @@
 /cgroup_regression_getdelays
 /cgroup_core01
 /cgroup_core02
+/cgroup_core03
diff --git a/testcases/kernel/controllers/cgroup/cgroup_core03.c b/testcases/kernel/controllers/cgroup/cgroup_core03.c
new file mode 100644
index 000000000..6ee6deafe
--- /dev/null
+++ b/testcases/kernel/controllers/cgroup/cgroup_core03.c
@@ -0,0 +1,130 @@
+// 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;
+}
+
+static int cg_run_nowait(const struct tst_cg_group *const cg)
+{
+	int pid;
+
+	pid = SAFE_FORK();
+	if (pid == 0) {
+		SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
+		if (tst_atomic_inc(data_ptr) == PID_NUM)
+			TST_CHECKPOINT_WAKE(0);
+		pause();
+	}
+
+	return pid;
+}
+
+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 nr;
+}
+
+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");
+
+	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");
+	}
+
+	memset(buf, 0, BUF_LEN);
+
+	for (i = 0; i < PID_NUM; i++)
+		pids[i] = cg_run_nowait(cg_child_test_simple);
+
+	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 < PID_NUM; i++)
+		TST_EXP_PASS_SILENT(wait_for_pid(pids[i]));
+
+	TST_EXP_VAL(cg_count_procs(cg_child_test_simple), 0);
+	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 []){ "base", NULL },
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_checkpoints = 1,
+};
-- 
2.35.3


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

Re: [LTP] [PATCH v12 1/2] tst_cgroup.c: Add a cgroup base controller

[Li Wang]

Hi Wei,

Seems we have to skip the cgroup.subtree_control cleanup
process inside tst_cg_cleanup(), otherwise, test complains that
there is error operation in line#1020 if the unified cgroup-V2
was mounted by LTP.

(no need to resend another patch, someone who merge this can help fix.)

--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -1012,7 +1012,8 @@ void tst_cg_cleanup(void)
        }

        for_each_ctrl(ctrl) {
-               if (!cgroup_ctrl_on_v2(ctrl) ||
!ctrl->ctrl_root->we_mounted_it)
+               if (!cgroup_ctrl_on_v2(ctrl) ||
!ctrl->ctrl_root->we_mounted_it
+                               || !strcmp(ctrl->ctrl_name, "base"))
                        continue;

                SAFE_FILE_PRINTFAT(ctrl->ctrl_root->mnt_dir.dir_fd,

With above fix:
Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v12 2/2] cgroup_core03.c: New case test cgroup kill feature

[Li Wang]

Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v12 1/2] tst_cgroup.c: Add a cgroup base controller

[Wei Gao via ltp]

On Sun, Apr 30, 2023 at 09:44:08PM +0800, Li Wang wrote:
> Hi Wei,
> 
> Seems we have to skip the cgroup.subtree_control cleanup
> process inside tst_cg_cleanup(), otherwise, test complains that
> there is error operation in line#1020 if the unified cgroup-V2
> was mounted by LTP.
> 
> (no need to resend another patch, someone who merge this can help fix.)
Thanks a lot :)
> 
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -1012,7 +1012,8 @@ void tst_cg_cleanup(void)
>         }
> 
>         for_each_ctrl(ctrl) {
> -               if (!cgroup_ctrl_on_v2(ctrl) ||
> !ctrl->ctrl_root->we_mounted_it)
> +               if (!cgroup_ctrl_on_v2(ctrl) ||
> !ctrl->ctrl_root->we_mounted_it
> +                               || !strcmp(ctrl->ctrl_name, "base"))
>                         continue;
> 
>                 SAFE_FILE_PRINTFAT(ctrl->ctrl_root->mnt_dir.dir_fd,
> 
> With above fix:
> Reviewed-by: Li Wang <liwang@redhat.com>
> 
> 
> -- 
> Regards,
> Li Wang

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

Re: [LTP] [PATCH v12 1/2] tst_cgroup.c: Add a cgroup base controller

[Petr Vorel]

Hi Wei, Li,

> Hi Wei,

> Seems we have to skip the cgroup.subtree_control cleanup
> process inside tst_cg_cleanup(), otherwise, test complains that
> there is error operation in line#1020 if the unified cgroup-V2
> was mounted by LTP.

> (no need to resend another patch, someone who merge this can help fix.)

> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -1012,7 +1012,8 @@ void tst_cg_cleanup(void)
>         }

>         for_each_ctrl(ctrl) {
> -               if (!cgroup_ctrl_on_v2(ctrl) ||
> !ctrl->ctrl_root->we_mounted_it)
> +               if (!cgroup_ctrl_on_v2(ctrl) ||
> !ctrl->ctrl_root->we_mounted_it
> +                               || !strcmp(ctrl->ctrl_name, "base"))
>                         continue;

Agree.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

>                 SAFE_FILE_PRINTFAT(ctrl->ctrl_root->mnt_dir.dir_fd,

> With above fix:

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

Re: [LTP] [PATCH v12 1/2] tst_cgroup.c: Add a cgroup base controller

[Petr Vorel]

Hi Li, Wei,

> Hi Wei,

> Seems we have to skip the cgroup.subtree_control cleanup
> process inside tst_cg_cleanup(), otherwise, test complains that
> there is error operation in line#1020 if the unified cgroup-V2
> was mounted by LTP.

> (no need to resend another patch, someone who merge this can help fix.)

> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -1012,7 +1012,8 @@ void tst_cg_cleanup(void)
>         }

>         for_each_ctrl(ctrl) {
> -               if (!cgroup_ctrl_on_v2(ctrl) ||
> !ctrl->ctrl_root->we_mounted_it)
> +               if (!cgroup_ctrl_on_v2(ctrl) ||
> !ctrl->ctrl_root->we_mounted_it
> +                               || !strcmp(ctrl->ctrl_name, "base"))
>                         continue;

>                 SAFE_FILE_PRINTFAT(ctrl->ctrl_root->mnt_dir.dir_fd,

Merged with this change. Thanks to both!

Kind regards,
Petr

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