Re: [PATCH] btrfs: handle errors properly in update_inline_extent_backref()

[Josef Bacik <josef@toxicpanda.com>]

On Wed, Aug 09, 2023 at 03:08:21PM +0800, Qu Wenruo wrote:
> [PROBLEM]
> Inside function update_inline_extent_backref(), we have several
> BUG_ON()s along with some ASSERT()s which can be triggered by corrupted
> filesystem.
> 
> [ANAYLYSE]
> Most of those BUG_ON()s and ASSERT()s are just a way of handling
> unexpected on-disk data.
> 
> Although we have tree-checker to rule out obviously incorrect extent
> tree blocks, it's not enough for those ones.
> 
> Thus we need proper error handling for them.
> 
> [FIX]
> Thankfully all the callers of update_inline_extent_backref() would
> eventually handle the errror by aborting the current transaction.
> 
> So this patch would do the proper error handling by:
> 
> - Make update_inline_extent_backref() to return int
>   The return value would be either 0 or -EUCLEAN.
> 
> - Replace BUG_ON()s and ASSERT()s with proper error handling
>   This includes:
>   * Dump the bad extent tree leaf
>   * Output an error message for the cause
>     This would include the extent bytenr, num_bytes (if needed),
>     the bad values and expected good values.
>   * Return -EUCLEAN
> 
>   Note here we remove all the WARN_ON()s, as eventually the transaction
>   would be aborted, thus a backtrace would be triggered anyway.
> 
> - Better comments on why we expect refs == 1 and refs_to_mode == -1 for
>   tree blocks
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef