[PATCH 0/3] Folio conversions in extent-io-tests

[PATCH 0/3] Folio conversions in extent-io-tests

[Matthew Wilcox (Oracle)]

extent-io-tests is the last user of find_lock_page() in my tree, so
let's convert these two functions.  test_find_delalloc() is quite a big
function, so I split the conversion up.  We could combine these two
patches if anyone has a strong opinion.  There's no more use of 'struct
page' in extent-io-tests after this.  Compile tested only.

Matthew Wilcox (Oracle) (3):
  btrfs: Convert test_find_delalloc() to use a folio
  btrfs: Convert test_find_delalloc() to use a folio, part two
  btrfs: Simplify dump_eb_and_memory_contents()

 fs/btrfs/tests/extent-io-tests.c | 89 +++++++++++++++-----------------
 1 file changed, 43 insertions(+), 46 deletions(-)

-- 
2.47.2

[PATCH 1/3] btrfs: Convert test_find_delalloc() to use a folio

[Matthew Wilcox (Oracle)]

Replace the 'locked_page' variable with 'locked_folio'.  Replaces
ten calls to compound_head() with one.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/btrfs/tests/extent-io-tests.c | 47 ++++++++++++++------------------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index 00da54f0164c..8bdf742d90fd 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -112,7 +112,7 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
 	struct inode *inode = NULL;
 	struct extent_io_tree *tmp;
 	struct page *page;
-	struct page *locked_page = NULL;
+	struct folio *locked_folio = NULL;
 	unsigned long index = 0;
 	/* In this test we need at least 2 file extents at its maximum size */
 	u64 max_bytes = BTRFS_MAX_EXTENT_SIZE;
@@ -168,7 +168,7 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
 			unlock_page(page);
 		} else {
 			get_page(page);
-			locked_page = page;
+			locked_folio = page_folio(page);
 		}
 	}
 
@@ -179,8 +179,7 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
 	btrfs_set_extent_bit(tmp, 0, sectorsize - 1, EXTENT_DELALLOC, NULL);
 	start = 0;
 	end = start + PAGE_SIZE - 1;
-	found = find_lock_delalloc_range(inode, page_folio(locked_page), &start,
-					 &end);
+	found = find_lock_delalloc_range(inode, locked_folio, &start, &end);
 	if (!found) {
 		test_err("should have found at least one delalloc");
 		goto out_bits;
@@ -191,8 +190,8 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
 		goto out_bits;
 	}
 	btrfs_unlock_extent(tmp, start, end, NULL);
-	unlock_page(locked_page);
-	put_page(locked_page);
+	folio_unlock(locked_folio);
+	folio_put(locked_folio);
 
 	/*
 	 * Test this scenario
@@ -201,17 +200,16 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
 	 *           |--- search ---|
 	 */
 	test_start = SZ_64M;
