Re: [PATCH 4/7] btrfs: convert PREALLOC to PERTRANS after record_root_in_trans

[Qu Wenruo <wqu@suse.com>]

在 2024/3/27 08:09, Boris Burkov 写道:
> The transaction is only able to free PERTRANS reservations for a root
> once that root has been recorded with the TRANS tag on the roots radix
> tree. Therefore, until we are sure that this root will get tagged, it
> isn't safe to convert. Generally, this is not an issue as *some*
> transaction will likely tag the root before long and this reservation
> will get freed in that transaction, but technically it could stick
> around until unmount and result in a warning about leaked metadata
> reservation space.
> 
> This path is most exercised by running the generic/269 xfstest with
> CONFIG_BTRFS_DEBUG.
> 
> Fixes: a6496849671a ("btrfs: fix start transaction qgroup rsv double free")

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

Thanks,
Qu
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
>   fs/btrfs/transaction.c | 17 ++++++++---------
>   1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index feffff91c6fe..1c449d1cea1b 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -745,14 +745,6 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
>   		h->reloc_reserved = reloc_reserved;
>   	}
>   
> -	/*
> -	 * Now that we have found a transaction to be a part of, convert the
> -	 * qgroup reservation from prealloc to pertrans. A different transaction
> -	 * can't race in and free our pertrans out from under us.
> -	 */
> -	if (qgroup_reserved)
> -		btrfs_qgroup_convert_reserved_meta(root, qgroup_reserved);
> -
>   got_it:
>   	if (!current->journal_info)
>   		current->journal_info = h;
> @@ -786,8 +778,15 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
>   		 * not just freed.
>   		 */
>   		btrfs_end_transaction(h);
> -		return ERR_PTR(ret);
> +		goto reserve_fail;
>   	}
> +	/*
> +	 * Now that we have found a transaction to be a part of, convert the
> +	 * qgroup reservation from prealloc to pertrans. A different transaction
> +	 * can't race in and free our pertrans out from under us.
> +	 */
> +	if (qgroup_reserved)
> +		btrfs_qgroup_convert_reserved_meta(root, qgroup_reserved);
>   
>   	return h;
>