Re: [PATCH mm-new v8 2/4] mm: khugepaged: refine scan progress number

[Vernon Yang]

On Tue, Feb 24, 2026 at 03:52:47AM +0000, Wei Yang wrote:
> On Sat, Feb 21, 2026 at 05:39:16PM +0800, Vernon Yang wrote:
> >From: Vernon Yang <yanglincheng@xxxxxxxxxx>
> >
> >Currently, each scan always increases "progress" by HPAGE_PMD_NR,
> >even if only scanning a single PTE/PMD entry.
> >
> >- When only scanning a sigle PTE entry, let me provide a detailed
> >  example:
> >
> >static int hpage_collapse_scan_pmd()
> >{
> >	for (addr = start_addr, _pte = pte; _pte < pte + HPAGE_PMD_NR;
> >	     _pte++, addr += PAGE_SIZE) {
> >		pte_t pteval = ptep_get(_pte);
> >		...
> >		if (pte_uffd_wp(pteval)) { <-- first scan hit
> >			result = SCAN_PTE_UFFD_WP;
> >			goto out_unmap;
> >		}
> >	}
> >}
> >
> >During the first scan, if pte_uffd_wp(pteval) is true, the loop exits
> >directly. In practice, only one PTE is scanned before termination.
> >Here, "progress += 1" reflects the actual number of PTEs scanned, but
> >previously "progress += HPAGE_PMD_NR" always.
> >
> >- When the memory has been collapsed to PMD, let me provide a detailed
> >  example:
> >
> >The following data is traced by bpftrace on a desktop system. After
> >the system has been left idle for 10 minutes upon booting, a lot of
> >SCAN_PMD_MAPPED or SCAN_NO_PTE_TABLE are observed during a full scan
> >by khugepaged.
> >
> >>From trace_mm_khugepaged_scan_pmd and trace_mm_khugepaged_scan_file, the
> >following statuses were observed, with frequency mentioned next to them:
> >
> >SCAN_SUCCEED          : 1
> >SCAN_EXCEED_SHARED_PTE: 2
> >SCAN_PMD_MAPPED       : 142
> >SCAN_NO_PTE_TABLE     : 178
> >total progress size   : 674 MB
> >Total time            : 419 seconds, include khugepaged_scan_sleep_millisecs
> >
> >The khugepaged_scan list save all task that support collapse into hugepage,
> >as long as the task is not destroyed, khugepaged will not remove it from
> >the khugepaged_scan list. This exist a phenomenon where task has already
> >collapsed all memory regions into hugepage, but khugepaged continues to
> >scan it, which wastes CPU time and invalid, and due to
> >khugepaged_scan_sleep_millisecs (default 10s) causes a long wait for
> >scanning a large number of invalid task, so scanning really valid task
> >is later.
> >
> >After applying this patch, when the memory is either SCAN_PMD_MAPPED or
> >SCAN_NO_PTE_TABLE, just skip it, as follow:
> >
> >SCAN_EXCEED_SHARED_PTE: 2
> >SCAN_PMD_MAPPED       : 147
> >SCAN_NO_PTE_TABLE     : 173
> >total progress size   : 45 MB
> >Total time            : 20 seconds
> >
> >SCAN_PTE_MAPPED_HUGEPAGE is the same, for detailed data, refer to
> >https://lore.kernel.org/linux-mm/4qdu7owpmxfh3ugsue775fxarw5g2gcggbxdf5psj75nnu7z2u@cv2uu2yocaxq
> >
> >Signed-off-by: Vernon Yang <yanglincheng@xxxxxxxxxx>
> >Reviewed-by: Dev Jain <dev.jain@xxxxxxx>
> >---
> > mm/khugepaged.c | 42 ++++++++++++++++++++++++++++++++----------
> > 1 file changed, 32 insertions(+), 10 deletions(-)
> >
> >diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> >index e2f6b68a0011..61e25cf5424b 100644
> >--- a/mm/khugepaged.c
> >+++ b/mm/khugepaged.c
> >@@ -68,7 +68,10 @@ enum scan_result {
> > static struct task_struct *khugepaged_thread __read_mostly;
> > static DEFINE_MUTEX(khugepaged_mutex);
> >
> >-/* default scan 8*HPAGE_PMD_NR ptes (or vmas) every 10 second */
> >+/*
> >+ * default scan 8*HPAGE_PMD_NR ptes, pmd_mapped, no_pte_table or vmas
> >+ * every 10 second.
> >+ */
> > static unsigned int khugepaged_pages_to_scan __read_mostly;
> > static unsigned int khugepaged_pages_collapsed;
> > static unsigned int khugepaged_full_scans;
> >@@ -1231,7 +1234,8 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > }
> >
> > static enum scan_result hpage_collapse_scan_pmd(struct mm_struct *mm,
> >-		struct vm_area_struct *vma, unsigned long start_addr, bool *mmap_locked,
> >+		struct vm_area_struct *vma, unsigned long start_addr,
> >+		bool *mmap_locked, unsigned int *cur_progress,
> > 		struct collapse_control *cc)
> > {
> > 	pmd_t *pmd;
> >@@ -1247,19 +1251,27 @@ static enum scan_result hpage_collapse_scan_pmd(struct mm_struct *mm,
> > 	VM_BUG_ON(start_addr & ~HPAGE_PMD_MASK);
> >
> > 	result = find_pmd_or_thp_or_none(mm, start_addr, &pmd);
> >-	if (result != SCAN_SUCCEED)
> >+	if (result != SCAN_SUCCEED) {
> >+		if (cur_progress)
> >+			*cur_progress = 1;
> > 		goto out;
> >+	}
>
> How about put cur_progress in struct collapse_control?
>
> Then we don't need to check cur_progress every time before modification.

Thank you for suggestion.

Placing it inside "struct collapse_control" makes the overall code
simpler, there also coincidentally has a 4-bytes hole, as shown below:

struct collapse_control {
        bool                       is_khugepaged;        /*     0     1 */

        /* XXX 3 bytes hole, try to pack */

        u32                        node_load[64];        /*     4   256 */

        /* XXX 4 bytes hole, try to pack */

        /* --- cacheline 4 boundary (256 bytes) was 8 bytes ago --- */
        nodemask_t                 alloc_nmask;          /*   264     8 */

        /* size: 272, cachelines: 5, members: 3 */
        /* sum members: 265, holes: 2, sum holes: 7 */
        /* last cacheline: 16 bytes */
};

But regardless of khugepaged or madvise(MADV_COLLAPSE), "cur_progress"
will be counted, while madvise(MADV_COLLAPSE) actually does not need to
be counted.

David, do we want to place "cur_progress" inside the "struct collapse_control"?
If Yes, it would be better to rename "cur_progress" to "pmd_progress",
as show below:

struct collapse_control {
        bool is_khugepaged;

        /* Num pages scanned per node */
        u32 node_load[MAX_NUMNODES];

        /*
         * Num pages scanned per pmd, include ptes,
         * pte_mapped_hugepage, pmd_mapped or no_pte_table.
         */
        unsigned int pmd_progress;

        /* nodemask for allocation fallback */
        nodemask_t alloc_nmask;
};

--
Cheers,
Vernon