All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v4 1/4] lib: add new cgroup test API
@ 2020-06-09  9:50 Li Wang
  2020-06-09  9:51 ` [LTP] [PATCH v4 2/4] mem: take use of new cgroup API Li Wang
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Li Wang @ 2020-06-09  9:50 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:
    v3 --> v4
        * remove tst_cgroup_mnt/new_path
        * tst_cgroup_get_path() return a pointer to new_path
        * add tst_cgroup_cpuset_read/write_files()

 doc/test-writing-guidelines.txt |  44 ++++
 include/tst_cgroup.h            |  39 +++
 include/tst_test.h              |   1 +
 lib/newlib_tests/test21.c       |  76 ++++++
 lib/tst_cgroup.c                | 420 ++++++++++++++++++++++++++++++++
 5 files changed, 580 insertions(+)
 create mode 100644 include/tst_cgroup.h
 create mode 100644 lib/newlib_tests/test21.c
 create mode 100644 lib/tst_cgroup.c

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index 93ca506d9..42b360da0 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -2053,6 +2053,50 @@ 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_move_current(PATH_TMP_CG_MEM);
+	tst_cgroup_mem_set_maxbytes(PATH_TMP_CG_MEM, MEMSIZE);
+
+	// do test under cgroup
+	...
+}
+
+static void setup(void)
+{
+	tst_cgroup_mount(TST_CGROUP_MEMCG, PATH_TMP_CG_MEM);
+}
+
+static void cleanup(void)
+{
+	tst_cgroup_umount(PATH_TMP_CG_MEM);
+}
+
+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..0bc73b542
--- /dev/null
+++ b/include/tst_cgroup.h
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Red Hat, Inc.
+ * Copyright (c) 2020 Li Wang <liwang@redhat.com>
+ */
+
+#ifndef TST_CGROUP_H
+#define TST_CGROUP_H
+
+#define PATH_TMP_CG_MEM	"/tmp/cgroup_mem"
+#define PATH_TMP_CG_CST	"/tmp/cgroup_cst"
+
+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 */
+};
+
+enum tst_cgroup_ver tst_cgroup_version(void);
+
+void tst_cgroup_mount(enum tst_cgroup_ctrl ctrl, const char *cgroup_dir);
+void tst_cgroup_umount(const char *cgroup_dir);
+
+void tst_cgroup_move_current(const char *cgroup_dir);
+void tst_cgroup_set_knob(const char *cgroup_dir, const char *knob, long value);
+
+void tst_cgroup_mem_set_maxbytes(const char *cgroup_dir, long memsz);
+int  tst_cgroup_mem_swapacct_enabled(const char *cgroup_dir);
+void tst_cgroup_mem_set_maxswap(const char *cgroup_dir, long memsz);
+
+void tst_cgroup_cpuset_read_files(const char *cgroup_dir, const char *filename, char *retbuf);
+void tst_cgroup_cpuset_write_files(const char *cgroup_dir, const char *filename, const char *buf);
+
+#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/newlib_tests/test21.c b/lib/newlib_tests/test21.c
new file mode 100644
index 000000000..c3fa5584e
--- /dev/null
+++ b/lib/newlib_tests/test21.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Red Hat, Inc.
+ * Copyright (c) 2020 Li Wang <liwang@redhat.com>
+ */
+
+/*
+ * Tests tst_cgroup.h APIs
+ */
+
+#include "tst_test.h"
+#include "tst_cgroup.h"
+
+#define PATH_CGROUP1 "/mnt/liwang1"
+#define PATH_CGROUP2 "/mnt/liwang2"
+#define MEMSIZE 1024 * 1024
+
+static void do_test(void)
+{
+	/* we should assign one or more memory nodes to cpuset.mems
+	 * and cpuset.cpus, otherwise, echo $$ > tasks gives ?no space
+	 * left on device? when trying to use cpuset.
+	 *
+	 * Or, setting cgroup.clone_children to 1 can help in automatically
+	 * inheriting memory and node setting from parent cgroup when
+	 * a child cgroup is created.
+	 */
+	tst_cgroup_set_knob(PATH_TMP_CG_CST, "../cgroup.clone_children", 1);
+
+	pid_t pid = SAFE_FORK();
+
+	switch (pid) {
+	case 0:
+		tst_cgroup_move_current(PATH_CGROUP1);
+		tst_cgroup_mem_set_maxbytes(PATH_CGROUP1, MEMSIZE);
+		tst_cgroup_mem_set_maxswap(PATH_CGROUP1, MEMSIZE);
+
+		tst_cgroup_move_current(PATH_CGROUP2);
+
+	break;
+	default:
+		tst_cgroup_move_current(PATH_TMP_CG_CST);
+
+		tst_cgroup_move_current(PATH_TMP_CG_MEM);
+		tst_cgroup_mem_set_maxbytes(PATH_TMP_CG_MEM, MEMSIZE);
+		tst_cgroup_mem_set_maxswap(PATH_TMP_CG_MEM, MEMSIZE);
+	break;
+	}
+
+	tst_res(TPASS, "Cgroup mount test");
+}
+
+static void setup(void)
+{
+	tst_cgroup_mount(TST_CGROUP_MEMCG, PATH_TMP_CG_MEM);
+	tst_cgroup_mount(TST_CGROUP_MEMCG, PATH_CGROUP1);
+
+	tst_cgroup_mount(TST_CGROUP_CPUSET, PATH_TMP_CG_CST);
+	tst_cgroup_mount(TST_CGROUP_CPUSET, PATH_CGROUP2);
+}
+
+static void cleanup(void)
+{
+	tst_cgroup_umount(PATH_TMP_CG_MEM);
+	tst_cgroup_umount(PATH_CGROUP1);
+
+	tst_cgroup_umount(PATH_TMP_CG_CST);
+	tst_cgroup_umount(PATH_CGROUP2);
+}
+
+static struct tst_test test = {
+	.test_all = do_test,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+};
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
new file mode 100644
index 000000000..f918e7e85
--- /dev/null
+++ b/lib/tst_cgroup.c
@@ -0,0 +1,420 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Red Hat, Inc.
+ * Copyright (c) 2020 Li Wang <liwang@redhat.com>
+ */
+
+#define TST_NO_DEFAULT_MAIN
+
+#include <stdio.h>
+#include <stdlib.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 enum tst_cgroup_ver tst_cg_ver;
+
+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");
+}
+
+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_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_cgroupN_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(v2: cgroup.procs) to its parent node. */
+	if (tst_cg_ver & TST_CGROUP_V1)
+		sprintf(s, "%s/tasks", mnt_path);
+	if (tst_cg_ver & TST_CGROUP_V2)
+		sprintf(s, "%s/cgroup.procs", mnt_path);
+
+	fd = open(s, O_WRONLY);
+	if (fd == -1)
+		tst_res(TWARN | TERRNO, "open %s", s);
+
+	if (tst_cg_ver & TST_CGROUP_V1)
+		snprintf(s_new, BUFSIZ, "%s/tasks", new_path);
+	if (tst_cg_ver & TST_CGROUP_V2)
+		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);
+
+	if (tst_cg_ver & TST_CGROUP_V1)
+		tst_res(TINFO, "Cgroup v1 unmount success");
+	if (tst_cg_ver & TST_CGROUP_V2)
+		tst_res(TINFO, "Cgroup v2 unmount success");
+}
+
+struct tst_cgroup_path {
+	char *mnt_path;
+	char *new_path;
+	struct tst_cgroup_path *next;
+};
+
+static struct tst_cgroup_path *tst_cgroup_paths;
+
+static void tst_cgroup_set_path(const char *cgroup_dir)
+{
+	char cgroup_new_dir[PATH_MAX];
+	struct tst_cgroup_path *tst_cgroup_path, *a;
+
+	if (!cgroup_dir)
+		tst_brk(TBROK, "Invalid cgroup dir, plese check cgroup_dir");
+
+	sprintf(cgroup_new_dir, "%s/ltp_%d", cgroup_dir, rand());
+
+	/* To store cgroup path in the 'path' list */
+	tst_cgroup_path = SAFE_MMAP(NULL, (sizeof(struct tst_cgroup_path)),
+			PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	tst_cgroup_path->mnt_path = SAFE_MMAP(NULL, (strlen(cgroup_dir) + 1),
+			PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	tst_cgroup_path->new_path = SAFE_MMAP(NULL, (strlen(cgroup_new_dir) + 1),
+			PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+
+	if (!tst_cgroup_paths) {
+		tst_cgroup_paths = tst_cgroup_path;
+	} else {
+		a = tst_cgroup_paths;
+		do {
+			if (!a->next) {
+				a->next = tst_cgroup_path;
+				break;
+			}
+			a = a->next;
+		} while (a);
+	}
+
+	sprintf(tst_cgroup_path->mnt_path, "%s", cgroup_dir);
+	sprintf(tst_cgroup_path->new_path, "%s", cgroup_new_dir);
+}
+
+static char *tst_cgroup_get_path(const char *cgroup_dir)
+{
+	struct tst_cgroup_path *a;
+
+	if (!tst_cgroup_paths)
+		return NULL;
+
+	a = tst_cgroup_paths;
+
+	while (strcmp(a->mnt_path, cgroup_dir) != 0){
+		if (!a->next) {
+			tst_res(TINFO, "%s is not found", cgroup_dir);
+			return NULL;
+		}
+		a = a->next;
+	};
+
+	return a->new_path;
+}
+
+static void tst_cgroup_del_path(const char *cgroup_dir)
+{
+	struct tst_cgroup_path *a, *b;
+
+	if (tst_cgroup_paths)
+		return;
+
+	a = b = tst_cgroup_paths;
+
+	while (strcmp(b->mnt_path, cgroup_dir) != 0) {
+		if (!b->next) {
+			tst_res(TINFO, "%s is not found", cgroup_dir);
+			return;
+		}
+		a = b;
+		b = b->next;
+	};
+
+	if (b == tst_cgroup_paths)
+		tst_cgroup_paths = b->next;
+	else
+		a->next = b->next;
+
+	SAFE_MUNMAP(b->mnt_path, sizeof(b->mnt_path));
+	SAFE_MUNMAP(b->new_path, sizeof(b->new_path));
+	SAFE_MUNMAP(b, sizeof(b));
+}
+
+void tst_cgroup_mount(enum tst_cgroup_ctrl ctrl, const char *cgroup_dir)
+{
+	char *cgroup_new_dir;
+	char knob_path[PATH_MAX];
+
+	tst_cg_ver = tst_cgroup_version();
+
+	tst_cgroup_set_path(cgroup_dir);
+	cgroup_new_dir = tst_cgroup_get_path(cgroup_dir);
+
+	if (tst_cg_ver & TST_CGROUP_V1) {
+		switch(ctrl) {
+		case TST_CGROUP_MEMCG:
+			tst_cgroup1_mount("memcg", "memory", cgroup_dir, cgroup_new_dir);
+		break;
+		case TST_CGROUP_CPUSET:
+			tst_cgroup1_mount("cpusetcg", "cpuset", cgroup_dir, cgroup_new_dir);
+		break;
+		default:
+			tst_brk(TBROK, "Invalid cgroup controller: %d", ctrl);
+		}
+	}
+
+	if (tst_cg_ver & TST_CGROUP_V2) {
+		tst_cgroup2_mount(cgroup_dir, cgroup_new_dir);
+
+		switch(ctrl) {
+		case TST_CGROUP_MEMCG:
+			sprintf(knob_path, "%s/cgroup.subtree_control", cgroup_dir);
+			SAFE_FILE_PRINTF(knob_path, "%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(const char *cgroup_dir)
+{
+	char *cgroup_new_dir;
+
+	cgroup_new_dir = tst_cgroup_get_path(cgroup_dir);
+	tst_cgroupN_umount(cgroup_dir, cgroup_new_dir);
+	tst_cgroup_del_path(cgroup_dir);
+}
+
+void tst_cgroup_set_knob(const char *cgroup_dir, const char *knob, long value)
+{
+	char *cgroup_new_dir;
+	char knob_path[PATH_MAX];
+
+	cgroup_new_dir = tst_cgroup_get_path(cgroup_dir);
+	sprintf(knob_path, "%s/%s", cgroup_new_dir, knob);
+	SAFE_FILE_PRINTF(knob_path, "%ld", value);
+}
+
+void tst_cgroup_move_current(const char *cgroup_dir)
+{
+	if (tst_cg_ver & TST_CGROUP_V1)
+		tst_cgroup_set_knob(cgroup_dir, "tasks", getpid());
+
+	if (tst_cg_ver & TST_CGROUP_V2)
+		tst_cgroup_set_knob(cgroup_dir, "cgroup.procs", getpid());
+}
+
+void tst_cgroup_mem_set_maxbytes(const char *cgroup_dir, long memsz)
+{
+	if (tst_cg_ver & TST_CGROUP_V1)
+		tst_cgroup_set_knob(cgroup_dir, "memory.limit_in_bytes", memsz);
+
+	if (tst_cg_ver & TST_CGROUP_V2)
+		tst_cgroup_set_knob(cgroup_dir, "memory.max", memsz);
+}
+
+int tst_cgroup_mem_swapacct_enabled(const char *cgroup_dir)
+{
+	char *cgroup_new_dir;
+	char knob_path[PATH_MAX];
+
+	cgroup_new_dir = tst_cgroup_get_path(cgroup_dir);
+
+	if (tst_cg_ver & TST_CGROUP_V1) {
+		sprintf(knob_path, "%s/%s",
+				cgroup_new_dir, "/memory.memsw.limit_in_bytes");
+
+		if ((access(knob_path, F_OK) == -1)) {
+			if (errno == ENOENT)
+				tst_res(TCONF, "memcg swap accounting is disabled");
+			else
+				tst_brk(TBROK | TERRNO, "failed to access %s", knob_path);
+		} else {
+			return 1;
+		}
+	}
+
+	if (tst_cg_ver & TST_CGROUP_V2) {
+		sprintf(knob_path, "%s/%s",
+				cgroup_new_dir, "/memory.swap.max");
+
+		if ((access(knob_path, F_OK) == -1)) {
+			if (errno == ENOENT)
+				tst_res(TCONF, "memcg swap accounting is disabled");
+			else
+				tst_brk(TBROK | TERRNO, "failed to access %s", knob_path);
+		} else {
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+void tst_cgroup_mem_set_maxswap(const char *cgroup_dir, long memsz)
+{
+	if (tst_cg_ver & TST_CGROUP_V1)
+		tst_cgroup_set_knob(cgroup_dir, "memory.memsw.limit_in_bytes", memsz);
+
+	if (tst_cg_ver & TST_CGROUP_V2)
+		tst_cgroup_set_knob(cgroup_dir, "memory.swap.max", memsz);
+}
+
+void tst_cgroup_cpuset_read_files(const char *cgroup_dir, const char *filename, char *retbuf)
+{
+	int fd;
+	char *cgroup_new_dir;
+	char knob_path[PATH_MAX];
+
+	cgroup_new_dir = tst_cgroup_get_path(cgroup_dir);
+
+	/*
+	 * try either '/dev/cpuset/XXXX' or '/dev/cpuset/cpuset.XXXX'
+	 * please see Documentation/cgroups/cpusets.txt from kernel src
+	 * for details
+	 */
+	sprintf(knob_path, "%s/%s", cgroup_new_dir, filename);
+	fd = open(knob_path, O_RDONLY);
+	if (fd == -1) {
+		if (errno == ENOENT) {
+			sprintf(knob_path, "%s/cpuset.%s",
+					cgroup_new_dir, filename);
+			fd = SAFE_OPEN(knob_path, O_RDONLY);
+		} else
+			tst_brk(TBROK | TERRNO, "open %s", knob_path);
+	}
+
+	if (read(fd, retbuf, sizeof(retbuf)) < 0)
+		tst_brk(TBROK | TERRNO, "read %s", knob_path);
+
+	close(fd);
+}
+
+void tst_cgroup_cpuset_write_files(const char *cgroup_dir, const char *filename, const char *buf)
+{
+	int fd;
+	char *cgroup_new_dir;
+	char knob_path[PATH_MAX];
+
+	cgroup_new_dir = tst_cgroup_get_path(cgroup_dir);
+
+	/*
+	 * try either '/dev/cpuset/XXXX' or '/dev/cpuset/cpuset.XXXX'
+	 * please see Documentation/cgroups/cpusets.txt from kernel src
+	 * for details
+	 */
+	sprintf(knob_path, "%s/%s", cgroup_new_dir, filename);
+	fd = open(knob_path, O_WRONLY);
+	if (fd == -1) {
+		if (errno == ENOENT) {
+			sprintf(knob_path, "%s/cpuset.%s", cgroup_new_dir, filename);
+			fd = SAFE_OPEN(knob_path, O_WRONLY);
+		} else
+			tst_brk(TBROK | TERRNO, "open %s", knob_path);
+	}
+
+	SAFE_WRITE(1, fd, buf, strlen(buf));
+
+	close(fd);
+}
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v4 2/4] mem: take use of new cgroup API
  2020-06-09  9:50 [LTP] [PATCH v4 1/4] lib: add new cgroup test API Li Wang
@ 2020-06-09  9:51 ` Li Wang
  2020-06-09  9:51 ` [LTP] [PATCH v4 3/4] mem: remove the old " Li Wang
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Li Wang @ 2020-06-09  9:51 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     |  3 +-
 testcases/kernel/mem/ksm/ksm02.c       | 12 ++++----
 testcases/kernel/mem/ksm/ksm03.c       | 12 +++-----
 testcases/kernel/mem/ksm/ksm04.c       | 21 ++++++--------
 testcases/kernel/mem/lib/mem.c         | 10 +++----
 testcases/kernel/mem/oom/oom03.c       | 22 ++++-----------
 testcases/kernel/mem/oom/oom04.c       | 14 ++++-----
 testcases/kernel/mem/oom/oom05.c       | 39 ++++++++------------------
 9 files changed, 57 insertions(+), 95 deletions(-)

diff --git a/testcases/kernel/mem/cpuset/cpuset01.c b/testcases/kernel/mem/cpuset/cpuset01.c
index 853f7fe55..8b1b6fcdf 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());
+	tst_cgroup_move_current(PATH_TMP_CG_CST);
+	tst_cgroup_cpuset_read_files(PATH_TMP_CG_CST, "cpus", buf);
+	tst_cgroup_cpuset_write_files(PATH_TMP_CG_CST, "cpus", buf);
+	tst_cgroup_cpuset_read_files(PATH_TMP_CG_CST, "mems", mems);
+	tst_cgroup_cpuset_write_files(PATH_TMP_CG_CST, "mems", mems);
 
 	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);
+	tst_cgroup_cpuset_write_files(PATH_TMP_CG_CST, "mems", buf);
 	snprintf(buf, BUFSIZ, "%d", nodes[1]);
-	write_cpuset_files(CPATH_NEW, "mems", buf);
+	tst_cgroup_cpuset_write_files(PATH_TMP_CG_CST, "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, PATH_TMP_CG_CST);
 	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(PATH_TMP_CG_CST);
 }
 
 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..c5dca1d9b 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__)
@@ -74,7 +75,7 @@ void write_memcg(void);
 
 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 write_cpusets(const char *cgroup_dir, long nd);
 void mount_mem(char *name, char *fs, char *options, char *path, char *path_new);
 void umount_mem(char *path, char *path_new);
 
diff --git a/testcases/kernel/mem/ksm/ksm02.c b/testcases/kernel/mem/ksm/ksm02.c
index 3d4c19deb..771f4dd65 100644
--- a/testcases/kernel/mem/ksm/ksm02.c
+++ b/testcases/kernel/mem/ksm/ksm02.c
@@ -59,13 +59,13 @@
 #ifdef HAVE_NUMA_V2
 #include <numaif.h>
 
-static int cpuset_mounted;
-
 static void verify_ksm(void)
 {
 	unsigned long nmask[MAXNODES / BITS_PER_LONG] = { 0 };
 	unsigned int node;
 
+	tst_cgroup_move_current(PATH_TMP_CG_CST);
+
 	node = get_a_numa_node();
 	set_node(nmask, node);
 
@@ -78,7 +78,7 @@ static void verify_ksm(void)
 	}
 	create_same_memory(size, num, unit);
 
-	write_cpusets(node);
+	write_cpusets(PATH_TMP_CG_CST, node);
 	create_same_memory(size, num, unit);
 }
 
@@ -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(PATH_TMP_CG_CST);
 }
 
 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, PATH_TMP_CG_CST);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/ksm/ksm03.c b/testcases/kernel/mem/ksm/ksm03.c
index 0e5131654..cc3695e95 100644
--- a/testcases/kernel/mem/ksm/ksm03.c
+++ b/testcases/kernel/mem/ksm/ksm03.c
@@ -59,11 +59,10 @@
 #include "mem.h"
 #include "ksm_common.h"
 
-static int memcg_mounted;
-
 static void verify_ksm(void)
 {
-	write_memcg();
+	tst_cgroup_move_current(PATH_TMP_CG_MEM);
+	tst_cgroup_mem_set_maxbytes(PATH_TMP_CG_MEM, TESTMEM);
 	create_same_memory(size, num, unit);
 }
 
@@ -79,8 +78,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, PATH_TMP_CG_MEM);
 }
 
 static void cleanup(void)
@@ -88,9 +86,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(PATH_TMP_CG_MEM);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/ksm/ksm04.c b/testcases/kernel/mem/ksm/ksm04.c
index e393dbd40..8af529d91 100644
--- a/testcases/kernel/mem/ksm/ksm04.c
+++ b/testcases/kernel/mem/ksm/ksm04.c
@@ -59,9 +59,6 @@
 #ifdef HAVE_NUMA_V2
 #include <numaif.h>
 
-static int cpuset_mounted;
-static int memcg_mounted;
-
 static void verify_ksm(void)
 {
 	unsigned long nmask[MAXNODES / BITS_PER_LONG] = { 0 };
@@ -70,7 +67,8 @@ static void verify_ksm(void)
 	node = get_a_numa_node();
 	set_node(nmask, node);
 
-	write_memcg();
+	tst_cgroup_move_current(PATH_TMP_CG_MEM);
+	tst_cgroup_mem_set_maxbytes(PATH_TMP_CG_MEM, TESTMEM);
 
 	if (set_mempolicy(MPOL_BIND, nmask, MAXNODES) == -1) {
 		if (errno != ENOSYS)
@@ -81,7 +79,8 @@ static void verify_ksm(void)
 	}
 	create_same_memory(size, num, unit);
 
-	write_cpusets(node);
+	tst_cgroup_move_current(PATH_TMP_CG_CST);
+	write_cpusets(PATH_TMP_CG_CST, node);
 	create_same_memory(size, num, unit);
 }
 
@@ -91,10 +90,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(PATH_TMP_CG_MEM);
+	tst_cgroup_umount(PATH_TMP_CG_CST);
 }
 
 static void setup(void)
@@ -110,10 +107,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, PATH_TMP_CG_MEM);
+	tst_cgroup_mount(TST_CGROUP_CPUSET, PATH_TMP_CG_CST);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index eca4c61c8..50bee39cb 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -686,13 +686,13 @@ void write_cpuset_files(char *prefix, char *filename, char *buf)
 	close(fd);
 }
 
-void write_cpusets(long nd)
+void write_cpusets(const char *cgroup_dir, long nd)
 {
 	char buf[BUFSIZ];
 	char cpus[BUFSIZ] = "";
 
 	snprintf(buf, BUFSIZ, "%ld", nd);
-	write_cpuset_files(CPATH_NEW, "mems", buf);
+	tst_cgroup_cpuset_write_files(cgroup_dir, "mems", buf);
 
 	gather_node_cpus(cpus, nd);
 	/*
@@ -701,14 +701,12 @@ void write_cpusets(long nd)
 	 * the value of cpuset.cpus.
 	 */
 	if (strlen(cpus) != 0) {
-		write_cpuset_files(CPATH_NEW, "cpus", cpus);
+		tst_cgroup_cpuset_write_files(cgroup_dir, "cpus", cpus);
 	} else {
 		tst_res(TINFO, "No CPUs in the node%ld; "
 				"using only CPU0", nd);
-		write_cpuset_files(CPATH_NEW, "cpus", "0");
+		tst_cgroup_cpuset_write_files(cgroup_dir, "cpus", "0");
 	}
-
-	SAFE_FILE_PRINTF(CPATH_NEW "/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..fc860c660 100644
--- a/testcases/kernel/mem/oom/oom03.c
+++ b/testcases/kernel/mem/oom/oom03.c
@@ -36,27 +36,19 @@
 
 #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_move_current(PATH_TMP_CG_MEM);
+	tst_cgroup_mem_set_maxbytes(PATH_TMP_CG_MEM, TESTMEM);
 
 	testoom(0, 0, ENOMEM, 1);
 
-	if (access(MEMCG_SW_LIMIT, 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);
+	if (tst_cgroup_mem_swapacct_enabled(PATH_TMP_CG_MEM)) {
+		tst_cgroup_mem_set_maxswap(PATH_TMP_CG_MEM, TESTMEM);
 		testoom(0, 1, ENOMEM, 1);
 	}
 
@@ -73,16 +65,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, PATH_TMP_CG_MEM);
 }
 
 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(PATH_TMP_CG_MEM);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/oom/oom04.c b/testcases/kernel/mem/oom/oom04.c
index 7185ef973..582663275 100644
--- a/testcases/kernel/mem/oom/oom04.c
+++ b/testcases/kernel/mem/oom/oom04.c
@@ -36,14 +36,14 @@
 
 #ifdef HAVE_NUMA_V2
 
-static int cpuset_mounted;
-
 static void verify_oom(void)
 {
 #ifdef TST_ABI32
 	tst_brk(TCONF, "test is not designed for 32-bit system.");
 #endif
 
+	tst_cgroup_move_current(PATH_TMP_CG_CST);
+
 	tst_res(TINFO, "OOM on CPUSET...");
 	testoom(0, 0, ENOMEM, 1);
 
@@ -53,7 +53,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");
+		tst_cgroup_cpuset_write_files(PATH_TMP_CG_CST, "memory_migrate", "1");
 		tst_res(TINFO, "OOM on CPUSET with mem migrate:");
 		testoom(0, 0, ENOMEM, 1);
 	}
@@ -69,8 +69,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, PATH_TMP_CG_CST);
 
 	/*
 	 * Some nodes do not contain memory, so use
@@ -82,15 +81,14 @@ static void setup(void)
 	if (ret < 0)
 		tst_brk(TBROK, "Failed to get a memory node "
 			      "using get_allowed_nodes()");
-	write_cpusets(memnode);
+	write_cpusets(PATH_TMP_CG_CST, memnode);
 }
 
 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(PATH_TMP_CG_CST);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/mem/oom/oom05.c b/testcases/kernel/mem/oom/oom05.c
index db24df6de..871f302e3 100644
--- a/testcases/kernel/mem/oom/oom05.c
+++ b/testcases/kernel/mem/oom/oom05.c
@@ -36,18 +36,16 @@
 
 #ifdef HAVE_NUMA_V2
 
-static int memcg_mounted;
-static int cpuset_mounted;
-
 static void verify_oom(void)
 {
-	int swap_acc_on = 1;
-
 #ifdef TST_ABI32
 	tst_brk(TCONF, "test is not designed for 32-bit system.");
 #endif
 
 	tst_res(TINFO, "OOM on CPUSET & MEMCG...");
+	tst_cgroup_move_current(PATH_TMP_CG_MEM);
+	tst_cgroup_move_current(PATH_TMP_CG_CST);
+	tst_cgroup_mem_set_maxbytes(PATH_TMP_CG_MEM, TESTMEM);
 	testoom(0, 0, ENOMEM, 1);
 
 	/*
@@ -56,29 +54,21 @@ 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");
+		tst_cgroup_cpuset_write_files(PATH_TMP_CG_CST, "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 (errno == ENOENT) {
-			tst_res(TCONF, "memcg swap accounting is disabled");
-			swap_acc_on = 0;
-		} else
-			tst_brk(TBROK|TERRNO, "access");
-	}
-
-	if (swap_acc_on) {
+	if (tst_cgroup_mem_swapacct_enabled(PATH_TMP_CG_MEM)) {
 		tst_res(TINFO, "OOM on CPUSET & MEMCG with "
 				"special memswap limitation:");
-		SAFE_FILE_PRINTF(MEMCG_SW_LIMIT, "%ld", TESTMEM);
+		tst_cgroup_mem_set_maxswap(PATH_TMP_CG_MEM, 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(PATH_TMP_CG_MEM, -1);
 		testoom(0, 0, ENOMEM, 1);
 	}
 }
@@ -93,11 +83,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, PATH_TMP_CG_MEM);
+	tst_cgroup_mount(TST_CGROUP_CPUSET, PATH_TMP_CG_CST);
 
 	/*
 	 * Some nodes do not contain memory, so use
@@ -109,17 +96,15 @@ void setup(void)
 	if (ret < 0)
 		tst_brk(TBROK, "Failed to get a memory node "
 			      "using get_allowed_nodes()");
-	write_cpusets(memnode);
+	write_cpusets(PATH_TMP_CG_CST, memnode);
 }
 
 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(PATH_TMP_CG_MEM);
+	tst_cgroup_umount(PATH_TMP_CG_CST);
 }
 
 static struct tst_test test = {
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v4 3/4] mem: remove the old cgroup API
  2020-06-09  9:50 [LTP] [PATCH v4 1/4] lib: add new cgroup test API Li Wang
  2020-06-09  9:51 ` [LTP] [PATCH v4 2/4] mem: take use of new cgroup API Li Wang
