<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small">Hi Jan,</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 1, 2020 at 9:57 PM Jan Stancek <<a href="mailto:jstancek@redhat.com" target="_blank">jstancek@redhat.com</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"><br>
<br>
----- Original Message -----<br>
> +<br>
> +[source,c]<br>
> +-------------------------------------------------------------------------------<br>
> +#include "tst_test.h"<br>
> +<br>
> +static void run(void)<br>
> +{<br>
> + ...<br>
> +<br>
> + tst_cgroup_move_current(PATH_TMP_CG1_MEM);<br>
> + tst_cgroup_mem_set_maxbytes(PATH_TMP_CG1_MEM, MEMSIZE);<br>
<br>
Goal for API is to hide differences between cgroup 1/2, but example above<br>
is passing cgroup <span class="gmail_default" style="font-size:small"></span>specific directory.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Sorry for the misleading info, actually that's no different to use any</div><div class="gmail_default" style="font-size:small">directory whatever you pass via parameter, it will recognize cgroup</div><div class="gmail_default" style="font-size:small">version to be mounted intelligently.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Here as I commented in the last email. the specific dir are typos which</div><div class="gmail_default" style="font-size:small">inherent from patch v1. We should fix them.</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">
<br>
My suggestion was to have directory parameter relative to cgroup mount,<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Patch v2 is already achieved this even includes more functions(i.e mount many same cgroup).</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">
I didn't consider there would be need for mounting cgroup more than once<br>
from single process. Is there such need?<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Yes right, the test21.c mounting many cgroups in a single process is just a test</div><div class="gmail_default" style="font-size:small"> to verify these APIs works fine, it meaningless in real situations.</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">
<br>
Since there's only one global 'tst_cgroup_mnt_path', is there need to have<br>
paths absolute? If we assume that single process will mount cgroup only once,<br>
then all paths could be relative to 'tst_cgroup_mnt_path', and test doesn't<br>
need to even use 'tst_cgroup_mnt_path'.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">No, the global 'tst_cgroup_mnt_path' is not only to pass mount directory but</div><div class="gmail_default" style="font-size:small">also for storing path when searching from get_cgroup_get_path().</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Why we need this? Because, if a testcase(i.e oom05.c) needs more than one cgroup<br></div><div class="gmail_default" style="font-size:small">subsystem(memory, cpuset) on RHEL7(cgroup-v1), it should mount two different </div><div class="gmail_default" style="font-size:small">directories and do some knob setting.</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">
<br>
> +<br>
> +static void tst_cgroup_set_path(const char *cgroup_dir)<br>
> +{<br>
> + struct tst_cgroup_path *tst_cgroup_path, *a;<br>
> +<br>
> + if (!cgroup_dir)<br>
> + tst_brk(TBROK, "Invalid cgroup dir, plese check cgroup_dir");<br>
> +<br>
> + sprintf(tst_cgroup_mnt_path, "%s", cgroup_dir);<br>
> + sprintf(tst_cgroup_new_path, "%s/ltp_%d", cgroup_dir, rand());<br>
> +<br>
> + /* To store cgroup path in the shared 'path' list */<br>
> + tst_cgroup_path = SAFE_MMAP(NULL, (sizeof(struct tst_cgroup_path)),<br>
> + PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED, -1, 0);<br>
<br>
I'm not sure I understand what is the reason to have tst_cgroup_path. Is it expected,<br>
that mount and umount are called by different processes? It might be easier<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">The shared 'tst_cgroup_path' is necessary especially for mounting</div><div class="gmail_default" style="font-size:small">different cgoups in setup(). Otherwise, it would be easy to get lost</div><div class="gmail_default" style="font-size:small">which directory for kind of cgroup type.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">And the worth to say, the random directory name for same cgroup</div><div class="gmail_default" style="font-size:small">mounting is also on purpose, though we mount same(i.e memory) </div><div class="gmail_default" style="font-size:small">cgroup in two places it still belongs to the one hierarchy, and create </div><div class="gmail_default" style="font-size:small">the same name of the new directory will be hit an error in EEXIST.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">static void tst_cgroup_set_path(const char *cgroup_dir)<br>{<br> ...<br> sprintf(tst_cgroup_mnt_path, "%s", cgroup_dir);<br> sprintf(tst_cgroup_new_path, "%s/ltp_%d", cgroup_dir, rand());<br></div><div class="gmail_default" style="font-size:small">}</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">
to define API as per-process and require same process to call mount and umount.<br>
<br>
> + tst_cgroup_path->mnt_path = SAFE_MALLOC(strlen(tst_cgroup_mnt_path));<br>
> + tst_cgroup_path->new_path = SAFE_MALLOC(strlen(tst_cgroup_new_path));<br>
<br>
Pointers are in shared memory, but content they point to is not, so it's accessible<br>
only from process that called tst_cgroup_set_path().<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Good catch. This should be also use shared memory.</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">
<br>
Can you describe all different scenarios you wanted to support?<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">Sure, we have to consider many scenarios:</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">1. mount only one cgroup in a single process on the system support cgroup v1 or v2 (i.e ksm03.c)</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">#include "tst_cgroup.h"<br>#define PATH_CGROUP /tmp/cgroup</div><div class="gmail_default" style="font-size:small"><br>static void run(void)<br>{<br> tst_cgroup_move_current(PATH_CGROUP);<br> tst_cgroup_mem_set_maxbytes(PATH_CGROUP, 1024*1024*1024);<br> // do your test<br>}<br><br>static void setup(void)<br>{<br> tst_cgroup_mount(TST_CGROUP_MEMCG, PATH_CGROUP);<br>}<br><br>static void cleanup(void)<br>{<br> tst_cgroup_umount(PATH_CGROUP);<br>}<br><br>static struct tst_test test = {<br> ...<br> .test_all = run,<br>};<br></div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"></div><div class="gmail_default" style="font-size:small">2. mount different cgroups in a single process on the system only support cgroup-v1 (i.e ksm04.c, oom05.c)</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">#include "tst_cgroup.h"<br>#define PATH_CGROUP1 /tmp/cgroup1</div><div class="gmail_default" style="font-size:small">#define PATH_CGROUP2 /tmp/cgroup2<br><br>static void run(void)<br>{<br> tst_cgroup_move_current(PATH_CGROUP1);</div><div class="gmail_default" style="font-size:small"> tst_cgroup_move_current(PATH_CGROUP2);</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"> // we have to recognize the correct cgroup path if we</div><div class="gmail_default" style="font-size:small"> // mount two or more cgroup subsystems in a single</div><div class="gmail_default" style="font-size:small"> // process, in case we get lost in knob setting</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"> // the tst_cgroup_get_path() helps to find and get</div><div class="gmail_default" style="font-size:small"> // correct path in tst_cgroup_mnt_path, tst_cgroup_new_path</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"> // that's also the reason why we need a shared list to</div><div class="gmail_default" style="font-size:small"> // store many cgoup pathes. And, this is extendible for </div><div class="gmail_default" style="font-size:small"> // mounting more cgroup subsystems in the future.</div><div class="gmail_default" style="font-size:small"><br> tst_cgroup_mem_set_maxbytes(PATH_CGROUP1, 1024*1024*1024);</div><div class="gmail_default" style="font-size:small"> // some cpuset configration<br>}<br><br>static void setup(void)<br>{<br> tst_cgroup_mount(TST_CGROUP_MEMCG, PATH_CGROUP1);</div><div class="gmail_default" style="font-size:small"> tst_cgroup_mount(TST_CGROUP_CPUSET, PATH_CGROUP2);<br>}<br><br>static void cleanup(void)<br>{<br> tst_cgroup_umount(PATH_CGROUP1);</div><div class="gmail_default" style="font-size:small"> tst_cgroup_umount(PATH_CGROUP2);<br>}<br><br>static struct tst_test test = {<br> ...<br> .test_all = run,<br>};<br></div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">3. mount different cgroups in different process on the system support v1 or v2</div><div class="gmail_default" style="font-size:small"><br></div></div><div class="gmail_default" style="font-size:small">#include "tst_cgroup.h"</div><div class="gmail_default">#define PATH_CGROUP1 /tmp/cgroup1</div><div class="gmail_default">#define PATH_CGROUP2 /tmp/cgroup2<br><br>static void run(void)<br>{</div><div class="gmail_default"> if (fork() == 0) {<br> tst_cgroup_move_current(PATH_CGROUP1);</div><div class="gmail_default"> tst_cgroup_mem_set_maxbytes(PATH_CGROUP1, 1024*1024*1024);</div><div class="gmail_default"> // test code</div><div class="gmail_default"> }<br></div><div class="gmail_default"><br></div><div class="gmail_default"><div class="gmail_default"> tst_cgroup_move_current(PATH_CGROUP2);</div><div class="gmail_default"> // some cpuset configuration </div><div class="gmail_default"> // and test code<br></div>}<br><br>static void setup(void)<br>{</div><div class="gmail_default"> // we do cgroup mount work unified in setup()</div><div class="gmail_default"> // whatever the cgroup is being used in the parent</div><div class="gmail_default"> // or children, there is also no need to care about the</div><div class="gmail_default"> // details of cgroup v1 or v2.</div><div class="gmail_default"><br> tst_cgroup_mount(TST_CGROUP_MEMCG, PATH_CGROUP1);</div><div class="gmail_default"> tst_cgroup_mount(TST_CGROUP_CPUSET, PATH_CGROUP2);<br>}<br><br>static void cleanup(void)<br>{<br> tst_cgroup_umount(PATH_CGROUP1);</div><div class="gmail_default"> tst_cgroup_umount(PATH_CGROUP2);<br>}<br><br>static struct tst_test test = {<br> ...<br> .test_all = run,<br></div><div class="gmail_default" style="font-size:small">};</div><div><br></div><div><br></div><div class="gmail_default" style="font-size:small">4. mount same cgroup in different process on the system support v1 or v2</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">#include "tst_cgroup.h"</div><div class="gmail_default">#define PATH_CGROUP1 /tmp/cgroup1</div><div class="gmail_default">#define PATH_CGROUP2 /tmp/cgroup2<br><br>static void run(void)<br>{</div><div class="gmail_default"> if (fork() == 0) {<br> tst_cgroup_move_current(PATH_CGROUP1);</div><div class="gmail_default"> tst_cgroup_mem_set_maxbytes(PATH_CGROUP1, 1024*1024*1024);</div><div class="gmail_default"> // test code1</div><div class="gmail_default"> }<br></div><div class="gmail_default"><br></div><div class="gmail_default"><div class="gmail_default"><div class="gmail_default"> tst_cgroup_move_current(PATH_CGROUP2);</div><div class="gmail_default"> tst_cgroup_mem_set_maxbytes(PATH_CGROUP2, 2048*2048);</div><div class="gmail_default"> // test code2</div></div>}<br><br>static void setup(void)<br>{</div><div class="gmail_default"> // we mount two memory cgroup in the parent</div><div class="gmail_default"> // but setting different value in corresponding </div><div class="gmail_default"> // knob for different process to test more configration</div><div class="gmail_default"><br> tst_cgroup_mount(TST_CGROUP_MEMCG, PATH_CGROUP1);</div><div class="gmail_default"> tst_cgroup_mount(TST_CGROUP_MEMCG, PATH_CGROUP2);<br>}<br><br>static void cleanup(void)<br>{<br> tst_cgroup_umount(PATH_CGROUP1);</div><div class="gmail_default"> tst_cgroup_umount(PATH_CGROUP2);<br>}<br><br>static struct tst_test test = {<br> ...<br> .test_all = run,<br></div><div class="gmail_default" style="font-size:small">};</div><div><br></div><div><br></div><div><div class="gmail_default" style="font-size:small">5. mount many cgroups on different process for future extendible work</div><div class="gmail_default" style="font-size:small">i.e.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">// tst_cgroup_mount(TST_CGROUP_MEMCG, PATH_CGROUP1);</div><div class="gmail_default">// tst_cgroup_mount(TST_CGROUP_CPUSET, PATH_CGROUP2);</div><div class="gmail_default"><div class="gmail_default">// tst_cgroup_mount(TST_CGROUP_CPUACT, PATH_CGROUP3);</div><div class="gmail_default"><div class="gmail_default">// tst_cgroup_mount(TST_CGROUP_PIDSCG, PATH_CGROUP4);</div><div class="gmail_default"><div class="gmail_default">// tst_cgroup_mount(TST_CGROUP_HUGTLB, PATH_CGROUP5);</div></div></div></div><div class="gmail_default" style="font-size:small">// ...</div><br></div><div><br></div><div><div class="gmail_default" style="font-size:small">Btw, I attach the patch v2.1 for better readable.</div></div><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>