Re: [PATCH v3] btrfs: fix creation of compressed inline extents that don't save space

[Qu Wenruo <wqu@suse.com>]

在 2026/9/15 05:10, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> If the compressed data of an inline extent is larger than or equals to the
> size of the uncompressed data, we are still allowing the creation of the
> compressed inline extent, which does not result in any benefits, quite the
> contrary as we waste metadata space and have to decompress when reading.
> 
> This is a recent regression introduced in commit 3eaf5f082c4c ("btrfs:
> extract inlined creation into a dedicated delalloc helper").
> 
> It happens because we are passing the block size to btrfs_compress_bio(),
> so we don't get -E2BIG from the compression code anymore, but we can not
> pass i_size either, because if i_size is smaller than sector size, we
> end up never creating lzo compressed inline extent for such small i_size
> values. So refuse the compressed result at run_delalloc_inline() if
> its size is not smaller than the uncompressesed size (i_size).
> 
> Fixes: 3eaf5f082c4c ("btrfs: extract inlined creation into a dedicated delalloc helper")
> Reported-by: Hanabishi <i.r.e.c.c.a.k.u.n+kernel.org@gmail.com>
> Link: https://lore.kernel.org/linux-btrfs/c97652a5-ac6b-4de6-aa23-3cdebc01d00b@gmail.com/
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Thanks,
Qu

> ---
> 
> V3: Fix being unable to create lzo compressed inline extents when the
>      data size (i_size) is smaller than the sector size (caught by
>      sashiko again).
> 
> V2: Check first if we can create an inline extent otherwise a too large
>      i_size would create a lot of work just to be discarded later.
> 
>   fs/btrfs/inode.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index a85a7c561cf8..2b4387db937e 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2339,12 +2339,27 @@ static int run_delalloc_inline(struct btrfs_inode *inode, struct folio *locked_f
>   		} else if (inode->prop_compress) {
>   			compress_type = inode->prop_compress;
>   		}
> +		/*
> +		 * We need to pass blocksize and not i_size, otherwise we can't
> +		 * create compressed inline extents for data smaller than sector
> +		 * size with lzo.
> +		 */
>   		cb = btrfs_compress_bio(inode, 0, blocksize, compress_type, compress_level, 0);
>   		if (IS_ERR(cb)) {
>   			cb = NULL;
>   			/* Just fall back to non-compressed case. */
>   		} else {
>   			compressed_size = cb->bbio.bio.bi_iter.bi_size;
> +			/*
> +			 * If we did not save space, it's pointless and wasteful
> +			 * to have an inline compressed extent, so fallback to
> +			 * an uncompressed inline extent.
> +			 */
> +			if (compressed_size >= i_size) {
> +				cleanup_compressed_bio(cb);
> +				cb = NULL;
> +				compressed_size = 0;
> +			}
>   		}
>   	}
>   	if (!can_cow_file_range_inline(inode, 0, i_size, compressed_size)) {