Re: [PATCH v4 5/9] iomap: simplify ->read_folio_range() error handling for reads

[Joanne Koong <joannelkoong@gmail.com>]

On Mon, Nov 17, 2025 at 12:46 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Tue, Nov 11, 2025 at 11:36:54AM -0800, Joanne Koong wrote:
> > Instead of requiring that the caller calls iomap_finish_folio_read()
> > even if the ->read_folio_range() callback returns an error, account for
> > this internally in iomap instead, which makes the interface simpler and
> > makes it match writeback's ->read_folio_range() error handling
> > expectations.
>
> Bisection of next-20251117 leads to this patch (commit
> f8eaf79406fe9415db0e7a5c175b50cb01265199)
>
> Here's the failure:
>
> generic/008       run fstests generic/008 at 2025-11-17 20:40:31
> page: refcount:5 mapcount:0 mapping:00000000101f858e index:0x4 pfn:0x12d4f8
> head: order:2 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
> memcg:ffff8881120315c0
> aops:xfs_address_space_operations ino:83 dentry name(?):"008.2222"
> flags: 0x8000000000014069(locked|uptodate|lru|private|head|reclaim|zone=2)
> raw: 8000000000014069 ffffea0004b69f48 ffffea0004a0a508 ffff8881139d83f0
> raw: 0000000000000004 ffff8881070b4420 00000005ffffffff ffff8881120315c0
> head: 8000000000014069 ffffea0004b69f48 ffffea0004a0a508 ffff8881139d83f0
> head: 0000000000000004 ffff8881070b4420 00000005ffffffff ffff8881120315c0
> head: 8000000000000202 ffffea0004b53e01 00000000ffffffff 00000000ffffffff
> head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000004
> page dumped because: VM_BUG_ON_FOLIO(success && folio_test_uptodate(folio))
> ------------[ cut here ]------------
> kernel BUG at mm/filemap.c:1538!
> Oops: invalid opcode: 0000 [#1] SMP NOPTI
> CPU: 1 UID: 0 PID: 2607 Comm: xfs_io Not tainted 6.18.0-rc1-ktest-00033-gf8eaf79406fe #151 NONE
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> RIP: 0010:folio_end_read+0x68/0x70
> Code: 8e e9 04 00 0f 0b 48 8b 07 48 89 c2 48 c1 ea 03 a8 08 74 00 83 e2 01 b8 09 00 00 00 74 c2 48 c7 c6 a0 6e 3e 82 e8 68 e9 04 00 <0f> 0b 90 0f 1f 44 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90
> RSP: 0018:ffff888112333870 EFLAGS: 00010286
> RAX: 000000000000004b RBX: ffffea0004b53e00 RCX: 0000000000000027
> RDX: ffff888179657c08 RSI: 0000000000000001 RDI: ffff888179657c00
> RBP: ffff888112333870 R08: 00000000fffbffff R09: ffff88817f1fdfa8
> R10: 0000000000000003 R11: 0000000000000000 R12: ffff8881070b4420
> R13: 0000000000000001 R14: ffff888112333b28 R15: ffffea0004b53e00
> FS:  00007f7b1ad91880(0000) GS:ffff8881f6b0b000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 000055d7455da018 CR3: 00000001111ac000 CR4: 0000000000750eb0
> PKRU: 55555554
> Call Trace:
>  <TASK>
>  iomap_read_end+0xac/0x130
>  iomap_readahead+0x1e1/0x330
>  xfs_vm_readahead+0x3d/0x50
>  read_pages+0x69/0x270
>  page_cache_ra_order+0x2c2/0x4d0
>  page_cache_async_ra+0x204/0x3c0
>  filemap_readahead.isra.0+0x67/0x80
>  filemap_get_pages+0x376/0x8a0
>  ? find_held_lock+0x31/0x90
>  ? try_charge_memcg+0x21a/0x750
>  ? lock_acquire+0xb2/0x290
>  ? __memcg_kmem_charge_page+0x160/0x3c0
>  filemap_read+0x106/0x4c0
>  ? __might_fault+0x35/0x80
>  generic_file_read_iter+0xbc/0x110
>  xfs_file_buffered_read+0xa9/0x110
>  xfs_file_read_iter+0x82/0xf0
>  vfs_read+0x277/0x360
>  __x64_sys_pread64+0x7a/0xa0
>  x64_sys_call+0x1b03/0x1da0
>  do_syscall_64+0x6a/0x2e0
>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> RIP: 0033:0x7f7b1b0efd07
> Code: 08 89 3c 24 48 89 4c 24 18 e8 55 76 fa ff 4c 8b 54 24 18 48 8b 54 24 10 41 89 c0 48 8b 74 24 08 8b 3c 24 b8 11 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 31 44 89 c7 48 89 04 24 e8 a5 76 fa ff 48 8b
> RSP: 002b:00007ffc4a4b8080 EFLAGS: 00000293 ORIG_RAX: 0000000000000011
> RAX: ffffffffffffffda RBX: 0000000000001000 RCX: 00007f7b1b0efd07
> RDX: 0000000000001000 RSI: 000055d7455d8000 RDI: 0000000000000003
> RBP: 0000000000001000 R08: 0000000000000000 R09: 0000000000000003
> R10: 0000000000001000 R11: 0000000000000293 R12: 0000000000000001
> R13: 0000000000020000 R14: 000000000001f000 R15: 0000000000001000
>  </TASK>
> Modules linked in:
> ---[ end trace 0000000000000000 ]---
> RIP: 0010:folio_end_read+0x68/0x70
> Code: 8e e9 04 00 0f 0b 48 8b 07 48 89 c2 48 c1 ea 03 a8 08 74 00 83 e2 01 b8 09 00 00 00 74 c2 48 c7 c6 a0 6e 3e 82 e8 68 e9 04 00 <0f> 0b 90 0f 1f 44 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90
> RSP: 0018:ffff888112333870 EFLAGS: 00010286
> RAX: 000000000000004b RBX: ffffea0004b53e00 RCX: 0000000000000027
> RDX: ffff888179657c08 RSI: 0000000000000001 RDI: ffff888179657c00
> RBP: ffff888112333870 R08: 00000000fffbffff R09: ffff88817f1fdfa8
> R10: 0000000000000003 R11: 0000000000000000 R12: ffff8881070b4420
> R13: 0000000000000001 R14: ffff888112333b28 R15: ffffea0004b53e00
> FS:  00007f7b1ad91880(0000) GS:ffff8881f6b0b000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 000055d7455da018 CR3: 00000001111ac000 CR4: 0000000000750eb0
> PKRU: 55555554
> Kernel panic - not syncing: Fatal exception
> Kernel Offset: disabled
> ---[ end Kernel panic - not syncing: Fatal exception ]---
>
> You're calling folio_end_read(folio, true) for a folio which is already
> marked uptodate!  I haven't looked through your patch to see what the
> problem is yet.  Very reproducible, you only have to run generic/008
> with a 1kB blocksize XFS.  And CONFIG_VM_DEBUG set, of course.

Ok, I see how we're getting into this state. For the case where all
the blocks get zeroed instead of read in, the
iomap_set_range_uptodate() call could have also called
folio_mark_uptodate() already.

This fixes the issue:
+++ b/fs/iomap/buffered-io.c
@@ -423,7 +423,9 @@ static void iomap_read_end(struct folio *folio,
size_t bytes_submitted)
                spin_lock_irq(&ifs->state_lock);
                if (!ifs->read_bytes_pending) {
                        WARN_ON_ONCE(bytes_submitted);
-                       end_read = true;
+                       spin_unlock_irq(&ifs->state_lock);
+                       folio_unlock(folio);
+                       return;
                } else {

I'll submit this fix to Christian's branch.

Thanks for giving the repro.