Re: [PATCH] btrfs: free qgroup configuration when quota enable fails

[Qu Wenruo <wqu@suse.com>]

在 2026/8/14 20:52, Guanjie Chen 写道:
> Quota enable adds qgroups to fs_info->qgroup_tree before the operation is
> guaranteed to succeed.  On failure, the current error path removes only the
> sysfs entries, leaving the qgroups and their relations in memory.
> 
> Use btrfs_free_qgroup_config() instead.  It is the existing qgroup teardown
> helper and removes the in-memory qgroups, their relations and the sysfs
> entries.
> 
> This is safe because the failure cleanup runs with qgroup_ioctl_lock held
> and before quota_root is published or quota accounting is enabled.  Qgroups
> already added to the tree are freed by btrfs_free_qgroup_config(), while an
> untransferred preallocation and quota_root are still released by the
> existing cleanup code.  Transaction handling is unchanged.
> 
> Signed-off-by: Guanjie Chen <jk.chen1095@gmail.com>

LLM disclosure.

In fact, for anyone who doesn't have any previous btrfs involvement, I 
have a strong feeling such error handling bug is fully exposed by LLM, 
not yourself.

> ---
>   fs/btrfs/qgroup.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1292,7 +1292,7 @@ out_free_root:
>   		btrfs_put_root(quota_root);
>   out:
>   	if (ret)
> -		btrfs_sysfs_del_qgroups(fs_info);
> +		btrfs_free_qgroup_config(fs_info);

And the error handling, no matter the older or the newer one, is 
incorrect in the first place.

In fact, the newer one is worse.

We can reach this out branch if btrfs_start_transaction() failed, but 
the fs_info->quota_root is already setup by another process.

In that case, the older code only removes the sysfs files, but keeps the 
qgroups in the rb tree.

But now all qgroups are removed unexpectedly.

>   	mutex_unlock(&fs_info->qgroup_ioctl_lock);
>   	if (ret && trans)
>   		btrfs_end_transaction(trans);