Re: [PATCH v2 0/2] btrfs: migrate extent_buffer::pages[] to folio and more cleanups

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

On 2023/12/5 02:56, David Sterba wrote:
> On Fri, Dec 01, 2023 at 04:36:53PM +1030, Qu Wenruo wrote:
>> [CHANGELOG]
>> v2:
>> - Adda new patch to do more cleanups for metadata page pointers usage
>>
>> This patchset would migrate extent_buffer::pages[] to folios[], then
>> cleanup the existing metadata page pointer usage to proper folios ones.
>>
>> This cleanup would help future higher order folios usage for metadata in
>> the following aspects:
>>
>> - No more need to iterate through the remaining pages for page flags
>>    We just call folio_set/mark/start_*() helpers, for the single folio.
>>    The helper would only set the flag (mostly for the leading page).
>>
>> - Single bio_add_folio() call for the whole eb
>>
>> - Better filio helpers naming
>>    PageUptodate() compared to folio_test_uptodate().
>>
>> The first patch would do a very simple conversion, then the 2nd patch do
>> the prepartion for the higher order folio situation.
>>
>> There are two locations which won't be converted to folios yet:
>>
>> - Subpage code
>>    There is no meaning to support higher order folio for subpage.
>>    The two conditions are just conflicting with each other.
>>
>> - Data page pointers
>>    That would be more useful in the future, before we going to support
>>    multi-page sectorsize.
>>
>> However the 2nd one would also add a new corner case:
>>
>> - Order mismatch in filemap and eb folios
>>    Unforatunately I don't have a better plan other than re-allocate the
>>    folios to the same order.
>>    Maybe in the future we would have better ways to handle it? Like
>>    migrating the pages to the higher order one?
>
> As long as it's a no-op for now this is OK, we can do the higher order
> allocation for eb pages afterwards.
>
Yep, it won't cause any problem for now.

Although this corner case is making me wondering if the new
alloc-then-attach is really any better than the original
alloc-and-attach solution.

If the mm (filemap) layer can allow us to allocate larger folios, it may
be much simpler.
The current code only needs to setlarge folio support for the mapping,
then go with high order fpg_flags.
The filemap code is already doing the retry and unalignment check.

But the existing filemap code would also try to reduce the order, which
can lead to other problems, like one extent buffer with multiple
different order folios.
Meanwhile alloc-and-attach gives us full control on the order, thus
allowing all-or-none (one single large folio, or all single page ones)
solution required by the 2nd patch.

Anyway, I would continue with the current alloc-then-attach method to
experiment the higher order folios allocation first to find out all the
pitfalls first.

Thanks,
Qu