Re: [LTP] [PATCH 1/4] controllers/memcg: update stress test to work under cgroup2

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello Li and Luke
Li Wang <liwang@redhat.com> writes:

> On Thu, Dec 2, 2021 at 6:24 AM Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> wrote:
>
>  Hi Li, 
>
>  On Wed, Dec 1, 2021 at 1:13 AM Li Wang <liwang@redhat.com> wrote:
>
>  Hi Luke,
>
>  Thanks for looking into this.
>
>  On Sat, Nov 27, 2021 at 8:05 AM Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> wrote:
>
>  Update tests to be able to work when memory controller is mounted under
>  cgroup2 hierarchy.
>
>  I'm thinking whether we could achieve these setup functions
>  more generic in cgroup_lib.sh, which to avoid the redundancy
>  behavior over and over again in each cgroup sub-test.

+1 this is very necessary IMO

>
>  Yes I agree. As I was doing the same things a few times I was beginning to wonder if there was a better way. I would be willing to look further into
>  expanding the cgroup_lib.sh and resubmitting my recent patches with
>  those changes.
>
> Thanks a lot!
>  
>  
>  About the compatibility of cgroup V1 and V2 in test, I'd suggest
>  following what the LTP C library did in include/tst_cgroup.h.
>  https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L2024
>
>  The solution may be briefly as:
>   
>    1. scan system mounted CGroup path, and judge the situation as one of below:
>       * only CGroup V2 mounted
>       * only CGroup V1 mounted
>       * both CGroup V2 and V1 mounted
>       * no CGroup mounted
>    2. make use of the system mounted CGroup V2 or TSKIP
>        * goto step 5
>    3. make use of the system mounted CGroup V1 
>        * goto step 5
>    4. do mount Cgroup as what we needed (V1 or V2) in test
>        * goto step 5
>    5. do cleanup 
>
>  Yes this would be the way to go through setting up a controller and checking everything.  
>  Going through the steps you mentioned for mounting a single controller and returning that path wouldn't be too hard but 
>  it seems to get more complicated when we want some guarantee of having multiple controllers in a hierarchy (if we even
>  would want to support something like that, which for testing purposes wouldnt seem unheard of).
>
> Right, it is the complicated part and you can take a reference how
> the current C API handles it.

Actually we can use the C API. This would avoid a whole bunch of
issues. It requires creating a utility which we can use from shell
(e.g. tst_cgctl).

We *may* have to track state somehow. Either we could run the utility as
a daemon initially and communicate with a socket to execute commands. Or
we could save/serialise some state to environment/shell
variables. Alternatively we can probably rescan the system each
time. The only state really required is the test's PID which is needed
to find the correct CGroup in the LTP sub-hierarchy.

Still that is probably easier than dealing with all of the corner cases
twice.

>
> TBH, I even think to skip the handling on mixed mounting with V1
> and V2, that is too messy/overthinking and not suggested using way.
>
> We'd better face the future and ideally as myself hoping,
> V2 will replace V1 everywhere someday:).

There are still controllers/features that don't exist on V2. Meanwhile
there are features that only exist on V2. So if someone wants both, then
they have to mount both. Regardless, this was the default in systemd, so
we are stuck with it for about 20 years and can't ignore it ;-)

>
>  
>  Maybe something mimicking the tst_cgroup_require() from the C api would be useful here? I imagine this is where we would
>  do the checking and mounting logic that you were describing. We would just also have to include checking if the controllers
>  we are requiring can be mounted / already exist together.
>
>  For example I am imaging something mimicking the C api something like:
>  tst_cgroup_require "cpu"
>  tst_cgroup_require "cpuset"
>  root_mount_point =$(tst_cgroup_get_mountpoint)
>
> I prefer this one a bit, not only it's consistent with C API but it also
> can do CGroup mounting in tst_cgroup_require for a system without
> V1 nor V2 mounting. Then I'd suggest having tst_cgroup_cleanup to
> help umount that which makes things more clear to understand.
>
> Anyway, it depends on the details achieve, maybe there is a better
> solution we haven't found.

Yes, or if it is a utility then

$ test_cpu_cg_dir = $(tst_cgroup require cpu)
$ test_cpu_cg_dir = $(tst_cgroup require memory)

maybe with an option to pass the PID to indetify the test. I guess there
might be some existing env variable the shell API sets we could use as well.

$ tst_cgroup cleanup --pid $MAIN_PID

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp