On 22/08/25 12:29 pm, Baolin Wang wrote:


On 2025/8/21 22:18, Lorenzo Stoakes wrote:
On Tue, Aug 19, 2025 at 07:42:02AM -0600, Nico Pache wrote:
From: Baolin Wang <baolin.w...@linux.alibaba.com>

When only non-PMD-sized mTHP is enabled (such as only 64K mTHP enabled),

I don't think this example is very useful, probably just remove it.

Also 'non-PMD-sized mTHP' implies there is such a thing as PMD-sized mTP :)

we should also allow kicking khugepaged to attempt scanning and collapsing

What is kicking? I think this should be rephrased to something like 'we should
also allow khugepaged to attempt scanning...'

64K mTHP. Modify hugepage_pmd_enabled() to support mTHP collapse, and

64K mTHP -> "of mTHP ranges". Put the 'Modify...' bit in a new paragraph to
be clear.

while we are at it, rename it to make the function name more clear.

To make this clearer let me suggest:

    In order for khugepaged to operate when only mTHP sizes are
    specified in sysfs, we must modify the predicate function that
    determines whether it ought to run to do so.

    This function is currently called hugepage_pmd_enabled(), this
    patch renames it to hugepage_enabled() and updates the logic to
    check to determine whether any valid orders may exist which would
    justify khugepaged running.

Thanks. This looks good to me.

Signed-off-by: Baolin Wang <baolin.w...@linux.alibaba.com>
Signed-off-by: Nico Pache <npa...@redhat.com>

---
  mm/khugepaged.c | 20 ++++++++++----------
  1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 2cadd07341de..81d2ffd56ab9 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -430,7 +430,7 @@ static inline int collapse_test_exit_or_disable(struct mm_struct *mm)
          mm_flags_test(MMF_DISABLE_THP_COMPLETELY, mm);
  }

-static bool hugepage_pmd_enabled(void)
+static bool hugepage_enabled(void)
  {
      /*
       * We cover the anon, shmem and the file-backed case here; file-backed
@@ -442,11 +442,11 @@ static bool hugepage_pmd_enabled(void)

The comment above this still references PMD-sized, please make sure to update comments when you change the described behaviour, as it is now incorrect:

    /*
     * We cover the anon, shmem and the file-backed case here; file-backed      * hugepages, when configured in, are determined by the global control.
     * Anon pmd-sized hugepages are determined by the pmd-size control.
     * Shmem pmd-sized hugepages are also determined by its pmd-size control,
     * except when the global shmem_huge is set to SHMEM_HUGE_DENY.
     */

Please correct this.

Sure. How about:

/*
 * We cover the anon, shmem and the file-backed case here; file-backed
 * hugepages, when configured in, are determined by the global control.
 * Anon hugepages are determined by its per-size mTHP control.
 * Shmem pmd-sized hugepages are also determined by its pmd-size control,
 * except when the global shmem_huge is set to SHMEM_HUGE_DENY.
 */

Looks good, had done something similar in my version.



      if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
          hugepage_global_enabled())
          return true;
-    if (test_bit(PMD_ORDER, &huge_anon_orders_always))
+    if (READ_ONCE(huge_anon_orders_always))
          return true;
-    if (test_bit(PMD_ORDER, &huge_anon_orders_madvise))
+    if (READ_ONCE(huge_anon_orders_madvise))
          return true;
-    if (test_bit(PMD_ORDER, &huge_anon_orders_inherit) &&
+    if (READ_ONCE(huge_anon_orders_inherit) &&
          hugepage_global_enabled())

I guess READ_ONCE() is probably sufficient here as memory ordering isn't
important here, right?

Yes, I think so.

Reply via email to