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

Dev Jain
2024-12-19T07:59:29Z

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,