diff mbox series

[v2,2/2] btrfs: add test for zone auto reclaim

Message ID 20210428084608.21213-3-johannes.thumshirn@wdc.com (mailing list archive)
State New, archived
Headers show
Series fstests: first few support patches for zoned btrfs | expand

Commit Message

Johannes Thumshirn April 28, 2021, 8:46 a.m. UTC 
Add a test for the patch titled "btrfs: zoned: automatically reclaim
zones".

This test creates a two file on a newly created FS in a way that when we
delete the first one, an auto reclaim process will be triggered by the FS.

After the reclaim process, it verifies that the data was moved to another
zone and old zone was successfully reset.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 common/config       |   1 +
 tests/btrfs/236     | 103 ++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/236.out |   2 +
 tests/btrfs/group   |   1 +
 4 files changed, 107 insertions(+)
 create mode 100755 tests/btrfs/236
 create mode 100644 tests/btrfs/236.out

Comments

Filipe Manana April 28, 2021, 9:24 a.m. UTC | #1 
On Wed, Apr 28, 2021 at 9:46 AM Johannes Thumshirn
<johannes.thumshirn@wdc.com> wrote:
>
> Add a test for the patch titled "btrfs: zoned: automatically reclaim
> zones".

Since this is already in Linus' tree, we could specify here the commit.

>
> This test creates a two file on a newly created FS in a way that when we
> delete the first one, an auto reclaim process will be triggered by the FS.
>
> After the reclaim process, it verifies that the data was moved to another
> zone and old zone was successfully reset.
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  common/config       |   1 +
>  tests/btrfs/236     | 103 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/236.out |   2 +
>  tests/btrfs/group   |   1 +
>  4 files changed, 107 insertions(+)
>  create mode 100755 tests/btrfs/236
>  create mode 100644 tests/btrfs/236.out
>
> diff --git a/common/config b/common/config
> index a47e462c7792..1a26934985dd 100644
> --- a/common/config
> +++ b/common/config
> @@ -226,6 +226,7 @@ export FSVERITY_PROG="$(type -P fsverity)"
>  export OPENSSL_PROG="$(type -P openssl)"
>  export ACCTON_PROG="$(type -P accton)"
>  export E2IMAGE_PROG="$(type -P e2image)"
> +export BLKZONE_PROG="$(type -P blkzone)"
>
>  # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
>  # newer systems have udevadm command but older systems like RHEL5 don't.
> diff --git a/tests/btrfs/236 b/tests/btrfs/236
> new file mode 100755
> index 000000000000..3be74196ec5d
> --- /dev/null
> +++ b/tests/btrfs/236
> @@ -0,0 +1,103 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Western Digital Corporation.  All Rights Reserved.
> +#
> +# FS QA Test 236
> +#
> +# Test that zone autoreclaim works as expected, that is: if the dirty
> +# threashold is exceeded the data gets relocated to new block group and the

threashold -> threshold

> +# old block group gets deleted. On block group deletion, the underlying device
> +# zone also needs to be reset.
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1       # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +       cd /
> +       rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +_supported_fs btrfs
> +_require_scratch
> +_require_btrfs_command inspect-internal dump-tree
> +_require_btrfs_command filesystem sync
> +_require_command "$BLKZONE_PROG" blkzone
> +_require_zoned_device "$SCRATCH_DEV"
> +
> +get_data_bg()
> +{
> +       $BTRFS_UTIL_PROG inspect-internal dump-tree -t CHUNK $SCRATCH_DEV |\
> +               grep -A 1 "CHUNK_ITEM" | grep -B 1 "type DATA" |\
> +               grep -Eo "CHUNK_ITEM [[:digit:]]+" | cut -d ' ' -f 2
> +}
> +
> +zonesize=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/chunk_sectors)
> +zonesize=$((zonesize << 9))
> +
> +_scratch_mkfs >/dev/null 2>&1
> +_scratch_mount -o commit=5 # 5s commit time to speed up test

Why not 1s? Would even be faster.

> +
> +uuid=$(findmnt -n -o UUID "$SCRATCH_MNT")
> +reclaim_threshold=75
> +echo $reclaim_threshold > /sys/fs/btrfs/"$uuid"/bg_reclaim_threshold
> +fill_percent=$((reclaim_threshold + 2))
> +rest_percent=$((90 - fill_percent)) # make sure we're not creating a new BG
> +fill_size=$((zonesize * fill_percent / 100))
> +rest=$((zonesize * rest_percent / 100))
> +
> +# step 1, fill FS over $fillsize
> +$XFS_IO_PROG -fdc "pwrite -W 0 $fill_size" $SCRATCH_MNT/$seq.test1 >> $seqres.full
> +$XFS_IO_PROG -fdc "pwrite -W 0 $rest" $SCRATCH_MNT/$seq.test2 >> $seqres.full
> +sleep 5 # need to make sure the transaction got committed

I don't get this.
Why not just call 'sync'? That commits the transaction too.
Doing regular buffered writes followed by sync is more intuitive and faster.

