<div dir="ltr"><div>Hi Richard and Li,<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Dec 2, 2021 at 1:50 AM Richard Palethorpe <<a href="mailto:rpalethorpe@suse.de">rpalethorpe@suse.de</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">Hello Li and Luke<br>
Li Wang <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>> writes:<br>
<br>
> On Thu, Dec 2, 2021 at 6:24 AM Luke Nowakowski-Krijger <<a href="mailto:luke.nowakowskikrijger@canonical.com" target="_blank">luke.nowakowskikrijger@canonical.com</a>> wrote:<br>
><br>
>  Hi Li, <br>
><br>
>  On Wed, Dec 1, 2021 at 1:13 AM Li Wang <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>> wrote:<br>
><br>
>  Hi Luke,<br>
><br>
>  Thanks for looking into this.<br>
><br>
>  On Sat, Nov 27, 2021 at 8:05 AM Luke Nowakowski-Krijger <<a href="mailto:luke.nowakowskikrijger@canonical.com" target="_blank">luke.nowakowskikrijger@canonical.com</a>> wrote:<br>
><br>
>  Update tests to be able to work when memory controller is mounted under<br>
>  cgroup2 hierarchy.<br>
><br>
>  I'm thinking whether we could achieve these setup functions<br>
>  more generic in cgroup_lib.sh, which to avoid the redundancy<br>
>  behavior over and over again in each cgroup sub-test.<br>
<br>
+1 this is very necessary IMO<br>
<br>
Actually we can use the C API. This would avoid a whole bunch of<br>
issues. It requires creating a utility which we can use from shell<br>
(e.g. tst_cgctl).<br>
<br>
We *may* have to track state somehow. Either we could run the utility as<br>
a daemon initially and communicate with a socket to execute commands. Or<br>
we could save/serialise some state to environment/shell<br>
variables. Alternatively we can probably rescan the system each<br>
time. The only state really required is the test's PID which is needed<br>
to find the correct CGroup in the LTP sub-hierarchy.<br>
<br>
Still that is probably easier than dealing with all of the corner cases<br>
twice.<br>
<br></blockquote><div>I rather like this idea. If I understand you correctly we could use it in an RPC sort of way which would make a lot of things simpler and use the existing C API which is nice. <br></div><div><br></div><div>My one question would be if we would want this daemon to run as a test suite utility, like it seems you are suggesting, or as a per process utility. <br></div><div><br></div><div>The nice part of having a daemon that we could fork off for every test that uses it would be that the cleanup / tracking of sub-groups would get cleaned up in the normal way when we want to close the daemon and just call tst_cgroup_cleanup(). The daemons state would be tied to the test that's issuing commands to it. We could also send out the commands via a shared buffer or pipe that we read and write to. <br></div><div><br></div><div>But is a daemon per test (that uses the cgroup shell api) overkill? It seems it would spare us from having to track the test PID to sub-hierarchy like you were mentioning. Or maybe there are some other drawbacks to the per-test daemon idea that I'm not seeing?<br></div><div><br></div><div>Looking forward to hearing what you think.<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>
> TBH, I even think to skip the handling on mixed mounting with V1<br>
> and V2, that is too messy/overthinking and not suggested using way.<br>
><br>
> We'd better face the future and ideally as myself hoping,<br>
> V2 will replace V1 everywhere someday:).<br>
<br>
There are still controllers/features that don't exist on V2. Meanwhile<br>
there are features that only exist on V2. So if someone wants both, then<br>
they have to mount both. Regardless, this was the default in systemd, so<br>
we are stuck with it for about 20 years and can't ignore it ;-)<br>
<br>
><br>
>  <br>
>  Maybe something mimicking the tst_cgroup_require() from the C api would be useful here? I imagine this is where we would<br>
>  do the checking and mounting logic that you were describing. We would just also have to include checking if the controllers<br>
>  we are requiring can be mounted / already exist together.<br>
><br>
>  For example I am imaging something mimicking the C api something like:<br>
>  tst_cgroup_require "cpu"<br>
>  tst_cgroup_require "cpuset"<br>
>  root_mount_point =$(tst_cgroup_get_mountpoint)<br>
><br>
> I prefer this one a bit, not only it's consistent with C API but it also<br>
> can do CGroup mounting in tst_cgroup_require for a system without<br>
> V1 nor V2 mounting. Then I'd suggest having tst_cgroup_cleanup to<br>
> help umount that which makes things more clear to understand.<br>
><br>
> Anyway, it depends on the details achieve, maybe there is a better<br>
> solution we haven't found.<br>
<br>
Yes, or if it is a utility then<br>
<br>
$ test_cpu_cg_dir = $(tst_cgroup require cpu)<br>
$ test_cpu_cg_dir = $(tst_cgroup require memory)<br>
<br>
maybe with an option to pass the PID to indetify the test. I guess there<br>
might be some existing env variable the shell API sets we could use as well.<br>
<br>
$ tst_cgroup cleanup --pid $MAIN_PID<br>
<br>
-- <br>
Thank you,<br>
Richard.<br></blockquote><div><br></div><div> Thanks, <br></div><div>- Luke<br></div></div></div>