All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v1 1/4] lib: add new cgroup test API
@ 2020-05-27  3:14 Li Wang
  2020-05-27  3:14 ` [LTP] [PATCH v1 2/4] mem: take use of new cgroup API Li Wang
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Li Wang @ 2020-05-27  3:14 UTC (permalink / raw)
  To: ltp

Many of our LTP tests need Control Group in the configuration,
this patch makes cgroup unified mounting at setup phase to be
possible. The method?is extracted from mem.h with the purpose
of?extendible for further developing, and trying?to compatible
the current two versions of cgroup,

It's hard to make all APIs be strictly consistent because there
are many differences between v1 and v2. But it?capsulate the detail
of cgroup mounting in high-level functions, which will be easier
to use cgroup without considering too much technical thing.? ?

Btw, test get passed on RHEL7(x86_64), RHEL8(ppc64le), Fedora32(x86_64).

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

Notes:
    RFC --> V1:
      * hide cgroup mount/umount with internal functions:
    	static void tst_cgroup1_mount(const char *name, ...)
    	static void tst_cgroup1_umount(const char *name, ...)
    	static void tst_cgroup2_mount(const char *name, ...)
    	static void tst_cgroup2_umount(const char *name, ...)
    
      * export unified cgroup APIs to LTP test users:
    	void tst_cgroup_mount(enum tst_cgroup_ctrl ctrl);
    	void tst_cgroup_umount(enum tst_cgroup_ctrl ctrl);
    	void tst_cgroup_move_current(enum tst_cgroup_ctrl ctrl);
    	void tst_cgroup_mem_set_maxbytes(long memsz);
    	void tst_cgroup_mem_set_maxswap(long memsz);

 doc/test-writing-guidelines.txt |  43 +++++
 include/tst_cgroup.h            |  45 ++++++
 include/tst_test.h              |   1 +
 lib/tst_cgroup.c                | 270 ++++++++++++++++++++++++++++++++
 4 files changed, 359 insertions(+)
 create mode 100644 include/tst_cgroup.h
 create mode 100644 lib/tst_cgroup.c

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index 93ca506d9..514739fa6 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -2053,6 +2053,49 @@ the prefix field of file pointed by path equals to the value passed to this func
 Also having a similar api pair TST_ASSERT_FILE_INT/STR(path, prefix, val) to assert
 the field value of file.
 
+2.2.36 Using Control Group
+^^^^^^^^^^^^^^^^^^^^^^^^^^
+Some of LTP tests need Control Group in their configuration, tst_cgroup.h provides
+APIs to make cgroup unified mounting at setup phase to be possible. The method?is
+extracted from mem.h with the purpose of?extendible for further developing, and
+trying to compatible the current two versions of cgroup.
+
+Considering there are many differences between cgroup v1 and v2, here we capsulate
+the detail of cgroup mounting in high-level functions, which will be easier to use
+cgroup without caring about too much technical thing.? ?
+
+Also, we do cgroup mount/umount work for the different hierarchy automatically.
+
+[source,c]
+-------------------------------------------------------------------------------
+#include "tst_test.h"
+
+static void run(void)
+{
+	...
+
+	tst_cgroup_mem_set_maxbytes(MEMSIZE);
+
+	// do test under cgroup
+	...
+}
+
+static void setup(void)
+{
+	tst_cgroup_mount(TST_CGROUP_MEMCG);
+}
+
+static void cleanup(void)
+{
+	tst_cgroup_umount(TST_CGROUP_MEMCG);
+}
+
+struct tst_test test = {
+	.test_all = run,
+	...
+};
+
+-------------------------------------------------------------------------------
 2.3 Writing a testcase in shell
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
new file mode 100644
index 000000000..c6790f762
--- /dev/null
+++ b/include/tst_cgroup.h
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Red Hat, Inc.
+ */
+
+#ifndef TST_CGROUP_H
+#define TST_CGROUP_H
+
+#define PATH_SYS_CGROUP		"/sys/fs/cgroup"
+
+enum tst_cgroup_ver {
+	TST_CGROUP_V1 = 1,
+	TST_CGROUP_V2 = 2,
+};
+
+enum tst_cgroup_ctrl {
+	TST_CGROUP_MEMCG = 1,
+	TST_CGROUP_CPUSET = 2,
+	/* add cgroup controller */
+};
+
+/* cgroup v1 */
+#define PATH_TMP_CG1_MEM	"/tmp/cgroup1_mem"
+#define PATH_TMP_CG1_CST	"/tmp/cgroup1_cpuset"
+#define PATH_CG1_MEM_LTP	PATH_TMP_CG1_MEM "/ltp"
+#define PATH_CG1_CST_LTP	PATH_TMP_CG1_CST "/ltp"
+#define PATH_MEMORY_LIMIT	PATH_CG1_MEM_LTP "/memory.limit_in_bytes"
+#define PATH_MEMORY_SW_LIMIT	PATH_CG1_MEM_LTP "/memory.memsw.limit_in_bytes"
+/* cgroup v1 */
+
+/* cgroup v2 */
+#define PATH_TMP_CGROUP2	"/tmp/cgroup2"	/* cgroup v2 has only single hierarchy */
+#define PATH_TMP_CG2_LTP	PATH_TMP_CGROUP2 "/ltp"
+#define PATH_MEMORY_MAX		PATH_TMP_CG2_LTP "/memory.max"
+#define PATH_MEMORY_SW_MAX	PATH_TMP_CG2_LTP "/memory.swap.max"
+/* cgroup v2 */
+
+enum tst_cgroup_ver tst_cgroup_version(void);
+void tst_cgroup_mount(enum tst_cgroup_ctrl ctrl);
+void tst_cgroup_umount(enum tst_cgroup_ctrl ctrl);
+void tst_cgroup_move_current(enum tst_cgroup_ctrl ctrl);
+void tst_cgroup_mem_set_maxbytes(long memsz);
+void tst_cgroup_mem_set_maxswap(long memsz);
+
+#endif /* TST_CGROUP_H */
diff --git a/include/tst_test.h b/include/tst_test.h
index 5bedb4f6f..b84f7b9dd 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -39,6 +39,7 @@
 #include "tst_capability.h"
 #include "tst_hugepage.h"
 #include "tst_assert.h"
+#include "tst_cgroup.h"
 
 /*
  * Reports testcase result.
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
new file mode 100644
index 000000000..4228f2730
--- /dev/null
+++ b/lib/tst_cgroup.c
@@ -0,0 +1,270 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Red Hat, Inc.
+ */
+
+#define TST_NO_DEFAULT_MAIN
+
+#include <stdio.h>
+#include <sys/mount.h>
+
+#include "tst_test.h"
+#include "tst_safe_macros.h"
+#include "tst_safe_stdio.h"
+#include "tst_cgroup.h"
+#include "tst_device.h"
+
+static int tst_cgroup_check(const char *cgroup)
+{
+	char line[PATH_MAX];
+	FILE *file;
+	int cg_check = 0;
+
+	file = SAFE_FOPEN("/proc/filesystems", "r");
+	while (fgets(line, sizeof(line), file)) {
+		if (strstr(line, cgroup) != NULL) {
+			cg_check = 1;
+			break;
+		}
+	}
+	SAFE_FCLOSE(file);
+
+	return cg_check;
+}
+
+enum tst_cgroup_ver tst_cgroup_version(void)
+{
+	if (tst_cgroup_check("cgroup2")) {
+		if (!tst_is_mounted("cgroup2") && tst_is_mounted("cgroup"))
+			return TST_CGROUP_V1;
+		else
+			return TST_CGROUP_V2;
+	}
+
+	if (tst_cgroup_check("cgroup"))
+		return TST_CGROUP_V1;
+
+	tst_brk(TCONF, "Cgroup is not configured");
+}
+
+/* cgroup v1 */
+static void tst_cgroup1_mount(const char *name, const char *option,
+			const char *mnt_path, const char *new_path)
+{
+	if (tst_is_mounted(mnt_path))
+		goto out;
+
+	SAFE_MKDIR(mnt_path, 0777);
+	if (mount(name, mnt_path, "cgroup", 0, option) == -1) {
+		if (errno == ENODEV) {
+			if (rmdir(mnt_path) == -1)
+				tst_res(TWARN | TERRNO, "rmdir %s failed", mnt_path);
+			tst_brk(TCONF,
+				 "Cgroup v1 is not configured in kernel");
+		}
+		tst_brk(TBROK | TERRNO, "mount %s", mnt_path);
+	}
+
+out:
+	SAFE_MKDIR(new_path, 0777);
+
+	tst_res(TINFO, "Cgroup(%s) v1 mount at %s success", option, mnt_path);
+}
+
+static void tst_cgroup1_umount(const char *mnt_path, const char *new_path)
+{
+	FILE *fp;
+	int fd;
+	char s_new[BUFSIZ], s[BUFSIZ], value[BUFSIZ];
+
+	if (!tst_is_mounted(mnt_path))
+		return;
+
+	/* Move all processes in task to its parent node. */
+	sprintf(s, "%s/tasks", mnt_path);
+	fd = open(s, O_WRONLY);
+	if (fd == -1)
+		tst_res(TWARN | TERRNO, "open %s", s);
+
+	snprintf(s_new, BUFSIZ, "%s/tasks", new_path);
+	fp = fopen(s_new, "r");
+	if (fp == NULL)
+		tst_res(TWARN | TERRNO, "fopen %s", s_new);
+	if ((fd != -1) && (fp != NULL)) {
+		while (fgets(value, BUFSIZ, fp) != NULL)
+			if (write(fd, value, strlen(value) - 1)
+			    != (ssize_t)strlen(value) - 1)
+				tst_res(TWARN | TERRNO, "write %s", s);
+	}
+	if (fd != -1)
+		close(fd);
+	if (fp != NULL)
+		fclose(fp);
+	if (rmdir(new_path) == -1)
+		tst_res(TWARN | TERRNO, "rmdir %s", new_path);
+	if (umount(mnt_path) == -1)
+		tst_res(TWARN | TERRNO, "umount %s", mnt_path);
+	if (rmdir(mnt_path) == -1)
+		tst_res(TWARN | TERRNO, "rmdir %s", mnt_path);
+
+	tst_res(TINFO, "Cgroup v1 unmount success");
+}
+/* cgroup v1 */
+
+/* cgroup v2 */
+static void tst_cgroup2_mount(const char *mnt_path, const char *new_path)
+{
+	if (tst_is_mounted(mnt_path))
+		goto out;
+
+	SAFE_MKDIR(mnt_path, 0777);
+	if (mount("cgroup2", mnt_path, "cgroup2", 0, NULL) == -1) {
+		if (errno == ENODEV) {
+			if (rmdir(mnt_path) == -1)
+				tst_res(TWARN | TERRNO, "rmdir %s failed",
+					 mnt_path);
+			tst_brk(TCONF,
+				 "Cgroup v2 is not configured in kernel");
+		}
+		tst_brk(TBROK | TERRNO, "mount %s", mnt_path);
+	}
+
+out:
+	SAFE_MKDIR(new_path, 0777);
+
+	tst_res(TINFO, "Cgroup v2 mount at %s success", mnt_path);
+}
+
+static void tst_cgroup2_umount(const char *mnt_path, const char *new_path)
+{
+	FILE *fp;
+	int fd;
+	char s_new[BUFSIZ], s[BUFSIZ], value[BUFSIZ];
+
+	if (!tst_is_mounted(mnt_path))
+		return;
+
+	/* Move all processes in cgroup.procs to its parent node. */
+	sprintf(s, "%s/cgroup.procs", mnt_path);
+	fd = open(s, O_WRONLY);
+	if (fd == -1)
+		tst_res(TWARN | TERRNO, "open %s", s);
+
+	snprintf(s_new, BUFSIZ, "%s/cgroup.procs", new_path);
+	fp = fopen(s_new, "r");
+	if (fp == NULL)
+		tst_res(TWARN | TERRNO, "fopen %s", s_new);
+	if ((fd != -1) && (fp != NULL)) {
+		while (fgets(value, BUFSIZ, fp) != NULL)
+			if (write(fd, value, strlen(value) - 1)
+			    != (ssize_t)strlen(value) - 1)
+				tst_res(TWARN | TERRNO, "write %s", s);
+	}
+	if (fd != -1)
+		close(fd);
+	if (fp != NULL)
+		fclose(fp);
+	if (rmdir(new_path) == -1)
+		tst_res(TWARN | TERRNO, "rmdir %s", new_path);
+	if (umount(mnt_path) == -1)
+		tst_res(TWARN | TERRNO, "umount %s", mnt_path);
+	if (rmdir(mnt_path) == -1)
+		tst_res(TWARN | TERRNO, "rmdir %s", mnt_path);
+
+	tst_res(TINFO, "Cgroup v2 unmount success");
+}
+/* cgroup v2 */
+
+static enum tst_cgroup_ver tst_cg_ver;
+
+void tst_cgroup_mount(enum tst_cgroup_ctrl ctrl)
+{
+	tst_cg_ver = tst_cgroup_version();
+
+	if (tst_cg_ver & TST_CGROUP_V1) {
+		switch(ctrl) {
+		case TST_CGROUP_MEMCG:
+			tst_cgroup1_mount("memcg", "memory", PATH_TMP_CG1_MEM, PATH_CG1_MEM_LTP);
+		break;
+		case TST_CGROUP_CPUSET:
+			tst_cgroup1_mount("cpusetcg", "cpuset", PATH_TMP_CG1_CST, PATH_CG1_CST_LTP);
+		break;
+		default:
+			tst_brk(TBROK, "Invalid cgroup controller: %d", ctrl);
+		}
+	}
+
+	if (tst_cg_ver & TST_CGROUP_V2) {
+		tst_cgroup2_mount(PATH_TMP_CGROUP2, PATH_TMP_CG2_LTP);
+
+		switch(ctrl) {
+		case TST_CGROUP_MEMCG:
+			SAFE_FILE_PRINTF(PATH_TMP_CGROUP2 "/cgroup.subtree_control", "%s", "+memory");
+		break;
+		case TST_CGROUP_CPUSET:
+			tst_brk(TCONF, "Cgroup v2 hasn't achieve cpuset subsystem");
+		break;
+		default:
+			tst_brk(TBROK, "Invalid cgroup controller: %d", ctrl);
+		}
+	}
+}
+
+void tst_cgroup_umount(enum tst_cgroup_ctrl ctrl)
+{
+	if (tst_cg_ver & TST_CGROUP_V1) {
+		switch(ctrl) {
+		case TST_CGROUP_MEMCG:
+			tst_cgroup1_umount(PATH_TMP_CG1_MEM, PATH_CG1_MEM_LTP);
+		break;
+		case TST_CGROUP_CPUSET:
+			tst_cgroup1_umount(PATH_TMP_CG1_CST, PATH_CG1_CST_LTP);
+		break;
+		default:
+			tst_brk(TBROK, "Invalid cgroup controller: %d", ctrl);
+		}
+	}
+
+	if (tst_cg_ver & TST_CGROUP_V2)
+		tst_cgroup2_umount(PATH_TMP_CGROUP2, PATH_TMP_CG2_LTP);
+}
+
+void tst_cgroup_move_current(enum tst_cgroup_ctrl ctrl)
+{
+	if (tst_cg_ver & TST_CGROUP_V1) {
+		switch(ctrl) {
+		case TST_CGROUP_MEMCG:
+			SAFE_FILE_PRINTF(PATH_CG1_MEM_LTP "/tasks", "%d", getpid());
+		break;
+		case TST_CGROUP_CPUSET:
+			SAFE_FILE_PRINTF(PATH_CG1_CST_LTP "/tasks", "%d", getpid());
+		break;
+		default:
+			tst_brk(TBROK, "Invalid cgroup controller: %d", ctrl);
+		}
+	}
+
+	if (tst_cg_ver & TST_CGROUP_V2)
+		SAFE_FILE_PRINTF(PATH_TMP_CG2_LTP "/cgroup.procs", "%d", getpid());
+}
+
+void tst_cgroup_mem_set_maxbytes(long memsz)
+{
+	tst_cgroup_move_current(TST_CGROUP_MEMCG);
+
+	if (tst_cg_ver & TST_CGROUP_V1)
+		SAFE_FILE_PRINTF(PATH_CG1_MEM_LTP "/memory.limit_in_bytes", "%ld", memsz);
+
+	if (tst_cg_ver & TST_CGROUP_V2)
+		SAFE_FILE_PRINTF(PATH_TMP_CG2_LTP "/memory.max", "%ld", memsz);
+}
+
+void tst_cgroup_mem_set_maxswap(long memsz)
+{
+	tst_cgroup_move_current(TST_CGROUP_MEMCG);
+
+	if (tst_cg_ver & TST_CGROUP_V1)
+		SAFE_FILE_PRINTF(PATH_CG1_MEM_LTP "/memory.memsw.limit_in_bytes", "%ld", memsz);
+	if (tst_cg_ver & TST_CGROUP_V2)
+		SAFE_FILE_PRINTF(PATH_TMP_CG2_LTP "/memory.swap.max", "%ld", memsz);
+}
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [LTP] [PATCH v1 2/4] mem: take use of new cgroup API
  2020-05-27  3:14 [LTP] [PATCH v1 1/4] lib: add new cgroup test API Li Wang
@ 2020-05-27  3:14 ` Li Wang
  2020-05-27  7:45   ` Jan Stancek
  2020-05-27  3:14 ` [LTP] [PATCH v1 3/4] mem: remove the old " Li Wang
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Li Wang @ 2020-05-27  3:14 UTC (permalink / raw)
  To: ltp

