Re: [PATCH v2 4/4] btrfs: keep folios locked inside run_delalloc_nocow()

Qu Wenruo
2025-08-04T22:53:44Z

在 2025/8/5 08:06, Wang Yugui 写道:
> Hi,
>
>> [BUG]
>> There is a very low chance that DEBUG_WARN() inside
>> btrfs_writepage_cow_fixup() can be triggered when
>> CONFIG_BTRFS_EXPERIMENTAL is enabled.
>>
>> This only happens after run_delalloc_nocow() failed.
>>
>> Unfortunately I haven't hit it for a while thus no real world dmesg for
>> now.
>>
>> [CAUSE]
>> There is a race window where after run_delalloc_nocow() failed, error
>> handling can race with writeback thread.
>>
>> Before we hit run_delalloc_nocow(), there is an inode with the following
>> dirty pages: (4K page size, 4K block size, no large folio)
>>
>> 0 4K 8K 12K 16K
>> |/////////|///////////|///////////|////////////|
>>
>> The inode also have NODATACOW flag, and the above dirty range will go
>> through different extents during run_delalloc_range():
>>
>> 0 4K 8K 12K 16K
>> | NOCOW | COW | COW | NOCOW |
>>
>> The race happen like this:
>>
>> writeback thread A | writeback thread B
>> ----------------------------------+--------------------------------------
>> Writeback for folio 0 |
>> run_delalloc_nocow() |
>> |- nocow_one_range() |
>> | For range [0, 4K), ret = 0 |
>> | |
>> |- fallback_to_cow() |
>> | For range [4K, 8K), ret = 0 |
>> | Folio 4K *UNLOCKED* |
>> | | Writeback for folio 4K
>> |- fallback_to_cow() | extent_writepage()
>> | For range [8K, 12K), failure | |- writepage_delalloc()
>> | | |
>> |- btrfs_cleanup_ordered_extents()| |
>> |- btrfs_folio_clear_ordered() | |
>> | Folio 0 still locked, safe | |
>> | | | Ordered extent already allocated.
>> | | | Nothing to do.
>> | | |- extent_writepage_io()
>> | | |- btrfs_writepage_cow_fixup()
>> |- btrfs_folio_clear_ordered() | |
>> Folio 4K hold by thread B, | |
>> UNSAFE! | |- btrfs_test_ordered()
>> | | Cleared by thread A,
>> | |
>> | |- DEBUG_WARN();
>>
>> This is only possible after run_delalloc_nocow() failure, as
>> cow_file_range() will keep all folios and io tree range locked, until
>> everything is finished or after error handling.
>>
>> The root cause is we allow fallback_to_cow() and nocow_one_range() to
>> unlock the folios after a successful run, so that during error handling
>> we're no longer safe to use btrfs_cleanup_ordered_extents() as the
>> folios are already unlocked.
>>
>> [FIX]
>> - Make fallback_to_cow() and nocow_one_range() to keep folios locked
>> after a successful run
>>
>> For fallback_to_cow() we can pass COW_FILE_RANGE_KEEP_LOCKED flag
>> into cow_file_range().
>>
>> For nocow_one_range() we have to remove the PAGE_UNLOCK flag from
>> extent_clear_unlock_delalloc().
>>
>> - Unlock folios if everything is fine in run_delalloc_nocow()
>>
>> - Use extent_clear_unlock_delalloc() to handle range [@start,
>> @cur_offset) inside run_delalloc_nocow()
>> Since folios are still locked, we do not need
>> cleanup_dirty_folios() to do the cleanup.
>>
>> extent_clear_unlock_delalloc() with "PAGE_START_WRIBACK |
>> PAGE_END_WRITEBACK" will clear the dirty flags.
>>
>> - Remove cleanup_dirty_folios()
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
> We need a 'Fix:' tag for kernel stable branch.

The problem is, the original error handling behavior is not correct from
the very beginning.

Thus I can not locate a good offending commit.

>
> It seems that this problem does not happen (or less frequency) on kernel
> 6.6.y/6.1.y.

Unfortunately those kernels do not have the proper error handling either.

Thanks,
Qu

>
> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2025/08/05
>
>
>