@ 2020-06-09  9:51 ` Li Wang
  2020-06-09  9:51 ` [LTP] [PATCH v4 4/4] mm: add cpuset01 to runtest file Li Wang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Li Wang @ 2020-06-09  9:51 UTC (permalink / raw)
  To: ltp

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

diff --git a/testcases/kernel/mem/include/mem.h b/testcases/kernel/mem/include/mem.h
index c5dca1d9b..42b12a230 100644
--- a/testcases/kernel/mem/include/mem.h
+++ b/testcases/kernel/mem/include/mem.h
@@ -60,24 +60,8 @@ 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
-
-void read_cpuset_files(char *prefix, char *filename, char *retbuf);
-void write_cpuset_files(char *prefix, char *filename, char *buf);
+/* cpuset/memcg - include/tst_cgroup.h */
 void write_cpusets(const char *cgroup_dir, 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 50bee39cb..2de3f83a6 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;
@@ -603,7 +596,6 @@ void test_ksm_merge_across_nodes(unsigned long nr_pages)
 /* THP */
 
 /* cpuset/memcg */
-
 static void gather_node_cpus(char *cpus, long nd)
 {
 	int ncpus = 0;
@@ -637,55 +629,6 @@ static void gather_node_cpus(char *cpus, long nd)
 	cpus[strlen(cpus) - 1] = '\0';
 }
 
-void read_cpuset_files(char *prefix, char *filename, char *retbuf)
-{
-	int fd;
-	char path[BUFSIZ];
-
-	/*
-	 * try either '/dev/cpuset/XXXX' or '/dev/cpuset/cpuset.XXXX'
-	 * please see Documentation/cgroups/cpusets.txt from kernel src
-	 * for details
-	 */
-	snprintf(path, BUFSIZ, "%s/%s", prefix, filename);
-	fd = open(path, O_RDONLY);
-	if (fd == -1) {
-		if (errno == ENOENT) {
-			snprintf(path, BUFSIZ, "%s/cpuset.%s",
-				 prefix, filename);
-			fd = SAFE_OPEN(path, O_RDONLY);
-		} else
-			tst_brk(TBROK | TERRNO, "open %s", path);
-	}
-	if (read(fd, retbuf, BUFSIZ) < 0)
-		tst_brk(TBROK | TERRNO, "read %s", path);
-	close(fd);
-}
-
-void write_cpuset_files(char *prefix, char *filename, char *buf)
-{
-	int fd;
-	char path[BUFSIZ];
-
-	/*
-	 * try either '/dev/cpuset/XXXX' or '/dev/cpuset/cpuset.XXXX'
-	 * please see Documentation/cgroups/cpusets.txt from kernel src
-	 * for details
-	 */
-	snprintf(path, BUFSIZ, "%s/%s", prefix, filename);
-	fd = open(path, O_WRONLY);
-	if (fd == -1) {
-		if (errno == ENOENT) {
-			snprintf(path, BUFSIZ, "%s/cpuset.%s",
-				 prefix, filename);
-			fd = SAFE_OPEN(path, O_WRONLY);
-		} else
-			tst_brk(TBROK | TERRNO, "open %s", path);
-	}
-	SAFE_WRITE(1, fd, buf, strlen(buf));
-	close(fd);
-}
-
 void write_cpusets(const char *cgroup_dir, long nd)
 {
 	char buf[BUFSIZ];
@@ -709,57 +652,6 @@ void write_cpusets(const char *cgroup_dir, long nd)
 	}
 }
 
