On Mon, May 11, 2026 at 12:58:03PM -0600, Nico Pache wrote:
>The following cleanup reworks all the max_ptes_* handling into helper
>functions. This increases the code readability and will later be used to
>implement the mTHP handling of these variables.
>
>With these changes we abstract all the madvise_collapse() special casing
>(dont respect the sysctls) away from the functions that utilize them. And

Nit: s/dont/do not/

>will be used later in this series to cleanly restrict the mTHP collapse
>behavior.
>
>No functional change is intended; however, we are now only reading the
>sysfs variables once per scan, whereas before these variables were being
>read on each loop iteration.
>
>Suggested-by: David Hildenbrand <[email protected]>
>Acked-by: David Hildenbrand (Arm) <[email protected]>
>Acked-by: Usama Arif <[email protected]>
>Signed-off-by: Nico Pache <[email protected]>
>---
> mm/khugepaged.c | 118 +++++++++++++++++++++++++++++++++---------------
> 1 file changed, 82 insertions(+), 36 deletions(-)
>
>diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>index f0e29d5c7b1f..f68853b3caa7 100644
>--- a/mm/khugepaged.c
>+++ b/mm/khugepaged.c
>@@ -348,6 +348,62 @@ static bool pte_none_or_zero(pte_t pte)
>       return pte_present(pte) && is_zero_pfn(pte_pfn(pte));
> }
> 
>+/**
>+ * collapse_max_ptes_none - Calculate maximum allowed none-page or zero-page
>+ * PTEs for the given collapse operation.
>+ * @cc: The collapse control struct
>+ * @vma: The vma to check for userfaultfd
>+ *
>+ * Return: Maximum number of none-page or zero-page PTEs allowed for the
>+ * collapse operation.
>+ */
>+static unsigned int collapse_max_ptes_none(struct collapse_control *cc,
>+              struct vm_area_struct *vma)
>+{
>+      // If the vma is userfaultfd-armed, allow no none-page or zero-page 
>PTEs.
>+      if (vma && userfaultfd_armed(vma))
>+              return 0;
>+      // for MADV_COLLAPSE, allow any none-page or zero-page PTEs.
>+      if (!cc->is_khugepaged)
>+              return HPAGE_PMD_NR;
>+      // For all other cases repect the user defined maximum.
>+      return khugepaged_max_ptes_none;

Nit: kernel code usually uses C-style comments. This could be:

/* For all other cases, respect the user-defined maximum. */

Also, s/repect/respect/.

>+}
>+
>+/**
>+ * collapse_max_ptes_shared - Calculate maximum allowed PTEs that map shared
>+ * anonymous pages for the given collapse operation.
>+ * @cc: The collapse control struct
>+ *
>+ * Return: Maximum number of PTEs that map shared anonymous pages for the
>+ * collapse operation
>+ */
>+static unsigned int collapse_max_ptes_shared(struct collapse_control *cc)
>+{
>+      // for MADV_COLLAPSE, do not restrict the number of PTEs that map shared
>+      // anonymous pages.

Ditto.

>+      if (!cc->is_khugepaged)
>+              return HPAGE_PMD_NR;
>+      return khugepaged_max_ptes_shared;
>+}
>+
>+/**
>+ * collapse_max_ptes_swap - Calculate the maximum allowed non-present PTEs or 
>the
>+ * maximum allowed non-present pagecache entries for the given collapse 
>operation.
>+ * @cc: The collapse control struct
>+ *
>+ * Return: Maximum number of non-present PTEs or the maximum allowed 
>non-present
>+ * pagecache entries for the collapse operation.
>+ */
>+static unsigned int collapse_max_ptes_swap(struct collapse_control *cc)
>+{
>+      // for MADV_COLLAPSE, do not restrict the number PTEs entries or
>+      // pagecache entries that are non-present.

Same here.

>+      if (!cc->is_khugepaged)
>+              return HPAGE_PMD_NR;
>+      return khugepaged_max_ptes_swap;
>+}
>+
> int hugepage_madvise(struct vm_area_struct *vma,
>                    vm_flags_t *vm_flags, int advice)
> {
>@@ -546,21 +602,19 @@ static enum scan_result 
>__collapse_huge_page_isolate(struct vm_area_struct *vma,
>       pte_t *_pte;
>       int none_or_zero = 0, shared = 0, referenced = 0;
>       enum scan_result result = SCAN_FAIL;
>+      unsigned int max_ptes_none = collapse_max_ptes_none(cc, vma);
>+      unsigned int max_ptes_shared = collapse_max_ptes_shared(cc);

Nit: could these be const, as David suggested earlier?

Nothing else jumped out at me. LGTM!

Reviewed-by: Lance Yang <[email protected]>

Reply via email to