On Tue, Feb 18, 2020 at 10:27:12AM -0500, Josef Bacik wrote:
> I want to add some extended statistic gathering for xfstests, but it's
> tricky to isolate xfstests from the rest of the host applications. The
> most straightforward way to do this is to run every test inside of it's
> own cgroup. From there we can monitor the activity of tasks in the
> specific cgroup using BPF.
>
> The support for this is pretty simple, allow users to pass -C <cgroup
> name>. We will create the path if it doesn't already exist, and
> validate we can add things to cgroup.procs. If we cannot it'll be
> disabled, otherwise we will use this when we do _run_seq by echo'ing the
> bash pid into cgroup.procs, which will cause any children to run under
> that cgroup.
>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
> v1->v2:
> - Changed it from a local.config option to a command line option.
> - Export CGROUP2_PATH for everything, utilize that path when generating our
> cgroup for the scripts to run in.
>
> check | 24 +++++++++++++++++++++++-
> common/cgroup2 | 2 --
> common/config | 1 +
> 3 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/check b/check
> index 2e148e57..df33628e 100755
> --- a/check
> +++ b/check
> @@ -72,6 +72,7 @@ check options
> --large-fs optimise scratch device for large filesystems
> -s section run only specified section from config file
> -S section exclude the specified section from the config file
> + -C cgroup_name run all the tests in the specified cgroup name
>
> testlist options
> -g group[,group...] include tests from these groups
> @@ -101,6 +102,10 @@ excluded from the list of tests to run from that test dir.
> external_file argument is a path to a single file containing a list of tests
> to exclude in the form of <test dir>/<test name>.
>
> +cgroup_name is just a plain name, or a path relative to the root cgroup path.
> +If CGROUP2_PATH does not point at where cgroup2 is mounted then adjust it
> +accordingly.
> +
> examples:
> check xfs/001
> check -g quick
> @@ -307,6 +312,7 @@ while [ $# -gt 0 ]; do
> ;;
> --large-fs) export LARGE_SCRATCH_DEV=yes ;;
> --extra-space=*) export SCRATCH_DEV_EMPTY_SPACE=${r#*=} ;;
> + -C) CGROUP=$2 ; shift ;;
>
> -*) usage ;;
> *) # not an argument, we've got tests now.
> @@ -509,11 +515,24 @@ _expunge_test()
> OOM_SCORE_ADJ="/proc/self/oom_score_adj"
> test -w ${OOM_SCORE_ADJ} && echo -1000 > ${OOM_SCORE_ADJ}
>
> +# Initialize the cgroup path if it doesn't already exist
> +if [ ! -z "$CGROUP" ]; then
> + CGROUP=${CGROUP2_PATH}/${CGROUP}
> + mkdir -p ${CGROUP}
> +
> + # If we can't write to cgroup.procs then unset cgroup
> + test -w ${CGROUP}/cgroup.procs || unset CGROUP
> +fi
> +
Do we need to fix up generic/563 to use this new $CGROUP value, when
set? That test explicitly moves tasks to new/temporary groups, but looks
like it resets back to the top-level CGROUP2_PATH group. I'm not sure
how much that really matters since presumably the next test should move
back into $CGROUP. Otherwise this looks reasonable enough to me.
Brian
> # ...and make the tests themselves somewhat more attractive to it, so that if
> # the system runs out of memory it'll be the test that gets killed and not the
> # test framework.
> _run_seq() {
> - bash -c "test -w ${OOM_SCORE_ADJ} && echo 250 > ${OOM_SCORE_ADJ}; exec ./$seq"
> + _extra="test -w ${OOM_SCORE_ADJ} && echo 250 > ${OOM_SCORE_ADJ};"
> + if [ ! -z "$CGROUP" ]; then
> + _extra+="echo $$ > ${CGROUP}/cgroup.procs;"
> + fi
> + bash -c "${_extra} exec ./$seq"
> }
>
> _detect_kmemleak
> @@ -615,6 +634,9 @@ for section in $HOST_OPTIONS_SECTIONS; do
> echo "MKFS_OPTIONS -- `_scratch_mkfs_options`"
> echo "MOUNT_OPTIONS -- `_scratch_mount_options`"
> fi
> + if [ ! -z "$CGROUP" ]; then
> + echo "CGROUP -- ${CGROUP}"
> + fi
> echo
> needwrap=true
>
> diff --git a/common/cgroup2 b/common/cgroup2
> index 8833c9c8..554bd238 100644
> --- a/common/cgroup2
> +++ b/common/cgroup2
> @@ -1,7 +1,5 @@
> # cgroup2 specific common functions
>
> -export CGROUP2_PATH="${CGROUP2_PATH:-/sys/fs/cgroup}"
> -
> _require_cgroup2()
> {
> if [ `findmnt -d backward -n -o FSTYPE -f ${CGROUP2_PATH}` != "cgroup2" ]; then
> diff --git a/common/config b/common/config
> index 9a9c7760..0eaf35c3 100644
> --- a/common/config
> +++ b/common/config
> @@ -259,6 +259,7 @@ case "$HOSTOS" in
> export E2FSCK_PROG=$(type -P e2fsck)
> export TUNE2FS_PROG=$(type -P tune2fs)
> export FSCK_OVERLAY_PROG=$(type -P fsck.overlay)
> + export CGROUP2_PATH="${CGROUP2_PATH:-/sys/fs/cgroup}"
> ;;
> esac
>
> --
> 2.24.1
>