-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] 15+ messages in thread

* [LTP] [PATCH v4 4/4] mm: add cpuset01 to runtest file
  2020-06-09  9:50 [LTP] [PATCH v4 1/4] lib: add new cgroup test API Li Wang
  2020-06-09  9:51 ` [LTP] [PATCH v4 2/4] mem: take use of new cgroup API Li Wang
  2020-06-09  9:51 ` [LTP] [PATCH v4 3/4] mem: remove the old " Li Wang
@ 2020-06-09  9:51 ` Li Wang
  2020-06-19  7:35   ` Petr Vorel
  2020-06-09 10:42 ` [LTP] [PATCH v4 1/4] lib: add new cgroup test API Jan Stancek
  2020-09-23 11:03 ` Richard Palethorpe
  4 siblings, 1 reply; 15+ messages in thread
From: Li Wang @ 2020-06-09  9:51 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] 15+ messages in thread

* [LTP] [PATCH v4 1/4] lib: add new cgroup test API
  2020-06-09  9:50 [LTP] [PATCH v4 1/4] lib: add new cgroup test API Li Wang
                   ` (2 preceding siblings ...)
  2020-06-09  9:51 ` [LTP] [PATCH v4 4/4] mm: add cpuset01 to runtest file Li Wang
@ 2020-06-09 10:42 ` Jan Stancek
  2020-06-10 10:12   ` Li Wang
  2020-09-23 11:03 ` Richard Palethorpe
  4 siblings, 1 reply; 15+ messages in thread
From: Jan Stancek @ 2020-06-09 10:42 UTC (permalink / raw)
  To: ltp


----- Original Message -----
> 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).
> 

No strong objections to v4, couple comments below (if you spin v5 because
of other reviews).

> +2.2.36 Using Control Group
> +^^^^^^^^^^^^^^^^^^^^^^^^^^

Would be nice if there was short description of each function.

> +static void tst_cgroup_set_path(const char *cgroup_dir)
> +{
> +	char cgroup_new_dir[PATH_MAX];
> +	struct tst_cgroup_path *tst_cgroup_path, *a;
> +
> +	if (!cgroup_dir)
> +		tst_brk(TBROK, "Invalid cgroup dir, plese check cgroup_dir");
> +
> +	sprintf(cgroup_new_dir, "%s/ltp_%d", cgroup_dir, rand());
> +
> +	/* To store cgroup path in the 'path' list */
> +	tst_cgroup_path = SAFE_MMAP(NULL, (sizeof(struct tst_cgroup_path)),
> +			PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

This looks like it could use just SAFE_MALLOC/SAFE_FREE.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v4 1/4] lib: add new cgroup test API
  2020-06-09 10:42 ` [LTP] [PATCH v4 1/4] lib: add new cgroup test API Jan Stancek
