Re: [PATCH mm-unstable v17 03/14] mm/khugepaged: rework max_ptes_* handling with helper functions
From: Lance Yang
Date: Tue May 12 2026 - 00:45:19 EST
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 <david@xxxxxxxxxx>
>Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
>Acked-by: Usama Arif <usama.arif@xxxxxxxxx>
>Signed-off-by: Nico Pache <npache@xxxxxxxxxx>
>---
> 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 <lance.yang@xxxxxxxxx>