For issue #611:
  https://github.com/linux-test-project/ltp/issues/611

Signed-off-by: Li Wang <liwang@redhat.com>
---
 testcases/kernel/mem/cpuset/cpuset01.c | 19 ++++++++++---------
 testcases/kernel/mem/include/mem.h     |  1 +
 testcases/kernel/mem/ksm/ksm02.c       |  6 ++----
 testcases/kernel/mem/ksm/ksm03.c       | 11 +++--------
 testcases/kernel/mem/ksm/ksm04.c       | 14 +++++---------
 testcases/kernel/mem/lib/mem.c         |  8 ++++----
 testcases/kernel/mem/oom/oom03.c       | 17 +++++++----------
 testcases/kernel/mem/oom/oom04.c       | 10 +++-------
 testcases/kernel/mem/oom/oom05.c       | 26 ++++++++++----------------
 9 files changed, 45 insertions(+), 67 deletions(-)

diff --git a/testcases/kernel/mem/cpuset/cpuset01.c b/testcases/kernel/mem/cpuset/cpuset01.c
index 853f7fe55..cecc4ba86 100644
--- a/testcases/kernel/mem/cpuset/cpuset01.c
+++ b/testcases/kernel/mem/cpuset/cpuset01.c
@@ -51,11 +51,11 @@ static void test_cpuset(void)
 	unsigned long nmask[MAXNODES / BITS_PER_LONG] = { 0 };
 	char mems[BUFSIZ], buf[BUFSIZ];
 
