[LTP] [PATCH 6/6] controllers: Expand cgroup_lib shell library

[Li Wang]

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
>


-- 
Regards,
Li Wang