Re: [PATCH v4 5/7] mm/khugepaged: Refactor the PTE state checks into a helper
"Zi Yan" <[email protected]>
| Newsgroups | org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On Tue Aug 11, 2026 at 8:48 AM EDT, Nico Pache (Red Hat) wrote: > For anonymous collapse, the collapse_scan_pmd() and > __collapse_huge_page_isolate() functions share a large portion of their > logic. These functions both check the state of the PTEs and verify the > following: > - max_pte_* values are not exceeded > - uffd is not active > - lazyfree properties > - non-anonymous > > Merge these checks into a helper collapse_check_pte() to reduce code > duplication. We also add a helper struct for this function called > pte_check_context which allows us to pass the required parameters in a > clean and elegant manner. > > A helper function is also introduced pte_check_fail() to provide a clean > interface to set the pte_check_context failure results and return > PTE_CHECK_FAIL state. This helps reduce code duplications across the new > collapse_check_pte function. > > Two slight modifications are done to the original functionality. We now > warn (instead of crash) if the anon test fails, and we leverage the > vm_normal_folio function instead of page->folio, this should be > functionally equivalent. > > No other functional changes intended. > > This patch is heavily based off work done by Lance Yang, but modified to > deal with conflicts and feedback received during the review cycle [1]. > > [1] https://lore.kernel.org/linux-mm/[email protected]/ > Suggested-by: David Hildenbrand <[email protected]> > Signed-off-by: Nico Pache (Red Hat) <[email protected]> > --- > mm/khugepaged.c | 298 +++++++++++++++++++++++++++++--------------------------- > 1 file changed, 157 insertions(+), 141 deletions(-) > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index 90d6e595d282..b7372aba4417 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -65,6 +65,12 @@ enum scan_result { > SCAN_PAGE_DIRTY_OR_WRITEBACK, > }; > > +enum pte_check_result { > + PTE_CHECK_SUCCEED, > + PTE_CHECK_FAIL, > + PTE_CHECK_CONTINUE, > +}; > + > #define CREATE_TRACE_POINTS > #include <trace/events/huge_memory.h> > > @@ -119,6 +125,20 @@ struct collapse_control { > DECLARE_BITMAP(mthp_present_ptes, MAX_PTRS_PER_PTE); > }; > > +struct pte_check_context { > + struct collapse_control *cc; > + struct vm_area_struct *vma; > + unsigned int order; > + struct folio *folio; > + int none_or_zero; > + int shared; > + int unmapped; > + enum scan_result result; > + unsigned int max_ptes_none; > + unsigned int max_ptes_swap; > + unsigned int max_ptes_shared; > +}; > + > /** > * struct khugepaged_scan - cursor for scanning > * @mm_head: the head of the mm list to scan > @@ -696,74 +716,131 @@ static void count_collapse_event(unsigned int order, enum vm_event_item vm_event > count_mthp_stat(order, mthp_event); > } > > +/* > + * pte_check_fail() - A simple helper to set the pte_check_context result and > + * return PTE_CHECK_FAIL. > + */ > +static enum pte_check_result pte_check_fail(struct pte_check_context *ctx, > + enum scan_result result) > +{ > + ctx->result = result; > + return PTE_CHECK_FAIL; > +} > + > +/* > + * collapse_check_pte() - Check if a PTE is suitable for collapse > + * > + * Check if a PTE is suitable for collapse based on the following criteria: > + * - max_pte_* values are not exceeded > + * - uffd is not active > + * - lazyfree properties are not present > + * - only anonymous pages are present > + * > + * a helper struct pte_check_context is used to pass and store relevant > + * information between the collapse_check_pte() function and the caller. > + * > + * Return: PTE_CHECK_SUCCEED if the PTE is suitable for collapse, > + * PTE_CHECK_FAIL if the PTE is not suitable for collapse, > + * PTE_CHECK_CONTINUE if the scan should continue to check the next PTE. I was thinking if we can make decisions based on scan_result instead of a new pte_check_result, but pte_check_context->unmapped also changes the result of collapse_check_pte(). And we will need a new SCAN_CONTINUE to match PTE_CHECK_CONTINUE. The changes look good to me. Acked-by: Zi Yan <[email protected]> -- Best Regards, Yan, Zi