<div dir="ltr"><div>Hi Richard, <br></div><div><br></div><div>I see what you mean, it would not work if both hierarchies exist. I will do something similar to the patch you linked as that has cleared up a lot of my misunderstandings about it. While I am there I will update the tests to use the newer test APIs. </div><div><br></div><div>Thanks for the review.</div><div><br></div><div>- Luke<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 16, 2021 at 2:09 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 Luke,<br>
<br>
Luke Nowakowski-Krijger <<a href="mailto:luke.nowakowskikrijger@canonical.com" target="_blank">luke.nowakowskikrijger@canonical.com</a>> writes:<br>
<br>
> Some tests no longer make sense under cgroup2, while other tests just<br>
> needed to be updated to use new parameters introduced by the cgroup2<br>
> interface.<br>
><br>
> Signed-off-by: Luke Nowakowski-Krijger <<a href="mailto:luke.nowakowskikrijger@canonical.com" target="_blank">luke.nowakowskikrijger@canonical.com</a>><br>
> ---<br>
> .../memcg/regression/memcg_regression_test.sh | 41 ++++++++++++++++---<br>
> .../memcg/regression/memcg_test_1.c | 12 +++++-<br>
> .../memcg/regression/memcg_test_3.c | 8 ++++<br>
> .../memcg/regression/memcg_test_4.sh | 18 ++++++--<br>
> 4 files changed, 68 insertions(+), 11 deletions(-)<br>
><br>
> diff --git a/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh b/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh<br>
> index c91a4069e..ad88d49d1 100755<br>
> --- a/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh<br>
> +++ b/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh<br>
> @@ -103,7 +103,12 @@ check_kernel_bug()<br>
> test_1()<br>
> {<br>
> mkdir memcg/0/<br>
> - echo 0 > memcg/0/memory.limit_in_bytes<br>
> +<br>
> + if [ "$cgroup_ver" = "cgroup2" ]; then<br>
> + echo 0 > memcg/0/memory.max<br>
> + else<br>
> + echo 0 > memcg/0/memory.limit_in_bytes<br>
> + fi<br>
> <br>
> ./memcg_test_1<br>
> <br>
> @@ -124,6 +129,16 @@ test_1()<br>
> #---------------------------------------------------------------------------<br>
> test_2()<br>
> {<br>
> + # for cgroup2 the default behaivor is to check the new memory limit and<br>
> + # then to start killing processes if oom. This test then doesen't<br>
> + # make sense as we don't expect EBUSY to be returned. The shrink<br>
> + # operation (write to memory.max in cgroup2) would kill the pid1 process<br>
> + # and exit.<br>
> + if [ "$cgroup_ver" = "cgroup2" ]; then<br>
> + tst_resm TCONF "cgroup2 found, skipping test"<br>
> + return<br>
> + fi<br>
> +<br>
> ./memcg_test_2 &<br>
> pid1=$!<br>
> sleep 1<br>
> @@ -177,12 +192,20 @@ test_2()<br>
> test_3()<br>
> {<br>
> mkdir memcg/0<br>
> - for pid in `cat memcg/tasks`; do<br>
> - echo $pid > memcg/0/tasks 2> /dev/null<br>
> + if [ "$cgroup_ver" = "cgroup2" ]; then<br>
> + memcg_procs=memcg/cgroup.procs<br>
> + memcg_subprocs=memcg/0/cgroup.procs<br>
> + else<br>
> + memcg_procs=memcg/tasks<br>
> + memcg_subprocs=memcg/0/tasks<br>
> + fi<br>
> +<br>
> + for pid in `cat $memcg_procs`; do<br>
> + echo $pid > $memcg_subprocs 2> /dev/null<br>
> done<br>
> <br>
> - for pid in `cat memcg/0/tasks`; do<br>
> - echo $pid > memcg/tasks 2> /dev/null<br>
> + for pid in `cat $memcg_subprocs`; do<br>
> + echo $pid > $memcg_procs 2> /dev/null<br>
> done<br>
> rmdir memcg/0<br>
> <br>
> @@ -218,13 +241,19 @@ test_4()<br>
> }<br>
> <br>
> # main<br>
> +cgroup_ver=$(grep "/sys/fs/cgroup" /proc/mounts | cut -d' ' -f1)<br>
> failed=0<br>
> mkdir memcg/<br>
> <br>
> for cur in $(seq 1 $TST_TOTAL); do<br>
> export TST_COUNT=$cur<br>
> <br>
> - mount -t cgroup -o memory xxx memcg/<br>
> + if [ "$cgroup_ver" = "cgroup2" ]; then<br>
> + mount -t cgroup2 xxx memcg/<br>
> + else<br>
> + mount -t cgroup -o memory xxx memcg/<br>
> + fi<br>
> +<br>
> if [ $? -ne 0 ]; then<br>
> tst_resm TFAIL "failed to mount memory subsystem"<br>
> failed=1<br>
> diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_1.c b/testcases/kernel/controllers/memcg/regression/memcg_test_1.c<br>
> index c7fb948fe..b9277e633 100644<br>
> --- a/testcases/kernel/controllers/memcg/regression/memcg_test_1.c<br>
> +++ b/testcases/kernel/controllers/memcg/regression/memcg_test_1.c<br>
> @@ -33,6 +33,16 @@<br>
> <br>
> #define FORKED_PROC_COUNT 10<br>
> <br>
> +static int open_cgroup_procs(void)<br>
> +{<br>
> + int fd = open("memcg/0/tasks", O_WRONLY);<br>
> + if (fd >= 0)<br>
> + return fd;<br>
> +<br>
> + fd = open("memcg/0/cgroup.procs", O_WRONLY);<br>
> + return fd;<br>
> +}<br>
> +<br>
> int main(void)<br>
> {<br>
> char buf[10];<br>
> @@ -40,7 +50,7 @@ int main(void)<br>
> int loop;<br>
> int pid;<br>
> int size = getpagesize();<br>
> - int fd = open("memcg/0/tasks", O_WRONLY);<br>
> + int fd = open_cgroup_procs();<br>
> <br>
> if (fd < 0)<br>
> return 1;<br>
> diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_3.c b/testcases/kernel/controllers/memcg/regression/memcg_test_3.c<br>
> index 75a6e1545..d5531fe87 100644<br>
> --- a/testcases/kernel/controllers/memcg/regression/memcg_test_3.c<br>
> +++ b/testcases/kernel/controllers/memcg/regression/memcg_test_3.c<br>
> @@ -66,6 +66,14 @@ static void setup(void)<br>
> SAFE_MKDIR(MNTPOINT, 0644);<br>
> <br>
> ret = mount("memcg", MNTPOINT, "cgroup", 0, "memory");<br>
> +<br>
> + if (!ret) {<br>
> + mount_flag = 1;<br>
> + return;<br>
> + }<br>
> +<br>
> + ret = mount("memcg", MNTPOINT, "cgroup2", 0, NULL);<br>
> +<br>
> if (ret) {<br>
> if (errno == ENOENT)<br>
> tst_brk(TCONF | TERRNO, "memcg not supported");<br>
> diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh b/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh<br>
> index 620031366..287864b81 100755<br>
> --- a/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh<br>
> +++ b/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh<br>
> @@ -22,9 +22,19 @@<br>
> ## ##<br>
> ################################################################################<br>
> <br>
> +cgroup_ver=$(grep "/sys/fs/cgroup" /proc/mounts | cut -d' ' -f1)<br>
<br>
Unfortunately this doesn't work when both V1 and V2 hierarchies are<br>
mounted. You need to check that the memory controller is enabled on<br>
V2. Also it's possible to mount CGroups at alternate locations.<br>
<br>
Please see the recent patch from Masayoshi:<br>
<a href="https://patchwork.ozlabs.org/project/ltp/patch/20211113041706.12893-1-msys.mizuma@gmail.com/" rel="noreferrer" target="_blank">https://patchwork.ozlabs.org/project/ltp/patch/20211113041706.12893-1-msys.mizuma@gmail.com/</a><br>
<br>
> +<br>
> +if [ "$cgroup_ver" = 'cgroup2' ]; then<br>
> + cgroup_proc=cgroup.procs<br>
> + cgroup_mem_limit=memory.max<br>
> +else<br>
> + cgroup_proc=tasks<br>
> + cgroup_mem_limit=memory.limit_in_bytes<br>
> +fi<br>
> +<br>
> # attach current task to memcg/0/<br>
> mkdir memcg/0<br>
> -echo $$ > memcg/0/tasks<br>
> +echo $$ > memcg/0/${cgroup_proc}<br>
> <br>
> ./memcg_test_4 &<br>
> pid=$!<br>
> @@ -35,13 +45,13 @@ sleep 1<br>
> sleep 1<br>
> <br>
> # shrink memory, and then 80M will be swapped<br>
> -echo 40M > memcg/0/memory.limit_in_bytes<br>
> +echo 40M > memcg/0/${cgroup_mem_limit}<br>
> <br>
> # turn off swap, and swapoff will be killed<br>
> swapoff -a<br>
> sleep 1<br>
> -echo $pid > memcg/tasks 2> /dev/null<br>
> -echo $$ > memcg/tasks 2> /dev/null<br>
> +echo $pid > memcg/${cgroup_proc} 2> /dev/null<br>
> +echo $$ > memcg/${cgroup_proc} 2> /dev/null<br>
> <br>
> # now remove the cgroup<br>
> rmdir memcg/0<br>
> -- <br>
> 2.32.0<br>
<br>
<br>
-- <br>
Thank you,<br>
Richard.<br>
</blockquote></div></div>