@ 2020-06-10 10:12   ` Li Wang
  2020-06-19  3:07     ` Li Wang
  0 siblings, 1 reply; 15+ messages in thread
From: Li Wang @ 2020-06-10 10:12 UTC (permalink / raw)
  To: ltp

On Tue, Jun 9, 2020 at 6:42 PM Jan Stancek <jstancek@redhat.com> wrote:

>
> ----- Original Message -----
> > 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).
> >
>
> No strong objections to v4, couple comments below (if you spin v5 because
> of other reviews).
>

Thanks for review!
Ok, I agree to keep patchv4 posts for more days in case other people have
comments.


>
> > +2.2.36 Using Control Group
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> Would be nice if there was short description of each function.
>

I'd add some code comments in the tst_cgroup.h header file. Which something
maybe like:

/* To mount/umount specified cgroup controller on 'cgroup_dir' path */
void tst_cgroup_mount(enum tst_cgroup_ctrl ctrl, const char *cgroup_dir);
void tst_cgroup_umount(const char *cgroup_dir);

/* To move current process PID to the mounted cgroup tasks */
void tst_cgroup_move_current(const char *cgroup_dir);

/* To set cgroup controller knob with new value */
void tst_cgroup_set_knob(const char *cgroup_dir, const char *knob, long
value);

