[PATCH 0/3] btrfs: small fixes related to qgroup deletion

[PATCH 0/3] btrfs: small fixes related to qgroup deletion

[Qu Wenruo]

There is a recent bug report that when deleting a qgroup that is not
fully exclusively owned, it can make all future qgroup operations to
fail.

It turns out to be a problem handling "ret > 0" cases when deleting a
qgroup.
The first patch is the root fix.

The seconds patch is to improve the error reporting of
quick_update_accounting().

Finally there is a pre-existing problem exposed by that "ret > 0"
handling, that if we use btrfs_remove_qgroup() on a qgroup that only
exists in the rb-tree, it will return -ENOENT meanwhile still deletes
the in-memory qgroup.

Fix that -ENOENT leak in the last patch.

Qu Wenruo (3):
  btrfs: qgroup: do not treat "ret > 0" as error when deleting a qgroup
  btrfs: qgroup: use qgroup_mark_inconsistent() in
    quick_update_accounting()
  btrfs: qgroup: do not leak -ENOENT from btrfs_remove_qgroup()

 fs/btrfs/qgroup.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

-- 
2.55.0

[PATCH 1/3] btrfs: qgroup: do not treat "ret > 0" as error when deleting a qgroup

[Qu Wenruo]

[BUG]
There is a bug report that when removing a child qgroup, if removing the
qgroup requires a rescan, it will delete the on-disk relation and qgroup
items but not the in-memory qgroup rb entry.

This makes all later qgroup rescan to fail.

[CAUSE]
The call chain looks like the following:

 btrfs_remove_qgroup()
 |- __del_qgroup_relation()
 |  |- quick_update_accounting()
 |     Which returns 1, showing this operation will require
 |     a qgroup rescan
 |- if (ret) goto out;
    Which skips the del_qgroup_rb() call, leaving the in-memory qgroup
    rb entry still intact.

Then later qgroup rescan/update will always fail and mark qgroup
inconsistent:

 btrfs_run_qgroups()
 |- update_qgroup_info_item()
 |  |- btrfs_search_slot()
 |     Which returns -ENOENT, as the qgroup info item is already
 |     deleted.
 |- qgroup_mark_inconsistent()
 |- return -ENOENT;

[FIX]
Do not treat "ret > 0" as an error.

Reported-by: c.f.a@posteo.de
Link: https://lore.kernel.org/linux-btrfs/14846642-85d3-4afb-ad99-1b74b22f5f90@posteo.de/
Fixes: 9c8b35b1ba21 ("btrfs: quota: Automatically update related qgroups or mark INCONSISTENT flags when assigning/deleting a qgroup relations.")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/qgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index bf06962d49f3..dfe3827ec369 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1853,7 +1853,7 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
 					struct btrfs_qgroup_list, next_group);
 		ret = __del_qgroup_relation(trans, qgroupid,
 					    list->group->qgroupid);
-		if (ret)
+		if (ret < 0)
 			goto out;
 	}
 
-- 
2.55.0

[PATCH 2/3] btrfs: qgroup: use qgroup_mark_inconsistent() in quick_update_accounting()

[Qu Wenruo]

When quick_update_accounting() fails to handle a qgroup which is not
fully exclusive (its exclusive number doesn't match the reference one),
we have to mark the qgroup inconsistent.

Unlike other locations we didn't use qgroup_mark_inconsistent(), thus
when such problem happens no extra error message is shown.

Use the helper in quick_update_accounting(), which does not only show
the reason of marking qgroup inconsistent, but also cancels any running
rescan and skips dirty accounting, avoiding wasting CPU time on already
inconsistent qgroup numbers.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/qgroup.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index dfe3827ec369..d3401a6df530 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1562,7 +1562,10 @@ static int quick_update_accounting(struct btrfs_fs_info *fs_info,
 	}
 out:
 	if (ret)
-		set_bit(BTRFS_QGROUP_STATUS_BIT_INCONSISTENT, &fs_info->qgroup_flags);
+		qgroup_mark_inconsistent(fs_info,
+	"unable to do quick excl updating for qgroup %hu/%llu, ret=%d",
+					 btrfs_qgroup_level(src),
+					 btrfs_qgroup_subvolid(src), ret);
 	return ret;
 }
 
-- 
2.55.0

[PATCH 3/3] btrfs: qgroup: do not leak -ENOENT from btrfs_remove_qgroup()

[Qu Wenruo]

If btrfs_remove_qgroup() is called on a qgroup that only exists in the
rb-tree, then del_qgroup_item() will return -ENOENT, and we tolerate
that -ENOENT.

However if the to-be-deleted qgroup has no parent qgroups, the @ret will
not be updated, and will leak to user space.

It will make the ioctl() caller incorrectly assume the operation failed.

Fix it by resetting @ret to 0 after a successful or tolerated
del_qgroup_item() call, so the -ENOENT from del_qgroup_item() won't be
leaked.

Assisted-by: LLM (exposing this pre-existing problem)
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/qgroup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index d3401a6df530..46de0a5e80d5 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1850,6 +1850,7 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
 	ret = del_qgroup_item(trans, qgroupid);
 	if (ret && ret != -ENOENT)
 		goto out;
+	ret = 0;
 
 	while (!list_empty(&qgroup->groups)) {
 		list = list_first_entry(&qgroup->groups,
-- 
2.55.0

Re: [PATCH 0/3] btrfs: small fixes related to qgroup deletion

[Filipe Manana]

On Thu, Oct 1, 2026 at 6:17 AM Qu Wenruo <wqu@suse.com> wrote:
>
> There is a recent bug report that when deleting a qgroup that is not
> fully exclusively owned, it can make all future qgroup operations to
> fail.
>
> It turns out to be a problem handling "ret > 0" cases when deleting a
> qgroup.
> The first patch is the root fix.
>
> The seconds patch is to improve the error reporting of
> quick_update_accounting().
>
> Finally there is a pre-existing problem exposed by that "ret > 0"
> handling, that if we use btrfs_remove_qgroup() on a qgroup that only
> exists in the rb-tree, it will return -ENOENT meanwhile still deletes
> the in-memory qgroup.
>
> Fix that -ENOENT leak in the last patch.
>
> Qu Wenruo (3):
>   btrfs: qgroup: do not treat "ret > 0" as error when deleting a qgroup
>   btrfs: qgroup: use qgroup_mark_inconsistent() in
>     quick_update_accounting()
>   btrfs: qgroup: do not leak -ENOENT from btrfs_remove_qgroup()

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good, thanks.

>
>  fs/btrfs/qgroup.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> --
> 2.55.0
>
>