Re: [PATCH] btrfs: extent_io: Handle memory allocation failure in __clear_extent_bit()

[Nikolay Borisov <nborisov@suse.com>]

On 18.04.19 г. 10:21 ч., Qu Wenruo wrote:
> There is a BUG_ON() in __clear_extent_bit() for memory allocation
> failure.
> 
> While comment of __clear_extent_bit() says it can return error, but we
> always return 0.
> 
> Some __clear_extent_bit() callers just ignore the return value, while
> some still expect error.
> 
> Let's return proper error for this memory allocation anyway, to remove
> that BUG_ON() as a first step, so at least we can continue test.

I remember Josef did some changes into this code and said that prealloc
shouldn't fail because this will cause mayhem down the road i.e. proper
error handling is missing. If anything I think it should be added first
and then remove the BUG_ONs.

> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/extent_io.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 828708f6510c..f1bfc632ef7b 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -743,7 +743,10 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>  
>  	if (state->start < start) {
>  		prealloc = alloc_extent_state_atomic(prealloc);
> -		BUG_ON(!prealloc);
> +		if (!prealloc) {
> +			err = -ENOMEM;
> +			goto out;
> +		}
>  		err = split_state(tree, state, prealloc, start);
>  		if (err)
>  			extent_io_tree_panic(tree, err);
> @@ -766,7 +769,10 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>  	 */
>  	if (state->start <= end && state->end > end) {
>  		prealloc = alloc_extent_state_atomic(prealloc);
> -		BUG_ON(!prealloc);
> +		if (!prealloc) {
> +			err = -ENOMEM;
> +			goto out;
> +		}
>  		err = split_state(tree, state, prealloc, end + 1);
>  		if (err)
>  			extent_io_tree_panic(tree, err);
> @@ -801,6 +807,8 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>  	if (prealloc)
>  		free_extent_state(prealloc);
>  
> +	if (err)
> +		return err;
>  	return 0;
>  
>  }
>