* Re: [PATCH] btrfs: fix creation of compressed inline extents that don't save space
2026-09-14 17:36 [PATCH] btrfs: fix creation of compressed inline extents that don't save space fdmanana
@ 2026-09-14 18:15 ` Hanabishi
2026-09-14 18:46 ` [PATCH v2] " fdmanana
2026-09-14 19:40 ` [PATCH v3] " fdmanana
2 siblings, 0 replies; 5+ messages in thread
From: Hanabishi @ 2026-09-14 18:15 UTC (permalink / raw)
To: fdmanana, linux-btrfs
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index a85a7c561cf8..420d78b55bba 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2339,7 +2339,7 @@ static int run_delalloc_inline(struct btrfs_inode *inode, struct folio *locked_f
> } else if (inode->prop_compress) {
> compress_type = inode->prop_compress;
> }
> - cb = btrfs_compress_bio(inode, 0, blocksize, compress_type, compress_level, 0);
> + cb = btrfs_compress_bio(inode, 0, i_size, compress_type, compress_level, 0);
> if (IS_ERR(cb)) {
> cb = NULL;
> /* Just fall back to non-compressed case. */
Yep, this works. 👍️
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2] btrfs: fix creation of compressed inline extents that don't save space
2026-09-14 17:36 [PATCH] btrfs: fix creation of compressed inline extents that don't save space fdmanana
2026-09-14 18:15 ` Hanabishi
@ 2026-09-14 18:46 ` fdmanana
2026-09-14 19:40 ` [PATCH v3] " fdmanana
2 siblings, 0 replies; 5+ messages in thread
From: fdmanana @ 2026-09-14 18:46 UTC (permalink / raw)
To: linux-btrfs
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()
instead of the inode's i_size. So unless the compressed size is greater
than or equals to the block size, we allow the creation of compressed
inline extents that waste metadata space.
Since that commit we are also trying the compression even when we can
not create an inline extent because its i_size is larger than the page
size. We check for that only after compressing.
The fix is to pass the i_size instead of the block size in the call to
btrfs_compress_bio(), but before we call this function, we also need
to make sure we can create an inline extent by calling
can_cow_file_range_inline() first to validate the i_size, otherwise
an excepcionally large i_size could trigger a lot of compression work
which would then be later discarded because the call to
can_cow_file_range_inline() that follows the btrfs_compress_bio() call
will return false.
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>
---
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 | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a85a7c561cf8..d06a328ddead 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2330,7 +2330,8 @@ static int run_delalloc_inline(struct btrfs_inode *inode, struct folio *locked_f
*/
btrfs_check_folio_write_protected(locked_folio);
- if (btrfs_inode_can_compress(inode) &&
+ if (can_cow_file_range_inline(inode, 0, i_size, 0) &&
+ btrfs_inode_can_compress(inode) &&
inode_need_compress(inode, 0, blocksize, true)) {
if (inode->defrag_compress > 0 &&
inode->defrag_compress < BTRFS_NR_COMPRESS_TYPES) {
@@ -2339,7 +2340,7 @@ static int run_delalloc_inline(struct btrfs_inode *inode, struct folio *locked_f
} else if (inode->prop_compress) {
compress_type = inode->prop_compress;
}
- cb = btrfs_compress_bio(inode, 0, blocksize, compress_type, compress_level, 0);
+ cb = btrfs_compress_bio(inode, 0, i_size, compress_type, compress_level, 0);
if (IS_ERR(cb)) {
cb = NULL;
/* Just fall back to non-compressed case. */
--
2.47.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v3] btrfs: fix creation of compressed inline extents that don't save space
2026-09-14 17:36 [PATCH] btrfs: fix creation of compressed inline extents that don't save space fdmanana
2026-09-14 18:15 ` Hanabishi
2026-09-14 18:46 ` [PATCH v2] " fdmanana
@ 2026-09-14 19:40 ` fdmanana
2026-09-14 21:33 ` Qu Wenruo
2 siblings, 1 reply; 5+ messages in thread
From: fdmanana @ 2026-09-14 19:40 UTC (permalink / raw)
To: linux-btrfs
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>
---
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)) {
--
2.47.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v3] btrfs: fix creation of compressed inline extents that don't save space
2026-09-14 19:40 ` [PATCH v3] " fdmanana
@ 2026-09-14 21:33 ` Qu Wenruo
0 siblings, 0 replies; 5+ messages in thread
From: Qu Wenruo @ 2026-09-14 21:33 UTC (permalink / raw)
To: fdmanana, linux-btrfs
在 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)) {
^ permalink raw reply [flat|nested] 5+ messages in thread