<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Richard Palethorpe via ltp <<a href="mailto:ltp@lists.linux.it" target="_blank">ltp@lists.linux.it</a>> wrote:<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">
So we have a choice of:<br>
<br>
A) Expect a particular configuration and refuse to run<br>
otherwise<br>
B) Expect no CGroups and mount them<br>
C) Try to use what is available<br>
D) (C) and try to mount them if nothing is present<br>
E) (C), (D) and try to cleanup what we created<br>
<br>
(A) and (B) are more simple for the LTP library, but require our users<br>
to do more work, perhaps a lot more. Depending on the test<br>
environment; disabling or reconfiguring CGroups may be<br>
difficult. Because many tests require CGroups to expose a bug in other<br>
subsystems (e.g. the memory subsystem), I do not think (A) and (B) are<br>
viable options. They may be acceptable for testing core CGroup<br>
features where a pristine system is required (e.g. first mount), but<br>
that is not the current focus of these changes.<br>
<br>
(C) is probably the simplest viable option, but this tries to<br>
implement (E). Mounting the CGroups if none are available simplifies<br>
testing within environments like Rapido where testing is done in the<br>
recovery console and no CGroups are mounted by the system manager. On<br>
the other hand it is quite simple for the user to mount some CGroups.<br>
<br>
Mounting is relatively simple so does not add much complexity, however<br>
cleaning up the mounts is broken so possibly we should only<br>
attempt (C) or (D). It appears that the test process has some references to<br>
the mount struct and so it can not be removed by the test, but I am<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Yes, and that's also the reason I like the drain_dir in this patch.</div></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">
partly guessing. Using MNT_DETACH at least allows the directory to be<br>
removed, but it's not clear if the root is eventually destroyed. It<br>
appears that a separate process (i.e. the umount command) can properly<br>
remove the mount and the CGroup is destroyed, but this is still not<br>
fully clear without more tracing.<br><br>
<span class="gmail_default" style="font-size:small">[...</span><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"><br>
+/* Determine if a mounted cgroup hierarchy (tree) is unique and record it if so.<br>
+ *<br>
+ * For CGroups V2 this is very simple as there is only one<br>
+ * hierarchy. We just record which controllers are available and check<br>
+ * if this matches what we read from any previous mounts to verify our<br>
+ * own logic (and possibly detect races).<br>
+ *<br>
+ * For V1 the set of controllers S is partitioned into sets {P_1, P_2,<br>
+ * ..., P_n} with one or more controllers in each partion. Each<br>
+ * partition P_n can be mounted multiple times, but the same<br>
+ * controller can not appear in more than one partition. Usually each<br>
+ * partition contains a single controller, but, for example, cpu and<br>
+ * cpuacct are often mounted together in the same partiion.<br>
+ *<br>
+ * Each controller partition has its own hierarchy/root/tree which we<br>
+ * must track and update independently.<br>
+ */<br>
+static void tst_cgroup_get_tree(const char *type, const char *path, char *opts)<br>
+{<br>
+ struct tst_cgroup_root *t = trees;<br>
+ struct tst_cgroup_ss *c;<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">why not naming *c to *ss? to make it tidier for readability just like what you did in other functions.</div><br></div><div><div class="gmail_default" style="font-size:small"> static void tst_cgroup_get_tree(const char *type, const char *path, char *opts)</div> {<br> struct tst_cgroup_root *t = trees;<br>- struct tst_cgroup_ss *c;<br>+ struct tst_cgroup_ss *ss;<br> uint32_t ss_field = 0;<br> int no_prefix = 0;<br> char buf[BUFSIZ];<br>@@ -290,9 +290,9 @@ backref:<br> t->ss_field = ss_field;<br> t->no_prefix = no_prefix;<br> <br>- tst_for_each_css(c) {<br>- if (t->ss_field & (1 << c->indx))<br>- c->tree = t;<br>+ tst_for_each_css(ss) {<br>+ if (t->ss_field & (1 << ss->indx))<br>+ ss->tree = t;<br> }<br><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">
<span class="gmail_default" style="font-size:small">[...]</span><br><br>
+void tst_cgroup_require(enum tst_cgroup_ctrl type,<br>
+ const struct tst_cgroup_opts *options)<br>
{<br>
- char *cgroup_new_dir;<br>
- char knob_path[PATH_MAX];<br>
+ struct tst_cgroup_ss *ss = tst_cgroup_get_ss(type);<br>
+ struct tst_cgroup_root *t;<br>
+ char path[PATH_MAX];<br>
<br>
- cgroup_new_dir = tst_cgroup_get_path(cgroup_dir);<br>
+ if (!options)<br>
+ options = &default_opts;<br>
<br>
- if (tst_cg_ver & TST_CGROUP_V1) {<br>
- sprintf(knob_path, "%s/%s",<br>
- cgroup_new_dir, "/memory.memsw.limit_in_bytes");<br>
+ if (ss->tree)<br>
+ goto ltpdir;<br>
<br>
- if ((access(knob_path, F_OK) == -1)) {<br>
- if (errno == ENOENT)<br>
- tst_res(TCONF, "memcg swap accounting is disabled");<br>
- else<br>
- tst_brk(TBROK | TERRNO, "failed to access %s", knob_path);<br>
- } else {<br>
- return 1;<br>
- }<br>
+ tst_cgroup_scan();<br>
+<br>
+ if (ss->tree)<br>
+ goto ltpdir;<br>
+<br>
+ if (!tst_cgroup_v2_mounted() && !options->only_mount_v1)<br>
+ tst_cgroup_mount_v2();<br>
+<br>
+ if (ss->tree)<br>
+ goto ltpdir;<br>
+<br>
+ tst_cgroup_mount_v1(type);<br>
+<br>
+ if (!ss->tree) {<br>
+ tst_brk(TCONF,<br>
+ "%s controller required, but not available", ss->name);<br>
}<br>
<br>
- if (tst_cg_ver & TST_CGROUP_V2) {<br>
- sprintf(knob_path, "%s/%s",<br>
- cgroup_new_dir, "/memory.swap.max");<br>
+ltpdir:<br>
+ t = ss->tree;<br>
<br>
- if ((access(knob_path, F_OK) == -1)) {<br>
- if (errno == ENOENT)<br>
- tst_res(TCONF, "memcg swap accounting is disabled");<br>
- else<br>
- tst_brk(TBROK | TERRNO, "failed to access %s", knob_path);<br>
- } else {<br>
- return 1;<br>
- }<br>
+ if (tst_cgroup_ss_on_v2(ss)) {<br>
+ tst_file_printfat(t->mnt_dir, "cgroup.subtree_control",<br>
+ "+%s", ss->name);<br>
}<br>
<br>
- return 0;<br>
+ sprintf(path, "%s/%s", t->path, ltp_cgroup_dir);<br>
+<br>
+ if (!mkdir(path, 0777)) {<br>
+ t->we_created_ltp_dir = 1;<br>
+ goto draindir;<br>
+ }<br>
+<br>
+ if (errno == EEXIST)<br>
+ goto draindir;<br>
+<br>
+ if (errno == EACCES) {<br>
+ tst_brk(TCONF | TERRNO,<br>
+ "Lack permission to make %s; premake it or run as root",<br>
+ path);<br>
+ }<br>
+<br>
+ tst_brk(TBROK | TERRNO, "mkdir(%s, 0777)", path);<br>
+<br>
+draindir:<br>
+ if (!t->ltp_dir)<br>
+ t->ltp_dir = SAFE_OPEN(path, O_PATH | O_DIRECTORY);<br>
+<br>
+ if (tst_cgroup_ss_on_v2(ss)) {<br>
+ SAFE_FILE_PRINTFAT(t->ltp_dir, "cgroup.subtree_control",<br>
+ "+%s", ss->name);<br>
+ } else {<br>
+ SAFE_FILE_PRINTFAT(t->ltp_dir, "cgroup.clone_children",<br>
+ "%d", 1);<br>
+<br>
+ if (type == TST_CGROUP_CPUSET)<br>
+ tst_cgroup_copy_cpuset(t);<br>
+ }<br>
+<br>
+ sprintf(path, "%s/%s/%s",<br>
+ t->path, ltp_cgroup_dir, ltp_cgroup_drain_dir);<br>
+<br>
+ if (!mkdir(path, 0777) || errno == EEXIST)<br>
+ goto testdir;<br>
+<br>
+ if (errno == EACCES) {<br>
+ tst_brk(TCONF | TERRNO,<br>
+ "Lack permission to make %s; grant access or run as root",<br>
+ path);<br>
+ }<br>
+<br>
+ tst_brk(TBROK | TERRNO, "mkdir(%s, 0777)", path);<br>
+<br>
+testdir:<br>
+ if (!t->drain_dir)<br>
+ t->drain_dir = SAFE_OPEN(path, O_PATH | O_DIRECTORY);<br>
+<br>
+ if (!test_cgroup_dir[0])<br>
+ sprintf(test_cgroup_dir, "test-%d", getpid());<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">As I was mentioned in 0/5 that maybe we should create test_cgroup_dir</div><div class="gmail_default" style="font-size:small">for different progress so that the test could use the same controller with</div><div class="gmail_default" style="font-size:small">various configurations in parallel<span class="gmail_default">.</span></div></div><div><br>e.g. child_1 set SIZE to memory.limit_in_bytes<br><span class="gmail_default" style="font-size:small"> </span>child_2 set SIZE/2 to memory.limit_in_bytes</div><div><br><span class="gmail_default" style="font-size:small">Any possibility </span><span class="gmail_default" style="font-size:small">to </span><span class="gmail_default" style="font-size:small">move</span> this to tst_cgroup_move_current?<br></div><div><br></div></div>-- <br><div dir="ltr"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>