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

Am 23.07.26 um 08:13 schrieb Qu Wenruo:
> 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
> 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]>


Wow that is a big hammer. Doesnt that break typical usecases (like databases on a file,
qemu/kvm aio+direct image files etc)?

Even worse, this does not fix the other GUP use cases that still exist? So its only
a partial fix. Have you considered to add my RFC patch on top to close another
problem that I can reproduce easily.