Re: [PATCH] fstests: btrfs: add a test case to make sure inconsitent qgroup won't leak reserved data space

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Fri, Feb 23, 2024 at 8:58 AM Qu Wenruo <wqu@xxxxxxxx> wrote:
>
> There is a kernel regression caused by commit e15e9f43c7ca ("btrfs:
> introduce BTRFS_QGROUP_RUNTIME_FLAG_NO_ACCOUNTING to skip qgroup
> accounting"), where if qgroup is inconsistent (not that hard to trigger)
> btrfs would leak its qgroup data reserved space, and cause a warning at
> unmount time.
>
> The test case would verify the behavior by:
>
> - Enable qgroup first
>
> - Intentionally mark qgroup inconsistent
>   This is done by taking a snapshot and assign it to a higher level
>   qgroup, meanwhile the source has no higher level qgroup.
>
> - Trigger a large enough write to cause qgroup data space leak
>
> - Unmount and check the dmesg for the qgroup rsv leak warning
>
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> ---
>  tests/btrfs/303     | 60 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/303.out |  2 ++
>  2 files changed, 62 insertions(+)
>  create mode 100755 tests/btrfs/303
>  create mode 100644 tests/btrfs/303.out
>
> diff --git a/tests/btrfs/303 b/tests/btrfs/303
> new file mode 100755
> index 00000000..44dbaeab
> --- /dev/null
> +++ b/tests/btrfs/303
> @@ -0,0 +1,60 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2024 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FS QA Test 303
> +#
> +# Make sure btrfs qgroup won't leak its reserved data space if qgroup is
> +# marked inconsistent.
> +#
> +# This exercises a regression introduced in v6.1 kernel by the following commit:
> +#
> +# e15e9f43c7ca ("btrfs: introduce BTRFS_QGROUP_RUNTIME_FLAG_NO_ACCOUNTING to skip qgroup accounting")
> +#
> +. ./common/preamble
> +_begin_fstest auto quick qgroup
> +
> +_supported_fs btrfs
> +_require_scratch
> +
> +_fixed_by_kernel_commit eb96e221937a \

Erroneous line here.

> +_fixed_by_kernel_commit xxxxxxxxxxxx \
> +       "btrfs: qgroup: always free reserved space for extent records"
> +
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount
> +
> +$BTRFS_UTIL_PROG quota enable $SCRATCH_MNT
> +$BTRFS_UTIL_PROG quota rescan -w $SCRATCH_MNT >> $seqres.full
> +
> +$BTRFS_UTIL_PROG qgroup create 1/0 $SCRATCH_MNT >> $seqres.full
> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subv1 >> $seqres.full
> +
> +# This would mark qgroup inconsistent, as the snapshot belows to a different
> +# higher level qgroup, we have to do full rescan on both source and snapshot.
> +# This can be very slow for large subvolumes, so btrfs only marks qgroup
> +# inconsitent and let users to determine when to do a full rescan

inconsitent -> inconsistent

> +$BTRFS_UTIL_PROG subvolume snapshot -i 1/0 $SCRATCH_MNT/subv1 $SCRATCH_MNT/snap1 >> $seqres.full
> +
> +# This write would lead to a qgroup extent record holding the reserved 128K.
> +# And for unpatched kernels, the reserved space would not be freed properly
> +# due to qgroup is inconsistent.
> +_pwrite_byte 0xcd 0 128K $SCRATCH_MNT/foobar >> $seqres.full
> +
> +# The qgroup leak detection is only triggered at unmount time.
> +_scratch_unmount
> +
> +# Check the dmesg warning for data rsv leak.
> +#
> +# If CONFIG_BTRFS_DEBUG is enabled, we would have a kernel wanring with

wanring -> warning

Otherwise it looks good, and you can have

Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>

Especially after removing the erroneous _fixed_by_kernel_commit line.
Thanks.

> +# backtrace, but for release builds, it's just a warning line.
> +# So here we manually check the warning message.
> +if _dmesg_since_test_start | grep -q "leak"; then
> +       _fail "qgroup data reserved space leaked"
> +fi
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/303.out b/tests/btrfs/303.out
> new file mode 100644
> index 00000000..d48808e6
> --- /dev/null
> +++ b/tests/btrfs/303.out
> @@ -0,0 +1,2 @@
> +QA output created by 303
> +Silence is golden
> --
> 2.42.0
>
>





[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux