<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small">Hi Richard,</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">This is a good improvement for the CG API. Nice work!!</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Feb 3, 2022 at 4:20 PM Richard Palethorpe via ltp <<a href="mailto:ltp@lists.linux.it" target="_blank">ltp@lists.linux.it</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail_default" style="font-size:small">...</span></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
--- a/include/tst_test.h<br>
+++ b/include/tst_test.h<br>
@@ -133,6 +133,14 @@ extern unsigned int tst_variant;<br>
<br>
 #define TST_NO_HUGEPAGES ((unsigned long)-1)<br>
<br>
+/* CGroups Kernel API version */<br>
+enum tst_cgroup_ver {<br>
+       TST_CGROUP_V1 = 1,<br>
+       TST_CGROUP_V2 = 2,<br>
+};<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">We can move this into 'tst_cgroup.h' if included that header in tst_test.h.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">(As we decide to integrate the CG structure in tst_test, it<br></div><div class="gmail_default" style="font-size:small">seems better to include that tst_cgorup.h file though that</div><div class="gmail_default" style="font-size:small">makes some binaries become larger.)</div><div class="gmail_default" style="font-size:small"><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+struct tst_cgroup_group;<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">As Cyril pointed out, this is useless.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
 struct tst_test {<br>
        /* number of tests available in test() function */<br>
        unsigned int tcnt;<br>
@@ -280,6 +288,12 @@ struct tst_test {<br>
<br>
        /* NULL terminated array of required commands */<br>
        const char *const *needs_cmds;<br>
+<br>
+       /* Requires a particular CGroup API version. */<br>
+       const enum tst_cgroup_ver needs_cgroup_ver;<br>
+<br>
+       /* {} terminated array of required CGroup controllers */<br>
<span class="gmail_default" style="font-size:small"></span>+       const char *const *needs_cgroup_controllers;<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Can we use abbreviations for both? just to keep them consistently :).</div><div class="gmail_default" style="font-size:small"></div><span class="gmail_default" style="font-size:small"></span>       const enum tst_cgroup_ver needs_cgroup_ver;<br><div class="gmail_default" style="font-size:small">       const char *const *needs_cgroup_ctrls;</div><br></div><div><div class="gmail_default" style="font-size:small">or,</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"><span class="gmail_default"> </span>      const enum tst_cgroup_ver needs_cgroup_version;<br></div><div class="gmail_default" style="font-size:small"><span class="gmail_default"></span>       const char *const *needs_cgroup_controllers;</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
 };<br>
