Re: [PATCH 5/6] btrfs: abort transaction on qgroup failures in create_pending_snapshot()

[Qu Wenruo <wqu@suse.com>]

在 2026/10/3 04:25, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> If we get an error when calling one of the qgroup functions, we jump to
> the 'fail' label without aborting the transaction. This is not a bug as up
> the call chain (transaction commit path), we will end up aborting the
> transaction. However having the explicit transaction abort in
> create_pending_snapshot() allows us to get a stack trace and log message
> that tells us exactly where we failed, useful for troubleshooting, and
> also adds consistency since in that function we abort the transaction in
> every other error path.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Thanks,
Qu

> ---
>   fs/btrfs/transaction.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index da4ffa7bda00..fcbcb4a208be 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -1920,14 +1920,21 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
>   	 * To co-operate with that hack, we do hack again.
>   	 * Or snapshot will be greatly slowed down by a subtree qgroup rescan
>   	 */
> -	if (btrfs_qgroup_mode(fs_info) == BTRFS_QGROUP_MODE_FULL)
> +	if (btrfs_qgroup_mode(fs_info) == BTRFS_QGROUP_MODE_FULL) {
>   		ret = qgroup_account_snapshot(trans, root, parent_root,
>   					      pending->inherit, objectid);
> -	else if (btrfs_qgroup_mode(fs_info) == BTRFS_QGROUP_MODE_SIMPLE)
> +		if (unlikely(ret < 0)) {
> +			btrfs_abort_transaction(trans, ret);
> +			goto fail;
> +		}
> +	} else if (btrfs_qgroup_mode(fs_info) == BTRFS_QGROUP_MODE_SIMPLE) {
>   		ret = btrfs_qgroup_inherit(trans, btrfs_root_id(root), objectid,
>   					   btrfs_root_id(parent_root), pending->inherit);
> -	if (unlikely(ret < 0))
> -		goto fail;
> +		if (unlikely(ret < 0)) {
> +			btrfs_abort_transaction(trans, ret);
> +			goto fail;
> +		}
> +	}
>   
>   	ret = btrfs_insert_dir_item(trans, &fname.disk_name,
>   				    parent_inode, &key, BTRFS_FT_DIR, index, NULL);