Re: [PATCH 0/4] btrfs: initial subpage support for zoned devices

[Qu Wenruo <wqu@suse.com>]

Ping?

I know this is a very niche scenario (subpage + zoned), and the change 
itself looks very scary, but the change should be safe for non-subpage 
routine (as the new lock all delalloc ranges covers the page would still 
at most lock one delalloc range for normal cases).

Furthermore without this series, there seems be no proper way to support 
subpage + zoned, unless we do a much larger change to merge 
extent_writepage_io() into run_delalloc_range() (which I believe it's 
still needed, but can be done in the future).

Thanks,
Qu

在 2024/2/19 16:38, Qu Wenruo 写道:
> [REPO]
> https://github.com/adam900710/linux/tree/subpage_delalloc
> 
> Please fetch the whole series for testing, as it relies on 3 submitted
> patches.
> 
> [BUG]
> When running subpage btrfs (sectorsize < PAGE_SIZE) with zoned device,
> btrfs can easily crash (with CONFIG_BTRFS_ASSERT enabled) with an
> ASSERT():
> 
>   assertion failed: block_start != EXTENT_MAP_HOLE, in fs/btrfs/extent_io.c:1384
>   ------------[ cut here ]------------
>   kernel BUG at fs/btrfs/extent_io.c:1384!
>   CPU: 2 PID: 1711 Comm: fsstress Tainted: G           OE      6.8.0-rc4-custom+ #9
>   Hardware name: QEMU KVM Virtual Machine, BIOS edk2-20231122-12.fc39 11/22/2023
>   pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>   pc : __extent_writepage_io+0x404/0x460 [btrfs]
>   lr : __extent_writepage_io+0x404/0x460 [btrfs]
>   Call trace:
>    __extent_writepage_io+0x404/0x460 [btrfs]
>    extent_write_locked_range+0x16c/0x460 [btrfs]
>    run_delalloc_cow+0x88/0x118 [btrfs]
>    btrfs_run_delalloc_range+0x128/0x228 [btrfs]
>    writepage_delalloc+0xb8/0x178 [btrfs]
>    __extent_writepage+0xc8/0x3a0 [btrfs]
>    extent_write_cache_pages+0x1cc/0x460 [btrfs]
>    extent_writepages+0x8c/0x120 [btrfs]
>    btrfs_writepages+0x18/0x30 [btrfs]
>    do_writepages+0x94/0x1f8
>    filemap_fdatawrite_wbc+0x88/0xc8
>    __filemap_fdatawrite_range+0x6c/0xa8
>    filemap_flush+0x24/0x38
>    btrfs_remap_file_range_prep+0x100/0x1a8 [btrfs]
>    btrfs_remap_file_range+0x2ec/0x448 [btrfs]
>    vfs_copy_file_range+0x4cc/0x520
>    __do_sys_copy_file_range+0xc4/0x2e8
>    __arm64_sys_copy_file_range+0x30/0xd0
>    invoke_syscall+0x78/0x100
>    el0_svc_common.constprop.0+0x48/0xf0
>    do_el0_svc+0x24/0x38
>    el0_svc+0x3c/0x138
>    el0t_64_sync_handler+0x120/0x130
>    el0t_64_sync+0x194/0x198
>   Code: 9108c021 90000be0 913d8000 9402bfad (d4210000)
>   ---[ end trace 0000000000000000 ]---
> 
> [CAUSE]
> There are several problems involved in this case:
> 
> - __extent_writepage_io() would always try writeback the whole page
> - extent_write_locked_range() would only lock the first delalloc range
> 
> This two limits combined, result the following page cache layout to
> cause the problem:
> 
>       0     4K     8K    12K    16K
>       |/////|      |/////|
> 
> - btrfs_run_delalloc_range() called on the above page
> - run_dealloc_cow() ran for range [0, 4K)
> - __extent_writepage_io() called for the whole page
> - __extent_writepage_io() submitted bio for [0, 4K)
> - __extent_writepage_io() try to submit IO for [8K, 12K)
>    But this range has no OE covered, and the existing extent map is a
>    hole, and triggered the ASSERT().
> 
>    Even if we ignore the ASSERT(), we would still hit other problems.
> 
> - run_delalloc_cow() would unlock the full page.
> - btrfs_run_delalloc_range() called again on the page
>    The page is no longer locked, triggering another ASSERT() on
>    PageLocked.
> 
> [FIX]
> This series would try to fix the problem by:
> 
> - making __extent_writepage_io() to only write the specified range
> - making writepage_delalloc() to lock all delalloc range first
>    Then btrfs_run_delalloc_range() for each locked range.
> 
>    This patch is a temporaray solution, until needed subpage interfaces
>    are introduced, and allowing me to do extra testing to make sure the
>    lock-in-one-go behavior is safe.
> 
>    This is a preparation for allowing subpage delalloc async submission.
> 
> - adding subpage interfaces to go through all subpage locked ranges
> 
> - using new subpage interfaces to make sure the full page is only
>    unlocked when the last writer lock owner is releasing the lock
> 
> But please note, this fix is only for the above mentioned problems.
> There can still be other problems related to zoned+subpage, but at least
> we won't trigger ASSERT()s and crash.
> 
> Qu Wenruo (4):
>    btrfs: make __extent_writepage_io() to write specified range only
>    btrfs: lock subpage ranges in one go for writepage_delalloc()
>    btrfs: subpage: introduce helpers to handle subpage delalloc locking
>    btrfs: migrate writepage_delalloc() to use subpage helpers
> 
>   fs/btrfs/extent_io.c |  95 +++++++++++++++++++++++-----
>   fs/btrfs/subpage.c   | 144 +++++++++++++++++++++++++++++++++++++++++--
>   fs/btrfs/subpage.h   |  10 ++-
>   3 files changed, 228 insertions(+), 21 deletions(-)
>