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

[Zi Yan <ziy@nvidia.com>]

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

> On 4/13/26 22:42, Zi Yan wrote:
>> On 13 Apr 2026, at 16:33, Matthew Wilcox wrote:
>>
>>> On Mon, Apr 13, 2026 at 03:20:23PM -0400, Zi Yan wrote:
>>>> After READ_ONLY_THP_FOR_FS Kconfig is removed, this check becomes dead
>>>> code.
>>>>
>>>> This changes hugepage_pmd_enabled() semantics. Previously, with
>>>> READ_ONLY_THP_FOR_FS enabled, hugepage_pmd_enabled() returned true whenever
>>>> /sys/kernel/mm/transparent_hugepage/enabled was set to "always" or
>>>> "madvise".
>>>>
>>>> After this change, hugepage_pmd_enabled() is governed only by the anon and
>>>> shmem PMD THP controls. As a result, khugepaged collapse for file-backed
>>>> folios no longer runs unconditionally under the top-level THP setting, and
>>>> now depends on the anon/shmem PMD configuration.
>>>
>>> This seems like it'll turn off khugepaged too easily.  I would have
>>> thought we'd want:
>>>
>>> -	if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>>> -	    hugepage_global_enabled())
>>> +	if (hugepage_global_enabled())
>>>  		return true;
>>
>
> 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.

>>
>>>
>>> ... or maybe this whole thing could be simplified?
>>
>> 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.

>
> 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.

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.

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


Best Regards,
Yan, Zi