Message-ID: <13b68fa0-8755-43d8-8504-d181c2d46134@gmail.com>
Date: Sun, 11 May 2025 15:08:05 +0100
From: Usama Arif <usamaarif642@...il.com>
To: David Hildenbrand <david@...hat.com>, Zi Yan <ziy@...dia.com>
Cc: Johannes Weiner <hannes@...xchg.org>, Yafang Shao <laoar.shao@...il.com>,
 Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
 shakeel.butt@...ux.dev, riel@...riel.com, baolin.wang@...ux.alibaba.com,
 lorenzo.stoakes@...cle.com, Liam.Howlett@...cle.com, npache@...hat.com,
 ryan.roberts@....com, linux-kernel@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH 0/1] prctl: allow overriding system THP policy to always



On 11/05/2025 09:15, David Hildenbrand wrote:
> On 10.05.25 01:34, Zi Yan wrote:
>> On 9 May 2025, at 18:42, David Hildenbrand wrote:
>>
>>>>>>> - madvise
>>>>>>>     The sysadmin gently encourages the use of THP, but it is only
>>>>>>> enabled when explicitly requested by the application.
>>>>
>>>> And this "user mode" or "manual mode", where applications self-manage
>>>> which parts of userspace they want to enroll.
>>>>
>>>> Both madvise() and unprivileged prctl() should work here as well,
>>>> IMO. There is no policy or security difference between them, it's just
>>>> about granularity and usability.
>>>>
>>>>>>> - never
>>>>>>>      The sysadmin discourages the use of THP, and "its use is only permitted
>>>>>>> with explicit approval" .
>>>>
>>>> This one I don't quite agree with, and IMO conflicts with what David
>>>> is saying as well.
>>>
>>> Yeah ... "never" does not mean "sometimes" in my reality :)
>>>
>>>>
>>>>>> "never" so far means "no thps, no exceptions". We've had serious THP
>>>>>> issues in the past, where our workaround until we sorted out the issue
>>>>>> for affected customers was to force-disable THPs on that system during boot.
>>>>>
>>>>> Right, that reflects the current behavior. What we aim to enhance is
>>>>> by adding the requirement that "its use is only permitted with
>>>>> explicit approval."
>>>>
>>>> I think you're conflating a safety issue with a security issue.
>>>>
>>>> David is saying there can be cases where the kernel is broken, and
>>>> "never" is a production escape hatch to disable the feature until a
>>>> kernel upgrade for the fix is possible. In such a case, it doesn't
>>>> make sense to override this decision based on any sort of workload
>>>> policy, privileged or not.
>>>>
>>>> The way I understand you is that you want enrollment (and/or
>>>> self-management) only for blessed applications. Because you don't
>>>> generally trust workloads in the wild enough to switch the global
>>>> default away from "never", given the semantics of always/madvise.
>>>
>>> Assuming "never" means "never" and "always" means "always" ( crazy, right? :) ), could be make use of "madvise" mode, which essentially means "VM_HUGEPAGE" takes control?
>>>
>>> We'd need
>>>
>>> a) A way to enable THP for a process. Changing the default/vma settings to VM_HUGEPAGE as discussed using a prctl could work.
>>>
>>> b) A way to ignore VM_HUGEPAGE for processes. Maybe the existing prctl to force-disable THPs could work?
>>
>> This means process level control overrides VMA level control, which
>> overrides global control, right?
>>
>> Intuitively, it should be that VMA level control overrides process level
>> control, which overrides global control, namely finer granularly control
>> precedes coarse one. But some apps might not use VMA level control
>> (e.g., madvise) carefully, we want to override that. Maybe ignoring VMA
>> level control is what we want?
> 
> Let's take a step back:
> 
> Current behavior is
> 
> 1) If anybody (global / process / VM) says "never" (never/PR_SET_THP_DISABLE/VM_NOHUGEPAGE), the behavior is "never".

Just to add here to the current behavior for completeness, if we have the global system setting set to never,
but the global hugepage level setting set to madvise, we do still get a THP, i.e. if I have:

