Re: [PATCH 2/3] btrfs: Convert test_find_delalloc() to use a folio, part two

[Qu Wenruo <wqu@suse.com>]

在 2025/6/14 04:37, Matthew Wilcox (Oracle) 写道:
> Replace the 'page' variable with 'folio'.  Removes six calls to
> compound_head().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/btrfs/tests/extent-io-tests.c | 32 +++++++++++++++++---------------
>   1 file changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
> index 8bdf742d90fd..36720b77b440 100644
> --- a/fs/btrfs/tests/extent-io-tests.c
> +++ b/fs/btrfs/tests/extent-io-tests.c
> @@ -111,7 +111,7 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
>   	struct btrfs_root *root = NULL;
>   	struct inode *inode = NULL;
>   	struct extent_io_tree *tmp;
> -	struct page *page;
> +	struct folio *folio;
>   	struct folio *locked_folio = NULL;
>   	unsigned long index = 0;
>   	/* In this test we need at least 2 file extents at its maximum size */
> @@ -152,23 +152,25 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
>   	btrfs_extent_io_tree_init(NULL, tmp, IO_TREE_SELFTEST);
>   
>   	/*
> -	 * First go through and create and mark all of our pages dirty, we pin
> -	 * everything to make sure our pages don't get evicted and screw up our
> +	 * First go through and create and mark all of our folios dirty, we pin
> +	 * everything to make sure our folios don't get evicted and screw up our
>   	 * test.
>   	 */
>   	for (index = 0; index < (total_dirty >> PAGE_SHIFT); index++) {
> -		page = find_or_create_page(inode->i_mapping, index, GFP_KERNEL);
> -		if (!page) {
> -			test_err("failed to allocate test page");
> +		folio = __filemap_get_folio(inode->i_mapping, index,
> +				FGP_LOCK | FGP_ACCESSED | FGP_CREAT,
> +				GFP_KERNEL);
> +		if (!folio) {
> +			test_err("failed to allocate test folio");
>   			ret = -ENOMEM;
>   			goto out;
>   		}
> -		SetPageDirty(page);
> +		folio_mark_dirty(folio);

Crashing immediately when loading the module.
(Need CONFIG_BTRFS_FS_RUN_SANITY_TESTS=y)

[   20.626710] BUG: kernel NULL pointer dereference, address: 
0000000000000000
[   20.628812] #PF: supervisor instruction fetch in kernel mode
[   20.630648] #PF: error_code(0x0010) - not-present page
[   20.632156] PGD 0 P4D 0
[   20.632893] Oops: Oops: 0010 [#1] SMP NOPTI
[   20.634052] CPU: 6 UID: 0 PID: 622 Comm: insmod Tainted: G 
OE       6.16.0-rc1-custom+ #253 PREEMPT(full)
[   20.636879] Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
[   20.638321] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
unknown 02/02/2022
[   20.640524] RIP: 0010:0x0
[   20.641290] Code: Unable to access opcode bytes at 0xffffffffffffffd6.
[   20.643075] RSP: 0018:ffffc90001587c88 EFLAGS: 00010246
[   20.644519] RAX: ffff88810b144670 RBX: 0000000000000000 RCX: 
0000000000000001
[   20.646490] RDX: 0000000000000000 RSI: ffffea0004128200 RDI: 
ffff88810b144670
[   20.648496] RBP: ffff88810b144500 R08: 0000000000000000 R09: 
ffffffff83549b20
[   20.650524] R10: 00000000000002c0 R11: 0000000000000000 R12: 
0000000000000000
[   20.652642] R13: ffff88810b1443a8 R14: ffffea0004128200 R15: 
0000000000001000
[   20.654778] FS:  00007f2e60e99740(0000) GS:ffff8882f45e2000(0000) 
knlGS:0000000000000000
[   20.657158] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   20.658980] CR2: ffffffffffffffd6 CR3: 00000001040b9000 CR4: 
00000000000006f0
[   20.661219] Call Trace:
[   20.662057]  <TASK>
[   20.662757]  btrfs_test_extent_io+0x17a/0xf40 [btrfs]
[   20.664266]  btrfs_run_sanity_tests.cold+0x84/0x11e [btrfs]
[   20.665839]  init_btrfs_fs+0x4d/0xb0 [btrfs]
[   20.667380]  ? __pfx_init_btrfs_fs+0x10/0x10 [btrfs]
[   20.668883]  do_one_initcall+0x76/0x250
[   20.670098]  do_init_module+0x62/0x250
[   20.671359]  init_module_from_file+0x85/0xc0
[   20.672586]  idempotent_init_module+0x148/0x340
[   20.673900]  __x64_sys_finit_module+0x6d/0xd0
[   20.675074]  do_syscall_64+0x54/0x1d0
[   20.676167]  entry_SYSCALL_64_after_hwframe+0x76/0x7e


Furthermore, the error handling of __filemap_get_folio() is incorrect.
That function returns either a valid folio, or an ERR_PTR(), no more NULL.

This applies to all folio calls like filemap_lock_folio() too.

Thanks,
Qu

>   		if (index) {
> -			unlock_page(page);
> +			folio_unlock(folio);
>   		} else {
> -			get_page(page);
> -			locked_folio = page_folio(page);
> +			folio_get(folio);
> +			locked_folio = folio;
>   		}
>   	}
>   
> @@ -283,14 +285,14 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
>   	 * Now to test where we run into a page that is no longer dirty in the
>   	 * range we want to find.
>   	 */
> -	page = find_get_page(inode->i_mapping,
> +	folio = filemap_get_folio(inode->i_mapping,
>   			     (max_bytes + SZ_1M) >> PAGE_SHIFT);
> -	if (!page) {
> -		test_err("couldn't find our page");
> +	if (!folio) {
> +		test_err("couldn't find our folio");
>   		goto out_bits;
>   	}
> -	ClearPageDirty(page);
> -	put_page(page);
> +	folio_clear_dirty(folio);
> +	folio_put(folio);
>   
>   	/* We unlocked it in the previous test */
>   	folio_lock(locked_folio);