[6/6] controllers: Expand cgroup_lib shell library

Expand the cgroup_lib library by using the tools/cgroup/tst_cgctl binary
utility to make calls to the Cgroup C API to simplify and centralize the
mounting and cleanup process of Cgroups.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 testcases/kernel/controllers/cgroup_lib.sh | 129 ++++++++++++++++++---
 1 file changed, 113 insertions(+), 16 deletions(-)

On Wed, Jan 5, 2022 at 6:01 PM Luke Nowakowski-Krijger
<luke.nowakowskikrijger@canonical.com> wrote:
>
> Expand the cgroup_lib library by using the tools/cgroup/tst_cgctl binary
> utility to make calls to the Cgroup C API to simplify and centralize the
> mounting and cleanup process of Cgroups.
>
> Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
> ---
>  testcases/kernel/controllers/cgroup_lib.sh | 129 ++++++++++++++++++---
>  1 file changed, 113 insertions(+), 16 deletions(-)
>
> diff --git a/testcases/kernel/controllers/cgroup_lib.sh b/testcases/kernel/controllers/cgroup_lib.sh
> index 7918b5636..6ab201b95 100644
> --- a/testcases/kernel/controllers/cgroup_lib.sh
> +++ b/testcases/kernel/controllers/cgroup_lib.sh
> @@ -5,22 +5,7 @@
>
>  . tst_test.sh
>
> -# Find mountpoint to given subsystem
> -# get_cgroup_mountpoint SUBSYSTEM
> -# RETURN: 0 if mountpoint found, otherwise 1
> -get_cgroup_mountpoint()

As we have renamed this function to cgroup_get_mountpoint(), so the
invoke part in test cases should be updated too. Maybe you're going
to complete that in following up patches, but as a completed patch set,
we'd better keep the code not broken in running.

cgroup/cgroup_regression_test.sh:
cpu_subsys_path=$(get_cgroup_mountpoint "cpu")
cgroup/cgroup_regression_test.sh:       local
subsys_path=$(get_cgroup_mountpoint $subsys)
cpuset/cpuset_regression_test.sh:
root_cpuset_dir=$(get_cgroup_mountpoint cpuset)

