[PATCH] xfstests: add a simple cgroup2 writeback test

[PATCH] xfstests: add a simple cgroup2 writeback test

[Shaohua Li]

From: Shaohua Li <shli@fb.com>

If filesystem supports cgroup2 writeback, the writeback IO should belong
to the cgroup which writes the file. To verify this, we set a write
bandwidth limit for a cgroup using block-throttling, the writeback IO
should be throttled according to the write bandwidth.

Thanks Dave Chinner's idea to use syncfs to wait for writeback
completion.

Signed-off-by: Shaohua Li <shli@fb.com>
---
 common/cgroup2        | 18 +++++++++++++
 tests/generic/463     | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/463.out |  3 +++
 tests/generic/group   |  1 +
 4 files changed, 97 insertions(+)
 create mode 100644 common/cgroup2
 create mode 100755 tests/generic/463
 create mode 100644 tests/generic/463.out

diff --git a/common/cgroup2 b/common/cgroup2
new file mode 100644
index 00000000..bf935fee
--- /dev/null
+++ b/common/cgroup2
@@ -0,0 +1,18 @@
+#!/bin/bash
+# cgroup2 specific common functions
+
+export CGROUP2_PATH="/sys/fs/cgroup"
+
+_require_cgroup2()
+{
+    if [ ! -f ${CGROUP2_PATH}/cgroup.subtree_control ]; then
+        _notrun "Test requires cgroup2 enabled"
+    fi
+}
+
+_get_scratch_dev_devt()
+{
+    ls -l $SCRATCH_DEV | awk '{printf("%s:%s", substr($5, 1, length($5)-1), 0)}'
+}
+
+/bin/true
diff --git a/tests/generic/463 b/tests/generic/463
new file mode 100755
index 00000000..620a14a4
--- /dev/null
+++ b/tests/generic/463
@@ -0,0 +1,75 @@
+#! /bin/bash
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/cgroup2
+
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+cgname=`mktemp -du ${CGROUP2_PATH}/test.XXXXXX`
+_cleanup()
+{
+    cd /
+    _scratch_unmount
+    sync
+    rmdir $cgname
+}
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+_require_cgroup2
+
+# Setup Filesystem
+_scratch_mkfs >/dev/null 2>&1 || _fail "mkfs failed"
+
+_scratch_mount || _fail "mount failed"
+
+test_speed()
+{
+    start=$(date +%s)
+    $XFS_IO_PROG -f -d -c "pwrite -b 1m 0 500m" "$SCRATCH_MNT/image" \
+        > /dev/null 2>&1
+    echo $(($(date +%s) - $start))
+}
+time=$(test_speed)
+
+if [ $time -gt 25 ]; then
+    echo "Disk is too slow, make sure disk can do 20MB/s at least"
+    status=1
+    exit 1
+fi
+
+echo "Disk speed is ok, start throttled writeback test"
+
+echo +io > ${CGROUP2_PATH}/cgroup.subtree_control
+mkdir $cgname
+echo "$(_get_scratch_dev_devt) wbps=$((5*1024*1024))" > $cgname/io.max
+
+run_writeback()
+{
+    start=$(date +%s)
+    $XFS_IO_PROG -f -c "pwrite 0 500m" -c "syncfs" "$SCRATCH_MNT/image" \
+        > /dev/null 2>&1
+    echo $(($(date +%s) - $start))
+}
+time=$(
+echo $BASHPID > $cgname/cgroup.procs;
+run_writeback
+)
+_within_tolerance "Throttled writeback runtime" $time 100 10% -v
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/463.out b/tests/generic/463.out
new file mode 100644
index 00000000..c69f87c2
--- /dev/null
+++ b/tests/generic/463.out
@@ -0,0 +1,3 @@
+QA output created by 463
+Disk speed is ok, start throttled writeback test
+Throttled writeback runtime is in range
diff --git a/tests/generic/group b/tests/generic/group
index f2a6cdad..ea7b6956 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -465,3 +465,4 @@
 460 auto quick rw
 461 auto shutdown stress
 462 auto quick dax
