Re: [PATCH 1/2] btrfs: do not try compression for data reloc inodes

在 2026/6/22 20:47, Filipe Manana 写道:
> On Sun, Jun 21, 2026 at 7:13 AM Qu Wenruo <[email protected]> wrote:
>>
>> [BUG]
>> There is a syzbot report that the check inside get_new_location()
>> triggered:
>>
>>   BTRFS info (device loop0): found 31 extents, stage: move data extents
>>   BTRFS info (device loop0): leaf 8908800 gen 16 total ptrs 28 free space 1676 owner 18446744073709551607
>>          item 0 key (256 INODE_ITEM 0) itemoff 3835 itemsize 160
>>                  inode generation 5 transid 0 size 0 nbytes 0
>>                  block group 0 mode 40755 links 1 uid 0 gid 0
>>                  rdev 0 sequence 0 flags 0x0
>>                  atime 1669132761.0
>>                  ctime 1669132761.0
>>                  mtime 1669132761.0
>>                  otime 0.0
>>          item 1 key (256 INODE_REF 256) itemoff 3823 itemsize 12
>>                  index 0 name_len 2
>>          item 2 key (258 INODE_ITEM 0) itemoff 3663 itemsize 160
>>                  inode generation 1 transid 16 size 733184 nbytes 106496
>>                  block group 0 mode 100600 links 0 uid 0 gid 0
>>                  rdev 0 sequence 24 flags 0x18
>>          item 3 key (258 EXTENT_DATA 0) itemoff 3595 itemsize 68
>>                  generation 16 type 0
>>                  inline extent data size 47 ram_bytes 4096 compression 1
>>   [...]
>>          item 27 key (18446744073709551611 ORPHAN_ITEM 258) itemoff 2376 itemsize 0
>>   BTRFS error (device loop0): unexpected non-zero offset in file extent item for data reloc inode 258 key offset 0 offset 9277520992061368337
>>   ------------[ cut here ]------------
>>   btrfs_abort_should_print_stack(__error)
>>
>> [CAUSE]
>> The above dump tree shows the first file extent item is inlined, which
>> should make no sense for data reloc inodes, as such inodes are just
>> representing where the data extents are in the relocation destination chunk.
>>
>> However the relocation path is just dirtying the data reloc inode
>> cluster by cluster. It's possible to have a single block, not adjacent
>> to any other data extents.
>>
>> Then relocation will dirty the first block of the data reloc inode, then
>> memory pressure forces the data reloc inode to be written back.
>>
>> In that case, since the syzbot has forced compression, we try to
>> compress the first block and if it can be compressed and inlined, an
>> inlined extent will be created.
> 
> If it were that simple, users would have encountered it and reported
> it, and fstests would have triggered this (we have several balance +
> fsstress + compression tests).
> 
> Something is missing here.
> A very important detail, which is not mentioned here at all, is that
> relocation works by preallocating extents (see
> prealloc_file_extent_cluster()) before dirtying pages/folios.
> 
> This means that flushing delalloc of the data reloc inode should
> always go into the nocow path.

Not really.

Since commit 3eaf5f082c4c ("btrfs: extract inlined creation into a 
dedicated delalloc helper"), we do not try nocow first, but inline first.

So even if we had a preallocated file extent for the first block, as 
long as the isize is no larger than 1 block, the write back path will 
still try inline first.

And since global force compression is set, we try compression and then 
inline the compressed data.


In fact, this can be reproduced on regular inodes:

  $ mkfs.btrfs -f /dev/test/scratch1
  $ sudo mount -o compress-force=zstd /dev/test/scratch1  /mnt/btrfs/
  $ sudo xfs_io -f -c "falloc 0 4k" -c sync /mnt/btrfs/foobar
  $ sudo xfs_io -f -c "pwrite 0 4k" -c sync /mnt/btrfs/foobar
  $ btrfs ins dump-tree -t 5 /dev/test/scratch1
  [...]
	item 4 key (257 INODE_ITEM 0) itemoff 15879 itemsize 160
		generation 9 transid 10 size 4096 nbytes 4096
		block group 0 mode 100600 links 1 uid 0 gid 0 rdev 0
		sequence 3 flags 0x10(PREALLOC)
		atime 1782166878.155000000 (2026-06-23 07:51:18)
		ctime 1782166892.349000000 (2026-06-23 07:51:32)
		mtime 1782166892.349000000 (2026-06-23 07:51:32)
		otime 1782166878.155000000 (2026-06-23 07:51:18)
	item 5 key (257 INODE_REF 256) itemoff 15863 itemsize 16
		index 2 namelen 6 name: foobar
	item 6 key (257 EXTENT_DATA 0) itemoff 15823 itemsize 40
		generation 10 type 0 (inline)
		inline extent data size 19 ram_bytes 4096 compression 3 (zstd)

Note that, inode 257 has PREALLOC flag, meaning it indeed went through 
preallocation before.

But still the final extent is still inlined.

I'll add the explanation in the next update.
> 
> Even if the nocow path would fallback into cow, which should never
> happen for a data reloc inode, we never try to compress and inline the
> fallback path - fallback_to_cow() -> cow_file_range() ->
> cow_one_range() - nothing here attempts inline extents (or
> compression).
> 
> What you are describing would be easy to convert into an fstests test case.

Not really. It requires a very specific cluster layout (only one block 
at the beginning of the bg, and no other block in the cluster).
And also very specific timing on when the writeback happens.

The required timing/layout means it's pretty hard to hit with regular 
stress runs.

Thanks,
Qu
> 
> Flushing delalloc of the data reloc inode should never reach
> btrfs_inode_can_compress() - if we end up there, then the problem is
> somewhere else.
> 
> Thanks.
> 
>>
>> Then the check in get_new_location() will check the file offset, without
>> checking if the file extent is inlined or not, resulting the above
>> failure.
>>
>> [FIX]
>> Do not allow compression for data reloc inodes in the first place.
>>
>> Reported-by: [email protected]
>> Link: https://lore.kernel.org/linux-btrfs/[email protected]/
>> Cc: [email protected]
>> Signed-off-by: Qu Wenruo <[email protected]>
>> ---
>>   fs/btrfs/btrfs_inode.h | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
>> index d5d81f9546c3..fff72f6cc1e8 100644
>> --- a/fs/btrfs/btrfs_inode.h
>> +++ b/fs/btrfs/btrfs_inode.h
>> @@ -476,6 +476,8 @@ static inline bool btrfs_inode_can_compress(const struct btrfs_inode *inode)
>>          if (inode->flags & BTRFS_INODE_NODATACOW ||
>>              inode->flags & BTRFS_INODE_NODATASUM)
>>                  return false;
>> +       if (btrfs_root_id(inode->root) == BTRFS_DATA_RELOC_TREE_OBJECTID)
>> +               return false;
>>          return true;
>>   }
>>
>> --
>> 2.54.0
>>
>>
>