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

["David Hildenbrand (Arm)" <david@kernel.org>]

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?

> 
> BTW, this conflicts with Patch 12 from Nico’s khugepaged for mTHP patchset.

Right. I guess it can be handled.

-- 
Cheers,

David