+463 cgroup writeback test
-- 
2.11.0

Re: [PATCH] xfstests: add a simple cgroup2 writeback test

[Eryu Guan]

On Tue, Oct 17, 2017 at 10:20:49PM -0700, Shaohua Li wrote:
> From: Shaohua Li <shli@fb.com>
> 
> If filesystem supports cgroup2 writeback, the writeback IO should belong
> to the cgroup which writes the file. To verify this, we set a write
> bandwidth limit for a cgroup using block-throttling, the writeback IO
> should be throttled according to the write bandwidth.
> 
> Thanks Dave Chinner's idea to use syncfs to wait for writeback
> completion.
> 
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
>  common/cgroup2        | 18 +++++++++++++
>  tests/generic/463     | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/463.out |  3 +++
>  tests/generic/group   |  1 +
>  4 files changed, 97 insertions(+)
>  create mode 100644 common/cgroup2
>  create mode 100755 tests/generic/463
>  create mode 100644 tests/generic/463.out
> 
> diff --git a/common/cgroup2 b/common/cgroup2
> new file mode 100644
> index 00000000..bf935fee
> --- /dev/null
> +++ b/common/cgroup2
> @@ -0,0 +1,18 @@
> +#!/bin/bash

No need to use "#!/bin/bash" for a common file to be sourced by other
scripts.

> +# cgroup2 specific common functions
> +
> +export CGROUP2_PATH="/sys/fs/cgroup"

Make it configurable? e.g.
export CGROUP2_PATH="${CGROUP2_PATH:-"/sys/fs/cgroup"}

Because on my fedora test host, cgroup is mounted at
/sys/fs/cgroup/unified by default.

> +
> +_require_cgroup2()
> +{
> +    if [ ! -f ${CGROUP2_PATH}/cgroup.subtree_control ]; then
> +        _notrun "Test requires cgroup2 enabled"
> +    fi

Please use tab for indention.

> +}
> +
> +_get_scratch_dev_devt()
> +{
> +    ls -l $SCRATCH_DEV | awk '{printf("%s:%s", substr($5, 1, length($5)-1), 0)}'
> +}

stat -c %t for major and stat -c %T for minor? And seems that the minor
device id returned here is always 0, is that intentional?

Also, you need to call _real_dev first in case $SCRATCH_DEV is a
symlink, e.g. device mapper device. And this helper can be moved to
common/rc, it's not cgroup2 specific.

I think you can factor out "get the device id" part of _sysfs_dev() into
a new helper, and use the new one in _sysfs_dev() and this test.

> +
> +/bin/true
> diff --git a/tests/generic/463 b/tests/generic/463
> new file mode 100755
> index 00000000..620a14a4
> --- /dev/null
> +++ b/tests/generic/463
> @@ -0,0 +1,75 @@
> +#! /bin/bash
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/cgroup2
> +
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +cgname=`mktemp -du ${CGROUP2_PATH}/test.XXXXXX`
> +_cleanup()
> +{
> +    cd /
> +    _scratch_unmount

check will do this automatically :)

> +    sync
> +    rmdir $cgname
> +}
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch
> +_require_cgroup2
> +
> +# Setup Filesystem
> +_scratch_mkfs >/dev/null 2>&1 || _fail "mkfs failed"
> +
> +_scratch_mount || _fail "mount failed"
> +
> +test_speed()
> +{
> +    start=$(date +%s)
> +    $XFS_IO_PROG -f -d -c "pwrite -b 1m 0 500m" "$SCRATCH_MNT/image" \
> +        > /dev/null 2>&1
> +    echo $(($(date +%s) - $start))
> +}
> +time=$(test_speed)
> +
> +if [ $time -gt 25 ]; then
> +    echo "Disk is too slow, make sure disk can do 20MB/s at least"
> +    status=1
> +    exit 1

We should call _notrun here if device speed is too slow, that's not a
failure, but a un-met test requirement.

> +fi
> +
> +echo "Disk speed is ok, start throttled writeback test"
> +
> +echo +io > ${CGROUP2_PATH}/cgroup.subtree_control

