linux-kernel - Re: [PATCH v10 10/13] khugepaged: kick khugepaged for enabling none-PMD-sized mTHPs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2fdeeb22-e511-4495-b4ad-2b26a5fcbb00@arm.com>
Date: Fri, 22 Aug 2025 13:06:12 +0530
From: Dev Jain <dev.jain@....com>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Nico Pache <npache@...hat.com>
Cc: linux-mm@...ck.org, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
 david@...hat.com, ziy@...dia.com, Liam.Howlett@...cle.com,
 ryan.roberts@....com, corbet@....net, rostedt@...dmis.org,
 mhiramat@...nel.org, mathieu.desnoyers@...icios.com,
 akpm@...ux-foundation.org, baohua@...nel.org, willy@...radead.org,
 peterx@...hat.com, wangkefeng.wang@...wei.com, usamaarif642@...il.com,
 sunnanyong@...wei.com, vishal.moola@...il.com,
 thomas.hellstrom@...ux.intel.com, yang@...amperecomputing.com,
 kirill.shutemov@...ux.intel.com, aarcange@...hat.com, raquini@...hat.com,
 anshuman.khandual@....com, catalin.marinas@....com, tiwai@...e.de,
 will@...nel.org, dave.hansen@...ux.intel.com, jack@...e.cz, cl@...two.org,
 jglisse@...gle.com, surenb@...gle.com, zokeefe@...gle.com,
 hannes@...xchg.org, rientjes@...gle.com, mhocko@...e.com,
 rdunlap@...radead.org, hughd@...gle.com
Subject: Re: [PATCH v10 10/13] khugepaged: kick khugepaged for enabling
 none-PMD-sized mTHPs


On 22/08/25 12:29 pm, Baolin Wang wrote:
>
>
> On 2025/8/21 22:18, Lorenzo Stoakes wrote:
>> On Tue, Aug 19, 2025 at 07:42:02AM -0600, Nico Pache wrote:
>>> From: Baolin Wang <baolin.wang@...ux.alibaba.com>
>>>
>>> When only non-PMD-sized mTHP is enabled (such as only 64K mTHP 
>>> enabled),
>>
>> I don't think this example is very useful, probably just remove it.
>>
>> Also 'non-PMD-sized mTHP' implies there is such a thing as PMD-sized 
>> mTP :)
>>
>>> we should also allow kicking khugepaged to attempt scanning and 
>>> collapsing
>>
>> What is kicking? I think this should be rephrased to something like 
>> 'we should
>> also allow khugepaged to attempt scanning...'
>>
>>> 64K mTHP. Modify hugepage_pmd_enabled() to support mTHP collapse, and
>>
>> 64K mTHP -> "of mTHP ranges". Put the 'Modify...' bit in a new 
>> paragraph to
>> be clear.
>>
>>> while we are at it, rename it to make the function name more clear.
>>
>> To make this clearer let me suggest:
>>
>>     In order for khugepaged to operate when only mTHP sizes are
>>     specified in sysfs, we must modify the predicate function that
>>     determines whether it ought to run to do so.
>>
>>     This function is currently called hugepage_pmd_enabled(), this
>>     patch renames it to hugepage_enabled() and updates the logic to
>>     check to determine whether any valid orders may exist which would
>>     justify khugepaged running.
>
> Thanks. This looks good to me.
>
>>> Signed-off-by: Baolin Wang <baolin.wang@...ux.alibaba.com>
>>> Signed-off-by: Nico Pache <npache@...hat.com>
>>
>>> ---
>>>   mm/khugepaged.c | 20 ++++++++++----------
>>>   1 file changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index 2cadd07341de..81d2ffd56ab9 100644
>>> --- a/mm/khugepaged.c
>>> +++ b/mm/khugepaged.c
>>> @@ -430,7 +430,7 @@ static inline int 
>>> collapse_test_exit_or_disable(struct mm_struct *mm)
>>>           mm_flags_test(MMF_DISABLE_THP_COMPLETELY, mm);
>>>   }
>>>
>>> -static bool hugepage_pmd_enabled(void)
>>> +static bool hugepage_enabled(void)
>>>   {
>>>       /*
>>>        * We cover the anon, shmem and the file-backed case here; 
>>> file-backed
>>> @@ -442,11 +442,11 @@ static bool hugepage_pmd_enabled(void)
>>
>> The comment above this still references PMD-sized, please make sure 
>> to update
>> comments when you change the described behaviour, as it is now 
>> incorrect:
>>
>>     /*
>>      * We cover the anon, shmem and the file-backed case here; 
>> file-backed
>>      * hugepages, when configured in, are determined by the global 
>> control.
>>      * Anon pmd-sized hugepages are determined by the pmd-size control.
>>      * Shmem pmd-sized hugepages are also determined by its pmd-size 
>> control,
>>      * except when the global shmem_huge is set to SHMEM_HUGE_DENY.
>>      */
>>
>> Please correct this.
>
> Sure. How about:
>
> /*
>  * We cover the anon, shmem and the file-backed case here; file-backed
>  * hugepages, when configured in, are determined by the global control.
>  * Anon hugepages are determined by its per-size mTHP control.
>  * Shmem pmd-sized hugepages are also determined by its pmd-size control,
>  * except when the global shmem_huge is set to SHMEM_HUGE_DENY.
>  */

Looks good, had done something similar in my version.


>
>>>       if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>>>           hugepage_global_enabled())
>>>           return true;
>>> -    if (test_bit(PMD_ORDER, &huge_anon_orders_always))
>>> +    if (READ_ONCE(huge_anon_orders_always))
>>>           return true;
>>> -    if (test_bit(PMD_ORDER, &huge_anon_orders_madvise))
>>> +    if (READ_ONCE(huge_anon_orders_madvise))
>>>           return true;
>>> -    if (test_bit(PMD_ORDER, &huge_anon_orders_inherit) &&
>>> +    if (READ_ONCE(huge_anon_orders_inherit) &&
>>>           hugepage_global_enabled())
>>
>> I guess READ_ONCE() is probably sufficient here as memory ordering isn't
>> important here, right?
>
> Yes, I think so.