<br>
 /*<br>
diff --git a/lib/newlib_tests/tst_cgroup01.c b/lib/newlib_tests/tst_cgroup01.c<br>
index 54a370362..62df9aab2 100644<br>
--- a/lib/newlib_tests/tst_cgroup01.c<br>
+++ b/lib/newlib_tests/tst_cgroup01.c<br>
@@ -22,7 +22,7 @@ static void do_test(void)<br>
<br>
 static void setup(void)<br>
 {<br>
-       cgopts.only_mount_v1 = !!only_mount_v1,<br>
+       cgopts.needs_ver = !!only_mount_v1 ? TST_CGROUP_V1 : 0;<br>
<br>
        tst_cgroup_scan();<br>
        tst_cgroup_print_config();<br>
diff --git a/lib/newlib_tests/tst_cgroup02.c b/lib/newlib_tests/tst_cgroup02.c<br>
index 64b0a1e94..d048c720a 100644<br>
--- a/lib/newlib_tests/tst_cgroup02.c<br>
+++ b/lib/newlib_tests/tst_cgroup02.c<br>
@@ -15,8 +15,6 @@ static struct tst_option opts[] = {<br>
        {NULL, NULL, NULL},<br>
 };<br>
 static struct tst_cgroup_opts cgopts;<br>
-static const struct tst_cgroup_group *cg;<br>
-static const struct tst_cgroup_group *cg_drain;<br>
 static struct tst_cgroup_group *cg_child;<br>
<br>
 static void do_test(void)<br>
@@ -24,12 +22,12 @@ static void do_test(void)<br>
        char buf[BUFSIZ];<br>
        size_t mem;<br>
<br>
-       if (!TST_CGROUP_VER_IS_V1(cg, "memory"))<br>
-               SAFE_CGROUP_PRINT(cg, "cgroup.subtree_control", "+memory");<br>
-       if (!TST_CGROUP_VER_IS_V1(cg, "cpuset"))<br>
-               SAFE_CGROUP_PRINT(cg, "cgroup.subtree_control", "+cpuset");<br>
+       if (!TST_CGROUP_VER_IS_V1(tst_cgroup, "memory"))<br>
+               SAFE_CGROUP_PRINT(tst_cgroup, "cgroup.subtree_control", "+memory");<br>
+       if (!TST_CGROUP_VER_IS_V1(tst_cgroup, "cpuset"))<br>
+               SAFE_CGROUP_PRINT(tst_cgroup, "cgroup.subtree_control", "+cpuset");<br>
<br>
-       cg_child = tst_cgroup_group_mk(cg, "child");<br>
+       cg_child = tst_cgroup_group_mk(tst_cgroup, "child");<br>
        if (!SAFE_FORK()) {<br>
                SAFE_CGROUP_PRINTF(cg_child, "cgroup.procs", "%d", getpid());<br>
<br>
@@ -47,19 +45,19 @@ static void do_test(void)<br>
                exit(0);<br>
        }<br>
<br>
-       SAFE_CGROUP_PRINTF(cg, "memory.max", "%zu", (1UL << 24) - 1);<br>
+       SAFE_CGROUP_PRINTF(tst_cgroup, "memory.max", "%zu", (1UL << 24) - 1);<br>
        SAFE_CGROUP_PRINTF(cg_child, "cgroup.procs", "%d", getpid());<br>
-       SAFE_CGROUP_SCANF(cg, "memory.current", "%zu", &mem);<br>
+       SAFE_CGROUP_SCANF(tst_cgroup, "memory.current", "%zu", &mem);<br>
        tst_res(TPASS, "memory.current = %zu", mem);<br>
<br>
        tst_reap_children();<br>
-       SAFE_CGROUP_PRINTF(cg_drain, "cgroup.procs", "%d", getpid());<br>
+       SAFE_CGROUP_PRINTF(tst_cgroup_drain, "cgroup.procs", "%d", getpid());<br>
        cg_child = tst_cgroup_group_rm(cg_child);<br>
 }<br>
<br>
 static void setup(void)<br>
 {<br>
-       cgopts.only_mount_v1 = !!only_mount_v1,<br>
+       cgopts.needs_ver = !!only_mount_v1 ? TST_CGROUP_V1 : 0;<br>
<br>
        tst_cgroup_scan();<br>
        tst_cgroup_print_config();<br>
@@ -67,14 +65,14 @@ static void setup(void)<br>
        tst_cgroup_require("memory", &cgopts);<br>
        tst_cgroup_require("cpuset", &cgopts);<br>
<br>
-       cg = tst_cgroup_get_test_group();<br>
-       cg_drain = tst_cgroup_get_drain_group();<br>
+       tst_cgroup_init();<br>
 }<br>
<br>
 static void cleanup(void)<br>
 {<br>
        if (cg_child) {<br>
-               SAFE_CGROUP_PRINTF(cg_drain, "cgroup.procs", "%d", getpid());<br>
+               SAFE_CGROUP_PRINTF(tst_cgroup_drain,<br>
+                                  "cgroup.procs", "%d", getpid());<br>
                cg_child = tst_cgroup_group_rm(cg_child);<br>
        }<br>
        if (!no_cleanup)<br>
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c<br>
index 10b65364b..e694d353b 100644<br>
--- a/lib/tst_cgroup.c<br>
+++ b/lib/tst_cgroup.c<br>
@@ -138,6 +138,14 @@ struct tst_cgroup_group {<br>
        struct cgroup_dir *dirs[ROOTS_MAX + 1];<br>
 };<br>
<br>
+/* If controllers are required via the tst_test struct then this is<br>
+ * populated with the test's CGroup.<br>
+ */<br>
+static struct tst_cgroup_group test_group;<br>
+static struct tst_cgroup_group drain_group;<br>
+const struct tst_cgroup_group *const tst_cgroup = &test_group;<br>
+const struct tst_cgroup_group *const tst_cgroup_drain = &drain_group;<br>
+<br>
 /* Always use first item for unified hierarchy */<br>
 static struct cgroup_root roots[ROOTS_MAX + 1];<br>
<br>
@@ -196,7 +204,7 @@ static struct cgroup_ctrl controllers[] = {<br>
        { }<br>
 };<br>
<br>
-static const struct tst_cgroup_opts default_opts = { 0 };<br>
+static const struct tst_cgroup_opts default_opts;<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">I'm wondering is there still a need this 'default_opts'?</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Since we have defined 'cg_opts' in do_cgroup_requires for the</div><div class="gmail_default" style="font-size:small">whole library. So that 'default_opts' is redundant from now on.</div></div><div><div class="gmail_default" style="font-size:small"><br></div></div><div> </div></div><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>