Re: [PATCH v3 2/6] mm: khugepaged: refine scan progress number

[David Hildenbrand (Red Hat)]

On 1/4/26 06:41, Vernon Yang wrote:
> Currently, each PMD scan always increases `progress` by HPAGE_PMD_NR,
> even if only scanning a single page. By counting the actual number of

"... a single pmd" ?

> pages scanned, the `progress` is tracked accurately.

"page table entries / pages scanned" ?

> Signed-off-by: Vernon Yang <yanglincheng@xxxxxxxxxx>
> ---
>   mm/khugepaged.c | 31 +++++++++++++++++++++++--------
>   1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 9f99f61689f8..4b124e854e2e 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -1247,7 +1247,7 @@ static int collapse_huge_page(struct mm_struct *mm, unsigned long address,
>   static int hpage_collapse_scan_pmd(struct mm_struct *mm,
>   				   struct vm_area_struct *vma,
>   				   unsigned long start_addr, bool *mmap_locked,
> -				   struct collapse_control *cc)
> +				   int *progress, struct collapse_control *cc)
>   {
>   	pmd_t *pmd;
>   	pte_t *pte, *_pte;
> @@ -1258,23 +1258,28 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm,
>   	unsigned long addr;
>   	spinlock_t *ptl;
>   	int node = NUMA_NO_NODE, unmapped = 0;
> +	int _progress = 0;

"cur_progress" ?

>   
>   	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) {
> +		_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) {
> +		_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) {
> +		_progress++;
>   		pte_t pteval = ptep_get(_pte);
>   		if (pte_none_or_zero(pteval)) {
>   			++none_or_zero;
> @@ -1410,6 +1415,9 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm,
>   		*mmap_locked = false;
>   	}
>   out:
> +	if (progress)
> +		*progress += _progress;
> +
>   	trace_mm_khugepaged_scan_pmd(mm, folio, referenced,
>   				     none_or_zero, result, unmapped);
>   	return result;
> @@ -2287,7 +2295,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
>   
>   static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr,
>   				    struct file *file, pgoff_t start,
> -				    struct collapse_control *cc)
> +				    int *progress, struct collapse_control *cc)
>   {
>   	struct folio *folio = NULL;
>   	struct address_space *mapping = file->f_mapping;
> @@ -2295,6 +2303,7 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr,
>   	int present, swap;
>   	int node = NUMA_NO_NODE;
>   	int result = SCAN_SUCCEED;
> +	int _progress = 0;

Same here.


Not sure if it would be cleaner to just let the parent increment its 
counter and returning instead the "cur_progress" from the function.

--
Cheers

David