-	read_cpuset_files(CPATH, "cpus", buf);
-	write_cpuset_files(CPATH_NEW, "cpus", buf);
-	read_cpuset_files(CPATH, "mems", mems);
-	write_cpuset_files(CPATH_NEW, "mems", mems);
-	SAFE_FILE_PRINTF(CPATH_NEW "/tasks", "%d", getpid());
+	read_cpuset_files(PATH_TMP_CG1_CST, "cpus", buf);
+	write_cpuset_files(PATH_CG1_CST_LTP, "cpus", buf);
+	read_cpuset_files(PATH_TMP_CG1_CST, "mems", mems);
+	write_cpuset_files(PATH_CG1_CST_LTP, "mems", mems);
+	tst_cgroup_move_current(TST_CGROUP_CPUSET);
 
 	child = SAFE_FORK();
 	if (child == 0) {
@@ -70,9 +70,9 @@ static void test_cpuset(void)
 	}
 
 	snprintf(buf, BUFSIZ, "%d", nodes[0]);
-	write_cpuset_files(CPATH_NEW, "mems", buf);
+	write_cpuset_files(PATH_CG1_CST_LTP, "mems", buf);
 	snprintf(buf, BUFSIZ, "%d", nodes[1]);
-	write_cpuset_files(CPATH_NEW, "mems", buf);
+	write_cpuset_files(PATH_CG1_CST_LTP, "mems", buf);
 
 	SAFE_WAITPID(child, &status, WUNTRACED | WCONTINUED);
 	if (WEXITSTATUS(status) != 0) {
@@ -85,7 +85,7 @@ static void test_cpuset(void)
 
 static void setup(void)
 {
-	mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
+	tst_cgroup_mount(TST_CGROUP_CPUSET);
 	ncpus = count_cpu();
 	if (get_allowed_nodes_arr(NH_MEMS | NH_CPUS, &nnodes, &nodes) < 0)
 		tst_brk(TBROK | TERRNO, "get_allowed_nodes_arr");
@@ -95,7 +95,7 @@ static void setup(void)
 
 static void cleanup(void)
 {
-	umount_mem(CPATH, CPATH_NEW);
+	tst_cgroup_umount(TST_CGROUP_CPUSET);
 }
 
 static void sighandler(int signo LTP_ATTRIBUTE_UNUSED)
@@ -183,6 +183,7 @@ static long count_cpu(void)
 
 static struct tst_test test = {
 	.needs_root = 1,
+	.forks_child = 1,
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_all = test_cpuset,
diff --git a/testcases/kernel/mem/include/mem.h b/testcases/kernel/mem/include/mem.h
index cce9c0497..f553651c2 100644
--- a/testcases/kernel/mem/include/mem.h
+++ b/testcases/kernel/mem/include/mem.h
@@ -2,6 +2,7 @@
 #define _MEM_H
 #include "config.h"
 #include "tst_test.h"
+#include "tst_cgroup.h"
 #include "ksm_helper.h"
 
 #if defined(__powerpc__) || defined(__powerpc64__)
diff --git a/testcases/kernel/mem/ksm/ksm02.c b/testcases/kernel/mem/ksm/ksm02.c
index 3d4c19deb..9f8b782cf 100644
--- a/testcases/kernel/mem/ksm/ksm02.c
+++ b/testcases/kernel/mem/ksm/ksm02.c
@@ -88,8 +88,7 @@ static void cleanup(void)
 		FILE_PRINTF(PATH_KSM "merge_across_nodes",
 				 "%d", merge_across_nodes);
 
-	if (cpuset_mounted)
-		umount_mem(CPATH, CPATH_NEW);
+	tst_cgroup_umount(TST_CGROUP_CPUSET);
 }
 
 static void setup(void)
@@ -105,8 +104,7 @@ static void setup(void)
 		SAFE_FILE_PRINTF(PATH_KSM "merge_across_nodes", "1");
 	}
 
-	mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
-	cpuset_mounted = 1;
+	tst_cgroup_mount(TST_CGROUP_CPUSET);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/ksm/ksm03.c b/testcases/kernel/mem/ksm/ksm03.c
index 0e5131654..c2c8ab529 100644
--- a/testcases/kernel/mem/ksm/ksm03.c
+++ b/testcases/kernel/mem/ksm/ksm03.c
@@ -59,11 +59,9 @@
 #include "mem.h"
 #include "ksm_common.h"
 
-static int memcg_mounted;
-
 static void verify_ksm(void)
 {
-	write_memcg();
+	tst_cgroup_mem_set_maxbytes(TESTMEM);
 	create_same_memory(size, num, unit);
 }
 
@@ -79,8 +77,7 @@ static void setup(void)
 	}
 
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
-	mount_mem("memcg", "cgroup", "memory", MEMCG_PATH, MEMCG_PATH_NEW);
-	memcg_mounted = 1;
+	tst_cgroup_mount(TST_CGROUP_MEMCG);
 }
 
 static void cleanup(void)
