Re: [PATCH 2/2] btrfs: check for commit error at btrfs_attach_transaction_barrier()

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2023/7/21 17:49, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> btrfs_attach_transaction_barrier() is used to get a handle pointing to the
> current running transaction if the transaction has not started its commit
> yet (its state is < TRANS_STATE_COMMIT_START). If the transaction commit
> has started, then we wait for the transaction to commit and finish before
> returning - however we completely ignore if the transaction was aborted
> due to some error during its commit, we simply return ERR_PT(-ENOENT),
> which makes the caller assume everything is fine and no errors happened.
>
> This could make an fsync return success (0) to user space when in fact we
> had a transaction abort and the target inode changes were therefore not
> persisted.
>
> Fix this by checking for the return value from btrfs_wait_for_commit(),
> and if it returned an error, return it back to the caller.
>
> Fixes: d4edf39bd5db ("Btrfs: fix uncompleted transaction")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Thanks,
Qu
> ---
>   fs/btrfs/transaction.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 8ab85465cdaa..4bb9716ad24a 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -826,8 +826,13 @@ btrfs_attach_transaction_barrier(struct btrfs_root *root)
>
>   	trans = start_transaction(root, 0, TRANS_ATTACH,
>   				  BTRFS_RESERVE_NO_FLUSH, true);
> -	if (trans == ERR_PTR(-ENOENT))
> -		btrfs_wait_for_commit(root->fs_info, 0);
> +	if (trans == ERR_PTR(-ENOENT)) {
> +		int ret;
> +
> +		ret = btrfs_wait_for_commit(root->fs_info, 0);
> +		if (ret)
> +			return ERR_PTR(ret);
> +	}
>
>   	return trans;
>   }