Re: [LTP] [PATCH] controllers/memcg: update regression tests to work under cgroup2

Richard Palethorpe
2021-11-16T10:10:06Z

Hello Luke,

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

> Some tests no longer make sense under cgroup2, while other tests just
> needed to be updated to use new parameters introduced by the cgroup2
> interface.
>
> Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
> ---
> .../memcg/regression/memcg_regression_test.sh | 41 ++++++++++++++++---
> .../memcg/regression/memcg_test_1.c | 12 +++++-
> .../memcg/regression/memcg_test_3.c | 8 ++++
> .../memcg/regression/memcg_test_4.sh | 18 ++++++--
> 4 files changed, 68 insertions(+), 11 deletions(-)
>
> diff --git a/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh b/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
> index c91a4069e..ad88d49d1 100755
> --- a/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
> +++ b/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
> @@ -103,7 +103,12 @@ check_kernel_bug()
> test_1()
> {
> mkdir memcg/0/
> - echo 0 > memcg/0/memory.limit_in_bytes
> +
> + if [ "$cgroup_ver" = "cgroup2" ]; then
> + echo 0 > memcg/0/memory.max
> + else
> + echo 0 > memcg/0/memory.limit_in_bytes
> + fi
>
> ./memcg_test_1
>
> @@ -124,6 +129,16 @@ test_1()
> #---------------------------------------------------------------------------
> test_2()
> {
> + # for cgroup2 the default behaivor is to check the new memory limit and
> + # then to start killing processes if oom. This test then doesen't
> + # make sense as we don't expect EBUSY to be returned. The shrink
> + # operation (write to memory.max in cgroup2) would kill the pid1 process
> + # and exit.
> + if [ "$cgroup_ver" = "cgroup2" ]; then
> + tst_resm TCONF "cgroup2 found, skipping test"
> + return
> + fi
> +
> ./memcg_test_2 &
> pid1=$!
> sleep 1
> @@ -177,12 +192,20 @@ test_2()
> test_3()
> {
> mkdir memcg/0
> - for pid in `cat memcg/tasks`; do
> - echo $pid > memcg/0/tasks 2> /dev/null
> + if [ "$cgroup_ver" = "cgroup2" ]; then
> + memcg_procs=memcg/cgroup.procs
> + memcg_subprocs=memcg/0/cgroup.procs
> + else
> + memcg_procs=memcg/tasks
> + memcg_subprocs=memcg/0/tasks
> + fi
> +
> + for pid in `cat $memcg_procs`; do
> + echo $pid > $memcg_subprocs 2> /dev/null
> done
>
> - for pid in `cat memcg/0/tasks`; do
> - echo $pid > memcg/tasks 2> /dev/null
> + for pid in `cat $memcg_subprocs`; do
> + echo $pid > $memcg_procs 2> /dev/null
> done
> rmdir memcg/0
>
> @@ -218,13 +241,19 @@ test_4()
> }
>
> # main
> +cgroup_ver=$(grep "/sys/fs/cgroup" /proc/mounts | cut -d' ' -f1)
> failed=0
> mkdir memcg/
>
> for cur in $(seq 1 $TST_TOTAL); do
> export TST_COUNT=$cur
>
> - mount -t cgroup -o memory xxx memcg/
> + if [ "$cgroup_ver" = "cgroup2" ]; then
> + mount -t cgroup2 xxx memcg/
> + else
> + mount -t cgroup -o memory xxx memcg/
> + fi
> +
> if [ $? -ne 0 ]; then
> tst_resm TFAIL "failed to mount memory subsystem"
> failed=1
> diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_1.c b/testcases/kernel/controllers/memcg/regression/memcg_test_1.c
> index c7fb948fe..b9277e633 100644
> --- a/testcases/kernel/controllers/memcg/regression/memcg_test_1.c
> +++ b/testcases/kernel/controllers/memcg/regression/memcg_test_1.c
> @@ -33,6 +33,16 @@
>
> #define FORKED_PROC_COUNT 10
>
> +static int open_cgroup_procs(void)
> +{
> + int fd = open("memcg/0/tasks", O_WRONLY);
> + if (fd >= 0)
> + return fd;
> +
> + fd = open("memcg/0/cgroup.procs", O_WRONLY);
> + return fd;
> +}
> +
> int main(void)
> {
> char buf[10];
> @@ -40,7 +50,7 @@ int main(void)
> int loop;
> int pid;
> int size = getpagesize();
> - int fd = open("memcg/0/tasks", O_WRONLY);
> + int fd = open_cgroup_procs();
>
> if (fd < 0)
> return 1;
> diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_3.c b/testcases/kernel/controllers/memcg/regression/memcg_test_3.c
> index 75a6e1545..d5531fe87 100644
> --- a/testcases/kernel/controllers/memcg/regression/memcg_test_3.c
> +++ b/testcases/kernel/controllers/memcg/regression/memcg_test_3.c
> @@ -66,6 +66,14 @@ static void setup(void)
> SAFE_MKDIR(MNTPOINT, 0644);
>
> ret = mount("memcg", MNTPOINT, "cgroup", 0, "memory");
> +
> + if (!ret) {
> + mount_flag = 1;
> + return;
> + }
> +
> + ret = mount("memcg", MNTPOINT, "cgroup2", 0, NULL);
> +
> if (ret) {
> if (errno == ENOENT)
> tst_brk(TCONF | TERRNO, "memcg not supported");
> diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh b/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
> index 620031366..287864b81 100755
> --- a/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
> +++ b/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
> @@ -22,9 +22,19 @@
> ## ##
> ################################################################################
>
> +cgroup_ver=$(grep "/sys/fs/cgroup" /proc/mounts | cut -d' ' -f1)

Unfortunately this doesn't work when both V1 and V2 hierarchies are
mounted. You need to check that the memory controller is enabled on
V2. Also it's possible to mount CGroups at alternate locations.

Please see the recent patch from Masayoshi:
https://patchwork.ozlabs.org/project/ltp/patch/20211113041706.12893-1-msys.mizuma@gmail.com/

> +
> +if [ "$cgroup_ver" = 'cgroup2' ]; then
> + cgroup_proc=cgroup.procs
> + cgroup_mem_limit=memory.max
> +else
> + cgroup_proc=tasks
> + cgroup_mem_limit=memory.limit_in_bytes
> +fi
> +
> # attach current task to memcg/0/
> mkdir memcg/0
> -echo $$ > memcg/0/tasks
> +echo $$ > memcg/0/${cgroup_proc}
>
> ./memcg_test_4 &
> pid=$!
> @@ -35,13 +45,13 @@ sleep 1
> sleep 1
>
> # shrink memory, and then 80M will be swapped
> -echo 40M > memcg/0/memory.limit_in_bytes
> +echo 40M > memcg/0/${cgroup_mem_limit}
>
> # turn off swap, and swapoff will be killed
> swapoff -a
> sleep 1
> -echo $pid > memcg/tasks 2> /dev/null
> -echo $$ > memcg/tasks 2> /dev/null
> +echo $pid > memcg/${cgroup_proc} 2> /dev/null
> +echo $$ > memcg/${cgroup_proc} 2> /dev/null
>
> # now remove the cgroup
> rmdir memcg/0
> --
> 2.32.0

--
Thank you,
Richard.