Re: [PATCH 2/8] mm/khugepaged: extract young page check into collapse_is_young() helper

[Nico Pache]

On Fri, Jul 10, 2026 at 1:41 AM Baolin Wang
<baolin.wang@xxxxxxxxxxxxxxxxx> wrote:
>
>
>
> On 7/6/26 11:44 PM, Nico Pache wrote:
> > The change deduplicates the "is this PTE young enough to count as
> > referenced" condition that was repeated in both
> > __collapse_huge_page_isolate() and collapse_scan_pmd(), extracting it into
> > a single inline helper function.
> >
> > Also move the comment and use it as the function header. While we are at
> > it, updated the comment to clarify that a young pte is a recently accessed
> > one.
> >
> > Signed-off-by: Nico Pache <npache@xxxxxxxxxx>
> > ---
> >   mm/khugepaged.c | 35 +++++++++++++++++++----------------
> >   1 file changed, 19 insertions(+), 16 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index b3985b854e77..48b008a3c891 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -675,6 +675,23 @@ static void release_pte_pages(pte_t *pte, pte_t *_pte,
> >       }
> >   }
> >
> > +/*
> > + * collapse_is_young() - Check for enough young pte to justify collapsing
> > + *
> > + * If collapse was initiated by khugepaged, check that the page has been
> > + * recently accessed (young pte) to justify collapsing the page.
> > + *
> > + * Return: true if the page has been recently accessed (young pte).
> > + */
> > +static inline bool collapse_is_young(struct collapse_control *cc, pte_t pteval,
> > +     struct folio *folio, struct vm_area_struct *vma, unsigned long addr)
> > +{
> > +     return cc->is_khugepaged &&
> > +            (pte_young(pteval) || folio_test_young(folio) ||
> > +             folio_test_referenced(folio) ||
> > +             mmu_notifier_test_young(vma->vm_mm, addr));
> > +}
>
> collapse_is_young() is somewhat confusing to me. Would using
> 'referenced' be a more appropriate name? collapse_folio_is_referenced()?

I went with collapse_is_referenced since it also checks the PTE not
just the folio.

>
> Also, it feels odd to put 'cc->is_khugepaged' in a helper whose purpose
> is to check whether a folio has been accessed. I think this helper
> should be more self-contained and focused solely on checking whether the
> folio was accessed.

While I don't wholeheartedly disagree, the point is to clean up the
code and abstract some of this logic away from the parent functions. I
prefer doing the khugepaged check inside this new helper because the
code is much cleaner.

Cheers,
-- Nico

>
> Just my 2 cents.
>