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

David Hildenbrand (Arm)
2026-04-14T11:02:13Z

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.

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

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

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?)

--
Cheers,

David