If for some reason direct IO is really needed, it also misses a
_require_odirect.

> +
> +zones_before=$($BLKZONE_PROG report $SCRATCH_DEV | grep -v -e em -e nw | wc -l)
> +echo "Before reclaim: $zones_before zones open" >> $seqres.full
> +old_data_zone=$(get_data_bg)
> +old_data_zone=$((old_data_zone >> 9))
> +printf "Old data zone 0x%x\n" $old_data_zone >> $seqres.full
> +
> +# step 2, delete the 1st $fill_size sized file to trigger reclaim
> +rm $SCRATCH_MNT/$seq.test1
> +$BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
> +sleep 10 # 1 transaction commit for 'rm' and one for balance
> +
> +# check that we don't have more zones open than before
> +zones_after=$($BLKZONE_PROG report $SCRATCH_DEV | grep -v -e em -e nw | wc -l)
> +echo "After reclaim: $zones_after zones open" >> $seqres.full
> +
> +# Check that old data zone was reset
> +old_wptr=$($BLKZONE_PROG report -o $old_data_zone -c 1 $SCRATCH_DEV |\
> +       grep -Eo "wptr 0x[[:xdigit:]]+" | cut -d ' ' -f 2)
> +if [ "$old_wptr" != "0x000000" ]; then
> +       _fail "Old wptr still at $old_wptr"
> +fi
> +
> +new_data_zone=$(get_data_bg)
> +new_data_zone=$((new_data_zone >> 9))
> +printf "New data zone 0x%x\n" $new_data_zone >> $seqres.full
> +
> +# Check that data was really relocated to a different zone
> +if [ $old_data_zone -eq $new_data_zone ]; then
> +       _fail "New zone same as old zone"

The use of _fail is usually discouraged. A simple echo would suffice here.

Other than that, it looks good, but I don't have an environment to
test zoned devices, so it's just eyeballing.

Thanks.

> +fi
> +
> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/btrfs/236.out b/tests/btrfs/236.out
> new file mode 100644
> index 000000000000..b6b6e0cad9a7
> --- /dev/null
> +++ b/tests/btrfs/236.out
> @@ -0,0 +1,2 @@
> +QA output created by 236
> +Silence is golden
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index 331dd432fac3..62c9c761e974 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -238,3 +238,4 @@
>  233 auto quick subvolume
>  234 auto quick compress rw
>  235 auto quick send
> +236 auto quick balance
> --
> 2.31.1
>
Johannes Thumshirn April 29, 2021, 9:44 a.m. UTC | #2 
On 28/04/2021 11:24, Filipe Manana wrote:
> The use of _fail is usually discouraged. A simple echo would suffice here.


Need to do echo + exit here, otherwise there will be errors in the shell script.

Everything else worked in.

Thanks a lot,
	Johannes
Filipe Manana April 29, 2021, 9:56 a.m. UTC | #3 
On Thu, Apr 29, 2021 at 10:44 AM Johannes Thumshirn
<Johannes.Thumshirn@wdc.com> wrote:
>
> On 28/04/2021 11:24, Filipe Manana wrote:
> > The use of _fail is usually discouraged. A simple echo would suffice here.
>
>
> Need to do echo + exit here, otherwise there will be errors in the shell script.

Errors? I don't understand, what kind of errors?

Thanks.
>
> Everything else worked in.
>
> Thanks a lot,
>         Johannes
Johannes Thumshirn April 29, 2021, 10:05 a.m. UTC | #4 
On 29/04/2021 11:56, Filipe Manana wrote:
> On Thu, Apr 29, 2021 at 10:44 AM Johannes Thumshirn
> <Johannes.Thumshirn@wdc.com> wrote:
>>
>> On 28/04/2021 11:24, Filipe Manana wrote:
>>> The use of _fail is usually discouraged. A simple echo would suffice here.
>>
>>
>> Need to do echo + exit here, otherwise there will be errors in the shell script.
> 
> Errors? I don't understand, what kind of errors?


For instance if the block group isn't removed, the dump-tree call will return two
block groups and then this calculation will fail:
new_data_zone=$(get_data_bg)
new_data_zone=$((new_data_zone >> 9))
Filipe Manana April 29, 2021, 10:06 a.m. UTC | #5 
On Thu, Apr 29, 2021 at 11:05 AM Johannes Thumshirn
<Johannes.Thumshirn@wdc.com> wrote:
>
> On 29/04/2021 11:56, Filipe Manana wrote:
> > On Thu, Apr 29, 2021 at 10:44 AM Johannes Thumshirn
> > <Johannes.Thumshirn@wdc.com> wrote:
> >>
> >> On 28/04/2021 11:24, Filipe Manana wrote:
> >>> The use of _fail is usually discouraged. A simple echo would suffice here.
> >>
> >>
> >> Need to do echo + exit here, otherwise there will be errors in the shell script.
> >
> > Errors? I don't understand, what kind of errors?
>
>
> For instance if the block group isn't removed, the dump-tree call will return two
> block groups and then this calculation will fail:
> new_data_zone=$(get_data_bg)
> new_data_zone=$((new_data_zone >> 9))

