<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">Thanks for your work, comments see below.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Dec 16, 2020 at 6:02 PM Richard Palethorpe via ltp <<a href="mailto:ltp@lists.linux.it">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">This is a request for comment on a new CGroups API. I have tried to<br>
keep the existing API functions, but there are many changes in the<br>
implementation. Please see the commit message to "CGroup API rewrite"<br>
in this series.<br>
<br>
A previous failed attempt to correct some problems and discussion is<br>
here: <a href="http://lists.linux.it/pipermail/ltp/2020-November/019586.html" rel="noreferrer" target="_blank">http://lists.linux.it/pipermail/ltp/2020-November/019586.html</a><br>
This is a much bigger change than I would like, but CGroups make it<br>
very difficult to do anything simply.<br>
<br>
I have done a direct conversion of the test cases to the new API, but<br>
I am not sure that it makes sense to call <span class="gmail_default" style="font-size:small"></span>tst_cgroup_move_current<br>
within the run method of a test because after the first iteration it<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Hmm, I feel that is a rare scenario in our real test. Mostly we</div><div class="gmail_default" style="font-size:small">just need to set it once in a process.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Also, another race condition we are facing is to set the same</div><div class="gmail_default" style="font-size:small">hierarchy in a different process parallel. i.e.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">// Child_1: set MEMSIZE maxbytes</div><div class="gmail_default" style="font-size:small">if (fork() == 0) {</div><div class="gmail_default" style="font-size:small">    <span class="gmail_default"></span>tst_cgroup_move_current(TST_CGROUP_MEMORY);</div><div class="gmail_default" style="font-size:small">    tst_cgroup_mem_set_maxbytes(MEMSIZE);</div><div class="gmail_default" style="font-size:small">}</div><div class="gmail_default" style="font-size:small">// Child_2: set MEMSIZE/2 maxbytes</div><div class="gmail_default" style="font-size:small">if (fork() == 0) {</div><div class="gmail_default">    <span class="gmail_default"></span>tst_cgroup_move_current(TST_CGROUP_MEMORY);</div><div class="gmail_default">    tst_cgroup_mem_set_maxbytes(MEMSIZE/2);</div><div class="gmail_default" style="font-size:small">}</div><br></div><div><div class="gmail_default" style="font-size:small">For the previous CGroup test-lib, we solved that via mount the</div><div class="gmail_default" style="font-size:small">same controller at different places. In this new CGroup lib, I guess</div><div class="gmail_default" style="font-size:small">creating dynamic directories in tst_cgroup_move_current might</div><div class="gmail_default" style="font-size:small">work for us, and I'll comment more about it in patch2/5.</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">
will be a NOP. There is also the issue that on the unified hierarchy<br>
when calling<br>
<br>
<span class="gmail_default" style="font-size:small"></span>tst_cgroup_move_current(TST_CGROUP_MEMORY);<br>
... testing ...<br>
tst_cgroup_move_current(TST_CGROUP_<span class="gmail_default" style="font-size:small"></span>CPUSET);<br>
<br>
the second call is a NOP as there is only one CGroup, but when the<br>
hierarchies are mounted seperately on V1 the current process will not<br>
be added to cpuset CGroup until the second call. We probably do not<br>
want different behaviour between commonly used hierarchies.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">That's true, but it is mainly caused by different versions of</div><div class="gmail_default" style="font-size:small">CGroup. We could NOT unify the unsupported behavior, so</div><div class="gmail_default" style="font-size:small">maybe the wiser choice is to let <span class="gmail_default"></span>_CPUSET test skipping(TCONF)</div><div class="gmail_default" style="font-size:small">directly on CGroup_V2?</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>
In a lot of cases, the test probably only requires the process to be<br>
moved into a CGroup during setup, but this requires an investigation<br>
of each test. Some tests scan for NUMA information and use this in the<br>
CGroup config which complicates matters. However it seems unlikely to<br>
me that the available NUMA nodes will change between test iterations<br>
unless we are testing hotplugging or failover.<br>
<br>
For tests which are actually testing CGroups themselves, most of the<br>
API is too high-level. Please see the kernel self tests for what may<br>
be required for those kinds of tests.<br>
<br>
This is not meant for full review and has not been tested on older<br>
setups yet.</blockquote></div><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>