@@ -88,9 +85,7 @@ static void cleanup(void)
 	if (access(PATH_KSM "merge_across_nodes", F_OK) == 0)
 		FILE_PRINTF(PATH_KSM "merge_across_nodes",
 				 "%d", merge_across_nodes);
-
-	if (memcg_mounted)
-		umount_mem(MEMCG_PATH, MEMCG_PATH_NEW);
+	tst_cgroup_umount(TST_CGROUP_MEMCG);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/ksm/ksm04.c b/testcases/kernel/mem/ksm/ksm04.c
index e393dbd40..6dc36b35a 100644
--- a/testcases/kernel/mem/ksm/ksm04.c
+++ b/testcases/kernel/mem/ksm/ksm04.c
@@ -70,7 +70,7 @@ static void verify_ksm(void)
 	node = get_a_numa_node();
 	set_node(nmask, node);
 
-	write_memcg();
+	tst_cgroup_mem_set_maxbytes(TESTMEM);
 
 	if (set_mempolicy(MPOL_BIND, nmask, MAXNODES) == -1) {
 		if (errno != ENOSYS)
@@ -91,10 +91,8 @@ static void cleanup(void)
 		FILE_PRINTF(PATH_KSM "merge_across_nodes",
 				 "%d", merge_across_nodes);
 
-	if (cpuset_mounted)
-		umount_mem(CPATH, CPATH_NEW);
-	if (memcg_mounted)
-		umount_mem(MEMCG_PATH, MEMCG_PATH_NEW);
+	tst_cgroup_umount(TST_CGROUP_CPUSET);
+	tst_cgroup_umount(TST_CGROUP_MEMCG);
 }
 
 static void setup(void)
@@ -110,10 +108,8 @@ static void setup(void)
 
 	parse_ksm_options(opt_sizestr, &size, opt_numstr, &num, opt_unitstr, &unit);
 
-	mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
-	cpuset_mounted = 1;
-	mount_mem("memcg", "cgroup", "memory", MEMCG_PATH, MEMCG_PATH_NEW);
-	memcg_mounted = 1;
+	tst_cgroup_mount(TST_CGROUP_MEMCG);
+	tst_cgroup_mount(TST_CGROUP_CPUSET);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index eca4c61c8..df1baa42a 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -692,7 +692,7 @@ void write_cpusets(long nd)
 	char cpus[BUFSIZ] = "";
 
 	snprintf(buf, BUFSIZ, "%ld", nd);
-	write_cpuset_files(CPATH_NEW, "mems", buf);
+	write_cpuset_files(PATH_CG1_CST_LTP, "mems", buf);
 
 	gather_node_cpus(cpus, nd);
 	/*
@@ -701,14 +701,14 @@ void write_cpusets(long nd)
 	 * the value of cpuset.cpus.
 	 */
 	if (strlen(cpus) != 0) {
-		write_cpuset_files(CPATH_NEW, "cpus", cpus);
+		write_cpuset_files(PATH_CG1_CST_LTP, "cpus", cpus);
 	} else {
 		tst_res(TINFO, "No CPUs in the node%ld; "
 				"using only CPU0", nd);
-		write_cpuset_files(CPATH_NEW, "cpus", "0");
+		write_cpuset_files(PATH_CG1_CST_LTP, "cpus", "0");
 	}
 
-	SAFE_FILE_PRINTF(CPATH_NEW "/tasks", "%d", getpid());
+	SAFE_FILE_PRINTF(PATH_CG1_CST_LTP "/tasks", "%d", getpid());
 }
 
 void umount_mem(char *path, char *path_new)
diff --git a/testcases/kernel/mem/oom/oom03.c b/testcases/kernel/mem/oom/oom03.c
index ce0b34c31..af3a565ce 100644
--- a/testcases/kernel/mem/oom/oom03.c
+++ b/testcases/kernel/mem/oom/oom03.c
@@ -36,27 +36,26 @@
 
 #ifdef HAVE_NUMA_V2
 
