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

[Baolin Wang]

On 7/15/26 2:04 PM, Nico Pache wrote:
> 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.

Sure. I don't have a strong opinion, let's see what others perfer :)