/* Set of functions to set knobs under the memory controller */
void tst_cgroup_mem_set_maxbytes(const char *cgroup_dir, long memsz);
int  tst_cgroup_mem_swapacct_enabled(const char *cgroup_dir);
void tst_cgroup_mem_set_maxswap(const char *cgroup_dir, long memsz);

/* Set of functions to read/write cpuset controller files content */
void tst_cgroup_cpuset_read_files(const char *cgroup_dir, const char
*filename, char *retbuf);
void tst_cgroup_cpuset_write_files(const char *cgroup_dir, const char
*filename, const char *buf);



>
> > +static void tst_cgroup_set_path(const char *cgroup_dir)
> > +{
> > +     char cgroup_new_dir[PATH_MAX];
> > +     struct tst_cgroup_path *tst_cgroup_path, *a;
> > +
> > +     if (!cgroup_dir)
> > +             tst_brk(TBROK, "Invalid cgroup dir, plese check
> cgroup_dir");
> > +
> > +     sprintf(cgroup_new_dir, "%s/ltp_%d", cgroup_dir, rand());
> > +
> > +     /* To store cgroup path in the 'path' list */
> > +     tst_cgroup_path = SAFE_MMAP(NULL, (sizeof(struct tst_cgroup_path)),
> > +                     PROT_READ | PROT_WRITE, MAP_PRIVATE |
> MAP_ANONYMOUS, -1, 0);
>
> This looks like it could use just SAFE_MALLOC/SAFE_FREE.
>

Agree.

And btw, seems we have to set cgroup.clone_children as 1, otherwise, we
can't write anything to the cpuset subsystem files.

    BROK: Failed to close FILE '/tmp/cgroup_cst/ltp_1804289383/tasks' at
tst_cgroup.c:296: ENOSPC (28)

So these lines will be add in to library as neccesary:

--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -251,6 +251,16 @@ void tst_cgroup_mount(enum tst_cgroup_ctrl ctrl, const
char *cgroup_dir)
                break;
                case TST_CGROUP_CPUSET:
                        tst_cgroup1_mount("cpusetcg", "cpuset", cgroup_dir,
cgroup_new_dir);
+                       /*
+                        * we should assign one or more memory nodes to
cpuset.mems
+                        * and cpuset.cpus, otherwise, echo $$ > tasks
gives ?no space
+                        * left on device: ENOSPC? when trying to use
cpuset.
+                        *
+                        * Or, setting cgroup.clone_children to 1 can help
in automatically
+                        * inheriting memory and node setting from parent
cgroup when a
+                        * child cgroup is created.
+                        */
+                       tst_cgroup_set_knob(cgroup_dir,
"../cgroup.clone_children", 1);
                break;


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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v4 1/4] lib: add new cgroup test API
  2020-06-10 10:12   ` Li Wang
@ 2020-06-19  3:07     ` Li Wang
  2020-06-22 13:10       ` Petr Vorel
  0 siblings, 1 reply; 15+ messages in thread
From: Li Wang @ 2020-06-19  3:07 UTC (permalink / raw)
  To: ltp

I have pushed the patchset with the following changes:

1. use SAFE_MALLOC to replace SAFE_MMAP
2. set 1 to cgroup.clone_children before creating dir
3. add more code comments in tst_cgroup.h

Let's see how things going on with these new APIs using in next.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v4 4/4] mm: add cpuset01 to runtest file
  2020-06-09  9:51 ` [LTP] [PATCH v4 4/4] mm: add cpuset01 to runtest file Li Wang
@ 2020-06-19  7:35   ` Petr Vorel
  0 siblings, 0 replies; 15+ messages in thread
From: Petr Vorel @ 2020-06-19  7:35 UTC (permalink / raw)
  To: ltp

Hi Li,

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

Thanks for fixing my regression :).

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v4 1/4] lib: add new cgroup test API
  2020-06-19  3:07     ` Li Wang
@ 2020-06-22 13:10       ` Petr Vorel
  2020-06-23  0:10         ` Li Wang
  0 siblings, 1 reply; 15+ messages in thread
From: Petr Vorel @ 2020-06-22 13:10 UTC (permalink / raw)
  To: ltp

Hi all,

> I have pushed the patchset with the following changes:
I've updated wiki. Please remember updating it when you touch doc/*.txt.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v4 1/4] lib: add new cgroup test API
  2020-06-22 13:10       ` Petr Vorel
@ 2020-06-23  0:10         ` Li Wang
  0 siblings, 0 replies; 15+ messages in thread
From: Li Wang @ 2020-06-23  0:10 UTC (permalink / raw)
  To: ltp

On Mon, Jun 22, 2020 at 10:20 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi all,
>
> > I have pushed the patchset with the following changes:
> I've updated wiki. Please remember updating it when you touch doc/*.txt.
>

Ah yes, I have forgotten that more than twice. Sorry for the inconvenience.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v4 1/4] lib: add new cgroup test API
  2020-06-09  9:50 [LTP] [PATCH v4 1/4] lib: add new cgroup test API Li Wang
                   ` (3 preceding siblings ...)
  2020-06-09 10:42 ` [LTP] [PATCH v4 1/4] lib: add new cgroup test API Jan Stancek
