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

[Wang Yugui <wangyugui@e16-tech.com>]

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.

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

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