Re: [PATCH 1/3] btrfs: qgroup: set quota enabled bit if quota disable fails flushing reservations

[Qu Wenruo <wqu@suse.com>]

在 2025/7/2 01:12, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Before waiting for the rescan worker to finish and flushing reservations,
> we clear the BTRFS_FS_QUOTA_ENABLED flag from fs_info. If we fail flushing
> reservations we leave with the flag not set which is not correct since
> quotas are still enabled - we must set back the flag on error paths, such
> as when we fail to start a transaction, except for error paths that abort
> a transaction. The reservation flushing happens very early before we do
> any operation that actually disables quotas and before we start a
> transaction, so set back BTRFS_FS_QUOTA_ENABLED if it fails.
> 
> Fixes: af0e2aab3b70 ("btrfs: qgroup: flush reservations during quota disable")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

I initially thought it will be a little racy when setting/clearing the 
ENABLED flag without qgroup_ioctl_mutex, but since we're already holding 
subvol_sem() for both enable and disable, it should be fine.

Thanks,
Qu

> ---
>   fs/btrfs/qgroup.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 42d3cfb84318..eb1bb57dee7d 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1334,11 +1334,14 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info)
>   
>   	/*
>   	 * We have nothing held here and no trans handle, just return the error
> -	 * if there is one.
> +	 * if there is one and set back the quota enabled bit since we didn't
> +	 * actually disable quotas.
>   	 */
>   	ret = flush_reservations(fs_info);
> -	if (ret)
> +	if (ret) {
> +		set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
>   		return ret;
> +	}
>   
>   	/*
>   	 * 1 For the root item