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

[Baolin Wang]

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()?

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.

Just my 2 cents.