Re: [PATCH RFC v2] btrfs: disable direct reads to avoid dirty folios without fs knowing

On Thu, Jul 23, 2026 at 12:17 PM Qu Wenruo <[email protected]> wrote:
>
>
>
> 在 2026/7/23 20:08, Filipe Manana 写道:
> > On Thu, Jul 23, 2026 at 7:16 AM Qu Wenruo <[email protected]> wrote:
> >>
> >> There is a bug report that a reproducer which doing the following
> >> workloads in two threads:
> >>
> >> - Direct read into memory mapped from page cache
> >> - Sync the above range
> >>
> >> This can lead to dirty folios without fs knowing, this can be a huge
> >
> > Can you please explain where, and why, the folio is dirtied?
>
>
> btrfs_check_read_bio()
> |- __iomap_dio_bio_end_io() from btrfs_bio_end_io()
>     |- bio_check_pages_dirty()
>        |- bio_dirty_fn()
>           |- bio_release_pages(bio, true)
>              |- __bio_release_pages(bio, mark_dirty == true)
>                 |- folio_mark_dirty()
>
> At least this is the one from the reproducer.

That should be part of the change log, it's not obvious at all.
I also wonder why a direct IO read dirties the folio if it's not
supposed to change file data.

>
> Thanks,
> Qu
>
> >
> > How does a direct IO read dirties a folio or a sync_file_range(2) call?
> >
> > Thanks.
> >
> >> problem for btrfs, as even on the very basic bs == ps cases without
> >> large folios, such reproducer can screw up the ordered extent accounting
> >> already:
> >>
> >>   ------------[ cut here ]------------
> >>   WARNING: fs/btrfs/ordered-data.c:390 at can_finish_ordered_extent.isra.0+0x56/0x1f0 [btrfs], CPU#1: kworker/u42:0/68
> >>   CPU: 1 UID: 0 PID: 68 Comm: kworker/u42:0 Tainted: G            E       7.2.0-rc4-custom+ #415 PREEMPT(full)  74dbeafab12c410178747d5bec9fb200ae56949f
> >>   Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 02/02/2022
> >>   Workqueue: btrfs-endio simple_end_io_work [btrfs]
> >>   RIP: 0010:can_finish_ordered_extent.isra.0+0x56/0x1f0 [btrfs]
> >>   Call Trace:
> >>    <TASK>
> >>    btrfs_finish_ordered_extent+0x39/0xd0 [btrfs 7d1ffd99bb9696179883f257e5fd838e2d1a0ca3]
> >>    end_bbio_data_write+0x1ff/0x280 [btrfs 7d1ffd99bb9696179883f257e5fd838e2d1a0ca3]
> >>    btrfs_bio_end_io+0x76/0xf0 [btrfs 7d1ffd99bb9696179883f257e5fd838e2d1a0ca3]
> >>    process_one_work+0x198/0x380
> >>    worker_thread+0x1c8/0x330
> >>    kthread+0xee/0x120
> >>    ret_from_fork+0x28f/0x310
> >>    ret_from_fork_asm+0x11/0x20
> >>    </TASK>
> >>   ---[ end trace 0000000000000000 ]---
> >>   BTRFS critical (device dm-3): bad ordered extent accounting, root=5 ino=257 OE offset=3465216 OE len=2826240 to_dec=319488 left=135168
> >>
> >> Unfortunately we have removed cow fixup mechanism, which is to work
> >> around such dirty folios by re-dirtying them and reserve space for them,
> >> across several kernel releases, meaning we can not easily revert a
> >> single commit to bring it back.
> >> And without doubt, that old cow fixup mechanism is not support larger
> >> folios.
> >>
> >> As a hot fix, disable btrfs direct reads for non-experimental builds for
> >> now, so this can buy some time before we find out a proper way to address
> >> this.
> >>
> >> Reported-by: Christian Borntraeger <[email protected]>
> >> Link: https://lore.kernel.org/linux-btrfs/[email protected]/
> >> Signed-off-by: Qu Wenruo <[email protected]>
> >> ---
> >> Changelog:
> >> v2:
> >> - Fallback to buffered IO to avoid failing existing direct IO users
> >>
> >> - Allow direct writes
> >>
> >> Reason for RFC:
> >> I'm not 100% sure if disabling direct IOs can fill all the holes.
> >>
> >> We still allow mmapping page cache into user spaces, thus I'm not sure
> >> if this is the only hole.
> >> ---
> >>   fs/btrfs/direct-io.c | 5 +++++
> >>   1 file changed, 5 insertions(+)
> >>
> >> diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
> >> index ed1779ccb4de..32b29a4faaf2 100644
> >> --- a/fs/btrfs/direct-io.c
> >> +++ b/fs/btrfs/direct-io.c
> >> @@ -1093,6 +1093,11 @@ ssize_t btrfs_direct_read(struct kiocb *iocb, struct iov_iter *to)
> >>          if (check_direct_read(inode_to_fs_info(inode), to, iocb->ki_pos))
> >>                  return 0;
> >>
> >> +#ifndef CONFIG_BTRFS_EXPERIMENTAL
> >> +       /* To avoid dirty folios without fs knowing through */
> >> +       return 0;
> >> +#endif
> >> +
> >>          btrfs_inode_lock(BTRFS_I(inode), BTRFS_ILOCK_SHARED);
> >>   again:
> >>          /*
> >> --
> >> 2.54.0
> >>
> >>
> >
>