Re: [PATCH 2/3] mm/khugepaged: use enum scan_result for result variables

[Zi Yan <ziy@nvidia.com>]

On 16 Dec 2025, at 6:11, Shivank Garg wrote:

> Use enum scan_result for local variables and the result pointer in
> khugepaged_scan_mm_slot(), instead of plain int. This improves code
> readability and clarifies intent,
>
> No functional change.
>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
>  mm/khugepaged.c | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 107146f012b1..65b1b778378a 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -547,7 +547,8 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,

The return type could be changed too.

>  	struct folio *folio = NULL;
>  	unsigned long addr = start_addr;
>  	pte_t *_pte;
> -	int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0;
> +	int none_or_zero = 0, shared = 0, referenced = 0;
> +	enum scan_result result = SCAN_FAIL;
>
>  	for (_pte = pte; _pte < pte + HPAGE_PMD_NR;
>  	     _pte++, addr += PAGE_SIZE) {
> @@ -786,7 +787,7 @@ static int __collapse_huge_page_copy(pte_t *pte, struct folio *folio,

Ditto.

>  		struct list_head *compound_pagelist)
>  {
>  	unsigned int i;
> -	int result = SCAN_SUCCEED;
> +	enum scan_result result = SCAN_SUCCEED;
>
>  	/*
>  	 * Copying pages' contents is subject to memory poison at any iteration.
> @@ -969,7 +970,7 @@ static int check_pmd_still_valid(struct mm_struct *mm,
>  				 pmd_t *pmd)

Ditto.

>  {
>  	pmd_t *new_pmd;
> -	int result = find_pmd_or_thp_or_none(mm, address, &new_pmd);
> +	enum scan_result result = find_pmd_or_thp_or_none(mm, address, &new_pmd);
>
>  	if (result != SCAN_SUCCEED)
>  		return result;
> @@ -993,7 +994,7 @@ static int __collapse_huge_page_swapin(struct mm_struct *mm,

Ditto.

>  	int swapped_in = 0;
>  	vm_fault_t ret = 0;
>  	unsigned long addr, end = start_addr + (HPAGE_PMD_NR * PAGE_SIZE);
> -	int result;
> +	enum scan_result result;
>  	pte_t *pte = NULL;
>  	spinlock_t *ptl;
>
> @@ -1100,7 +1101,7 @@ static int collapse_huge_page(struct mm_struct *mm, unsigned long address,

Ditto.

>  	pgtable_t pgtable;
>  	struct folio *folio;
>  	spinlock_t *pmd_ptl, *pte_ptl;
> -	int result = SCAN_FAIL;
> +	enum scan_result result = SCAN_FAIL;
>  	struct vm_area_struct *vma;
>  	struct mmu_notifier_range range;
>
> @@ -1253,8 +1254,8 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm,
>  {
Ditto.

>  	pmd_t *pmd;
>  	pte_t *pte, *_pte;
> -	int result = SCAN_FAIL, referenced = 0;
> -	int none_or_zero = 0, shared = 0;
> +	int none_or_zero = 0, shared = 0, referenced = 0;
> +	enum scan_result result = SCAN_FAIL;
>  	struct page *page = NULL;
>  	struct folio *folio = NULL;
>  	unsigned long addr;
> @@ -1492,7 +1493,8 @@ static int set_huge_pmd(struct vm_area_struct *vma, unsigned long addr,

Same here.

>  int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>  			    bool install_pmd)
>  {

And here.

> -	int nr_mapped_ptes = 0, result = SCAN_FAIL;
> +	enum scan_result result = SCAN_FAIL;
> +	int nr_mapped_ptes = 0;
>  	unsigned int nr_batch_ptes;
>  	struct mmu_notifier_range range;
>  	bool notified = false;
> @@ -1866,7 +1868,8 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,

Same here.

>  	pgoff_t index = 0, end = start + HPAGE_PMD_NR;
>  	LIST_HEAD(pagelist);
>  	XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
> -	int nr_none = 0, result = SCAN_SUCCEED;
> +	enum scan_result result = SCAN_SUCCEED;
> +	int nr_none = 0;
>  	bool is_shmem = shmem_file(file);
>
>  	VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
> @@ -2296,7 +2299,7 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr,

Same here.

>  	XA_STATE(xas, &mapping->i_pages, start);
>  	int present, swap;
>  	int node = NUMA_NO_NODE;
> -	int result = SCAN_SUCCEED;
> +	enum scan_result result = SCAN_SUCCEED;
>
>  	present = 0;
>  	swap = 0;
> @@ -2394,7 +2397,7 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr,
>  	return result;
>  }
>
> -static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
> +static unsigned int khugepaged_scan_mm_slot(unsigned int pages, enum scan_result *result,
>  					    struct collapse_control *cc)
>  	__releases(&khugepaged_mm_lock)
>  	__acquires(&khugepaged_mm_lock)
> @@ -2555,7 +2558,7 @@ static void khugepaged_do_scan(struct collapse_control *cc)
>  	unsigned int progress = 0, pass_through_head = 0;
>  	unsigned int pages = READ_ONCE(khugepaged_pages_to_scan);
>  	bool wait = true;
> -	int result = SCAN_SUCCEED;
> +	enum scan_result result = SCAN_SUCCEED;
>
>  	lru_add_drain_all();
>
> @@ -2790,7 +2793,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
>
>  	for (addr = hstart; addr < hend; addr += HPAGE_PMD_SIZE) {
>  		bool retried = false;
> -		int result = SCAN_FAIL;
> +		enum scan_result result = SCAN_FAIL;
>
>  		if (!mmap_locked) {
>  retry:
> -- 

In addition, the return types of find_pmd_or_thp_or_none(),
hugepage_vma_revalidate(), alloc_charge_folio(), and check_pmd_state() need to be changed to enum scan_result too.

Best Regards,
Yan, Zi