-static int memcg_mounted;
-
 static void verify_oom(void)
 {
 #ifdef TST_ABI32
 	tst_brk(TCONF, "test is not designed for 32-bit system.");
 #endif
 
-	SAFE_FILE_PRINTF(MEMCG_PATH_NEW "/tasks", "%d", getpid());
-	SAFE_FILE_PRINTF(MEMCG_LIMIT, "%ld", TESTMEM);
+	tst_cgroup_mem_set_maxbytes(TESTMEM);
 
 	testoom(0, 0, ENOMEM, 1);
 
-	if (access(MEMCG_SW_LIMIT, F_OK) == -1) {
+	if ((access(PATH_MEMORY_SW_LIMIT, F_OK) == -1) ||
+			(access(PATH_MEMORY_SW_MAX, F_OK) == -1)) {
 		if (errno == ENOENT)
 			tst_res(TCONF,
 				"memcg swap accounting is disabled");
 		else
 			tst_brk(TBROK | TERRNO, "access");
 	} else {
-		SAFE_FILE_PRINTF(MEMCG_SW_LIMIT, "%ld", TESTMEM);
+		tst_cgroup_mem_set_maxswap(TESTMEM);
+
 		testoom(0, 1, ENOMEM, 1);
 	}
 
@@ -73,16 +72,14 @@ static void setup(void)
 {
 	overcommit = get_sys_tune("overcommit_memory");
 	set_sys_tune("overcommit_memory", 1, 1);
-	mount_mem("memcg", "cgroup", "memory", MEMCG_PATH, MEMCG_PATH_NEW);
-	memcg_mounted = 1;
+	tst_cgroup_mount(TST_CGROUP_MEMCG);
 }
 
 static void cleanup(void)
 {
 	if (overcommit != -1)
 		set_sys_tune("overcommit_memory", overcommit, 0);
-	if (memcg_mounted)
-		umount_mem(MEMCG_PATH, MEMCG_PATH_NEW);
+	tst_cgroup_umount(TST_CGROUP_MEMCG);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/oom/oom04.c b/testcases/kernel/mem/oom/oom04.c
index 7185ef973..553e6fc0a 100644
--- a/testcases/kernel/mem/oom/oom04.c
+++ b/testcases/kernel/mem/oom/oom04.c
@@ -36,8 +36,6 @@
 
 #ifdef HAVE_NUMA_V2
 
-static int cpuset_mounted;
-
 static void verify_oom(void)
 {
 #ifdef TST_ABI32
@@ -53,7 +51,7 @@ static void verify_oom(void)
 		 * is in charge of cpuset.memory_migrate, we can write
 		 * 1 to cpuset.memory_migrate to enable the migration.
 		 */
-		write_cpuset_files(CPATH_NEW, "memory_migrate", "1");
+		write_cpuset_files(PATH_CG1_CST_LTP, "memory_migrate", "1");
 		tst_res(TINFO, "OOM on CPUSET with mem migrate:");
 		testoom(0, 0, ENOMEM, 1);
 	}
@@ -69,8 +67,7 @@ static void setup(void)
 	overcommit = get_sys_tune("overcommit_memory");
 	set_sys_tune("overcommit_memory", 1, 1);
 
-	mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
-	cpuset_mounted = 1;
+	tst_cgroup_mount(TST_CGROUP_CPUSET);
 
 	/*
 	 * Some nodes do not contain memory, so use
@@ -89,8 +86,7 @@ static void cleanup(void)
 {
 	if (overcommit != -1)
 		set_sys_tune("overcommit_memory", overcommit, 0);
-	if (cpuset_mounted)
-		umount_mem(CPATH, CPATH_NEW);
+	tst_cgroup_umount(TST_CGROUP_CPUSET);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/oom/oom05.c b/testcases/kernel/mem/oom/oom05.c
index db24df6de..7a33edef2 100644
--- a/testcases/kernel/mem/oom/oom05.c
+++ b/testcases/kernel/mem/oom/oom05.c
@@ -36,9 +36,6 @@
 
 #ifdef HAVE_NUMA_V2
 
-static int memcg_mounted;
-static int cpuset_mounted;
-
 static void verify_oom(void)
 {
 	int swap_acc_on = 1;
@@ -48,6 +45,7 @@ static void verify_oom(void)
 #endif
 
 	tst_res(TINFO, "OOM on CPUSET & MEMCG...");
+	tst_cgroup_mem_set_maxbytes(TESTMEM);
 	testoom(0, 0, ENOMEM, 1);
 
 	/*
@@ -56,13 +54,14 @@ static void verify_oom(void)
 	 * 1 to cpuset.memory_migrate to enable the migration.
 	 */
 	if (is_numa(NULL, NH_MEMS, 2)) {
-		write_cpuset_files(CPATH_NEW, "memory_migrate", "1");
+		write_cpuset_files(PATH_CG1_CST_LTP, "memory_migrate", "1");
 		tst_res(TINFO, "OOM on CPUSET & MEMCG with "
 				"cpuset.memory_migrate=1");
 		testoom(0, 0, ENOMEM, 1);
 	}
 
-	if (access(MEMCG_SW_LIMIT, F_OK) == -1) {
+	if ((access(PATH_MEMORY_SW_LIMIT, F_OK) == -1) ||
+			(access(PATH_MEMORY_SW_MAX, F_OK) == -1)) {
 		if (errno == ENOENT) {
 			tst_res(TCONF, "memcg swap accounting is disabled");
 			swap_acc_on = 0;
@@ -73,12 +72,12 @@ static void verify_oom(void)
 	if (swap_acc_on) {
 		tst_res(TINFO, "OOM on CPUSET & MEMCG with "
 				"special memswap limitation:");
-		SAFE_FILE_PRINTF(MEMCG_SW_LIMIT, "%ld", TESTMEM);
+		tst_cgroup_mem_set_maxswap(TESTMEM);
 		testoom(0, 0, ENOMEM, 1);
 
 		tst_res(TINFO, "OOM on CPUSET & MEMCG with "
 				"disabled memswap limitation:");
-		SAFE_FILE_PRINTF(MEMCG_SW_LIMIT, "-1");
+		tst_cgroup_mem_set_maxswap(-1);
 		testoom(0, 0, ENOMEM, 1);
 	}
 }
@@ -93,11 +92,8 @@ void setup(void)
 	overcommit = get_sys_tune("overcommit_memory");
 	set_sys_tune("overcommit_memory", 1, 1);
 
-	mount_mem("memcg", "cgroup", "memory", MEMCG_PATH, MEMCG_PATH_NEW);
-	memcg_mounted = 1;
-	mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
-	cpuset_mounted = 1;
-	write_memcg();
+	tst_cgroup_mount(TST_CGROUP_MEMCG);
+	tst_cgroup_mount(TST_CGROUP_CPUSET);
 
 	/*
 	 * Some nodes do not contain memory, so use
@@ -116,10 +112,8 @@ void cleanup(void)
 {
 	if (overcommit != -1)
 		set_sys_tune("overcommit_memory", overcommit, 0);
-	if (cpuset_mounted)
-		umount_mem(CPATH, CPATH_NEW);
-	if (memcg_mounted)
-		umount_mem(MEMCG_PATH, MEMCG_PATH_NEW);
+	tst_cgroup_umount(TST_CGROUP_CPUSET);
+	tst_cgroup_umount(TST_CGROUP_MEMCG);
 }
 
 static struct tst_test test = {
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [LTP] [PATCH v1 3/4] mem: remove the old cgroup API
  2020-05-27  3:14 [LTP] [PATCH v1 1/4] lib: add new cgroup test API Li Wang
  2020-05-27  3:14 ` [LTP] [PATCH v1 2/4] mem: take use of new cgroup API Li Wang
@ 2020-05-27  3:14 ` Li Wang
  2020-05-27  3:14 ` [LTP] [PATCH v1 4/4] mm: add cpuset01 to runtest file Li Wang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Li Wang @ 2020-05-27  3:14 UTC (permalink / raw)
  To: ltp

Signed-off-by: Li Wang <liwang@redhat.com>
---
 testcases/kernel/mem/include/mem.h | 16 +--------
 testcases/kernel/mem/lib/mem.c     | 58 ------------------------------
 2 files changed, 1 insertion(+), 73 deletions(-)

diff --git a/testcases/kernel/mem/include/mem.h b/testcases/kernel/mem/include/mem.h
index f553651c2..0e089fae2 100644
--- a/testcases/kernel/mem/include/mem.h
+++ b/testcases/kernel/mem/include/mem.h
@@ -60,24 +60,10 @@ void test_ksm_merge_across_nodes(unsigned long nr_pages);
 void check_hugepage(void);
 void write_memcg(void);
 
-/* cpuset/memcg */
-
-#define CPATH			"/dev/cpuset"
-#define CPATH_NEW		CPATH "/1"
-#define MEMCG_PATH		"/dev/cgroup"
-#define MEMCG_PATH_NEW		MEMCG_PATH "/1"
-#define MEMCG_LIMIT		MEMCG_PATH_NEW "/memory.limit_in_bytes"
-#define MEMCG_SW_LIMIT		MEMCG_PATH_NEW "/memory.memsw.limit_in_bytes"
-#if HAVE_SYS_EVENTFD_H
-#define PATH_OOMCTRL		MEMCG_PATH_NEW "/memory.oom_control"
-#define PATH_EVTCTRL		MEMCG_PATH_NEW "/cgroup.event_control"
-#endif
-
+/* cpuset/memcg - include/tst_cgroup.h */
 void read_cpuset_files(char *prefix, char *filename, char *retbuf);
 void write_cpuset_files(char *prefix, char *filename, char *buf);
 void write_cpusets(long nd);
-void mount_mem(char *name, char *fs, char *options, char *path, char *path_new);
-void umount_mem(char *path, char *path_new);
 
 /* shared */
 unsigned int get_a_numa_node(void);
diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index df1baa42a..3ccba142e 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -312,13 +312,6 @@ void check_hugepage(void)
 		tst_brk(TCONF, "Huge page is not supported.");
 }
 
-void write_memcg(void)
-{
-	SAFE_FILE_PRINTF(MEMCG_LIMIT, "%ld", TESTMEM);
-
-	SAFE_FILE_PRINTF(MEMCG_PATH_NEW "/tasks", "%d", getpid());
-}
-
 struct ksm_merge_data {
 	char data;
 	unsigned int mergeable_size;
@@ -711,57 +704,6 @@ void write_cpusets(long nd)
 	SAFE_FILE_PRINTF(PATH_CG1_CST_LTP "/tasks", "%d", getpid());
 }
 
-void umount_mem(char *path, char *path_new)
-{
-	FILE *fp;
-	int fd;
-	char s_new[BUFSIZ], s[BUFSIZ], value[BUFSIZ];
-
-	/* Move all processes in task to its parent node. */
-	sprintf(s, "%s/tasks", path);
-	fd = open(s, O_WRONLY);
-	if (fd == -1)
-		tst_res(TWARN | TERRNO, "open %s", s);
-
-	snprintf(s_new, BUFSIZ, "%s/tasks", path_new);
-	fp = fopen(s_new, "r");
-	if (fp == NULL)
-		tst_res(TWARN | TERRNO, "fopen %s", s_new);
-	if ((fd != -1) && (fp != NULL)) {
-		while (fgets(value, BUFSIZ, fp) != NULL)
-			if (write(fd, value, strlen(value) - 1)
-			    != (ssize_t)strlen(value) - 1)
-				tst_res(TWARN | TERRNO, "write %s", s);
-	}
-	if (fd != -1)
-		close(fd);
-	if (fp != NULL)
-		fclose(fp);
-	if (rmdir(path_new) == -1)
-		tst_res(TWARN | TERRNO, "rmdir %s", path_new);
-	if (umount(path) == -1)
-		tst_res(TWARN | TERRNO, "umount %s", path);
-	if (rmdir(path) == -1)
-		tst_res(TWARN | TERRNO, "rmdir %s", path);
-}
-
-void mount_mem(char *name, char *fs, char *options, char *path, char *path_new)
-{
-	SAFE_MKDIR(path, 0777);
-	if (mount(name, path, fs, 0, options) == -1) {
-		if (errno == ENODEV) {
-			if (rmdir(path) == -1)
-				tst_res(TWARN | TERRNO, "rmdir %s failed",
-					 path);
-			tst_brk(TCONF,
-				 "file system %s is not configured in kernel",
-				 fs);
-		}
-		tst_brk(TBROK | TERRNO, "mount %s", path);
-	}
-	SAFE_MKDIR(path_new, 0777);
-}
-
 /* shared */
 
 /* Warning: *DO NOT* use this function in child */
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [LTP] [PATCH v1 4/4] mm: add cpuset01 to runtest file
  2020-05-27  3:14 [LTP] [PATCH v1 1/4] lib: add new cgroup test API Li Wang
  2020-05-27  3:14 ` [LTP] [PATCH v1 2/4] mem: take use of new cgroup API Li Wang
  2020-05-27  3:14 ` [LTP] [PATCH v1 3/4] mem: remove the old " Li Wang
@ 2020-05-27  3:14 ` Li Wang
  2020-05-27  7:40 ` [LTP] [PATCH v1 1/4] lib: add new cgroup test API Jan Stancek
  2020-05-27  7:47 ` Jan Stancek
  4 siblings, 0 replies; 10+ messages in thread
From: Li Wang @ 2020-05-27  3:14 UTC (permalink / raw)
  To: ltp

This test was moved to stress test since commit ae8fa55a8, but stress
runtest has been removed from commit e752f7c19674 :).

Here adding back it as a general memory test without '-I 3600'.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 runtest/mm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/runtest/mm b/runtest/mm
index a09f39c1e..612a4d066 100644
--- a/runtest/mm
+++ b/runtest/mm
@@ -73,6 +73,8 @@ ksm06 ksm06
 ksm06_1 ksm06 -n 10
 ksm06_2 ksm06 -n 10000
 
+cpuset01 cpuset01
+
 oom01 oom01
 oom02 oom02
 oom03 oom03
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [LTP] [PATCH v1 1/4] lib: add new cgroup test API
  2020-05-27  3:14 [LTP] [PATCH v1 1/4] lib: add new cgroup test API Li Wang
                   ` (2 preceding siblings ...)
  2020-05-27  3:14 ` [LTP] [PATCH v1 4/4] mm: add cpuset01 to runtest file Li Wang
@ 2020-05-27  7:40 ` Jan Stancek
  2020-05-27  8:00   ` Li Wang
  2020-05-27  7:47 ` Jan Stancek
  4 siblings, 1 reply; 10+ messages in thread
From: Jan Stancek @ 2020-05-27  7:40 UTC (permalink / raw)
  To: ltp




----- Original Message -----
> +
> +void tst_cgroup_mem_set_maxbytes(long memsz)
> +{
> +	tst_cgroup_move_current(TST_CGROUP_MEMCG);

It seems a bit unexpected, that setting a limit also moves current
process to cgroup. If test forks two processes, it has to set maxbytes
twice, to get the desired side-effect.

> +
> +	if (tst_cg_ver & TST_CGROUP_V1)
> +		SAFE_FILE_PRINTF(PATH_CG1_MEM_LTP "/memory.limit_in_bytes", "%ld", memsz);
> +
> +	if (tst_cg_ver & TST_CGROUP_V2)
> +		SAFE_FILE_PRINTF(PATH_TMP_CG2_LTP "/memory.max", "%ld", memsz);
> +}
> +
> +void tst_cgroup_mem_set_maxswap(long memsz)
> +{
> +	tst_cgroup_move_current(TST_CGROUP_MEMCG);
> +
> +	if (tst_cg_ver & TST_CGROUP_V1)
> +		SAFE_FILE_PRINTF(PATH_CG1_MEM_LTP "/memory.memsw.limit_in_bytes", "%ld",
> memsz);
> +	if (tst_cg_ver & TST_CGROUP_V2)
> +		SAFE_FILE_PRINTF(PATH_TMP_CG2_LTP "/memory.swap.max", "%ld", memsz);
> +}
> --
> 2.21.1
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread
* [LTP] [PATCH v1 2/4] mem: take use of new cgroup API
  2020-05-27  3:14 ` [LTP] [PATCH v1 2/4] mem: take use of new cgroup API Li Wang
@ 2020-05-27  7:45   ` Jan Stancek
  2020-05-27 10:10     ` Li Wang
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Stancek @ 2020-05-27  7:45 UTC (permalink / raw)
  To: ltp

> diff --git a/testcases/kernel/mem/cpuset/cpuset01.c
> b/testcases/kernel/mem/cpuset/cpuset01.c
> index 853f7fe55..cecc4ba86 100644
> --- a/testcases/kernel/mem/cpuset/cpuset01.c
> +++ b/testcases/kernel/mem/cpuset/cpuset01.c
> @@ -51,11 +51,11 @@ static void test_cpuset(void)
>  	unsigned long nmask[MAXNODES / BITS_PER_LONG] = { 0 };
>  	char mems[BUFSIZ], buf[BUFSIZ];
>  
> -	read_cpuset_files(CPATH, "cpus", buf);
> -	write_cpuset_files(CPATH_NEW, "cpus", buf);
> -	read_cpuset_files(CPATH, "mems", mems);
> -	write_cpuset_files(CPATH_NEW, "mems", mems);
> -	SAFE_FILE_PRINTF(CPATH_NEW "/tasks", "%d", getpid());
> +	read_cpuset_files(PATH_TMP_CG1_CST, "cpus", buf);
> +	write_cpuset_files(PATH_CG1_CST_LTP, "cpus", buf);
> +	read_cpuset_files(PATH_TMP_CG1_CST, "mems", mems);
> +	write_cpuset_files(PATH_CG1_CST_LTP, "mems", mems);

This mixes generic api with cgroup1. It currently relies on implementation
detail of tst_cgroup_mount(), which isn't visible just by looking at this test.

We should make it generic or make it clear that test is for cgroup1 only:

setup()
  if (tst_cgroup_version() != TST_CGROUP_V1)
    TCONF


> diff --git a/testcases/kernel/mem/oom/oom03.c
> b/testcases/kernel/mem/oom/oom03.c
> index ce0b34c31..af3a565ce 100644
> --- a/testcases/kernel/mem/oom/oom03.c
> +++ b/testcases/kernel/mem/oom/oom03.c
> @@ -36,27 +36,26 @@
>  
>  #ifdef HAVE_NUMA_V2
>  
> -static int memcg_mounted;
> -
>  static void verify_oom(void)
>  {
>  #ifdef TST_ABI32
>  	tst_brk(TCONF, "test is not designed for 32-bit system.");
>  #endif
>  
> -	SAFE_FILE_PRINTF(MEMCG_PATH_NEW "/tasks", "%d", getpid());
> -	SAFE_FILE_PRINTF(MEMCG_LIMIT, "%ld", TESTMEM);
> +	tst_cgroup_mem_set_maxbytes(TESTMEM);
>  
>  	testoom(0, 0, ENOMEM, 1);
>  
> -	if (access(MEMCG_SW_LIMIT, F_OK) == -1) {
> +	if ((access(PATH_MEMORY_SW_LIMIT, F_OK) == -1) ||
> +			(access(PATH_MEMORY_SW_MAX, F_OK) == -1)) {

This could be tst_cgroup_mem_swapacct_enabled(), without need for test
to probe specific cgroup[12] files.


^ permalink raw reply	[flat|nested] 10+ messages in thread
* [LTP] [PATCH v1 1/4] lib: add new cgroup test API
  2020-05-27  3:14 [LTP] [PATCH v1 1/4] lib: add new cgroup test API Li Wang
                   ` (3 preceding siblings ...)
  2020-05-27  7:40 ` [LTP] [PATCH v1 1/4] lib: add new cgroup test API Jan Stancek
@ 2020-05-27  7:47 ` Jan Stancek
  4 siblings, 0 replies; 10+ messages in thread
From: Jan Stancek @ 2020-05-27  7:47 UTC (permalink / raw)
  To: ltp



----- Original Message -----
> +
> +static void tst_cgroup1_umount(const char *mnt_path, const char *new_path)
> +{
> +	FILE *fp;
> +	int fd;
> +	char s_new[BUFSIZ], s[BUFSIZ], value[BUFSIZ];
> +
> +	if (!tst_is_mounted(mnt_path))
> +		return;
> +
> +	/* Move all processes in task to its parent node. */
> +	sprintf(s, "%s/tasks", mnt_path);
> +	fd = open(s, O_WRONLY);
> +	if (fd == -1)
> +		tst_res(TWARN | TERRNO, "open %s", s);
> +
> +	snprintf(s_new, BUFSIZ, "%s/tasks", new_path);
> +	fp = fopen(s_new, "r");
> +	if (fp == NULL)
> +		tst_res(TWARN | TERRNO, "fopen %s", s_new);
> +	if ((fd != -1) && (fp != NULL)) {
> +		while (fgets(value, BUFSIZ, fp) != NULL)
> +			if (write(fd, value, strlen(value) - 1)
> +			    != (ssize_t)strlen(value) - 1)
> +				tst_res(TWARN | TERRNO, "write %s", s);
> +	}
> +	if (fd != -1)
> +		close(fd);
> +	if (fp != NULL)
> +		fclose(fp);
> +	if (rmdir(new_path) == -1)
> +		tst_res(TWARN | TERRNO, "rmdir %s", new_path);
> +	if (umount(mnt_path) == -1)
> +		tst_res(TWARN | TERRNO, "umount %s", mnt_path);
> +	if (rmdir(mnt_path) == -1)
> +		tst_res(TWARN | TERRNO, "rmdir %s", mnt_path);

This sequence looks almost identical to tst_cgroup2_umount(),
would be nice if code could be shared in some way.


^ permalink raw reply	[flat|nested] 10+ messages in thread
* [LTP] [PATCH v1 1/4] lib: add new cgroup test API
  2020-05-27  7:40 ` [LTP] [PATCH v1 1/4] lib: add new cgroup test API Jan Stancek
@ 2020-05-27  8:00   ` Li Wang
  2020-05-27  9:15     ` Jan Stancek
  0 siblings, 1 reply; 10+ messages in thread
From: Li Wang @ 2020-05-27  8:00 UTC (permalink / raw)
  To: ltp

On Wed, May 27, 2020 at 3:40 PM Jan Stancek <jstancek@redhat.com> wrote:

>
>
>
> ----- Original Message -----
> > +
> > +void tst_cgroup_mem_set_maxbytes(long memsz)
> > +{
> > +     tst_cgroup_move_current(TST_CGROUP_MEMCG);
>
> It seems a bit unexpected, that setting a limit also moves current
> process to cgroup. If test forks two processes, it has to set maxbytes
> twice, to get the desired side-effect.
>

Yes, I didn't aware of that before.
Maybe we can remove the tst_cgroup_move_current() from here and invoke it
additionally?

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200527/c07fc6e5/attachment.htm>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [LTP] [PATCH v1 1/4] lib: add new cgroup test API
  2020-05-27  8:00   ` Li Wang
@ 2020-05-27  9:15     ` Jan Stancek
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Stancek @ 2020-05-27  9:15 UTC (permalink / raw)
  To: ltp



----- Original Message -----
> On Wed, May 27, 2020 at 3:40 PM Jan Stancek <jstancek@redhat.com> wrote:
> 
> >
> >
> >
> > ----- Original Message -----
> > > +
> > > +void tst_cgroup_mem_set_maxbytes(long memsz)
> > > +{
> > > +     tst_cgroup_move_current(TST_CGROUP_MEMCG);
> >
> > It seems a bit unexpected, that setting a limit also moves current
> > process to cgroup. If test forks two processes, it has to set maxbytes
> > twice, to get the desired side-effect.
> >
> 
> Yes, I didn't aware of that before.
> Maybe we can remove the tst_cgroup_move_current() from here and invoke it
> additionally?

Agreed, some cpuset tests already do it that way.


^ permalink raw reply	[flat|nested] 10+ messages in thread
* [LTP] [PATCH v1 2/4] mem: take use of new cgroup API
  2020-05-27  7:45   ` Jan Stancek
@ 2020-05-27 10:10     ` Li Wang
  0 siblings, 0 replies; 10+ messages in thread
From: Li Wang @ 2020-05-27 10:10 UTC (permalink / raw)
  To: ltp

On Wed, May 27, 2020 at 3:45 PM Jan Stancek <jstancek@redhat.com> wrote:

> > diff --git a/testcases/kernel/mem/cpuset/cpuset01.c
> > b/testcases/kernel/mem/cpuset/cpuset01.c
> > index 853f7fe55..cecc4ba86 100644
> > --- a/testcases/kernel/mem/cpuset/cpuset01.c
> > +++ b/testcases/kernel/mem/cpuset/cpuset01.c
> > @@ -51,11 +51,11 @@ static void test_cpuset(void)
> >       unsigned long nmask[MAXNODES / BITS_PER_LONG] = { 0 };
> >       char mems[BUFSIZ], buf[BUFSIZ];
> >
> > -     read_cpuset_files(CPATH, "cpus", buf);
> > -     write_cpuset_files(CPATH_NEW, "cpus", buf);
> > -     read_cpuset_files(CPATH, "mems", mems);
> > -     write_cpuset_files(CPATH_NEW, "mems", mems);
> > -     SAFE_FILE_PRINTF(CPATH_NEW "/tasks", "%d", getpid());
> > +     read_cpuset_files(PATH_TMP_CG1_CST, "cpus", buf);
> > +     write_cpuset_files(PATH_CG1_CST_LTP, "cpus", buf);
> > +     read_cpuset_files(PATH_TMP_CG1_CST, "mems", mems);
> > +     write_cpuset_files(PATH_CG1_CST_LTP, "mems", mems);
>
> This mixes generic api with cgroup1. It currently relies on implementation
> detail of tst_cgroup_mount(), which isn't visible just by looking at this
> test.
>

Yes, here just uses many path macros, which makes things easy since mem/lib
achieved many functions for cpuset files, I don't want to break the struct
so keep it similar as previous. Anyway, we can reconstruct that but that is
not the intention of this patchset.

Btw, the test has no chance to go there because in setup() the
tst_cgroup_mount(TST_CGROUP_CPUSET)
will get TCONF if the system uses cgroup-v2.

>
> We should make it generic or make it clear that test is for cgroup1 only:
>

But sure, we can add this, it makes code clear to readers.


>
> setup()
>   if (tst_cgroup_version() != TST_CGROUP_V1)
>     TCONF
>
>
> > diff --git a/testcases/kernel/mem/oom/oom03.c
> > b/testcases/kernel/mem/oom/oom03.c
> > index ce0b34c31..af3a565ce 100644
> > --- a/testcases/kernel/mem/oom/oom03.c
> > +++ b/testcases/kernel/mem/oom/oom03.c
> > @@ -36,27 +36,26 @@
> >
> >  #ifdef HAVE_NUMA_V2
> >
> > -static int memcg_mounted;
> > -
> >  static void verify_oom(void)
> >  {
> >  #ifdef TST_ABI32
> >       tst_brk(TCONF, "test is not designed for 32-bit system.");
> >  #endif
> >
> > -     SAFE_FILE_PRINTF(MEMCG_PATH_NEW "/tasks", "%d", getpid());
> > -     SAFE_FILE_PRINTF(MEMCG_LIMIT, "%ld", TESTMEM);
> > +     tst_cgroup_mem_set_maxbytes(TESTMEM);
> >
> >       testoom(0, 0, ENOMEM, 1);
> >
> > -     if (access(MEMCG_SW_LIMIT, F_OK) == -1) {
> > +     if ((access(PATH_MEMORY_SW_LIMIT, F_OK) == -1) ||
> > +                     (access(PATH_MEMORY_SW_MAX, F_OK) == -1)) {
>
> This could be tst_cgroup_mem_swapacct_enabled(), without need for test
> to probe specific cgroup[12] files.
>

Sounds good. I will have a try.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200527/068c1d55/attachment.htm>

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-05-27 10:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-27  3:14 [LTP] [PATCH v1 1/4] lib: add new cgroup test API Li Wang
2020-05-27  3:14 ` [LTP] [PATCH v1 2/4] mem: take use of new cgroup API Li Wang
2020-05-27  7:45   ` Jan Stancek
2020-05-27 10:10     ` Li Wang
2020-05-27  3:14 ` [LTP] [PATCH v1 3/4] mem: remove the old " Li Wang
2020-05-27  3:14 ` [LTP] [PATCH v1 4/4] mm: add cpuset01 to runtest file Li Wang
2020-05-27  7:40 ` [LTP] [PATCH v1 1/4] lib: add new cgroup test API Jan Stancek
2020-05-27  8:00   ` Li Wang
2020-05-27  9:15     ` Jan Stancek
2020-05-27  7:47 ` Jan Stancek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.