Re: [PATCH mm-new v4 2/6] mm: khugepaged: refine scan progress number

["David Hildenbrand (Red Hat)" <david@kernel.org>]

On 1/11/26 13:19, Vernon Yang wrote:
> Currently, each scan always increases "progress" by HPAGE_PMD_NR,
> even if only scanning a single pte.
> 
> This patch does not change the original semantics of "progress", it
> simply uses the exact number of PTEs counted to replace HPAGE_PMD_NR.
> 
> 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.
> 
> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> ---
>   mm/khugepaged.c | 28 ++++++++++++++++++++++------
>   1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 2e570f83778c..5c6015ac7b5e 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -1249,6 +1249,7 @@ 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,
> +						int *cur_progress,
>   						struct collapse_control *cc)
>   {
>   	pmd_t *pmd;
> @@ -1264,19 +1265,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 = HPAGE_PMD_NR;
>   		goto out;
> +	}
>   
>   	memset(cc->node_load, 0, sizeof(cc->node_load));
>   	nodes_clear(cc->alloc_nmask);
>   	pte = pte_offset_map_lock(mm, pmd, start_addr, &ptl);
>   	if (!pte) {
> +		if (cur_progress)
> +			*cur_progress = HPAGE_PMD_NR;
>   		result = SCAN_NO_PTE_TABLE;
>   		goto out;
>   	}
>   
>   	for (addr = start_addr, _pte = pte; _pte < pte + HPAGE_PMD_NR;
>   	     _pte++, addr += PAGE_SIZE) {
> +		if (cur_progress)
> +			*cur_progress += 1;
> +
>   		pte_t pteval = ptep_get(_pte);
>   		if (pte_none_or_zero(pteval)) {
>   			++none_or_zero;
> @@ -2297,6 +2306,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>   
>   static enum scan_result hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr,
>   						 struct file *file, pgoff_t start,
> +						 int *cur_progress,
>   						 struct collapse_control *cc)
>   {
>   	struct folio *folio = NULL;
> @@ -2337,6 +2347,9 @@ static enum scan_result hpage_collapse_scan_file(struct mm_struct *mm, unsigned
>   			continue;
>   		}
>   
> +		if (cur_progress)
> +			*cur_progress += folio_nr_pages(folio);
> +

Okay, I had another look and I think the file path is confusing. We're 
scanning xarray entries. But then, we only count some entries and not 
others.

Can we just keep that alone in this patch? That is, always indicate a 
progress of HPAGE_PMD_NR right at the start of the function?

-- 
Cheers

David