On Mon, May 11, 2026 at 10:45 PM Lance Yang <[email protected]> wrote: > > > 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]>
Ack on all the above thank you ! >
