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

[Richard Palethorpe]

Hello Luke and Li,

Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> writes:

> Hi Richard and Li,
>
>  Hmm, why do we need that utility as a daemon in the background? 
>  Isn't it easier to execute a binary utility to get the CGroup path
>  only when needed? Just like Richard mentioned the alternative
>  way, rescan system each time and only distinguish correct CGroup
>  path via the test PID.
>
> Yes this would be easier if we only wanted to get access to the path. I was getting ahead of myself and thinking about using the C-api to keep track of creating sub-cgroups and all the state that comes with that. Which we wouldn't want to do for a shell utility because creation/cleanup of subgroups is for the test to take care of and
> that is more trivial in a shell environment.

Yes; tests which require complicated hierarchies have to cleanup
themselves. I think it would be a waste of time to add a generic
sub-directory cleanup mechanism until we have a number of tests which
would benefit from it.

Especially because some controllers might place special restrictions on
directory and process manipulation. (cpuset for example?).

>
>   
>  
>  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.
>
>  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?
>
>  I think yes, starting a daemon per test is not wise.
>
>  Another drawback I can think of is that will definitely affect paralleling things,
>  we must guarantee the CGroup mounted by testA shouldn't be scanned/used
>  by testB, otherwise, it will fail in the cleanup phase. But, we can make the LTP 
>  test mounted CGroup path is transparent to others just by adding a special string
>  like "ltp-cgroup".
>
> If I understand it correctly, all the cgroup tests are mounted and created like "controller_mount_point/ltp/test-$PID" where every test shares the mount_point and ltp dir. And when tst_cgroup_cleanup() gets called it only cleans up its own test_dir and removes the ltp dir and unmounts the mount point if it was the first test to do it. So
> none of the tests should be touching each other's directories and so
> what you're describing should already be taken care of. Maybe I'm not
> understanding you correctly.

I think maybe you are talking about two different things. However IMO
daemons should be avoided in general. There is no hard requirement here
to have a long running process.

>
> I think the only problem with the binary utility approach where we rescan every time we execute it is that 
> 1) The test-$PID dir that the test would be created with the PID of the program which if we were executing the utility could be different between calls. This could be easily solved by adding an arg to tst_cgroup_opts for a specific PID, which would be the test that is calling it. 
> 2) We lose the reference between calls to the root->test_dir that is filled in when we call tst_cgroup_require(), which is what does the cleanup of the test specific dir. This is where I believe Richard was mentioning passing the PID as "tst_cgroup cleanup --pid $MAIN_PID". Which if we wanted to use the C api for this we would have to
> expose it to knowing about specific PIDs? Or for the root->test_dir to
> be reset somewhere?

I suppose we also need to remember if the current test created the ltp
subdirectory or mounted any controllers.

We could do this by just printing the required command line options to
stdout when the utility exits. Then save this to a variable for the next
use.

>
>  -- 
>  Regards,
>  Li Wang
>
> Let me know if this makes sense and what you think about it, I might be getting confused somewhere. But if I understand you correctly I believe that the binary utility approach where we rescan and call tst_cgroup_require() or tst_cgroup_cleanup() is a good approach.
>
> Best, 
> - Luke

Yes, sounds good.

-- 
Thank you,
Richard.