Re: [PATCH 2/2] btrfs: test qgroup deletion races with squota writes

[Anand Jain <anajain.sg@gmail.com>]

It appears that transaction commit could race with the testcase.


--------
SECTION       -- gen
FSTYP         -- btrfs
PLATFORM      -- Linux/x86_64 vm1 7.1.0-rc3-gf5d9f327e4b2 #14 SMP
PREEMPT_DYNAMIC Sun May 17 21:25:06 +08 2026
MKFS_OPTIONS  -- -f /dev/vde
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/vde
/mnt/scratch

btrfs/348    10s ... - output mismatch (see
/Volumes/work/ws/fstests/results//gen/btrfs/348.out.bad)
    --- tests/btrfs/348.out	2026-05-18 14:04:31.427635000 +0800
    +++ /Volumes/work/ws/fstests/results//gen/btrfs/348.out.bad
2026-05-18 17:13:33.000000000 +0800
    @@ -5,6 +5,4 @@
     ERROR: unable to destroy quota group: Device or resource busy
     ERROR: unable to create quota group: Invalid argument
     same txn destroy race
    -ERROR: unable to destroy quota group: Device or resource busy
     pre-existing data destroy
    -ERROR: unable to destroy quota group: Device or resource busy
    ...
    (Run 'diff -u /Volumes/work/ws/fstests/tests/btrfs/348.out
/Volumes/work/ws/fstests/results//gen/btrfs/348.out.bad'  to see the
entire di)

HINT: You _MAY_ be missing kernel fix:
      XXXXXXXXXXXX btrfs: check for subvolume before deleting squota qgroup

HINT: You _MAY_ be missing kernel fix:
      0c309d66dacd btrfs: forbid creating subvol qgroups

Ran: btrfs/348
Failures: btrfs/348
Failed 1 of 1 tests


Thanks, Anand







On 13/5/26 08:43, Boris Burkov wrote:
> When using simple quotas, an extent's EXTENT_OWNER_REF can long outlive
> the subvolume that created it, since the extent can pick up additional
> references that keep it alive after the owning subvolume is deleted.
> 
> Several lifecycle bugs around the owning qgroup arise from this:
> 
>   1. Freeing an extent whose owner qgroup is gone: must not cause an
>      underflow nor a transaction abort.
> 
>   2. Creating an extent in the same transaction that the owner subvolume
>      is deleted: the qgroup may already be gone by the time the squota
>      delta is recorded, leaving behind an EXTENT_OWNER_REF that points
>      at a qgroup that never accumulated a usage delta.  Manually
>      re-creating that qgroup and then freeing the extent would underflow
>      it.
> 
>   3. Destroying a live subvolume's qgroup while delayed refs for newly
>      allocated tree blocks have not yet flushed: rfer/excl read as 0,
>      so the destroy looked safe, but the alloc delta was silently lost.
> 
>   4. Destroying the qgroup of a live subvolume whose data predates
>      'btrfs quota enable --simple': pre-existing extents are not
>      accounted (generation < qgroup_enable_gen), so rfer/excl stay 0
>      permanently and a usage-only check happily destroys a qgroup
>      belonging to a still-mounted, still-writeable subvolume.
> 
> Add four cases covering these scenarios.  On a fixed kernel all four
> 'qgroup destroy' (and the re-create in case 2) operations are refused
> because the subvol check rejects the request; on an unfixed kernel
> they would succeed and leave the filesystem with broken accounting or
> trigger a transaction abort.
> 
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
>  tests/btrfs/348     | 136 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/348.out |  10 ++++
>  2 files changed, 146 insertions(+)
>  create mode 100755 tests/btrfs/348
>  create mode 100644 tests/btrfs/348.out
> 
> diff --git a/tests/btrfs/348 b/tests/btrfs/348
> new file mode 100755
> index 00000000..ab816301
> --- /dev/null
> +++ b/tests/btrfs/348
> @@ -0,0 +1,136 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Meta Platforms, Inc.  All Rights Reserved.
> +#
> +# FS QA Test 348
> +#
> +# Test various race conditions between qgroup deletion and squota writes
> +#
> +. ./common/preamble
> +_begin_fstest auto qgroup subvol clone
> +
> +# Import common functions.
> +. ./common/reflink
> +
> +# real QA test starts here
> +
> +_require_scratch_reflink
> +_require_cp_reflink
> +_require_scratch_enable_simple_quota
> +
> +_fixed_by_kernel_commit XXXXXXXXXXXX \
> +	"btrfs: check for subvolume before deleting squota qgroup"
> +_fixed_by_kernel_commit 0c309d66dacd \
> +	"btrfs: forbid creating subvol qgroups"
> +
> +subv1=$SCRATCH_MNT/subv1
> +subv2=$SCRATCH_MNT/subv2
> +
> +prepare()
> +{
> +	_scratch_mkfs >> $seqres.full
> +	_scratch_mount
> +	$BTRFS_UTIL_PROG quota enable --simple $SCRATCH_MNT
> +	$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.
> +free_from_deleted_owner()
> +{
> +	echo "free from deleted owner"
> +	prepare
> +	local 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
> +	local 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
> +	$BTRFS_UTIL_PROG qgroup create 0/$subvid1 $SCRATCH_MNT >> $seqres.full
> +	rm $subv2/f
> +	_scratch_unmount
> +}
> +
> +# Create a subvol, destroy its qgroup before delayed refs update rfer/excl.
> +# On an unfixed kernel the qgroup destroy succeeds (rfer == 0), silently
> +# losing the alloc delta. On a fixed kernel the destroy is refused because
> +# the ROOT_ITEM still exists.
> +same_txn_destroy_race()
> +{
> +	echo "same txn destroy race"
> +	prepare
> +
> +	# Commit so the next subvol create starts a fresh transaction.
> +	$BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
> +
> +	# New subvol -- tree block alloc delayed ref is pending, rfer/excl=0.
> +	local subv3=$SCRATCH_MNT/subv3
> +	$BTRFS_UTIL_PROG subvolume create $subv3 >> $seqres.full
> +	local subvid3=$(_btrfs_get_subvolid $SCRATCH_MNT subv3)
> +
> +	# rfer/excl still 0 (delayed refs not flushed). Destroy should fail
> +	# because the ROOT_ITEM exists.
> +	$BTRFS_UTIL_PROG qgroup destroy 0/$subvid3 $SCRATCH_MNT 2>&1
> +
> +	_scratch_unmount
> +}
> +
> +# Enable squotas on a filesystem that already has a subvolume with data.
> +# Pre-existing extents have generation < qgroup_enable_gen, so the qgroup's
> +# rfer/excl stay 0 permanently. On an unfixed kernel can_delete_squota_qgroup
> +# only checks rfer/excl, so the qgroup can be destroyed for a live subvol.
> +# On a fixed kernel, the subvol check prevents deletion.
> +pre_existing_data_destroy()
> +{
> +	echo "pre-existing data destroy"
> +	_scratch_mkfs >> $seqres.full
> +	_scratch_mount
> +
> +	# Create subvol and write data BEFORE enabling squotas.
> +	$BTRFS_UTIL_PROG subvolume create $subv1 >> $seqres.full
> +	$XFS_IO_PROG -fc "pwrite -q 0 128K" $subv1/f
> +	sync
> +
> +	# Enable squotas. Pre-existing data is not accounted (gen < enable_gen).
> +	$BTRFS_UTIL_PROG quota enable --simple $SCRATCH_MNT
> +	$BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
> +
> +	local subvid1=$(_btrfs_get_subvolid $SCRATCH_MNT subv1)
> +
> +	# Destroy the qgroup. rfer/excl = 0 because data predates squotas.
> +	# Should fail because the ROOT_ITEM still exists.
> +	$BTRFS_UTIL_PROG qgroup destroy 0/$subvid1 $SCRATCH_MNT 2>&1
> +
> +	_scratch_unmount
> +}
> +
> +free_from_deleted_owner
> +add_to_deleted_owner
> +same_txn_destroy_race
> +pre_existing_data_destroy
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/348.out b/tests/btrfs/348.out
> new file mode 100644
> index 00000000..0a1cf11f
> --- /dev/null
> +++ b/tests/btrfs/348.out
> @@ -0,0 +1,10 @@
> +QA output created by 348
> +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
> +same txn destroy race
> +ERROR: unable to destroy quota group: Device or resource busy
> +pre-existing data destroy
> +ERROR: unable to destroy quota group: Device or resource busy