Re: [PATCH] btrfs: do not clear page dirty at extent_write_cache_pages()

[Qu Wenruo <wqu@suse.com>]

在 2024/5/20 15:34, Qu Wenruo 写道:
> 
> 
> 在 2024/5/20 13:00, Qu Wenruo 写道:
>> [PROBLEM]
>> Currently we call folio_clear_dirty_for_io() for the locked dirty folio
>> inside extent_write_cache_pages().
>>
>> However this call is not really subpage aware, it's from the older days
>> where one page can only have one sector.
>>
>> But with nowadays subpage support, we can have multiple sectors inside
>> one page, thus if we clear the whole page dirty flag, it would make the
>> subpage and page dirty flags desynchronize.
>>
>> Thankfully this is not a big deal as our current subpage routine always
>> call __extent_writepage_io() for all the subpage dirty ranges, thus it
>> would ensure there is no subpage range dirty left.
>>
>> [FIX]
>> So here we just drop the folio_clear_dirty_for_io() call, and let
>> __extent_writepage_io() and extent_clear_unlock_delalloc() (which is for
>> compression path) to handle the dirty page and subapge clearing.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Please drop the patch.
> 
> Weirdly with this one, generic/027 would hang on locking the page...

More weirdly, this only happens for aarch64 subpage cases...

> 
> Thanks,
> Qu
>> ---
>> This patch is independent from the subpage zoned fixes, thus it can be
>> applied either before or after the subpage zoned fixes.
>> ---
>>   fs/btrfs/extent_io.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index 7275bd919a3e..a8fc0fcfa69f 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -2231,8 +2231,7 @@ static int extent_write_cache_pages(struct 
>> address_space *mapping,
>>                   folio_wait_writeback(folio);
>>               }
>> -            if (folio_test_writeback(folio) ||
>> -                !folio_clear_dirty_for_io(folio)) {
>> +            if (folio_test_writeback(folio)) {
>>                   folio_unlock(folio);
>>                   continue;
>>               }
>