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
