Re: [RFC PATCH 10/12] khugepaged: Skip PTE range if a larger mTHP is already mapped

[Dev Jain <dev.jain@arm.com>]

On 19/12/24 9:10 am, John Hubbard wrote:
> On 12/18/24 1:34 AM, Dev Jain wrote:
>> On 18/12/24 1:06 pm, Ryan Roberts wrote:
>>> On 16/12/2024 16:51, Dev Jain wrote:
>>>> We may hit a situation wherein we have a larger folio mapped. It is 
>>>> incorrect
>>>> to go ahead with the collapse since some pages will be unmapped, 
>>>> leading to
>>>> the entire folio getting unmapped. Therefore, skip the 
>>>> corresponding range.
> ...
>>> It would be good if you can spell out the desired policy when 
>>> khugepaged hits
>>> partially unmapped large folios and unaligned large folios. I think 
>>> the simple
>>> approach is to always collapse them to fully mapped, aligned folios 
>>> even if the
>>> resulting order is smaller than the original. But I'm not sure 
>>> that's definitely
>>> going to always be the best thing.
>>>
>>> Regardless, I'm struggling to understand the logic in this patch. 
>>> Taking the
>>> order of a folio based on having hit one of it's pages says anything 
>>> about
>>> whether the whole of that folio is mapped or not or it's alignment. 
>>> And it's not
>>> clear to me how we would get to a situation where we are scanning 
>>> for a lower
>>> order and find a (fully mapped, aligned) folio of higher order in 
>>> the first place.
>>>
>>> Let's assume the desired policy is that khugepaged should always 
>>> collapse to
>>> naturally aligned large folios. If there happens to be an existing 
>>> aligned
>>> order-4 folio that is fully mapped, we will identify that for 
>>> collapse as part
>>> of the scan for order-4. At that point, we should just notice that 
>>> it is already
>>> an aligned order-4 folio and bypass collapse. Of course we may have 
>>> already
>>> chosen to collapse it into a higher order, but we should definitely 
>>> not get to a
>>> lower order before we notice it.
>>>
>>> Hmm... I guess if the sysfs thp settings have been changed then 
>>> things could get
>>> spicy... if order-8 was previously enabled and we have an order-8 
>>> folio, then it
>>> get's disabled and khugepaged is scanning for order-4 (which is 
>>> still enabled)
>>> then hits the order-8; what's the expected policy? rework into 2 
>>> order-4 folios
>>> or leave it as as single order-8?
>>
>> Exactly, sorry, I should have made it clear in the patch description 
>> that I am
>> handling the following scenario: there is a long running system on 
>> which we are
>> using order-8 folios, and now we decide to downgrade to order-4. Will 
>> it be a
>> good idea to take the pain of splitting order-8 to 16 order-4 folios? 
>> This should
>> be a rare situation in the first place, so I have currently decided 
>> to ignore the
>> folios set up by the previous sysfs setting and only focus on 
>> collapsing fresh memory.
>>
>> Thinking again, a sys-admin deciding to downgrade order of folios, 
>> should do that in
>> the hopes of reducing internal fragmentation or increasing swap speed 
>> etc, so it makes
>> sense to shatter large folios....maybe we can have a sysfs tunable 
>> for this?
>
> Maybe we should not support it (at runtime) at all. We are trying to 
> build
> systems that don't require incredibly detailed sysadmin involvement, and
> this level of tweaking qualifies, thoroughly, as "incredibly detailed
> sysadmin micromanagement", imho.

Ryan pointed out one thing: what about unaligned, or partially mapped large
folios? For the previous sysfs settings, it may happen that we have an 
unaligned
order-8 folio, let us say it got unaligned due to mremap(). Then it is a 
good
idea to start from the order-4 aligned page and start collapsing memory so
that we can take advantage of the contig bit. Otherwise if it is a 
fully-mapped
aligned order-8 folio, then we anyways are abusing the contig bit advantage
so collapsing is pointless.
>
> Apologies for not having gone through the series in detail yet, but this
> point jumped out at me.
>
> thanks,