Re: [PATCH] btrfs: replace BUG_ON() with error return in cache_save_setup()

[Qu Wenruo <wqu@suse.com>]

在 2026/3/28 15:52, Teng Liu 写道:
> In cache_save_setup(), if create_free_space_inode() succeeds but the
> subsequent lookup_free_space_inode() still fails on retry, the
> BUG_ON(retries) will crash the kernel. This can happen due to I/O
> errors or transient failures, not just programming bugs.
> 
> Replace the BUG_ON with proper error handling that returns -EIO through
> the existing cleanup path. The callers already handle this gracefully:
> disk_cache_state defaults to BTRFS_DC_ERROR, so the space cache simply
> won't be written for that block group.
> 
> Signed-off-by: Teng Liu <27rabbitlt@gmail.com>
> ---
>   fs/btrfs/block-group.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index ebf507909..9f71362ba 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -3343,7 +3343,13 @@ static int cache_save_setup(struct btrfs_block_group *block_group,
>   	}
>   
>   	if (IS_ERR(inode)) {
> -		BUG_ON(retries);
> +		if (retries) {
> +			btrfs_debug(fs_info,

You mentioned:

  > This can happen due to I/O
  > errors or transient failures, not just programming bugs.

If so, btrfs_debug() is definitely not noisy enough, as it's disabled by 
default.

> +				    "free space inode not found after creation for block group %llu",
> +				    block_group->start);
> +			ret = -EIO;

Furthermore, use the original error code from PTR_ERR(inode) is more 
meaningful, in case there is other causes of the inode lookup.

And showing the error code inside the error message will help indicating 
the bug.

Otherwise looks good to me.

> +			goto out_free;
> +		}
>   		retries++;
>   
>   		if (block_group->ro)