Re: [PATCH 7.2 v2 05/12] mm/khugepaged: remove READ_ONLY_THP_FOR_FS check in hugepage_pmd_enabled()

[Zi Yan]

On 14 Apr 2026, at 14:14, David Hildenbrand (Arm) wrote:

> On 4/14/26 18:30, Zi Yan wrote:
>> On 14 Apr 2026, at 7:02, David Hildenbrand (Arm) wrote:
>>
>>> On 4/13/26 22:42, Zi Yan wrote:
>>>>
>>>>
>>>
>>> I assume such a change should come before patch #4, as it seems to affect
>>> the functionality that depended on CONFIG_READ_ONLY_THP_FOR_FS.
>>
>> If the goal is to have a knob of khugepaged for all files, yes I will move
>> the change before Patch 4.
>>
>>>
>>>> I thought about this, but it means khugepaged is turned on regardless of
>>>> anon and shmem configs. I tend to think the original code was a bug,
>>>> since enabling CONFIG_READ_ONLY_THP_FOR_FS would enable khugepaged all
>>>> the time.
>>>
>>> There might be some FS mapping to collapse? So that makes sense to
>>> some degree.
>>>
>>> I really don't like the side-effects of "/sys/kernel/mm/transparent_hugepage/enabled".
>>> Like, enabling khugepaged+PMD for files.
>>>
>>
>> I am not a fan either, but I was not sure about another sysfs knob.
>>
>
> Yeah, it would be better if we could avoid it. But the dependency on the
> global toggle as it is today is a bit weird.
>
>>>>
>>>>
>>>> Alternatives could be:
>>>> 1. to add a file-backed khhugepaged config, but another sysfs?
>>>
>>> Maybe that would be the time to decouple file THP logic from
>>> hugepage_global_enabled()/hugepage_global_always().
>>>
>>> In particular, as pagecache folio allocation doesn't really care about __thp_vma_allowable_orders() IIRC.
>>>
>>> I'm thinking about something like the following:
>>>
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index b2a6060b3c20..fb3a4fd84fe0 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -184,15 +184,6 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
>>>                                                    forced_collapse);
>>>
>>>         if (!vma_is_anonymous(vma)) {
>>> -               /*
>>> -                * Enforce THP collapse requirements as necessary. Anonymous vmas
>>> -                * were already handled in thp_vma_allowable_orders().
>>> -                */
>>> -               if (!forced_collapse &&
>>> -                   (!hugepage_global_enabled() || (!(vm_flags & VM_HUGEPAGE) &&
>>> -                                                   !hugepage_global_always())))
>>> -                       return 0;
>>> -
>>>                 /*
>>>                  * Trust that ->huge_fault() handlers know what they are doing
>>>                  * in fault path.
>>
>> Looks reasonable.
>
> I don't think there is other interaction with FS and the global toggle
> besides this and the one you are adjusting, right?
>
>>
>>>
>>> Then, we might indeed just want a khugepaged toggle whether to enable it at
>>> all in files. (or just a toggle to disable khugeapged entirely?)
>>>
>>
>> I think hugepage_global_enabled() should be enough to decide whether khugepaged
>> should run or not.
>
> That would also be an option and would likely avoid other toggles.
>
> So __thp_vma_allowable_orders() would allows THPs in any case for FS,
> but hugepage_global_enabled() would control whether khugepaged runs (for
> fs).
>
> It gives less flexibility, but likely that's ok.
>
>>
>> Currently, we have thp_vma_allowable_orders() to filter each VMAs and I do not
>> see a reason to use hugepage_pmd_enabled() to guard khugepaged daemon. I am
>> going to just remove hugepage_pmd_enabled() and replace it with
>> hugepage_global_enabled(). Let me know your thoughts.
>
> Can you send a quick draft of what you have in mind?