@ 2020-09-23 11:03 ` Richard Palethorpe
  2020-09-23 12:23   ` Li Wang
  4 siblings, 1 reply; 15+ messages in thread
From: Richard Palethorpe @ 2020-09-23 11:03 UTC (permalink / raw)
  To: ltp

Hello,

Li Wang <liwang@redhat.com> writes:

> 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).

This appears to be broken on SUSE.

> +enum tst_cgroup_ver tst_cgroup_version(void)
> +{
> +	if (tst_cgroup_check("cgroup2")) {
> +		if (!tst_is_mounted("cgroup2") && tst_is_mounted("cgroup"))

Unfortunately this is backwards: if a cgroup controller is mounted
(e.g. memory) in the v1 separated hierarchy then it is not available in
the cgroup v2 unified hierarchy even though the v2 unified hierarchy
exists.

So ksm{03,04} fails with:

safe_file_ops.c:299: BROK: Failed to close FILE
'/tmp/cgroup_mem/cgroup.subtree_control' at tst_cgroup.c:301: ENOENT (2)

because we have the memory controller (in fact all the controllers)
mounted in the V1 way, but we also have the cgroup2 mounted. If I
unmount the memory controller from V1 then the test passes.

Another potential problem is that it may be possible to remove access to
controllers some other way in which case you have to check
cgroup.controllers to see what is available. OTOH maybe this is not
possible because you mount a new cgroup2 hierarchy?

-- 
Thank you,
Richard.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v4 1/4] lib: add new cgroup test API
  2020-09-23 11:03 ` Richard Palethorpe
@ 2020-09-23 12:23   ` Li Wang
  2020-09-23 13:14     ` Richard Palethorpe
  0 siblings, 1 reply; 15+ messages in thread
From: Li Wang @ 2020-09-23 12:23 UTC (permalink / raw)
  To: ltp

On Wed, Sep 23, 2020 at 7:03 PM Richard Palethorpe <rpalethorpe@suse.de>
wrote:

> Hello,
>
> Li Wang <liwang@redhat.com> writes:
>
> > 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).
>
> This appears to be broken on SUSE.
>
> > +enum tst_cgroup_ver tst_cgroup_version(void)
> > +{
> > +     if (tst_cgroup_check("cgroup2")) {
> > +             if (!tst_is_mounted("cgroup2") && tst_is_mounted("cgroup"))
>
> Unfortunately this is backwards: if a cgroup controller is mounted
> (e.g. memory) in the v1 separated hierarchy then it is not available in
> the cgroup v2 unified hierarchy even though the v2 unified hierarchy
> exists.
>

That's on purpose because we don't want the user to mix v1 and v2 in using.
In other words, if a version of cgroup has been chosen on a SUT then LTP
follows in the same version.

The cgroup lib choose cgroup version likes:
      * system doesn't support any cgroup(v1, v2): TCONF
      * system only support cgroup v1: choose v1
      * system only support cgroup v2: choose v2
      * system support v1 & v2 but mounting v1: chosse v1   <=== this is
what you met
      * system support v1 & v2 but mounting v2: choose v2
      * system support v1 & v2 but mounting none: choose v2



>
> So ksm{03,04} fails with:
>
> safe_file_ops.c:299: BROK: Failed to close FILE
> '/tmp/cgroup_mem/cgroup.subtree_control' at tst_cgroup.c:301: ENOENT (2)
>
> because we have the memory controller (in fact all the controllers)
> mounted in the V1 way, but we also have the cgroup2 mounted. If I
>

Hmm, this is a situation that system mount v1&v2 at the same time.

So the ksm03/4 choose v2 automatically but hit a problem in cgroup_v2 using.
I pesonally think it mainly imputes to the SUT config in cgroup, because
that
makes cgroup_v2 has no actuall controllers in the cgroup.controllers. After
umounting all V1 hierarchies, the controllers get back into V2.



> unmount the memory controller from V1 then the test passes.
>
> Another potential problem is that it may be possible to remove access to
> controllers some other way in which case you have to check
> cgroup.controllers to see what is available. OTOH maybe this is not
> possible because you mount a new cgroup2 hierarchy?
>

I'm not sure about this.

To be honest, I hope a Linux distribution provides a pure way in cgroup
using, which means just mounting V1 or V2 but not to mix together. Or, do
you think LTP should handle the situation for the v1&v2 mixing mount?

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v4 1/4] lib: add new cgroup test API
  2020-09-23 12:23   ` Li Wang
@ 2020-09-23 13:14     ` Richard Palethorpe
  2020-09-23 14:40       ` Li Wang
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Palethorpe @ 2020-09-23 13:14 UTC (permalink / raw)
  To: ltp

Hello Li,

Li Wang <liwang@redhat.com> writes:

> On Wed, Sep 23, 2020 at 7:03 PM Richard Palethorpe <rpalethorpe@suse.de>
> wrote:
>
>> Hello,
>>
>> Li Wang <liwang@redhat.com> writes:
>>
>> > 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).
>>
>> This appears to be broken on SUSE.
>>
>> > +enum tst_cgroup_ver tst_cgroup_version(void)
>> > +{
>> > +     if (tst_cgroup_check("cgroup2")) {
>> > +             if (!tst_is_mounted("cgroup2") && tst_is_mounted("cgroup"))
>>
>> Unfortunately this is backwards: if a cgroup controller is mounted
>> (e.g. memory) in the v1 separated hierarchy then it is not available in
>> the cgroup v2 unified hierarchy even though the v2 unified hierarchy
>> exists.
>>
>
> That's on purpose because we don't want the user to mix v1 and v2 in using.
> In other words, if a version of cgroup has been chosen on a SUT then LTP
> follows in the same version.
>
> The cgroup lib choose cgroup version likes:
>       * system doesn't support any cgroup(v1, v2): TCONF
>       * system only support cgroup v1: choose v1
>       * system only support cgroup v2: choose v2
>       * system support v1 & v2 but mounting v1: chosse v1   <=== this is
> what you met

Perhaps you meant:

if (tst_is_mounted("cgroup"))
   cg_ver = TST_CGROUP_V1;
else
   cg_ver = TST_CGROUP_V2;

There is no point trying to use V2 if the controller is active in V1.

>       * system support v1 & v2 but mounting v2: choose v2
>       * system support v1 & v2 but mounting none: choose v2
>
>
>
>>
>> So ksm{03,04} fails with:
>>
>> safe_file_ops.c:299: BROK: Failed to close FILE
>> '/tmp/cgroup_mem/cgroup.subtree_control' at tst_cgroup.c:301: ENOENT (2)
>>
>> because we have the memory controller (in fact all the controllers)
>> mounted in the V1 way, but we also have the cgroup2 mounted. If I
>>
>
> Hmm, this is a situation that system mount v1&v2 at the same time.
>
> So the ksm03/4 choose v2 automatically but hit a problem in cgroup_v2 using.
> I pesonally think it mainly imputes to the SUT config in cgroup, because
> that
> makes cgroup_v2 has no actuall controllers in the cgroup.controllers. After
> umounting all V1 hierarchies, the controllers get back into V2.
>

yes

>
>
>> unmount the memory controller from V1 then the test passes.
>>
>> Another potential problem is that it may be possible to remove access to
>> controllers some other way in which case you have to check
>> cgroup.controllers to see what is available. OTOH maybe this is not
>> possible because you mount a new cgroup2 hierarchy?
>>
>
> I'm not sure about this.
>
> To be honest, I hope a Linux distribution provides a pure way in cgroup
> using, which means just mounting V1 or V2 but not to mix together. Or, do
> you think LTP should handle the situation for the v1&v2 mixing mount?

For now, I think we just need to use V1 if it is mounted. I don't like
the fact we have both mounted, but it seems most software can handle it,
so LTP should as well.

-- 
Thank you,
Richard.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v4 1/4] lib: add new cgroup test API
  2020-09-23 13:14     ` Richard Palethorpe
@ 2020-09-23 14:40       ` Li Wang
  2020-09-24  8:55         ` Richard Palethorpe
  0 siblings, 1 reply; 15+ messages in thread
From: Li Wang @ 2020-09-23 14:40 UTC (permalink / raw)
  To: ltp

On Wed, Sep 23, 2020 at 9:14 PM Richard Palethorpe <rpalethorpe@suse.de>
wrote:

> ...
> > That's on purpose because we don't want the user to mix v1 and v2 in
> using.
> > In other words, if a version of cgroup has been chosen on a SUT then LTP
> > follows in the same version.
> >
> > The cgroup lib choose cgroup version likes:
> >       * system doesn't support any cgroup(v1, v2): TCONF
> >       * system only support cgroup v1: choose v1
> >       * system only support cgroup v2: choose v2
> >       * system support v1 & v2 but mounting v1: chosse v1   <=== this is
> > what you met
>
> Perhaps you meant:
>
> if (tst_is_mounted("cgroup"))
>    cg_ver = TST_CGROUP_V1;
> else
>    cg_ver = TST_CGROUP_V2;
>

This is not enough for the version choice. As tst_is_mounted(?cgroup?) can't
distinguish precisely what "cgroup" or "cgroup2" has been mounted. Because
the function is fuzzy matching a string.



> >
> > To be honest, I hope a Linux distribution provides a pure way in cgroup
> > using, which means just mounting V1 or V2 but not to mix together. Or, do
> > you think LTP should handle the situation for the v1&v2 mixing mount?
>
> For now, I think we just need to use V1 if it is mounted. I don't like
> the fact we have both mounted, but it seems most software can handle it,
> so LTP should as well.
>

Agreed.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v4 1/4] lib: add new cgroup test API
  2020-09-23 14:40       ` Li Wang
@ 2020-09-24  8:55         ` Richard Palethorpe
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Palethorpe @ 2020-09-24  8:55 UTC (permalink / raw)
  To: ltp

Hello,

Li Wang <liwang@redhat.com> writes:

>> Perhaps you meant:
>>
>> if (tst_is_mounted("cgroup"))
>>    cg_ver = TST_CGROUP_V1;
>> else
>>    cg_ver = TST_CGROUP_V2;
>>
>
> This is not enough for the version choice. As tst_is_mounted(?cgroup?) can't
> distinguish precisely what "cgroup" or "cgroup2" has been mounted. Because
> the function is fuzzy matching a string.
>> >
>> > To be honest, I hope a Linux distribution provides a pure way in cgroup
>> > using, which means just mounting V1 or V2 but not to mix together. Or, do
>> > you think LTP should handle the situation for the v1&v2 mixing mount?
>>
>> For now, I think we just need to use V1 if it is mounted. I don't like
>> the fact we have both mounted, but it seems most software can handle it,
>> so LTP should as well.
>>
>
> Agreed.

OK, I will send a patch unless you send one first.

-- 
Thank you,
Richard.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-09-24  8:55 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-09  9:50 [LTP] [PATCH v4 1/4] lib: add new cgroup test API Li Wang
2020-06-09  9:51 ` [LTP] [PATCH v4 2/4] mem: take use of new cgroup API Li Wang
2020-06-09  9:51 ` [LTP] [PATCH v4 3/4] mem: remove the old " Li Wang
2020-06-09  9:51 ` [LTP] [PATCH v4 4/4] mm: add cpuset01 to runtest file Li Wang
2020-06-19  7:35   ` Petr Vorel
2020-06-09 10:42 ` [LTP] [PATCH v4 1/4] lib: add new cgroup test API Jan Stancek
2020-06-10 10:12   ` Li Wang
2020-06-19  3:07     ` Li Wang
2020-06-22 13:10       ` Petr Vorel
2020-06-23  0:10         ` Li Wang
2020-09-23 11:03 ` Richard Palethorpe
2020-09-23 12:23   ` Li Wang
2020-09-23 13:14     ` Richard Palethorpe
2020-09-23 14:40       ` Li Wang
2020-09-24  8:55         ` Richard Palethorpe

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.