* [PATCH v3 1/2] btrfs: lzo: fix space saving checks
2026-09-21 0:44 [PATCH v3 0/2] btrfs: remove the tailing zeros from compressed inline extents Qu Wenruo
@ 2026-09-21 0:44 ` Qu Wenruo
2026-09-21 0:44 ` [PATCH v3 2/2] btrfs: remove the trailing zeros from compressed inline extent Qu Wenruo
2026-09-21 1:06 ` [PATCH v3 0/2] btrfs: remove the tailing zeros from compressed inline extents Qu Wenruo
2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2026-09-21 0:44 UTC (permalink / raw)
To: linux-btrfs
[BUG]
Since commit 3be8a788eed3 ("btrfs: lzo: introduce lzo_compress_bio()
helper"), the space saving check on lzo is broken:
- Space saving check is always done based on block size
This prevents lzo_compress_bio() from doing any compression if the @len
is smaller than sectorsize.
This prevents proper inlined extent creation.
- Space saving check is not including the padding zeros
If the remaining space in the output folio is not enough, we add
padding zeros (up to 3 bytes), which contributes to the total
compressed size.
But that is not checked.
[CAUSE]
The function copy_compressed_data_to_bio() is doing the space saving
check wrong:
/* With the range copied, we're larger than the original range. */
if (((*total_out + copy_len) >> sectorsize_bits) >=
max_out >> sectorsize_bits)
- If the input length is smaller than sectorsize (inlined case)
Then @max_out >> sectorsize_bits returns 0, the above check always
returns true and copy_compressed_data_to_bio() always returns -E2BIG and
failed to create an lzo compressed inlined extent.
This check is LZO specific, as other algorithms do not have any block
aligned check during compression.
Both remaining algorithms rely on regular input/output size check, and
the final common space saving check at compress_file_range().
And for the extra padding bytes, there is no space saving check at all.
[FIX]
- Do a simpler space saving check
By just checking whether the compressed size meets or exceeds the input
size. If so reject it with -E2BIG.
With this fixed, lzo_compress_bio() can properly handle a range that is
smaller than a sector, which is required to get better compression ratio
for inlined extents.
- Do space saving check before padding
This will ensure that the compressed size never meets or exceeds the
input size.
Fixes: 3be8a788eed3 ("btrfs: lzo: introduce lzo_compress_bio() helper")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/lzo.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index 2f0996692da0..4e7837dec332 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -224,8 +224,7 @@ static int copy_compressed_data_to_bio(struct btrfs_fs_info *fs_info,
u32 foffset = *total_out & (fsize - 1);
/* With the range copied, we're larger than the original range. */
- if (((*total_out + copy_len) >> sectorsize_bits) >=
- max_out >> sectorsize_bits)
+ if (*total_out + copy_len >= max_out)
return -E2BIG;
if (!*out_folio) {
@@ -253,6 +252,8 @@ static int copy_compressed_data_to_bio(struct btrfs_fs_info *fs_info,
ASSERT(*out_folio);
/* The remaining size is not enough, pad it with zeros */
+ if (*total_out + sector_bytes_left >= max_out)
+ return -E2BIG;
folio_zero_range(*out_folio, offset_in_folio(*out_folio, *total_out), sector_bytes_left);
return write_and_queue_folio(out_bio, out_folio, total_out, sector_bytes_left);
}
--
2.55.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v3 2/2] btrfs: remove the trailing zeros from compressed inline extent
2026-09-21 0:44 [PATCH v3 0/2] btrfs: remove the tailing zeros from compressed inline extents Qu Wenruo
2026-09-21 0:44 ` [PATCH v3 1/2] btrfs: lzo: fix space saving checks Qu Wenruo
@ 2026-09-21 0:44 ` Qu Wenruo
2026-09-21 1:06 ` [PATCH v3 0/2] btrfs: remove the tailing zeros from compressed inline extents Qu Wenruo
2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2026-09-21 0:44 UTC (permalink / raw)
To: linux-btrfs
[BEHAVIOR CHANGE]
After commit 3eaf5f082c4c ("btrfs: extract inlined creation into a
dedicated delalloc helper"), btrfs changed its behavior when generating
compressed inlined extents.
Previously the compression input was the file range [0, i_size), but
after that commit the input is file range [0, sectorsize).
This means the decompression handling needs to have a buffer that is no
smaller than sectorsize, or the decompression will fail.
This has already caused problems for other projects, like u-boot and
btrfs-restore from btrfs-progs.
Although those projects are fixed with a more robust decompression path,
this kernel change also causes extra space usage for compressed inlined
extents:
All doing a 3K writes with content filled with 0xcd
Before:
item 6 key (257 EXTENT_DATA 0) itemoff 15794 itemsize 69
generation 9 type 0 (inline)
inline extent data size 48 ram_bytes 3072 compression 2 (lzo) encryption 0
After:
item 6 key (257 EXTENT_DATA 0) itemoff 15781 itemsize 82
generation 9 type 0 (inline)
inline extent data size 61 ram_bytes 3072 compression 2 (lzo) encryption 0
This behavior change also increased the lzo compressed size from 48 bytes to 61 bytes.
[FIX WITH A NEW BEHAVIOR CHANGE]
The previous patch "btrfs: lzo: fix space saving checks" fixed a
regression in lzo_compress_bio() where it didn't properly handle ranges
smaller than a sector.
With that regression fixed, we can finally pass the range [0, i_size) into
btrfs_compress_bio(), and this gets rid of the trailing zeros,
restoring the old behavior, along with the older and better
compression ratio.
However we need a tiny behavior change to ensure a stable i_size and
prevent races between inline writeback and isize expansion.
This is done by an early check rejecting i_size >= blocksize.
For i_size < blocksize, btrfs_setsize() will call
btrfs_cont_expand() and try to lock the folio, so btrfs_setsize() will
wait for writeback to finish before expanding i_size, killing the race.
For i_size > blocksize, we do not need to inline anyway.
The above two cases match the old behaviors.
But for i_size == blocksize, previously we allowed a compressed inline
extent as long as the compressed data is small enough.
However this exposed a race window without the new rejection, where the
isize is 4K and blocksize is 4K:
Writeback | Setsize
-------------------------------------+----------------------------------
btrfs_run_delalloc_range() |
|- run_delalloc_inline() |
| | btrfs_setsize()
| | |- btrfs_cont_expand()
| | | Doing nothing as the oldsize
| | | is already aligned
| | |- i_size_write()
| | Now the isize is 128K
|- i_size = i_size_read() |
| Which is 128K |
|- btrfs_compress_bio(len = 128K) |
This leads to incorrect end. |
So here to keep the i_size stable during writeback, we have to reject
any i_size >= blocksize, causing a minor behavior change to compressed
inline extents.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/inode.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 53f4532593b3..0a795bb460c8 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2325,6 +2325,27 @@ static int run_delalloc_inline(struct btrfs_inode *inode, struct folio *locked_f
int ret;
ASSERT(folio_pos(locked_folio) == 0);
+
+ /*
+ * It's possible that the i_size is expanded while writeback is running.
+ * There is no special locking to prevent isize being updated.
+ *
+ * But for unaligned i_size, btrfs_setsize() will need to lock the
+ * folio (which is held by us) before updating the isize, so if
+ * i_size < blocksize btrfs_setsize() will wait for us.
+ *
+ * The only problem is when i_size == blocksize and compression is on.
+ * In that case we previously allowed inline as long as the compressed
+ * data is smaller than blocksize.
+ * But since the i_size is already aligned, btrfs_setsize() would not
+ * need to lock the folio, so btrfs_setsize() can race with us.
+ *
+ * Thus here we reject i_size >= blocksize to prevent racing with
+ * btrfs_setsize() expansion.
+ */
+ if (i_size >= blocksize)
+ return 1;
+
/*
* If an mmap writer could modify the folio while we copy it into an
* inline extent we might see only part of their modification then
@@ -2342,12 +2363,7 @@ 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);
+ 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.55.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v3 0/2] btrfs: remove the tailing zeros from compressed inline extents
2026-09-21 0:44 [PATCH v3 0/2] btrfs: remove the tailing zeros from compressed inline extents Qu Wenruo
2026-09-21 0:44 ` [PATCH v3 1/2] btrfs: lzo: fix space saving checks Qu Wenruo
2026-09-21 0:44 ` [PATCH v3 2/2] btrfs: remove the trailing zeros from compressed inline extent Qu Wenruo
@ 2026-09-21 1:06 ` Qu Wenruo
2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2026-09-21 1:06 UTC (permalink / raw)
To: linux-btrfs
在 2026/9/21 10:14, Qu Wenruo 写道:
> [CHANGELOG]
> v3:
> - Introduce an early i_size >= blocksize rejection to prevent race
> Previously we allow inline extents for i_size == blocksize case, as
> long as the compressed data can still be inlined.
>
> But that introduce a race with setsize(), which is masked by using
> blocksize for compression.
>
> When reverting back to using i_size for compression, we can get a much
> larger i_size than expectation.
>
> Close the race window by always requireing i_size < blocksize, which
> makes btrfs_setsize() to always wait for the folio held by us before
> updating the i_size.
>
> This unfortunately introduce a behavior change, so all the fixes tags
> are dropped.
Please drop the whole series.
Filipe's merged v3 fix is good enough, and the extra i_size related race
is very tricky to handle correctly.
The existing blocksize based compression so far is the safest solution,
the saved a dozen of bytes are not worthy for the extra races.
Thanks,
Qu
>
> This is exposed by Sashiko during review of v1 and v2 patchsets, and
> reproduced once during fstests runs, resulting the following
> warning without the new rejection:
>
> [ 497.553473] BTRFS info (device dm-2 state M): use lzo compression, level 1
> [ 497.599454] BTRFS info (device dm-2 state M): use zstd compression, level 3
> [ 497.661405] ------------[ cut here ]------------
> [ 497.661414] WARNING: zstd.c:41 at zstd_compress_bio+0x120a/0x1fb0 [btrfs], CPU#1: kworker/u47:5/3431
> [ 497.666534] Modules linked in: dm_flakey btrfs(OE) nls_ascii nls_cp437 vfat fat i2c_i801 joydev psmouse pcspkr iTCO_wdt i2c_smbus mousedev xor raid6_pq loop vsock_loopback vmw_vsock_virtio_transport_common vmw_vsock_vmci_transport vsock vmw_vmci qemu_fw_cfg ext4 crc16 mbcache jbd2 dm_mod virtio_net net_failover serio_raw virtio_rng virtio_balloon virtio_gpu virtio_scsi failover virtio_dma_buf lpc_ich virtio_blk [last unloaded: btrfs]
> [ 497.677435] CPU: 1 UID: 0 PID: 3431 Comm: kworker/u47:5 Tainted: G OE 7.3.0-rc3-custom+ #464 PREEMPT(full) 8f6fa45f5906dd4b1abb78057674460bd6af91c9
> [ 497.681556] Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> [ 497.683051] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 02/02/2022
> [ 497.685450] Workqueue: writeback wb_workfn (flush-btrfs-222)
> [ 497.687235] RIP: 0010:zstd_compress_bio+0x120a/0x1fb0 [btrfs]
> [ 497.689061] Code: fd ff ff 48 89 ef e8 85 65 00 00 e9 db fd ff ff 48 89 df e8 38 e0 4b c2 e9 57 fe ff ff 48 89 d7 e8 2b e0 4b c2 e9 6f fa ff ff <0f> 0b e9 2c f0 ff ff e8 0a c5 ec c1 49 8d bf b8 01 00 00 48 b8 00
> [ 497.694043] RSP: 0018:ffff88810a17ece8 EFLAGS: 00010202
> [ 497.695504] RAX: 0000000000000011 RBX: ffff888102615e28 RCX: 0000001000000015
> [ 497.697482] RDX: 0000000100000011 RSI: 0000000000000002 RDI: ffff88810a17ec58
> [ 497.699455] RBP: 0000000000000011 R08: ffffffff846ca856 R09: ffff88810a17ec50
> [ 497.701424] R10: ffffed102142fd8b R11: 0000000000000010 R12: 0000000000000003
> [ 497.703444] R13: ffff888112695a00 R14: 000000000048f5e9 R15: ffff88819f5b0000
> [ 497.705410] FS: 0000000000000000(0000) GS:ffff8882aed9b000(0000) knlGS:0000000000000000
> [ 497.707610] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 497.709251] CR2: 000055967f60f000 CR3: 0000000111372000 CR4: 0000000000f50ef0
> [ 497.711388] PKRU: 55555554
> [ 497.712204] Call Trace:
> [ 497.712923] <TASK>
> [ 497.713575] ? _raw_spin_unlock_bh+0xe/0x20
> [ 497.714781] ? zstd_find_workspace.isra.0+0x283/0x8b0 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.717434] ? kmem_cache_alloc_noprof+0x194/0x420
> [ 497.718798] ? zstd_get_workspace+0xde/0x2c0 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.721241] ? zstd_get_workspace+0x2c0/0x2c0 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.723730] ? __asan_memset+0x27/0x50
> [ 497.724907] ? btrfs_free_compr_folio+0x2c0/0x2c0 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.727535] btrfs_compress_bio+0x683/0xb90 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.729989] btrfs_run_delalloc_range+0x12d9/0x1ba0 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.732712] ? extent_writepage_io+0xe40/0xe40 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.735231] ? btrfs_copy_subpage_dirty_bitmap+0x98/0x620 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.737989] ? submit_uncompressed_range+0x260/0x260 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.740665] ? btrfs_folio_set_lock+0x15c/0x420 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.743162] writepage_delalloc+0x7d8/0x1270 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.745646] ? find_lock_delalloc_range+0x7b0/0x7b0 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.748277] ? tag_pages_for_writeback+0x205/0x2d0
> [ 497.749653] ? folio_mkclean+0x163/0x3a0
> [ 497.750784] ? __traceiter_remove_migration_pte+0xb0/0xb0
> [ 497.752297] ? folio_wait_writeback+0xa8/0x200
> [ 497.753574] extent_write_cache_pages+0xb59/0x1e30 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.756187] ? btrfs_do_readpage+0x2600/0x2600 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.758797] btrfs_writepages+0x1c2/0x4a0 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.761179] ? virtqueue_add_inbuf_ctx+0x2820/0x2820
> [ 497.762655] ? extent_write_locked_range+0x8f0/0x8f0 [btrfs 1621afd7f58f5561128c1bbfbe7eb177867269dc]
> [ 497.765264] ? blk_mq_tag_update_sched_shared_tags+0xf0/0xf0
> [ 497.766854] ? sg_init_table+0x19/0x60
> [ 497.767967] do_writepages+0x212/0x550
> [ 497.769062] ? _raw_spin_lock+0x84/0xe0
> [ 497.770185] ? writeback_set_ratelimit+0x120/0x120
> [ 497.771574] __writeback_single_inode+0xeb/0xba0
> [ 497.772902] ? _raw_spin_lock+0x84/0xe0
> [ 497.774015] ? sync_lazytime+0x330/0x330
> [ 497.775167] ? _raw_spin_unlock+0xe/0x20
> [ 497.776288] ? wbc_attach_and_unlock_inode+0x346/0x610
> [ 497.777741] writeback_sb_inodes+0x5a3/0xff0
> [ 497.779007] ? asm_common_interrupt+0x26/0x40
> [ 497.780399] ? __writeback_single_inode+0xba0/0xba0
> [ 497.781862] ? queue_io+0x2a3/0x480
> [ 497.782957] ? wb_update_bandwidth+0xd0/0xd0
> [ 497.784258] wb_writeback+0x1d9/0x780
> [ 497.785338] ? writeback_inodes_wb.constprop.0+0x1d0/0x1d0
> [ 497.786935] ? _raw_write_lock_irq+0xe0/0xe0
> [ 497.788340] ? psi_group_change+0x38d/0x750
> [ 497.789711] wb_workfn+0x1f6/0xc10
> [ 497.790877] ? _raw_spin_unlock+0xe/0x20
> [ 497.792082] ? sync_inode_metadata+0xd0/0xd0
> [ 497.793392] ? __schedule+0xe8d/0x5ec0
> [ 497.794549] ? max_active_store+0x100/0x100
> [ 497.795838] ? _raw_spin_lock_irq+0x8a/0xe0
> [ 497.797113] process_one_work+0x721/0x10b0
> [ 497.798376] ? io_schedule_timeout+0x130/0x130
> [ 497.799801] ? pwq_dec_nr_in_flight+0xfd0/0xfd0
> [ 497.801092] ? _raw_spin_lock_irq+0x8a/0xe0
> [ 497.802271] ? _raw_write_lock_irq+0xe0/0xe0
> [ 497.803477] worker_thread+0x536/0xda0
> [ 497.804548] ? rescuer_thread+0x1480/0x1480
> [ 497.805905] kthread+0x352/0x450
> [ 497.806874] ? recalc_sigpending+0x15c/0x200
> [ 497.808114] ? kthread_affine_node+0x300/0x300
> [ 497.809389] ret_from_fork+0x473/0x710
> [ 497.810484] ? exit_thread+0x70/0x70
> [ 497.811537] ? __switch_to+0x3ab/0xcf0
> [ 497.812645] ? kthread_affine_node+0x300/0x300
> [ 497.813930] ret_from_fork_asm+0x11/0x20
> [ 497.815060] </TASK>
> [ 497.815718] ---[ end trace 0000000000000000 ]---
> [ 497.817260] BTRFS warning (device dm-2): zstd compression level 3 failed, error 64 root 263 inode 264 offset 0
> [ 497.840873] BTRFS info (device dm-2 state M): use no compression
>
> v2:
> - Remove the block based space saving checks completely from lzo
> Zlib and ZSTD do not have block based space saving in the first place.
> It's the caller's responsibility to check, and inlined and regular
> extents have different requirements.
>
> This simplify the first patch a lot.
>
> - Update the cover letter
> Filipe's v3 fix is already merged, so we can not force push a fix but
> to co-operate the v3 fix.
>
> Commit 3eaf5f082c4c ("btrfs: extract inlined creation into a dedicated
> delalloc helper") changed the compression input from [0, i_size) to [0,
> blocksize), which caused two problems:
>
> - Other tools unable to decompress the inlined extent
> U-boot and btrfs-restore are affected, as they only allocated a buffer
> which is @ram_bytes sized.
> That buffer is too small to contain the decompressed data, which is
> @sectorsize.
>
> Those projects are fixed to have a more robust decompression routine
> which can handle both cases now.
>
> - Worse ratio for those compressed inline extent.
>
> For the same 3K 0xcd filled range, the results are small but
> observable, 48 vs 61 bytes.
>
> Filipe's v1 fix is very close to a proper fix, but btrfs will unable to
> create inlined extents for lzo.
> It turns out to be another bug in the copy_compressed_data_to_bio()
> function.
>
> Which is doing a premature block size based space saving checks,
> meanwhile all other algorithms do not have block sized based checks, but
> only a simple "@compressed >= @input" check.
> Zlib and Zstd all rely on the caller to do proper space saving checks,
> as regular and inlined extents have different requirements.
>
> So this series is to properly fix the bug, firstly fix the lzo
> regression which prevents inlined extent creation, then restore the old
> i_size based inline extent creation.
>
> To prevent i_size being updated during writeback, add a new i_size <
> blocksize requirement, so btrfs_setsize() will always wait for us before
> updating the i_size during expansion.
>
> However this introduces a new behavior change, that we can no longer
> create block sized (ram_bytes) inlined extents.
>
>
> Qu Wenruo (2):
> btrfs: lzo: fix space saving checks
> btrfs: remove the trailing zeros from compressed inline extent
>
> fs/btrfs/inode.c | 28 ++++++++++++++++++++++------
> fs/btrfs/lzo.c | 5 +++--
> 2 files changed, 25 insertions(+), 8 deletions(-)
>
^ permalink raw reply [flat|nested] 4+ messages in thread