Make sure if io is available first in cgroup.controllers? Because on my
fedora rawhide test host, cgroup.controllers is empty, because all io,
cpu and mem controls are bound to cgroup v1 by default. I think this
check could be integrated with _require_cgroup2, e.g.

_require_cgroup2 io

> +mkdir $cgname
> +echo "$(_get_scratch_dev_devt) wbps=$((5*1024*1024))" > $cgname/io.max
> +
> +run_writeback()
> +{
> +    start=$(date +%s)
> +    $XFS_IO_PROG -f -c "pwrite 0 500m" -c "syncfs" "$SCRATCH_MNT/image" \
> +        > /dev/null 2>&1
> +    echo $(($(date +%s) - $start))
> +}
> +time=$(
> +echo $BASHPID > $cgname/cgroup.procs;
> +run_writeback
> +)
> +_within_tolerance "Throttled writeback runtime" $time 100 10% -v
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/463.out b/tests/generic/463.out
> new file mode 100644
> index 00000000..c69f87c2
> --- /dev/null
> +++ b/tests/generic/463.out
> @@ -0,0 +1,3 @@
> +QA output created by 463
> +Disk speed is ok, start throttled writeback test
> +Throttled writeback runtime is in range
> diff --git a/tests/generic/group b/tests/generic/group
> index f2a6cdad..ea7b6956 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -465,3 +465,4 @@
>  460 auto quick rw
>  461 auto shutdown stress
>  462 auto quick dax
> +463 cgroup writeback test

s/test/auto/, I don't see the meaning of a 'test' group :)

Thanks,
Eryu

> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] xfstests: add a simple cgroup2 writeback test

[Brian Foster]

On Thu, Oct 26, 2017 at 04:57:40PM +0800, Eryu Guan wrote:
> On Tue, Oct 17, 2017 at 10:20:49PM -0700, Shaohua Li wrote:
> > From: Shaohua Li <shli@fb.com>
> > 
> > If filesystem supports cgroup2 writeback, the writeback IO should belong
> > to the cgroup which writes the file. To verify this, we set a write
> > bandwidth limit for a cgroup using block-throttling, the writeback IO
> > should be throttled according to the write bandwidth.
> > 
> > Thanks Dave Chinner's idea to use syncfs to wait for writeback
> > completion.
> > 
> > Signed-off-by: Shaohua Li <shli@fb.com>
> > ---

Ping..

Shaohua,

Were you planning to incorporate Eryu's feedback and repost this (and
the associated xfs writeback patch)?

Brian

