Re: [PATCH v1 2/2] mm: mTHP stats for pagecache folio allocations

[Ryan Roberts <ryan.roberts@arm.com>]

On 17/07/2024 09:02, David Hildenbrand wrote:
>>> Sorry, busy with other stuff.
>>>
>>> Indicating only what really exists sounds cleaner. But I wonder how we would
>>> want to handle in general orders that are effectively non-existant?
>>
>> I'm not following your distinction between orders that don't "really exist" and
>> orders that are "effectively non-existant".
> 
> I'm questioning whether there should be a distinction at all. We should just
> hide what is either non-existant (not implemented) or non-functional.

Great we are on the same page.

> 
>>
>> I guess the real supported orders are:
>>
>>    anon:
>>      min order: 2
>>      max order: PMD_ORDER
>>    anon-shmem:
>>      min order: 1
>>      max order: MAX_PAGECACHE_ORDER
>>    tmpfs-shmem:
>>      min order: PMD_ORDER <= 11 ? PMD_ORDER : NONE
>>      max order: PMD_ORDER <= 11 ? PMD_ORDER : NONE
>>    file:
>>      min order: 1
>>      max order: MAX_PAGECACHE_ORDER
> 
> That's my understanding. But not sure about anon-shmem really supporting
> order-1, maybe we do.

Oh, I thought we only had the restriction for anon folios now (due to deferred
split queue), so assumed it would just work. With Gavin's
THP_ORDERS_ALL_FILE_DEFAULT change, that certainly implies that shmem must
support order-1. If it doesn't then we we might want to tidy that further.

Baolin, perhaps you can confirm either way?

> 
>>
>> But today, controls and stats are exposed for:
>>
>>    anon:
>>      min order: 2
>>      max order: PMD_ORDER
>>    anon-shmem:
>>      min order: 2
>>      max order: PMD_ORDER
>>    tmpfs-shmem:
>>      min order: PMD_ORDER
>>      max order: PMD_ORDER
>>    file:
>>      min order: Nothing yet (this patch proposes 1)
>>      max order: Nothing yet (this patch proposes MAX_PAGECACHE_ORDER)
>>
>> So I think there is definitely a bug for shmem where the minimum order control
>> should be order-1 but its currently order-2.
> 
> Maybe, did not play with that yet. Likely order-1 will work. (although probably
> of questionable use :) )

You might have to expand on why its of "questionable use". I'd assume it has the
same amount of value as using order-1 for regular page cache pages? i.e. half
the number of objects to manage for the same amount of memory.

> 
>>
>> I also wonder about PUD-order for DAX? We don't currently have a stat/control.
>> If we wanted to add it in future, if we take the "expose all stats/controls for
>> all orders" approach, we would end up extending all the way to PUD-order and all
>> the orders between PMD and PUD would be dummy for all memory types. That really
>> starts to feel odd, so I still favour only populating what's really supported.
> 
> I would go further and say that calling the fsdax thing a THP is borderline
> wrong and we should not expose any new toggles for it that way.
> 
> It really behaves much more like hugetlb folios that can be PTE-mapped ... we
> cannot split these things, and they are not allocated from the buddy. So I
> wouldn't worry about fsdax for now.
> 
> fsdax support for compound pages (now large folios) probably never should have
> been glued to any THP toggle.

Yeah fair enough. I wasn't really arguing for adding any dax controls; I was
just trying to think of examples as to why adding dummy controls might be a bad
idea.

> 
>>
>> I propose to fix shmem (extend down to 1, stop at MAX_PAGECACHE_ORDER) and
>> continue with the approach of "indicating only what really exists" for v2.
>>
>> Shout if you disagree.
> 
> Makes sense.

Excellent. I posted v2, which has these changes, yesterday afternoon. :)