Would it be possible to add FGP_FIXED_ORDER for __filemap_get_folio()?

Would it be possible to add FGP_FIXED_ORDER for __filemap_get_folio()?

[Qu Wenruo]

[-- Attachment #1.1.1: Type: text/plain, Size: 1331 bytes --]

Hi,

I'm recently converting btrfs (for now only metadata) to go larger 
(order > 0) folios, and so far it looks pretty good (can already pass 
local fstests runs).

But to achieve that btrfs metadata is not utilizing 
__filemap_get_folio() at all, but manually allocating larger folios, 
then attaching them to the filemap.

This is due to 2 factors:

- We want fixed folio size
- We have our own fallback path

   The whole idea is go large or go bust, either we got a large folio
   matching our request exactly, or we fall back to the old per-page
   allocation (the cross-page handling infrastructure is here anyway).

AFAIK the main block is the folio allocation loop for larger folios in 
__filemap_get_folio().
It would go NORETRY and NOWARN, and after failure reduce the order and 
retry until order is 0.

But if a filesystem really wants to support multi-page block size (eg, 
4K page size with 16K block size), the developers may really want to 
push all the work to folio layer, without handling all the cross-page 
hassle.

I'm wondering if it's possible (well, my naive brain would say it's 
pretty easy) to add something like FGP_FIXED_ORDER?
And what can be the possible pitfalls?
(Too many ENOMEM failure even withou NORETRY? Or just no real world 
users so far?)

Thanks,
Qu

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7027 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: Would it be possible to add FGP_FIXED_ORDER for __filemap_get_folio()?

[Matthew Wilcox]

On Wed, Dec 13, 2023 at 08:52:47AM +1030, Qu Wenruo wrote:
> I'm recently converting btrfs (for now only metadata) to go larger (order >
> 0) folios, and so far it looks pretty good (can already pass local fstests
> runs).
> 
> But to achieve that btrfs metadata is not utilizing __filemap_get_folio() at
> all, but manually allocating larger folios, then attaching them to the
> filemap.
> 
> This is due to 2 factors:
> 
> - We want fixed folio size
> - We have our own fallback path
> 
>   The whole idea is go large or go bust, either we got a large folio
>   matching our request exactly, or we fall back to the old per-page
>   allocation (the cross-page handling infrastructure is here anyway).

In the happy case, are all folios attached to the mapping of the same
order?  Or might you have one folio of eg, order-5 and another of
order-2 because they're different types of metadata?

I ask because we have patches out (not merged yet) that allow for
setting min/max order, and what you're asking for sounds like it could
be accommodated by that.

Re: Would it be possible to add FGP_FIXED_ORDER for __filemap_get_folio()?

[Qu Wenruo]

[-- Attachment #1.1.1: Type: text/plain, Size: 2134 bytes --]



On 2023/12/13 15:07, Matthew Wilcox wrote:
> On Wed, Dec 13, 2023 at 08:52:47AM +1030, Qu Wenruo wrote:
>> I'm recently converting btrfs (for now only metadata) to go larger (order >
>> 0) folios, and so far it looks pretty good (can already pass local fstests
>> runs).
>>
>> But to achieve that btrfs metadata is not utilizing __filemap_get_folio() at
>> all, but manually allocating larger folios, then attaching them to the
>> filemap.
>>
>> This is due to 2 factors:
>>
>> - We want fixed folio size
>> - We have our own fallback path
>>
>>    The whole idea is go large or go bust, either we got a large folio
>>    matching our request exactly, or we fall back to the old per-page
>>    allocation (the cross-page handling infrastructure is here anyway).
> 
> In the happy case, are all folios attached to the mapping of the same
> order?

Yep.

> Or might you have one folio of eg, order-5 and another of
> order-2 because they're different types of metadata?

Only happy case order (4 for example), or 0 order for sad cases.

All other cases are not valid.

In the future, we may get rid of sad cases completely, if the ENOMEM 
rate is low enough and we have handled all ENOMEM allocation properly.

> 
> I ask because we have patches out (not merged yet) that allow for
> setting min/max order, and what you're asking for sounds like it could
> be accommodated by that.

For the current usage, we're asking for something like a allowed order 
bitmap. (We only set 1 for order 0 and target order).

But that allowance for order 0 is purely because we already have 
cross-page handling (only for metadata).

For the future usage, I really believe most filesystem won't be happy 
with some random smaller-than-expected orders, which means they need to 
implement their own cross-folio handling.
(If they have already implemented such handling, they would already 
support multi-page block size, which doesn't look true for now)

Although that min/max would still help for the future multi-page data 
support, we would just set the same value for min/max.

Thanks,
Qu

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7027 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]