[LTP] [RFC][PATCH] tst_cgroup: Attempt to use CGroups V2 then V1 instead of guessing

Richard Palethorpe rpalethorpe@suse.com
Fri Sep 25 14:19:41 CEST 2020


The best way to find out if we can do something is to try it, we don't
check if the system has enough RAM and PIDs available before calling
fork() in safe_fork(). Currently we try to guess what cgroups are
currently in use then try to use the same version. We guess by
grepping the mounts and filesystem files, these files need to be
parsed in a structured way and even then, it is not the job of all
tests which *use* cgroups to test that if cgroup(2) is present in
mounts or filesystem that it can then be used.

The cpuset group is only available on V1 and we can mount and use V1
even if V2 is active. Just because the system has V2 active does not
mean we cannot execute tests which require V1. This is one example
where trying to figure out ahead of time what can or can't be used
results in less testing.

Furthermore removing these checks results in a ~50% reduction in code
and this is without correct parsing and checking of mounts and
filesystems. We also have to consider that just because one V1
controller is mounted, this does not prevent all V2 controllers from
being used. So someone may mount V1 cpuset for legacy reasons, while
using V2 for other controllers. To account for this we would need to
check each controller individually.

Note that the above may be a valid thing to do in a test checking
specific cgroup behavior, but this library code is meant for use by
all tests which need cgroups for some reason.
---
 include/tst_cgroup.h |   2 -
 lib/tst_cgroup.c     | 118 ++++++++++++++-----------------------------
 2 files changed, 39 insertions(+), 81 deletions(-)

diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index 77780e0d6..62d381ce9 100644
--- a/include/tst_cgroup.h
+++ b/include/tst_cgroup.h
@@ -21,8 +21,6 @@ enum tst_cgroup_ctrl {
 	/* add cgroup controller */
 };
 
-enum tst_cgroup_ver tst_cgroup_version(void);
-
 /* 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);
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index ba413d874..887423bc6 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -21,47 +21,6 @@
 static enum tst_cgroup_ver tst_cg_ver;
 static int clone_children;
 
-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)
-{
-        enum tst_cgroup_ver cg_ver;
-
-        if (tst_cgroup_check("cgroup2")) {
-                if (!tst_is_mounted("cgroup2") && tst_is_mounted("cgroup"))
-                        cg_ver = TST_CGROUP_V1;
-                else
-                        cg_ver = TST_CGROUP_V2;
-
-                goto out;
-        }
-
-        if (tst_cgroup_check("cgroup"))
-                cg_ver = TST_CGROUP_V1;
-
-        if (!cg_ver)
-                tst_brk(TCONF, "Cgroup is not configured");
-
-out:
-        return cg_ver;
-}
-
 static void tst_cgroup1_mount(const char *name, const char *option,
 			const char *mnt_path, const char *new_path)
 {
@@ -100,26 +59,18 @@ out:
 	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)
+static int cgroup2_mount(const char *mnt_path, const char *new_path)
 {
-	if (tst_is_mounted(mnt_path))
-		goto out;
+	if (!tst_is_mounted(mnt_path)) {
+		SAFE_MKDIR(mnt_path, 0777);
 
-	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);
+		if (mount("cgroup2", mnt_path, "cgroup2", 0, NULL) == -1)
+			return -1;
 	}
 
-out:
 	SAFE_MKDIR(new_path, 0777);
 
-	tst_res(TINFO, "Cgroup v2 mount at %s success", mnt_path);
+	return 0;
 }
 
 static void tst_cgroupN_umount(const char *mnt_path, const char *new_path)
@@ -274,39 +225,48 @@ 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 (ctrl == TST_CGROUP_CPUSET) {
+		tst_res(TINFO, "CGroup V2 lacks cpuset subsystem, trying V1");
+		goto cgroup_v1;
 	}
 
-	if (tst_cg_ver & TST_CGROUP_V2) {
-		tst_cgroup2_mount(cgroup_dir, cgroup_new_dir);
+	if (cgroup2_mount(cgroup_dir, cgroup_new_dir)) {
+		tst_res(TINFO | TERRNO, "Mounting CGroup V2 failed, trying V1");
+		goto cgroup_v1;
+	}
+
+	tst_res(TINFO, "Mounted CGroup V2");
+
+	switch(ctrl) {
+	case TST_CGROUP_MEMCG:
+		sprintf(knob_path, "%s/cgroup.subtree_control", cgroup_dir);
+		if (FILE_PRINTF(knob_path, "%s", "+memory")) {
+			tst_res(TINFO,
+				"Can't add V2 memory controller, this might be because it is mounted as V1");
+			break;
+		}
+		tst_cg_ver = TST_CGROUP_V2;
+		return;
+	default:
+		tst_brk(TBROK, "Invalid cgroup controller: %d", ctrl);
+	}
 
-		switch(ctrl) {
-		case TST_CGROUP_MEMCG:
-			sprintf(knob_path, "%s/cgroup.subtree_control", cgroup_dir);
-			SAFE_FILE_PRINTF(knob_path, "%s", "+memory");
+cgroup_v1:
+	switch(ctrl) {
+	case TST_CGROUP_MEMCG:
+		tst_cgroup1_mount("memcg", "memory", cgroup_dir, cgroup_new_dir);
 		break;
-		case TST_CGROUP_CPUSET:
-			tst_brk(TCONF, "Cgroup v2 hasn't achieve cpuset subsystem");
+	case TST_CGROUP_CPUSET:
+		tst_cgroup1_mount("cpusetcg", "cpuset", cgroup_dir, cgroup_new_dir);
 		break;
-		default:
-			tst_brk(TBROK, "Invalid cgroup controller: %d", ctrl);
-		}
+	default:
+		tst_brk(TBROK, "Invalid cgroup controller: %d", ctrl);
 	}
+
+	tst_cg_ver = TST_CGROUP_V1;
 }
 
 void tst_cgroup_umount(const char *cgroup_dir)
-- 
2.28.0



More information about the ltp mailing list