[PATCH 0/2] btrfs: fix generic/795 crash for bs > ps cases

[PATCH 0/2] btrfs: fix generic/795 crash for bs > ps cases

[Qu Wenruo]

During my development of the incoming data_size feature, it turns out
that we have a new crash triggered by the new test case, generic/795 on
bs > ps cases.

It turns out that for cached inode items (btrfs_delayed_node) we can
skip the inode mapping flag settings.

This bug is fixed by the first path.

The second patch is to introduce the extra debug ASSERT()s, which
greatly reduced the search range by showing the mapping's minimal folio
order.

Qu Wenruo (2):
  btrfs: initialize inode mapping flags for cached inodes
  btrfs: add extra ASSERT()s to make sure the folio size is correct

 fs/btrfs/extent_io.c | 19 +++++++++++++++++++
 fs/btrfs/inode.c     |  3 ++-
 2 files changed, 21 insertions(+), 1 deletion(-)

-- 
2.54.0

[PATCH 1/2] btrfs: initialize inode mapping flags for cached inodes

[Qu Wenruo]

[BUG]
When running generic/795 with 8K block size, 4K page size, the test
always fails, triggering some ASSERT()s related to folio size:

 795 (241074): drop_caches: 3
 assertion failed: IS_ALIGNED(start, blocksize) && IS_ALIGNED(end + 1, blocksize), in extent_io.c:1404 (blocksize=8192 root=262 ino=258 start=16826368 end=16830463 mapping min order=0)
 ------------[ cut here ]------------
 kernel BUG at extent_io.c:1404!
 Oops: invalid opcode: 0000 [#1] SMP
 CPU: 8 UID: 0 PID: 241105 Comm: fsstress Tainted: G           OE       7.2.0-rc5-custom+ #442 PREEMPT(full)  f4bfb352566f3949f29c233ce6f735050a03b245
 Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 02/02/2022
 RIP: 0010:assert_folio_range.cold+0x3d/0x3f [btrfs]
 Call Trace:
  <TASK>
  btrfs_read_folio+0x9e/0x170 [btrfs 4cd1dd93b341b8ef766643f9512f4a86259567a3]
  prepare_one_folio.constprop.0+0x104/0x2a0 [btrfs 4cd1dd93b341b8ef766643f9512f4a86259567a3]
  btrfs_buffered_write+0x285/0xa50 [btrfs 4cd1dd93b341b8ef766643f9512f4a86259567a3]
  btrfs_do_write_iter+0x1aa/0x210 [btrfs 4cd1dd93b341b8ef766643f9512f4a86259567a3]
  iter_file_splice_write+0x31a/0x540
  direct_splice_actor+0x53/0x170
  splice_direct_to_actor+0xe9/0x240
  do_splice_direct+0x76/0xb0
  vfs_copy_file_range+0x1fd/0x630
  __x64_sys_copy_file_range+0xf9/0x220
  do_syscall_64+0xe1/0x790
  entry_SYSCALL_64_after_hwframe+0x4b/0x53
  </TASK>
 ---[ end trace 0000000000000000 ]---

The ASSERT() itself is added by a later patch.
The crash is triggered with that new debug patch, and without this fix.

[CAUSE]
In the above case, the start 16826368 is properly 8K aligned, but the
end (16830463 + 1) is not 8K aligned.
Furthermore the mapping's minimal folio order is 0, not the expected 1
for 8K block size with 4K page size.

So this means some inodes do not have btrfs_set_inode_mapping_order()
called on it.

The missing btrfs_set_inode_mapping_order() call happens for cached
inodes, through the following events:

- btrfs_create_new_inode() called for inode X
  Which properly sets minimal folio order for the vfs inode.

- btrfs_update_inode() called for inode X
  Which calls btrfs_delayed_update_inode() to create a delayed_node
  into root->delayed_nodes xarray.

- Drop cache/memory pressure, evicting in-memory inode X
  Which evicted the inode X, but delayed_node is still in
  root->delayed_nodes for future reuse.

- btrfs_iget() for inode X called again

  btrfs_iget()
  |- btrfs_iget_locked()
  |  |- iget5_locked_rcu()
  |     Which creates a new vfs_inode for btrfs, whose mapping still
  |     has the minimal order as 0.
  |
  |- btrfs_read_locked_inode()
     |- btrfs_fill_inode()
     |  |- btrfs_get_delayed_node()
     |     Which found out the previous node, and use that delayed
     |     node to initialize the new inode.
     |
     |- filled = true;
     |- if (filled) goto cache_index;
        Which skips the btrfs_update_inode_mapping_flags() and
	btrfs_set_inode_mapping_order() calls.
	So the inode still has minimal folio order set as 0, not
	the required 1.

Thus later page cache read will get a folio whose size is smaller than
block size, as the mapping has its minimal folio order set as 0 not 1,
then trigger the ASSERT().

[FIX]
Move the btrfs_update_inode_mapping_flags() and
btrfs_set_inode_mapping_order() calls under cache_index label,
so that the mapping flags and minimal folio order is always set
no matter if we have a cached inode.

Assisted-by: LLM (analysis)
Fixes: ecde48a1a6b3 ("btrfs: expose per-inode stable writes flag")
Fixes: cc38d178ff33 ("btrfs: enable large data folio support under CONFIG_BTRFS_EXPERIMENTAL")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 78143e241ca4..fafd7a74eaa7 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4107,10 +4107,11 @@ static int btrfs_read_locked_inode(struct btrfs_inode *inode, struct btrfs_path
 
 	btrfs_inode_split_flags(btrfs_inode_flags(leaf, inode_item),
 				&inode->flags, &inode->ro_flags);
+
+cache_index:
 	btrfs_update_inode_mapping_flags(inode);
 	btrfs_set_inode_mapping_order(inode);
 
-cache_index:
 	/*
 	 * If we were modified in the current generation and evicted from memory
 	 * and then re-read we need to do a full sync since we don't have any
-- 
2.54.0

[PATCH 2/2] btrfs: add extra ASSERT()s to make sure the folio size is correct

[Qu Wenruo]

Inspired by the previous crash exposed by generic/795, we want to make
sure every folio from btrfs page cache is properly aligned to block
size.

This is especially important for bs > ps support, as every btrfs
infrastructure, e.g. extent map and extent state, requires strong block
alignment checks.

Furthermore, also output the minimal folio order from the inode mapping,
which is the determining factor during debugging, helping a lot pinning
down the final cause.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/extent_io.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index c7c3f138fb69..5641eb88e3d0 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1392,6 +1392,22 @@ static void lock_extents_for_read(struct btrfs_inode *inode, u64 start, u64 end,
 	}
 }
 
+static void assert_folio_range(const struct btrfs_inode *inode,
+			       u64 start, u64 end)
+{
+	const u32 blocksize = inode->root->fs_info->sectorsize;
+
+	/*
+	 * For btrfs page cache, a folio always contains at least one block,
+	 * so the range should always be block size aligned.
+	 */
+	ASSERT(IS_ALIGNED(start, blocksize) && IS_ALIGNED(end + 1, blocksize),
+	       "blocksize=%u root=%lld ino=%llu start=%llu end=%llu mapping min order=%u",
+	       blocksize, btrfs_root_id(inode->root), btrfs_ino(inode),
+	       start, end,
+	       mapping_min_folio_order(inode->vfs_inode.i_mapping));
+}
+
 int btrfs_read_folio(struct file *file, struct folio *folio)
 {
 	struct inode *vfs_inode = folio->mapping->host;
@@ -1407,6 +1423,7 @@ int btrfs_read_folio(struct file *file, struct folio *folio)
 	struct fsverity_info *vi = NULL;
 	int ret;
 
+	assert_folio_range(inode, start, end);
 	lock_extents_for_read(inode, start, end, &cached_state);
 	if (folio_pos(folio) < i_size_read(vfs_inode))
 		vi = fsverity_get_info(vfs_inode);
@@ -1914,6 +1931,7 @@ static noinline_for_stack int extent_writepage_io(struct btrfs_inode *inode,
 	ASSERT(start >= folio_start, "start=%llu folio_start=%llu", start, folio_start);
 	ASSERT(end <= folio_end, "start=%llu len=%u folio_start=%llu folio_size=%zu",
 	       start, len, folio_start, folio_size(folio));
+	assert_folio_range(inode, folio_start, folio_end - 1);
 
 	/*
 	 * We are about to checksum and write out the data, so it must not be
@@ -2976,6 +2994,7 @@ void btrfs_readahead(struct readahead_control *rac)
 	struct extent_map *em_cached = NULL;
 	struct fsverity_info *vi = NULL;
 
+	assert_folio_range(inode, start, end);
 	lock_extents_for_read(inode, start, end, &cached_state);
 	/* We don't use cached state for a bulk unlock, just free it. */
 	btrfs_free_extent_state(cached_state);
-- 
2.54.0