-	locked_page = find_lock_page(inode->i_mapping,
+	locked_folio = filemap_lock_folio(inode->i_mapping,
 				     test_start >> PAGE_SHIFT);
-	if (!locked_page) {
-		test_err("couldn't find the locked page");
+	if (!locked_folio) {
+		test_err("couldn't find the locked folio");
 		goto out_bits;
 	}
 	btrfs_set_extent_bit(tmp, sectorsize, max_bytes - 1, EXTENT_DELALLOC, NULL);
 	start = test_start;
 	end = start + PAGE_SIZE - 1;
-	found = find_lock_delalloc_range(inode, page_folio(locked_page), &start,
-					 &end);
+	found = find_lock_delalloc_range(inode, locked_folio, &start, &end);
 	if (!found) {
 		test_err("couldn't find delalloc in our range");
 		goto out_bits;
@@ -227,8 +225,8 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
 		goto out_bits;
 	}
 	btrfs_unlock_extent(tmp, start, end, NULL);
-	/* locked_page was unlocked above */
-	put_page(locked_page);
+	/* locked_folio was unlocked above */
+	folio_put(locked_folio);
 
 	/*
 	 * Test this scenario
@@ -236,16 +234,15 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
 	 *                    |--- search ---|
 	 */
 	test_start = max_bytes + sectorsize;
-	locked_page = find_lock_page(inode->i_mapping, test_start >>
-				     PAGE_SHIFT);
-	if (!locked_page) {
-		test_err("couldn't find the locked page");
+	locked_folio = filemap_lock_folio(inode->i_mapping,
+			test_start >> PAGE_SHIFT);
+	if (!locked_folio) {
+		test_err("couldn't find the locked folio");
 		goto out_bits;
 	}
 	start = test_start;
 	end = start + PAGE_SIZE - 1;
-	found = find_lock_delalloc_range(inode, page_folio(locked_page), &start,
-					 &end);
+	found = find_lock_delalloc_range(inode, locked_folio, &start, &end);
 	if (found) {
 		test_err("found range when we shouldn't have");
 		goto out_bits;
@@ -265,8 +262,7 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
 	btrfs_set_extent_bit(tmp, max_bytes, total_dirty - 1, EXTENT_DELALLOC, NULL);
 	start = test_start;
 	end = start + PAGE_SIZE - 1;
-	found = find_lock_delalloc_range(inode, page_folio(locked_page), &start,
-					 &end);
+	found = find_lock_delalloc_range(inode, locked_folio, &start, &end);
 	if (!found) {
 		test_err("didn't find our range");
 		goto out_bits;
@@ -297,7 +293,7 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
 	put_page(page);
 
 	/* We unlocked it in the previous test */
-	lock_page(locked_page);
+	folio_lock(locked_folio);
 	start = test_start;
 	end = start + PAGE_SIZE - 1;
 	/*
@@ -306,8 +302,7 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
 	 * this changes at any point in the future we will need to fix this
 	 * tests expected behavior.
 	 */
-	found = find_lock_delalloc_range(inode, page_folio(locked_page), &start,
-					 &end);
+	found = find_lock_delalloc_range(inode, locked_folio, &start, &end);
 	if (!found) {
 		test_err("didn't find our range");
 		goto out_bits;
@@ -328,8 +323,8 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
 		dump_extent_io_tree(tmp);
 	btrfs_clear_extent_bits(tmp, 0, total_dirty - 1, (unsigned)-1);
 out:
-	if (locked_page)
-		put_page(locked_page);
+	if (locked_folio)
+		folio_put(locked_folio);
 	process_page_range(inode, 0, total_dirty - 1,
 			   PROCESS_UNLOCK | PROCESS_RELEASE);
 	iput(inode);
-- 
2.47.2

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

[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);
 		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);
-- 
2.47.2

[PATCH 3/3] btrfs: Simplify dump_eb_and_memory_contents()

[Matthew Wilcox (Oracle)]

Remove the casting from folio back to page and remove the use of memcmp
for a single byte.  Change the type to u8 so we can do direct array
arithmetic instead of doing complicated casts.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/btrfs/tests/extent-io-tests.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index 36720b77b440..f5f6accbdc21 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -664,17 +664,17 @@ static int test_find_first_clear_extent_bit(void)
 	return ret;
 }
 
-static void dump_eb_and_memory_contents(struct extent_buffer *eb, void *memory,
-					const char *test_name)
+static void dump_eb_and_memory_contents(const struct extent_buffer *eb,
+		const u8 *memory, const char *test_name)
 {
 	for (int i = 0; i < eb->len; i++) {
-		struct page *page = folio_page(eb->folios[i >> PAGE_SHIFT], 0);
-		void *addr = page_address(page) + offset_in_page(i);
+		struct folio *folio = eb->folios[i / PAGE_SIZE];
+		u8 *addr = folio_address(folio) + i % PAGE_SIZE;
 
-		if (memcmp(addr, memory + i, 1) != 0) {
+		if (*addr != memory[i]) {
 			test_err("%s failed", test_name);
 			test_err("eb and memory diffs at byte %u, eb has 0x%02x memory has 0x%02x",
-				 i, *(u8 *)addr, *(u8 *)(memory + i));
+				 i, *addr, memory[i]);
 			return;
 		}
 	}
-- 
2.47.2

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

[Qu Wenruo]

在 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);

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

[Qu Wenruo]

在 2025/6/14 09:05, Qu Wenruo 写道:
> 
> 
> 在 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

This is from the a_ops, which is NULL for testing inodes.

So here we should not call folio_mark_dirty(), as it will call 
dirty_folio() callbacks unconditionally no matter if the mapping has 
such call backs.

Instead we should call noop_dirty_folio() instead, which is the 
equivalent of SetPageDirty().

Thanks,
Qu

> [   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);
> 

Re: [PATCH 1/3] btrfs: Convert test_find_delalloc() to use a folio

[Matthew Wilcox]

On Fri, Jun 13, 2025 at 08:07:00PM +0100, Matthew Wilcox (Oracle) wrote:
> @@ -201,17 +200,16 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
>  	 *           |--- search ---|
>  	 */
>  	test_start = SZ_64M;
> -	locked_page = find_lock_page(inode->i_mapping,
> +	locked_folio = filemap_lock_folio(inode->i_mapping,
>  				     test_start >> PAGE_SHIFT);
> -	if (!locked_page) {
> -		test_err("couldn't find the locked page");
> +	if (!locked_folio) {
> +		test_err("couldn't find the locked folio");
>  		goto out_bits;
>  	}
>  	btrfs_set_extent_bit(tmp, sectorsize, max_bytes - 1, EXTENT_DELALLOC, NULL);
>  	start = test_start;
>  	end = start + PAGE_SIZE - 1;
> -	found = find_lock_delalloc_range(inode, page_folio(locked_page), &start,
> -					 &end);
> +	found = find_lock_delalloc_range(inode, locked_folio, &start, &end);
>  	if (!found) {
>  		test_err("couldn't find delalloc in our range");
>  		goto out_bits;

Hm.  How much do you test the failure paths here?  It seems to me that
the 'locked_folio' is still locked at this point ...

> @@ -328,8 +323,8 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
>  		dump_extent_io_tree(tmp);
>  	btrfs_clear_extent_bits(tmp, 0, total_dirty - 1, (unsigned)-1);
>  out:
> -	if (locked_page)
> -		put_page(locked_page);
> +	if (locked_folio)
> +		folio_put(locked_folio);

And here we put it without unlocking it, which should cause the page
allocator to squawk at you.

Re: [PATCH 1/3] btrfs: Convert test_find_delalloc() to use a folio

[Qu Wenruo]

在 2025/6/19 12:20, Matthew Wilcox 写道:
> On Fri, Jun 13, 2025 at 08:07:00PM +0100, Matthew Wilcox (Oracle) wrote:
>> @@ -201,17 +200,16 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
>>   	 *           |--- search ---|
>>   	 */
>>   	test_start = SZ_64M;
>> -	locked_page = find_lock_page(inode->i_mapping,
>> +	locked_folio = filemap_lock_folio(inode->i_mapping,
>>   				     test_start >> PAGE_SHIFT);
>> -	if (!locked_page) {
>> -		test_err("couldn't find the locked page");
>> +	if (!locked_folio) {
>> +		test_err("couldn't find the locked folio");
>>   		goto out_bits;
>>   	}
>>   	btrfs_set_extent_bit(tmp, sectorsize, max_bytes - 1, EXTENT_DELALLOC, NULL);
>>   	start = test_start;
>>   	end = start + PAGE_SIZE - 1;
>> -	found = find_lock_delalloc_range(inode, page_folio(locked_page), &start,
>> -					 &end);
>> +	found = find_lock_delalloc_range(inode, locked_folio, &start, &end);
>>   	if (!found) {
>>   		test_err("couldn't find delalloc in our range");
>>   		goto out_bits;
> 
> Hm.  How much do you test the failure paths here?  It seems to me that
> the 'locked_folio' is still locked at this point ...

Yep, you're right, the error paths here should have the folio unlocked
(all the error handling after fielmap_lock_folio()).

It's just very rare to have a commit that won't pass selftest pushed to 
upstream.

Mind to fix it in another patch or you wish us to handle it before your 
series?

Thanks,
Qu

> 
>> @@ -328,8 +323,8 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
>>   		dump_extent_io_tree(tmp);
>>   	btrfs_clear_extent_bits(tmp, 0, total_dirty - 1, (unsigned)-1);
>>   out:
>> -	if (locked_page)
>> -		put_page(locked_page);
>> +	if (locked_folio)
>> +		folio_put(locked_folio);
> 
> And here we put it without unlocking it, which should cause the page
> allocator to squawk at you.
> 
> 

Re: [PATCH 1/3] btrfs: Convert test_find_delalloc() to use a folio

[David Sterba]

On Thu, Jun 19, 2025 at 03:50:01AM +0100, Matthew Wilcox wrote:
> On Fri, Jun 13, 2025 at 08:07:00PM +0100, Matthew Wilcox (Oracle) wrote:
> > @@ -201,17 +200,16 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
> >  	 *           |--- search ---|
> >  	 */
> >  	test_start = SZ_64M;
> > -	locked_page = find_lock_page(inode->i_mapping,
> > +	locked_folio = filemap_lock_folio(inode->i_mapping,
> >  				     test_start >> PAGE_SHIFT);
> > -	if (!locked_page) {
> > -		test_err("couldn't find the locked page");
> > +	if (!locked_folio) {
> > +		test_err("couldn't find the locked folio");
> >  		goto out_bits;
> >  	}
> >  	btrfs_set_extent_bit(tmp, sectorsize, max_bytes - 1, EXTENT_DELALLOC, NULL);
> >  	start = test_start;
> >  	end = start + PAGE_SIZE - 1;
> > -	found = find_lock_delalloc_range(inode, page_folio(locked_page), &start,
> > -					 &end);
> > +	found = find_lock_delalloc_range(inode, locked_folio, &start, &end);
> >  	if (!found) {
> >  		test_err("couldn't find delalloc in our range");
> >  		goto out_bits;
> 
> Hm.  How much do you test the failure paths here?  It seems to me that
> the 'locked_folio' is still locked at this point ...

I think we don't paid too much attention to the failure paths of tests,
it means one has to go to check the new code first. If the tests pass
then they will clean up everything, in case of error it's usually
followed by reboot.

Re: [PATCH 0/3] Folio conversions in extent-io-tests

[David Sterba]

On Fri, Jun 13, 2025 at 08:06:59PM +0100, Matthew Wilcox (Oracle) wrote:
> extent-io-tests is the last user of find_lock_page() in my tree, so
> let's convert these two functions.  test_find_delalloc() is quite a big
> function, so I split the conversion up.  We could combine these two
> patches if anyone has a strong opinion.  There's no more use of 'struct
> page' in extent-io-tests after this.  Compile tested only.

Thanks, the patch splitting is OK. The conversion is straightforward,
we'll fix the tests so it works in the incomplete environment.

Re: [PATCH 1/3] btrfs: Convert test_find_delalloc() to use a folio

[David Sterba]

On Thu, Jun 19, 2025 at 03:11:16PM +0930, Qu Wenruo wrote:
> 
> 
> 在 2025/6/19 12:20, Matthew Wilcox 写道:
> > On Fri, Jun 13, 2025 at 08:07:00PM +0100, Matthew Wilcox (Oracle) wrote:
> >> @@ -201,17 +200,16 @@ static int test_find_delalloc(u32 sectorsize, u32 nodesize)
> >>   	 *           |--- search ---|
> >>   	 */
> >>   	test_start = SZ_64M;
> >> -	locked_page = find_lock_page(inode->i_mapping,
> >> +	locked_folio = filemap_lock_folio(inode->i_mapping,
> >>   				     test_start >> PAGE_SHIFT);
> >> -	if (!locked_page) {
> >> -		test_err("couldn't find the locked page");
> >> +	if (!locked_folio) {
> >> +		test_err("couldn't find the locked folio");
> >>   		goto out_bits;
> >>   	}
> >>   	btrfs_set_extent_bit(tmp, sectorsize, max_bytes - 1, EXTENT_DELALLOC, NULL);
> >>   	start = test_start;
> >>   	end = start + PAGE_SIZE - 1;
> >> -	found = find_lock_delalloc_range(inode, page_folio(locked_page), &start,
> >> -					 &end);
> >> +	found = find_lock_delalloc_range(inode, locked_folio, &start, &end);
> >>   	if (!found) {
> >>   		test_err("couldn't find delalloc in our range");
> >>   		goto out_bits;
> > 
> > Hm.  How much do you test the failure paths here?  It seems to me that
> > the 'locked_folio' is still locked at this point ...
> 
> Yep, you're right, the error paths here should have the folio unlocked
> (all the error handling after fielmap_lock_folio()).
> 
> It's just very rare to have a commit that won't pass selftest pushed to 
> upstream.
> 
> Mind to fix it in another patch or you wish us to handle it before your 
> series?

Please fix it separately before the series, this needs runtime testing
and is specific to the selftest environment, while the folio conversions
are API-level and straightforward.