[root@vm4 vmuser]# cat /sys/kernel/mm/transparent_hugepage/enabled
always madvise [never]
[root@vm4 vmuser]# cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/enabled
always inherit [madvise] never

And then MADV_HUGEPAGE some region, it still gives me a THP.

> 
> 2) In "madvise" mode, only "VM_HUGEPAGE" gets THP unless PR_SET_THP_DISABLE is set. So per-process overrides per-VMA.
> 
> 3) In "always" mode, everything gets THP unless per-VMA (VM_NOHUGEPAGE) or per-process (PR_SET_THP_DISABLE) disables it.
> 
> 
> Interestingly, PR_SET_THP_DISABLE used to mimic exactly what I proposed for the other direction (change default of VM_HUGEPAGE), except that it wouldn't modify already existing mappings. Worth looking at 1860033237d4. Not sure if that commit was the right call, but it's the semantics we have today.
> 
> That commit notes:
> 
> "It should be noted, that the new implementation makes PR_SET_THP_DISABLE master override to any per-VMA setting, which was not the case previously."
> 
> 
> Whatever we do, we have to be careful to not create more mess or inconsistency.
> 
> Especially, if anybody sets VM_NOHUGEPAGE or PR_SET_THP_DISABLE, we must not use THPs, ever.
> 


I thought I will also summarize what the real world usecases are that we want to solve:

1) global system policy=madvise, process wants "always" policy for itself: We can have different types of workloads stacked on the same host, some of them benefit from always having THPs,
others will incur a regression (either its a performance regression or they are completely memory bound and even a very slight increase in memory will cause them to OOM).
So we want to selectively have "always" set for just those workloads (processes). (This is how workloads are deployed in our (Metas) fleet at this moment.)

2) global system policy=always, process wants "madvise" policy for itself: Same reasoning as 1, just that the host has a different default policy and we don't want the workloads (processes) that regress with always getting THPs to do so.
(We hope this is us (meta) in the future, if a majority of workloads show that they benefit from always, we flip the default host setting to "always" and workloads that regress can opt-out and be "madvise".
New services developed will then be tested with always by default. Always is also the default defconfig option upstream, so I would imagine this is faced by others as well.)

3) global system policy=never, process wants "madvise" policy for itself: This is what Yafang mentioned in [1]. sysadmins dont want to switch the global policy to madvise, but are willing to accept certain processes to madvise.
But David mentioned in [2] that never means no thps, no exceptions and the only way to solve some issues in the past has been to disable THPs completely.

Please feel free to add to the above list. I thought it would be good to list them out so that the solution can be derived with them in mind.

In terms of doing this with prctl, I was able to make prototypes for the 2 approaches that have been discussed:

a) have prctl change how existing and new VMAs have VM_HUGEPAGE set for the process including after fork+exec, as proposed by David. This prototype is available at [3]. This will solve problem 1 discussed above, but I don't think this
approach can be used to solve problems 2 and 3? There isnt a way where we can have a process change VMA setting so that after prctl, all future allocations are on madvise basis and not global policy (even if always). IOW we will need
some change in thp_vma_allowable_orders to have it done on process level basis.

b) have prctl override global policy *only* for hugepages that "inherit" global and only if global is "madvise" or "always". This prototype is available at [4]. The way I did this will achieve usecase 1 and 2, but not 3 (It can very easily be modified to get 3, but didn't do it as there maybe still is a discussion on what should be allowed when global=never?). I do prefer this method as I think it might be simpler overall and achieves both usecases.

[1] https://lore.kernel.org/all/CALOAHbBAVELx-fwyoQUH_ypFvT_Zd5ZLjSkAPXxShgCua8ifpA@mail.gmail.com/
[2] https://lore.kernel.org/all/41e60fa0-2943-4b3f-ba92-9f02838c881b@redhat.com/
[3] https://github.com/uarif1/linux/commit/209373cdeda93a55a699e2eee29f88f4e64ac8a5
[4] https://github.com/uarif1/linux/commit/e85c8edbcb4165c00026f0058b71e85f77da23f4


Thanks,
Usama

Powered by blists - more mailing lists