From: Boris Burkov <boris@bur.io>
To: linux-btrfs@vger.kernel.org, kernel-team@fb.com, fstests@vger.kernel.org
Subject: [PATCH] btrfs/304: test qgroup deletion
Date: Mon, 22 Jan 2024 15:06:28 -0800 [thread overview]
Message-ID: <c9fa8fa558e307a5d0d28545ff69433ae8324f4c.1705964751.git.boris@bur.io> (raw)
When using squotas, an extent's OWNER_REF can long outlive the subvolume
that is the owner, since it could pick up a different reference that
keeps it around, but the subvolume can go away.
Test this case, as originally, it resulted in a read only btrfs.
Since we can blow up the subvolume in the same transaction as the extent
is written, we can also increment the usage of a non-existent subvolume.
This leaves an OWNER_REF behind with no corresponding incremented usage
in a qgroup, so if we re-create that qgroup, we can then underflow its
usage.
Both of these cases are fixed in the kernel by disallowing
creating subvol qgroups and by disallowing deleting qgroups that still
have usage.
Signed-off-by: Boris Burkov <boris@bur.io>
---
common/btrfs | 10 +++++
tests/btrfs/301 | 14 +------
tests/btrfs/304 | 90 +++++++++++++++++++++++++++++++++++++++++++++
tests/btrfs/304.out | 6 +++
4 files changed, 108 insertions(+), 12 deletions(-)
create mode 100755 tests/btrfs/304
create mode 100644 tests/btrfs/304.out
diff --git a/common/btrfs b/common/btrfs
index f91f8dd86..c8593c1f9 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -775,3 +775,13 @@ _has_btrfs_sysfs_feature_attr()
test -e /sys/fs/btrfs/features/$feature_attr
}
+
+_enable_quota()
+{
+ local mode=$1
+
+ [ $mode == "n" ] && return
+ arg=$([ $mode == "s" ] && echo "--simple")
+
+ $BTRFS_UTIL_PROG quota enable $arg $SCRATCH_MNT
+}
diff --git a/tests/btrfs/301 b/tests/btrfs/301
index db4697247..b3ee66cd9 100755
--- a/tests/btrfs/301
+++ b/tests/btrfs/301
@@ -157,16 +157,6 @@ do_enospc_falloc()
do_falloc $file $sz
}
-enable_quota()
-{
- local mode=$1
-
- [ $mode == "n" ] && return
- arg=$([ $mode == "s" ] && echo "--simple")
-
- $BTRFS_UTIL_PROG quota enable $arg $SCRATCH_MNT
-}
-
get_subvid()
{
_btrfs_get_subvolid $SCRATCH_MNT subv
@@ -186,7 +176,7 @@ prepare()
{
_scratch_mkfs >> $seqres.full
_scratch_mount
- enable_quota "s"
+ _enable_quota "s"
$BTRFS_UTIL_PROG subvolume create $subv >> $seqres.full
local subvid=$(get_subvid)
set_subvol_limit $subvid $limit
@@ -397,7 +387,7 @@ enable_mature()
# Sync before enabling squotas to reliably *not* count the writes
# we did before enabling.
sync
- enable_quota "s"
+ _enable_quota "s"
set_subvol_limit $subvid $limit
_scratch_cycle_mount
usage=$(get_subvol_usage $subvid)
diff --git a/tests/btrfs/304 b/tests/btrfs/304
new file mode 100755
index 000000000..3fce0591c
--- /dev/null
+++ b/tests/btrfs/304
@@ -0,0 +1,90 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Meta Platforms, Inc. All Rights Reserved.
+#
+# FS QA Test 304
+#
+# Test various race conditions between qgroup deletion and squota writes
+#
+. ./common/preamble
+_begin_fstest auto quick qgroup subvol clone
+
+# Override the default cleanup function.
+# _cleanup()
+# {
+# cd /
+# rm -r -f $tmp.*
+# }
+
+# Import common functions.
+. ./common/reflink
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs btrfs
+_require_scratch_reflink
+_require_cp_reflink
+_require_scratch_enable_simple_quota
+_require_no_compress
+
+_fixed_by_kernel_commit xxxxxxxxxxxx "btrfs: forbid deleting live subvol qgroup"
+_fixed_by_kernel_commit xxxxxxxxxxxx "btrfs: forbid creating subvol qgroups"
+
+subv1=$SCRATCH_MNT/subv1
+subv2=$SCRATCH_MNT/subv2
+
+prepare()
+{
+ _scratch_mkfs >> $seqres.full
+ _scratch_mount
+ _enable_quota "s"
+ $BTRFS_UTIL_PROG subvolume create $subv1 >> $seqres.full
+ $BTRFS_UTIL_PROG subvolume create $subv2 >> $seqres.full
+ $XFS_IO_PROG -fc "pwrite -q 0 128K" $subv1/f
+ _cp_reflink $subv1/f $subv2/f
+}
+
+# An extent can long outlive its owner. Test this by deleting the owning
+# subvolume, committing the transaction, then deleting the reflinked copy.
+# Deleting the copy will attempt to free space from the missing owner, which
+# should be a no-op.
+free_from_deleted_owner()
+{
+ echo "free from deleted owner"
+ prepare
+ subvid1=$(_btrfs_get_subvolid $SCRATCH_MNT subv1)
+
+ $BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
+ $BTRFS_UTIL_PROG subvolume delete $subv1 >> $seqres.full
+ $BTRFS_UTIL_PROG qgroup destroy 0/$subvid1 $SCRATCH_MNT >> $seqres.full
+ $BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
+ rm $subv2/f
+ _scratch_unmount
+}
+
+# A race where we delete the owner in the same transaction as writing the
+# extent leads to incrementing the squota usage of the missing qgroup.
+# This leaves behind an owner ref with an owner id that cannot exist, so
+# freeing the extent now frees from that qgroup, but there has never
+# been a corresponding usage to free.
+add_to_deleted_owner()
+{
+ echo "add to deleted owner"
+ prepare
+ subvid1=$(_btrfs_get_subvolid $SCRATCH_MNT subv1)
+
+ $BTRFS_UTIL_PROG subvolume delete $subv1 >> $seqres.full
+ $BTRFS_UTIL_PROG qgroup destroy 0/$subvid1 $SCRATCH_MNT >> $seqres.full
+ $BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
+ $BTRFS_UTIL_PROG qgroup create 0/$subvid1 $SCRATCH_MNT >> $seqres.full
+ rm $subv2/f
+ _scratch_unmount
+}
+
+free_from_deleted_owner
+add_to_deleted_owner
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/304.out b/tests/btrfs/304.out
new file mode 100644
index 000000000..55bbc64f5
--- /dev/null
+++ b/tests/btrfs/304.out
@@ -0,0 +1,6 @@
+QA output created by 304
+free from deleted owner
+ERROR: unable to destroy quota group: Device or resource busy
+add to deleted owner
+ERROR: unable to destroy quota group: Device or resource busy
+ERROR: unable to create quota group: Invalid argument
--
2.43.0
next reply other threads:[~2024-01-22 23:05 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-22 23:06 Boris Burkov [this message]
2024-01-23 6:01 ` [PATCH] btrfs/304: test qgroup deletion David Disseldorp
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c9fa8fa558e307a5d0d28545ff69433ae8324f4c.1705964751.git.boris@bur.io \
--to=boris@bur.io \
--cc=fstests@vger.kernel.org \
--cc=kernel-team@fb.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).