> >  common/cgroup2        | 18 +++++++++++++
> >  tests/generic/463     | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/463.out |  3 +++
> >  tests/generic/group   |  1 +
> >  4 files changed, 97 insertions(+)
> >  create mode 100644 common/cgroup2
> >  create mode 100755 tests/generic/463
> >  create mode 100644 tests/generic/463.out
> > 
> > diff --git a/common/cgroup2 b/common/cgroup2
> > new file mode 100644
> > index 00000000..bf935fee
> > --- /dev/null
> > +++ b/common/cgroup2
> > @@ -0,0 +1,18 @@
> > +#!/bin/bash
> 
> No need to use "#!/bin/bash" for a common file to be sourced by other
> scripts.
> 
> > +# cgroup2 specific common functions
> > +
> > +export CGROUP2_PATH="/sys/fs/cgroup"
> 
> Make it configurable? e.g.
> export CGROUP2_PATH="${CGROUP2_PATH:-"/sys/fs/cgroup"}
> 
> Because on my fedora test host, cgroup is mounted at
> /sys/fs/cgroup/unified by default.
> 
> > +
> > +_require_cgroup2()
> > +{
> > +    if [ ! -f ${CGROUP2_PATH}/cgroup.subtree_control ]; then
> > +        _notrun "Test requires cgroup2 enabled"
> > +    fi
> 
> Please use tab for indention.
> 
> > +}
> > +
> > +_get_scratch_dev_devt()
> > +{
> > +    ls -l $SCRATCH_DEV | awk '{printf("%s:%s", substr($5, 1, length($5)-1), 0)}'
> > +}
> 
> stat -c %t for major and stat -c %T for minor? And seems that the minor
> device id returned here is always 0, is that intentional?
> 
> Also, you need to call _real_dev first in case $SCRATCH_DEV is a
> symlink, e.g. device mapper device. And this helper can be moved to
> common/rc, it's not cgroup2 specific.
> 
> I think you can factor out "get the device id" part of _sysfs_dev() into
> a new helper, and use the new one in _sysfs_dev() and this test.
> 
> > +
> > +/bin/true
> > diff --git a/tests/generic/463 b/tests/generic/463
> > new file mode 100755
> > index 00000000..620a14a4
> > --- /dev/null
> > +++ b/tests/generic/463
> > @@ -0,0 +1,75 @@
> > +#! /bin/bash
> > +
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +. ./common/cgroup2
> > +
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +cgname=`mktemp -du ${CGROUP2_PATH}/test.XXXXXX`
> > +_cleanup()
> > +{
> > +    cd /
> > +    _scratch_unmount
> 
> check will do this automatically :)
> 
> > +    sync
> > +    rmdir $cgname
> > +}
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_scratch
> > +_require_cgroup2
> > +
> > +# Setup Filesystem
> > +_scratch_mkfs >/dev/null 2>&1 || _fail "mkfs failed"
> > +
> > +_scratch_mount || _fail "mount failed"
> > +
> > +test_speed()
> > +{
> > +    start=$(date +%s)
> > +    $XFS_IO_PROG -f -d -c "pwrite -b 1m 0 500m" "$SCRATCH_MNT/image" \
> > +        > /dev/null 2>&1
> > +    echo $(($(date +%s) - $start))
> > +}
> > +time=$(test_speed)
> > +
> > +if [ $time -gt 25 ]; then
> > +    echo "Disk is too slow, make sure disk can do 20MB/s at least"
> > +    status=1
> > +    exit 1
> 
> We should call _notrun here if device speed is too slow, that's not a
> failure, but a un-met test requirement.
> 
> > +fi
> > +
> > +echo "Disk speed is ok, start throttled writeback test"
> > +
> > +echo +io > ${CGROUP2_PATH}/cgroup.subtree_control
> 
> Make sure if io is available first in cgroup.controllers? Because on my
> fedora rawhide test host, cgroup.controllers is empty, because all io,
> cpu and mem controls are bound to cgroup v1 by default. I think this
> check could be integrated with _require_cgroup2, e.g.
> 
> _require_cgroup2 io
> 
> > +mkdir $cgname
> > +echo "$(_get_scratch_dev_devt) wbps=$((5*1024*1024))" > $cgname/io.max
> > +
> > +run_writeback()
> > +{
> > +    start=$(date +%s)
> > +    $XFS_IO_PROG -f -c "pwrite 0 500m" -c "syncfs" "$SCRATCH_MNT/image" \
> > +        > /dev/null 2>&1
> > +    echo $(($(date +%s) - $start))
> > +}
> > +time=$(
> > +echo $BASHPID > $cgname/cgroup.procs;
> > +run_writeback
> > +)
> > +_within_tolerance "Throttled writeback runtime" $time 100 10% -v
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/generic/463.out b/tests/generic/463.out
> > new file mode 100644
> > index 00000000..c69f87c2
> > --- /dev/null
> > +++ b/tests/generic/463.out
> > @@ -0,0 +1,3 @@
> > +QA output created by 463
> > +Disk speed is ok, start throttled writeback test
> > +Throttled writeback runtime is in range
> > diff --git a/tests/generic/group b/tests/generic/group
> > index f2a6cdad..ea7b6956 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -465,3 +465,4 @@
> >  460 auto quick rw
> >  461 auto shutdown stress
> >  462 auto quick dax
> > +463 cgroup writeback test
> 
> s/test/auto/, I don't see the meaning of a 'test' group :)
> 
> Thanks,
> Eryu
> 
> > -- 
> > 2.11.0
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html