Btw, it would be great you can write simple test demos to verify the
new shell API works well.
Just like what people done at: ltp/lib/newlib_tests/shell/*

> -{
> -       local subsystem=$1
> -       local mntpoint
> -
> -       [ $# -eq 0 ] && tst_brk TBROK "get_cgroup_mountpoint: subsystem not defined"
> -
> -       mntpoint=$(grep cgroup /proc/mounts | grep -w $subsystem | awk '{ print $2 }')
> -       [ -z "$mntpoint" ] && return 1
> -
> -       echo $mntpoint
> -       return 0
> -}
> +_cgroup_state=
>
>  # Check if given subsystem is supported and enabled
>  # is_cgroup_subsystem_available_and_enabled SUBSYSTEM
> @@ -37,3 +22,115 @@ is_cgroup_subsystem_available_and_enabled()
>
>         return 1
>  }
> +
> +# Find mountpoint of the given controller
> +# USAGE: cgroup_get_mountpoint CONTROLLER
> +# RETURNS: Prints the mountpoint of the given controller
> +# Must call cgroup_require before calling
> +cgroup_get_mountpoint()
> +{
> +       local ctrl=$1
> +       local mountpoint
> +
> +       [ $# -eq 0 ] && tst_brk TBROK "cgroup_get_mountpoint: controller not defined"
> +       [ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_get_mountpoint: No previous state found. Forgot to call cgroup_require?"
> +
> +       mountpoint=$(echo "$_cgroup_state" | grep -w "$ctrl" | awk '{ print $4 }')
> +       echo "$mountpoint"
> +
> +       return 0
> +}
> +
> +# Get the test path of a given controller that has been created by the cgroup C API
> +# USAGE: cgroup_get_test_path CONTROLLER
> +# RETURNS: Prints the path to the test direcory
> +# Must call cgroup_require before calling
> +cgroup_get_test_path()
> +{
> +       local ctrl="$1"
> +       local mountpoint
> +       local test_path
> +
> +       [ $# -eq 0 ] && tst_brk TBROK "cgroup_get_test_path: controller not defined"
> +       [ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_get_test_path: No previous state found. Forgot to call cgroup_require?"
> +
> +       mountpoint=$(cgroup_get_mountpoint "$ctrl")
> +
> +       test_path="$mountpoint/ltp/test-$$"
> +
> +       [ ! -d "$test_path" ] && tst_brk TBROK "cgroup_get_test_path: No test path found. Forgot to call cgroup_require?"
> +
> +       echo "$test_path"
> +
> +       return 0
> +}
> +
> +# Gets the cgroup version of the given controller
> +# USAGE: cgroup_get_version CONTROLLER
> +# RETURNS: "V1" if version 1 and "V2" if version 2
> +# Must call cgroup_require before calling
> +cgroup_get_version()
> +{
> +       local ctrl="$1"
> +
> +       [ $# -eq 0 ] && tst_brk TBROK "cgroup_get_version: controller not defined"
> +       [ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_get_version: No previous state found. Forgot to call cgroup_require?"
> +
> +       version=$(echo "$_cgroup_state" | grep -w "$ctrl" | awk '{ print $2 }')
> +       [ "$version" = "" ] && tst_brk TBROK "cgroup_get_version: Could not find controller $ctrl"
> +
> +       echo "$version"
> +
> +       return 0
> +}
> +
> +# Cleans up any setup done by calling cgroup_require.
> +# USAGE: cgroup_cleanup
> +# Can be safely called even when no setup has been done
> +cgroup_cleanup()
> +{
> +       [ "$_cgroup_state" = "" ] && return 0
?> +
> +       tst_cgctl cleanup "$_cgroup_state"

I don't understand what exactly the $_cgroup_state do here,
isn't that just 0 or 1 which returned by 'tst_cgctl require "$ctrl" $$' ?
Maybe you can add brief comments on this variable.

> +
> +       return 0
> +}
> +
> +# Get the task list of the given controller
> +# USAGE: cgroup_get_task_list CONTROLLER
> +# RETURNS: prints out "cgroup.procs" if version 2 otherwise "tasks"
> +# Must call cgroup_require before calling
> +cgroup_get_task_list()
> +{
> +       local ctrl="$1"
> +       local version
> +
> +       version=$(cgroup_get_version "$ctrl")
> +
> +       if [ "$version" = "V2" ]; then
> +               echo "cgroup.procs"
> +       else
> +               echo "tasks"
> +       fi
> +
> +       return 0
> +}
> +
> +# Mounts and configures the given controller
> +# USAGE: cgroup_require CONTROLLER
> +cgroup_require()
> +{
> +       local ctrl="$1"
> +
> +       [ $# -eq 0 ] && tst_brk TBROK "cgroup_require: controller not defined"

We have already do parameter "$1" checking in
is_cgroup_subsystem_avalibale_and_enbale,
so here looks reductant.

And, I think we don't need to invoke that independent function at any
other place
if we put that in this cgroup_require() function. It might be good to delete
is_cgroup_subsystem_avalibale_and_enbale and move the process to here.

> +
> +       if ! is_cgroup_subsystem_available_and_enabled "$ctrl"; then
> +               tst_brk TBROK "cgroup_require: Controller not available or not enabled"

Maybe using TCONF here is more elegent.

> +       fi
> +
> +       _cgroup_state=$(tst_cgctl require "$ctrl" $$)
> +
> +       [ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_require: No state was set after call. Controller '$ctrl' maybe does not exist?"
> +
> +       return 0
> +}
> --
> 2.32.0
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>

> > +cgroup_cleanup()
> > +{
> > +       [ "$_cgroup_state" = "" ] && return 0
> ?> +
> > +       tst_cgctl cleanup "$_cgroup_state"
>
> I don't understand what exactly the $_cgroup_state do here,
> isn't that just 0 or 1 which returned by 'tst_cgctl require "$ctrl" $$' ?

Ah, I got it, that comes from a string printed by tst_cgroup_print_config.
(Sorry I was read these codes from top to down.)

> +# Find mountpoint of the given controller
> +# USAGE: cgroup_get_mountpoint CONTROLLER
> +# RETURNS: Prints the mountpoint of the given controller
> +# Must call cgroup_require before calling
> +cgroup_get_mountpoint()
> +{
> +       local ctrl=$1
> +       local mountpoint
> +
> +       [ $# -eq 0 ] && tst_brk TBROK "cgroup_get_mountpoint: controller not defined"
> +       [ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_get_mountpoint: No previous state found. Forgot to call cgroup_require?"
> +
> +       mountpoint=$(echo "$_cgroup_state" | grep -w "$ctrl" | awk '{ print $4 }')
> +       echo "$mountpoint"
> +
> +       return 0
> +}
> +
> +# Get the test path of a given controller that has been created by the cgroup C API
> +# USAGE: cgroup_get_test_path CONTROLLER
> +# RETURNS: Prints the path to the test direcory
> +# Must call cgroup_require before calling
> +cgroup_get_test_path()
> +{
> +       local ctrl="$1"
> +       local mountpoint
> +       local test_path
> +
> +       [ $# -eq 0 ] && tst_brk TBROK "cgroup_get_test_path: controller not defined"
> +       [ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_get_test_path: No previous state found. Forgot to call cgroup_require?"
> +
> +       mountpoint=$(cgroup_get_mountpoint "$ctrl")
> +
> +       test_path="$mountpoint/ltp/test-$$"
> +
> +       [ ! -d "$test_path" ] && tst_brk TBROK "cgroup_get_test_path: No test path found. Forgot to call cgroup_require?"
> +
> +       echo "$test_path"
> +
> +       return 0
> +}
> +
> +# Gets the cgroup version of the given controller
> +# USAGE: cgroup_get_version CONTROLLER
> +# RETURNS: "V1" if version 1 and "V2" if version 2
> +# Must call cgroup_require before calling
> +cgroup_get_version()
> +{
> +       local ctrl="$1"
> +
> +       [ $# -eq 0 ] && tst_brk TBROK "cgroup_get_version: controller not defined"
> +       [ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_get_version: No previous state found. Forgot to call cgroup_require?"
> +
> +       version=$(echo "$_cgroup_state" | grep -w "$ctrl" | awk '{ print $2 }')

This won't work on my x86_64 KVM platform, maybe we need: grep -w "^$ctrl".

# echo $$
1801

# ./tst_cgctl require memory 1801
Detected Controllers:
memory V1 @ /sys/fs/cgroup/memory Required
cpu V1 @ /sys/fs/cgroup/cpu,cpuacct
cpuset V1 @ /sys/fs/cgroup/cpuset
Detected Roots:
/sys/fs/cgroup/memory Mounted_Root=no Created_Ltp_Dir=no
Created_Drain_Dir=no Test_Id=test-1801
/sys/fs/cgroup/cpu,cpuacct Mounted_Root=no Created_Ltp_Dir=no
Created_Drain_Dir=no Test_Id=NULL
/sys/fs/cgroup/cpuset Mounted_Root=no Created_Ltp_Dir=no
Created_Drain_Dir=no Test_Id=NULL

# _cgroup_state=$(./tst_cgctl require memory 1801)

# echo "$_cgroup_state" | grep -w "memory" | awk '{ print $2 }'
V1
Mounted_Root=no

# echo "$_cgroup_state" | grep -w "memory" | awk '{ print $4 }'
/sys/fs/cgroup/memory
Created_Drain_Dir=no

# ./tst_cgctl cleanup "$_cgroup_state"
tst_cgroup.c:414: TBROK: Could not find root from config. Roots
changing between calls?


> +       [ "$version" = "" ] && tst_brk TBROK "cgroup_get_version: Could not find controller $ctrl"
> +
> +       echo "$version"
> +
> +       return 0
> +}
> +

Hi Li,

On Tue, Jan 11, 2022 at 2:51 AM Li Wang <liwang@redhat.com> wrote:

> > +# Find mountpoint of the given controller
> > +# USAGE: cgroup_get_mountpoint CONTROLLER
> > +# RETURNS: Prints the mountpoint of the given controller
> > +# Must call cgroup_require before calling
> > +cgroup_get_mountpoint()
> > +{
> > +       local ctrl=$1
> > +       local mountpoint
> > +
> > +       [ $# -eq 0 ] && tst_brk TBROK "cgroup_get_mountpoint: controller
> not defined"
> > +       [ "$_cgroup_state" = "" ] && tst_brk TBROK
> "cgroup_get_mountpoint: No previous state found. Forgot to call
> cgroup_require?"
> > +
> > +       mountpoint=$(echo "$_cgroup_state" | grep -w "$ctrl" | awk '{
> print $4 }')
> > +       echo "$mountpoint"
> > +
> > +       return 0
> > +}
> > +
> > +# Get the test path of a given controller that has been created by the
> cgroup C API
> > +# USAGE: cgroup_get_test_path CONTROLLER
> > +# RETURNS: Prints the path to the test direcory
> > +# Must call cgroup_require before calling
> > +cgroup_get_test_path()
> > +{
> > +       local ctrl="$1"
> > +       local mountpoint
> > +       local test_path
> > +
> > +       [ $# -eq 0 ] && tst_brk TBROK "cgroup_get_test_path: controller
> not defined"
> > +       [ "$_cgroup_state" = "" ] && tst_brk TBROK
> "cgroup_get_test_path: No previous state found. Forgot to call
> cgroup_require?"
> > +
> > +       mountpoint=$(cgroup_get_mountpoint "$ctrl")
> > +
> > +       test_path="$mountpoint/ltp/test-$$"
> > +
> > +       [ ! -d "$test_path" ] && tst_brk TBROK "cgroup_get_test_path: No
> test path found. Forgot to call cgroup_require?"
> > +
> > +       echo "$test_path"
> > +
> > +       return 0
> > +}
> > +
> > +# Gets the cgroup version of the given controller
> > +# USAGE: cgroup_get_version CONTROLLER
> > +# RETURNS: "V1" if version 1 and "V2" if version 2
> > +# Must call cgroup_require before calling
> > +cgroup_get_version()
> > +{
> > +       local ctrl="$1"
> > +
> > +       [ $# -eq 0 ] && tst_brk TBROK "cgroup_get_version: controller
> not defined"
> > +       [ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_get_version:
> No previous state found. Forgot to call cgroup_require?"
> > +
> > +       version=$(echo "$_cgroup_state" | grep -w "$ctrl" | awk '{ print
> $2 }')
>
> This won't work on my x86_64 KVM platform, maybe we need: grep -w "^$ctrl".
>
> # echo $$
> 1801
>
> # ./tst_cgctl require memory 1801
> Detected Controllers:
> memory V1 @ /sys/fs/cgroup/memory Required
> cpu V1 @ /sys/fs/cgroup/cpu,cpuacct
> cpuset V1 @ /sys/fs/cgroup/cpuset
> Detected Roots:
> /sys/fs/cgroup/memory Mounted_Root=no Created_Ltp_Dir=no
> Created_Drain_Dir=no Test_Id=test-1801
> /sys/fs/cgroup/cpu,cpuacct Mounted_Root=no Created_Ltp_Dir=no
> Created_Drain_Dir=no Test_Id=NULL
> /sys/fs/cgroup/cpuset Mounted_Root=no Created_Ltp_Dir=no
> Created_Drain_Dir=no Test_Id=NULL
>
> # _cgroup_state=$(./tst_cgctl require memory 1801)
>
> # echo "$_cgroup_state" | grep -w "memory" | awk '{ print $2 }'
> V1
> Mounted_Root=no
>
> # echo "$_cgroup_state" | grep -w "memory" | awk '{ print $4 }'
> /sys/fs/cgroup/memory
> Created_Drain_Dir=no
>
> # ./tst_cgctl cleanup "$_cgroup_state"
> tst_cgroup.c:414: TBROK: Could not find root from config. Roots
> changing between calls?
>
>
> > +       [ "$version" = "" ] && tst_brk TBROK "cgroup_get_version: Could
> not find controller $ctrl"
> > +
> > +       echo "$version"
> > +
> > +       return 0
> > +}
> > +
>
>
Ah I see that the grepping goes awry when there is already a V1 controller
which has the controller name in the mount path. Thank you for checking
this, I will fix this in the next version.


> --
> Regards,
> Li Wang
>
>
Thanks for the review,
- Luke