Re: [PATCH] btrfs: use FGP_STABLE to wait for folio writeback

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2024/10/15 00:46, David Sterba 写道:
> On Mon, Oct 14, 2024 at 03:06:31PM +1030, Qu Wenruo wrote:
>> __filemap_get_folio() provides the flag FGP_STABLE to wait for
>> writeback.
>>
>> There are two call sites doing __filemap_get_folio() then
>> folio_wait_writeback():
>>
>> - btrfs_truncate_block()
>> - defrag_prepare_one_folio()
>>
>> We can directly utilize that flag instead of manually calling
>> folio_wait_writeback().
>
> We can do that but I'm missing a justification for that. The explicit
> writeback calls are done at a different points than what FGP_STABLE
> does. So what's the difference?
>

TL;DR, it's not safe to read folio before waiting for writeback in theory.

There is a possible race, mostly related to my previous attempt of
subpage partial uptodate support:

              Thread A          |           Thread B
-------------------------------+-----------------------------
extent_writepage_io()          |
|- submit_one_sector()         |
   |- folio_set_writeback()     |
      The folio is partial dirty|
      and uninvolved sectors are|
      not uptodate              |
                                | btrfs_truncate_block()
                                | |- btrfs_do_readpage()
                                |   |- submit_one_folio
                                |      This will read all sectors
                                |      from disk, but that writeback
                                |      sector is not yet finished

In this case, we can read out garbage from disk, since the write is not
yet finished.

This is not yet possible, because we always read out the whole page so
in that case thread B won't trigger a read.


But this already shows the way we wait for writeback is not safe.
And that's why no other filesystems manually wait for writeback after
reading the folio.

Thus I think doing FGP_STABLE is way more safer, and that's why all
other fses are doing this way instead.

Thanks,
Qu