Re: [PATCH 3/3] btrfs: do not BUG_ON() on failure to migrate space when replacing extents

[Boris Burkov <boris@bur.io>]

On Mon, Jun 06, 2022 at 10:41:19AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> At btrfs_replace_file_extents(), if we fail to migrate reserved metadata
> space from the transaction block reserve into the local block reserve,
> we trigger a BUG_ON(). This is because it should not be possible to have
> a failure here, as we reserved more space when we started the transaction
> than the space we want to migrate. However having a BUG_ON() is way too
> drastic, we can perfectly handle the failure and return the error to the
> caller. So just do that instead, and add a WARN_ON() to make it easier
> to notice the failure if it evers happens (which is particularly useful
> for fstests, and the warning will trigger a failure of a test case).
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Boris Burkov <boris@bur.io>
> ---
>  fs/btrfs/file.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 29de433b7804..da41a0c371bc 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -2719,7 +2719,8 @@ int btrfs_replace_file_extents(struct btrfs_inode *inode,
>  
>  	ret = btrfs_block_rsv_migrate(&fs_info->trans_block_rsv, rsv,
>  				      min_size, false);
> -	BUG_ON(ret);
> +	if (WARN_ON(ret))
> +		goto out_trans;
>  	trans->block_rsv = rsv;
>  
>  	cur_offset = start;
> @@ -2838,7 +2839,8 @@ int btrfs_replace_file_extents(struct btrfs_inode *inode,
>  
>  		ret = btrfs_block_rsv_migrate(&fs_info->trans_block_rsv,
>  					      rsv, min_size, false);
> -		BUG_ON(ret);	/* shouldn't happen */
> +		if (WARN_ON(ret))
> +			break;
>  		trans->block_rsv = rsv;
>  
>  		cur_offset = drop_args.drop_end;
> -- 
> 2.35.1
>