Ok, I now see it, I was only noticing the very last _fail, which is
not problematic. Sounds good then.

Thanks.
diff mbox series

Patch

diff --git a/common/config b/common/config
index a47e462c7792..1a26934985dd 100644
--- a/common/config
+++ b/common/config
@@ -226,6 +226,7 @@  export FSVERITY_PROG="$(type -P fsverity)"
 export OPENSSL_PROG="$(type -P openssl)"
 export ACCTON_PROG="$(type -P accton)"
 export E2IMAGE_PROG="$(type -P e2image)"
+export BLKZONE_PROG="$(type -P blkzone)"
 
 # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
 # newer systems have udevadm command but older systems like RHEL5 don't.
diff --git a/tests/btrfs/236 b/tests/btrfs/236
new file mode 100755
index 000000000000..3be74196ec5d
--- /dev/null
+++ b/tests/btrfs/236
@@ -0,0 +1,103 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Western Digital Corporation.  All Rights Reserved.
+#
+# FS QA Test 236
+#
+# Test that zone autoreclaim works as expected, that is: if the dirty
+# threashold is exceeded the data gets relocated to new block group and the
+# old block group gets deleted. On block group deletion, the underlying device
+# zone also needs to be reset.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+_supported_fs btrfs
+_require_scratch
+_require_btrfs_command inspect-internal dump-tree
+_require_btrfs_command filesystem sync
+_require_command "$BLKZONE_PROG" blkzone
+_require_zoned_device "$SCRATCH_DEV"
+
+get_data_bg()
+{
+	$BTRFS_UTIL_PROG inspect-internal dump-tree -t CHUNK $SCRATCH_DEV |\
+		grep -A 1 "CHUNK_ITEM" | grep -B 1 "type DATA" |\
+		grep -Eo "CHUNK_ITEM [[:digit:]]+" | cut -d ' ' -f 2
+}
+
+zonesize=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/chunk_sectors)
+zonesize=$((zonesize << 9))
+
+_scratch_mkfs >/dev/null 2>&1
+_scratch_mount -o commit=5 # 5s commit time to speed up test
+
+uuid=$(findmnt -n -o UUID "$SCRATCH_MNT")
+reclaim_threshold=75
+echo $reclaim_threshold > /sys/fs/btrfs/"$uuid"/bg_reclaim_threshold
+fill_percent=$((reclaim_threshold + 2))
+rest_percent=$((90 - fill_percent)) # make sure we're not creating a new BG
+fill_size=$((zonesize * fill_percent / 100))
+rest=$((zonesize * rest_percent / 100))
+
+# step 1, fill FS over $fillsize
+$XFS_IO_PROG -fdc "pwrite -W 0 $fill_size" $SCRATCH_MNT/$seq.test1 >> $seqres.full
+$XFS_IO_PROG -fdc "pwrite -W 0 $rest" $SCRATCH_MNT/$seq.test2 >> $seqres.full
+sleep 5 # need to make sure the transaction got committed
+
+zones_before=$($BLKZONE_PROG report $SCRATCH_DEV | grep -v -e em -e nw | wc -l)
+echo "Before reclaim: $zones_before zones open" >> $seqres.full
+old_data_zone=$(get_data_bg)
+old_data_zone=$((old_data_zone >> 9))
+printf "Old data zone 0x%x\n" $old_data_zone >> $seqres.full
+
+# step 2, delete the 1st $fill_size sized file to trigger reclaim
+rm $SCRATCH_MNT/$seq.test1
+$BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
+sleep 10 # 1 transaction commit for 'rm' and one for balance
+
+# check that we don't have more zones open than before
+zones_after=$($BLKZONE_PROG report $SCRATCH_DEV | grep -v -e em -e nw | wc -l)
+echo "After reclaim: $zones_after zones open" >> $seqres.full
+
+# Check that old data zone was reset
+old_wptr=$($BLKZONE_PROG report -o $old_data_zone -c 1 $SCRATCH_DEV |\
+	grep -Eo "wptr 0x[[:xdigit:]]+" | cut -d ' ' -f 2)
+if [ "$old_wptr" != "0x000000" ]; then
+	_fail "Old wptr still at $old_wptr"
+fi
+
+new_data_zone=$(get_data_bg)
+new_data_zone=$((new_data_zone >> 9))
+printf "New data zone 0x%x\n" $new_data_zone >> $seqres.full
+
+# Check that data was really relocated to a different zone
+if [ $old_data_zone -eq $new_data_zone ]; then
+	_fail "New zone same as old zone"
+fi
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/btrfs/236.out b/tests/btrfs/236.out
new file mode 100644
index 000000000000..b6b6e0cad9a7
--- /dev/null
+++ b/tests/btrfs/236.out
@@ -0,0 +1,2 @@ 
+QA output created by 236
+Silence is golden
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 331dd432fac3..62c9c761e974 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -238,3 +238,4 @@ 
 233 auto quick subvolume
 234 auto quick compress rw
 235 auto quick send
+236 auto quick balance