Re: [PATCH 12/42] btrfs: make Private2 lifespan more consistent

[Josef Bacik <josef@toxicpanda.com>]

On 4/15/21 1:04 AM, Qu Wenruo wrote:
> Currently btrfs uses page Private2 bit to incidate if we have ordered
> extent for the page range.
> 
> But the lifespan of it is not consistent, during regular writeback path,
> there are two locations to clear the same PagePrivate2:
> 
>      T ----- Page marked Dirty
>      |
>      + ----- Page marked Private2, through btrfs_run_dealloc_range()
>      |
>      + ----- Page cleared Private2, through btrfs_writepage_cow_fixup()
>      |       in __extent_writepage_io()
>      |       ^^^ Private2 cleared for the first time
>      |
>      + ----- Page marked Writeback, through btrfs_set_range_writeback()
>      |       in __extent_writepage_io().
>      |
>      + ----- Page cleared Private2, through
>      |       btrfs_writepage_endio_finish_ordered()
>      |       ^^^ Private2 cleared for the second time.
>      |
>      + ----- Page cleared Writeback, through
>              btrfs_writepage_endio_finish_ordered()
> 
> Currently PagePrivate2 is mostly to prevent ordered extent accounting
> being executed for both endio and invalidatepage.
> Thus only the one who cleared page Private2 is responsible for ordered
> extent accounting.
> 
> But the fact is, in btrfs_writepage_endio_finish_ordered(), page
> Private2 is cleared and ordered extent accounting is executed
> unconditionally.
> 
> The race prevention only happens through btrfs_invalidatepage(), where
> we wait the page writeback first, before checking the Private2 bit.
> 
> This means, Private2 is also protected by Writeback bit, and there is no
> need for btrfs_writepage_cow_fixup() to clear Priavte2.
> 
> This patch will change btrfs_writepage_cow_fixup() to just
> check PagePrivate2, not to clear it.
> The clear will happen either in btrfs_invalidatepage() or
> btrfs_writepage_endio_finish_ordered().
> 
> This makes the Private2 bit easier to understand, just meaning the page
> has unfinished ordered extent attached to it.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef