在 2024/2/24 03:37, Anand Jain 写道:
On 2/23/24 15:05, Qu Wenruo 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>
Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>
looks good.
Reviewed-by: Anand Jain <anand.jain@xxxxxxxxxx>
Thanks for the review.
Queued for the upcoming pull request.
So for the btrfs fstests part, you'll do the pull request to upstream
fstests, right?
In that case, if we have some conflicting test case number, how do we
resolve it?
It would be resolved by you or the author would be notified and need a
resend?
And do we have a branch to base our new test cases upon? (Mostly to
avoid the number conflicting)
Thanks,
Qu
Thanks, Anand
---
tests/btrfs/303 | 59 +++++++++++++++++++++++++++++++++++++++++++++
tests/btrfs/303.out | 2 ++
2 files changed, 61 insertions(+)
create mode 100755 tests/btrfs/303
create mode 100644 tests/btrfs/303.out
---
Changelog:
v2:
- Fix various spelling errors
- Remove a copied _fixed_by_kernel_commit line
Which was used to align the number of 'x', but forgot to remove
diff --git a/tests/btrfs/303 b/tests/btrfs/303
new file mode 100755
index 00000000..9f7605ab
--- /dev/null
+++ b/tests/btrfs/303
@@ -0,0 +1,59 @@
+#! /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 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 belongs 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
+# inconsistent and let users to determine when to do a full rescan
+$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 warning with
+# 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
