[PATCH 0/3] mm/khugepaged: minor cleanups

[PATCH 0/3] mm/khugepaged: minor cleanups

[Shivank Garg]

This small series contains minor cleanups in mm/khugepaged and
no functional changes are intended.

Thanks,

Shivank Garg (3):
  mm/khugepaged: remove unnecessary goto 'skip' label
  mm/khugepaged: use enum scan_result for result variables
  mm/khugepaged: make khugepaged_collapse_control static

 mm/khugepaged.c | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)


base-commit: c642ecda5b136882e518d8303863473c0d21ab2f
-- 
2.43.0

[PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label

[Shivank Garg]

Replace 'goto skip' with actual logic for better code readability.

No functional change.

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 mm/khugepaged.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 6c8c35d3e0c9..107146f012b1 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2442,14 +2442,15 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
 			break;
 		}
 		if (!thp_vma_allowable_order(vma, vma->vm_flags, TVA_KHUGEPAGED, PMD_ORDER)) {
-skip:
 			progress++;
 			continue;
 		}
 		hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE);
 		hend = round_down(vma->vm_end, HPAGE_PMD_SIZE);
-		if (khugepaged_scan.address > hend)
-			goto skip;
+		if (khugepaged_scan.address > hend) {
+			progress++;
+			continue;
+		}
 		if (khugepaged_scan.address < hstart)
 			khugepaged_scan.address = hstart;
 		VM_BUG_ON(khugepaged_scan.address & ~HPAGE_PMD_MASK);
-- 
2.43.0

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

[Shivank Garg]

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,
 	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,
 		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)
 {
 	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,
 	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,
 	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,
 {
 	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,
 int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
 			    bool install_pmd)
 {
-	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,
 	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,
 	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:
-- 
2.43.0

[PATCH 3/3] mm/khugepaged: make khugepaged_collapse_control static

[Shivank Garg]

The global variable 'khugepaged_collapse_control' is not used outside of
mm/khugepaged.c. Make it static to limit its scope.

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 mm/khugepaged.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 65b1b778378a..d130357a2d88 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -829,7 +829,7 @@ static void khugepaged_alloc_sleep(void)
 	remove_wait_queue(&khugepaged_wait, &wait);
 }
 
-struct collapse_control khugepaged_collapse_control = {
+static struct collapse_control khugepaged_collapse_control = {
 	.is_khugepaged = true,
 };
 
-- 
2.43.0

Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label

[Zi Yan]

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

> Replace 'goto skip' with actual logic for better code readability.
>
> No functional change.
>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
>  mm/khugepaged.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>

Best Regards,
Yan, Zi

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

[Zi Yan]

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

Re: [PATCH 3/3] mm/khugepaged: make khugepaged_collapse_control static

[Zi Yan]

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

> The global variable 'khugepaged_collapse_control' is not used outside of
> mm/khugepaged.c. Make it static to limit its scope.
>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
>  mm/khugepaged.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>

Best Regards,
Yan, Zi

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

[Garg, Shivank]

On 12/16/2025 9:08 PM, Zi Yan wrote:
> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
> 

>> @@ -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)
>>  {

>> -- 
> 
> 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.
> 
Hi Zi,

Thanks for the review and suggestions.
I'll update the return types and send V2.

Thanks,
Shivank

Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label

[Wei Yang]

On Tue, Dec 16, 2025 at 11:11:38AM +0000, Shivank Garg wrote:
>Replace 'goto skip' with actual logic for better code readability.
>
>No functional change.
>
>Signed-off-by: Shivank Garg <shivankg@amd.com>
>---
> mm/khugepaged.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>index 6c8c35d3e0c9..107146f012b1 100644
>--- a/mm/khugepaged.c
>+++ b/mm/khugepaged.c
>@@ -2442,14 +2442,15 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
> 			break;
> 		}
> 		if (!thp_vma_allowable_order(vma, vma->vm_flags, TVA_KHUGEPAGED, PMD_ORDER)) {
>-skip:
> 			progress++;
> 			continue;
> 		}
> 		hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE);
> 		hend = round_down(vma->vm_end, HPAGE_PMD_SIZE);
>-		if (khugepaged_scan.address > hend)
>-			goto skip;
>+		if (khugepaged_scan.address > hend) {
>+			progress++;
>+			continue;
>+		}

Hi, Shivank

The change here looks good, while I come up with an question.

The @progress here seems record two things:

  * number of pages scaned
  * number of vma skipped

While in very rare case, we may miss to count the second case.

For example, we have following vmas in a process:

     vma1             vma2
    +----------------+------------+
    |2M              |1M          |
    +----------------+------------+

Let's assume vma1 is exactly HPAGE_PMD_SIZE and also HPAGE_PMD_SIZE aligned.
But vma2 is only half of HPAGE_PMD_SIZE.

When scan finish vma1 and start on vma2, we would have hstart = hend =
address. So we continue here but would not do real scan, since address == hend.

I am thinking whether this could handle it:

		if (khugepaged_scan.address > hend || hend <= hstart) {
			progress++;
			continue;
		}

Do you thinks I am correct on this?

> 		if (khugepaged_scan.address < hstart)
> 			khugepaged_scan.address = hstart;
> 		VM_BUG_ON(khugepaged_scan.address & ~HPAGE_PMD_MASK);
>-- 
>2.43.0
>

-- 
Wei Yang
Help you, Help me

Re: [PATCH 3/3] mm/khugepaged: make khugepaged_collapse_control static

[Wei Yang]

On Tue, Dec 16, 2025 at 11:11:42AM +0000, Shivank Garg wrote:
>The global variable 'khugepaged_collapse_control' is not used outside of
>mm/khugepaged.c. Make it static to limit its scope.
>
>Signed-off-by: Shivank Garg <shivankg@amd.com>

Reviewed-by: Wei Yang <richard.weiyang@gmail.com>

>---
> mm/khugepaged.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>index 65b1b778378a..d130357a2d88 100644
>--- a/mm/khugepaged.c
>+++ b/mm/khugepaged.c
>@@ -829,7 +829,7 @@ static void khugepaged_alloc_sleep(void)
> 	remove_wait_queue(&khugepaged_wait, &wait);
> }
> 
>-struct collapse_control khugepaged_collapse_control = {
>+static struct collapse_control khugepaged_collapse_control = {
> 	.is_khugepaged = true,
> };
> 
>-- 
>2.43.0
>

-- 
Wei Yang
Help you, Help me

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

[Garg, Shivank]

On 12/16/2025 9:08 PM, Zi Yan wrote:
> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
> 

> 
>>  int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>>  			    bool install_pmd)
>>  {
> 
> And here.
Since this function is declared in khugepaged.h, I need to
move the enum definition to that header. I see two options for handling
the CONFIG_TRANSPARENT_HUGEPAGE check:

1. Define enum OUTSIDE the ifdef: This allows the static inline stub
   to also return enum scan_result, keeping the API consistent.
2. Define enum INSIDE the ifdef: The enum is hidden when THP is disabled,
   forcing the stub to return int 0 instead.

The only external caller (uprobes.c) of collapse_pte_mapped_thp currently
ignores the return value.

What is the more preferred way for enum definition in such scenario?

Thanks,
Shivank


> 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

Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label

[David Hildenbrand (Red Hat)]

On 12/16/25 12:11, Shivank Garg wrote:
> Replace 'goto skip' with actual logic for better code readability.
> 
> No functional change.
> 
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
>   mm/khugepaged.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 6c8c35d3e0c9..107146f012b1 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2442,14 +2442,15 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>   			break;
>   		}
>   		if (!thp_vma_allowable_order(vma, vma->vm_flags, TVA_KHUGEPAGED, PMD_ORDER)) {
> -skip:
>   			progress++;
>   			continue;
>   		}
>   		hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE);
>   		hend = round_down(vma->vm_end, HPAGE_PMD_SIZE);
> -		if (khugepaged_scan.address > hend)
> -			goto skip;
> +		if (khugepaged_scan.address > hend) {
> +			progress++;
> +			continue;
> +		}
>   		if (khugepaged_scan.address < hstart)
>   			khugepaged_scan.address = hstart;
>   		VM_BUG_ON(khugepaged_scan.address & ~HPAGE_PMD_MASK);

Oh yes, please

Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>

-- 
Cheers

David

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

[David Hildenbrand (Red Hat)]

On 12/18/25 06:40, Garg, Shivank wrote:
> 
> 
> On 12/16/2025 9:08 PM, Zi Yan wrote:
>> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
>>
> 
>>
>>>   int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>>>   			    bool install_pmd)
>>>   {
>>
>> And here.
> Since this function is declared in khugepaged.h, I need to
> move the enum definition to that header. I see two options for handling
> the CONFIG_TRANSPARENT_HUGEPAGE check:
> 
> 1. Define enum OUTSIDE the ifdef: This allows the static inline stub
>     to also return enum scan_result, keeping the API consistent.
> 2. Define enum INSIDE the ifdef: The enum is hidden when THP is disabled,
>     forcing the stub to return int 0 instead.
> 
> The only external caller (uprobes.c) of collapse_pte_mapped_thp currently
> ignores the return value.

Probably best to not expose that enum (especially when nobody cares ...) 
and instead expose a new void function for uprobe purposes.

Maybe

void collapse_pte_mapped_thp(...)
{
	try_collapse_pte_mapped_thp();
}

Maybe something like that?

-- 
Cheers

David

Re: [PATCH 3/3] mm/khugepaged: make khugepaged_collapse_control static

[David Hildenbrand (Red Hat)]

On 12/16/25 12:11, Shivank Garg wrote:
> The global variable 'khugepaged_collapse_control' is not used outside of
> mm/khugepaged.c. Make it static to limit its scope.
> 
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---

Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>

-- 
Cheers

David

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

[Zi Yan]

On 18 Dec 2025, at 4:17, David Hildenbrand (Red Hat) wrote:

> On 12/18/25 06:40, Garg, Shivank wrote:
>>
>>
>> On 12/16/2025 9:08 PM, Zi Yan wrote:
>>> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
>>>
>>
>>>
>>>>   int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>>>>   			    bool install_pmd)
>>>>   {
>>>
>>> And here.
>> Since this function is declared in khugepaged.h, I need to
>> move the enum definition to that header. I see two options for handling
>> the CONFIG_TRANSPARENT_HUGEPAGE check:
>>
>> 1. Define enum OUTSIDE the ifdef: This allows the static inline stub
>>     to also return enum scan_result, keeping the API consistent.
>> 2. Define enum INSIDE the ifdef: The enum is hidden when THP is disabled,
>>     forcing the stub to return int 0 instead.
>>
>> The only external caller (uprobes.c) of collapse_pte_mapped_thp currently
>> ignores the return value.
>
> Probably best to not expose that enum (especially when nobody cares ...) and instead expose a new void function for uprobe purposes.
>
> Maybe
>
> void collapse_pte_mapped_thp(...)
> {
> 	try_collapse_pte_mapped_thp();
> }
>
> Maybe something like that?

Sounds good to me.

Best Regards,
Yan, Zi

Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label

[Liam R. Howlett]

* Shivank Garg <shivankg@amd.com> [251216 06:12]:
> Replace 'goto skip' with actual logic for better code readability.
> 
> No functional change.
> 
> Signed-off-by: Shivank Garg <shivankg@amd.com>

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  mm/khugepaged.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 6c8c35d3e0c9..107146f012b1 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2442,14 +2442,15 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>  			break;
>  		}
>  		if (!thp_vma_allowable_order(vma, vma->vm_flags, TVA_KHUGEPAGED, PMD_ORDER)) {
> -skip:
>  			progress++;
>  			continue;
>  		}
>  		hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE);
>  		hend = round_down(vma->vm_end, HPAGE_PMD_SIZE);
> -		if (khugepaged_scan.address > hend)
> -			goto skip;
> +		if (khugepaged_scan.address > hend) {
> +			progress++;
> +			continue;
> +		}
>  		if (khugepaged_scan.address < hstart)
>  			khugepaged_scan.address = hstart;
>  		VM_BUG_ON(khugepaged_scan.address & ~HPAGE_PMD_MASK);
> -- 
> 2.43.0
> 

Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label

[Garg, Shivank]

On 12/17/2025 8:18 AM, Wei Yang wrote:
> On Tue, Dec 16, 2025 at 11:11:38AM +0000, Shivank Garg wrote:
>> Replace 'goto skip' with actual logic for better code readability.
>>
>> No functional change.
>>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>> mm/khugepaged.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 6c8c35d3e0c9..107146f012b1 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -2442,14 +2442,15 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>> 			break;
>> 		}
>> 		if (!thp_vma_allowable_order(vma, vma->vm_flags, TVA_KHUGEPAGED, PMD_ORDER)) {
>> -skip:
>> 			progress++;
>> 			continue;
>> 		}
>> 		hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE);
>> 		hend = round_down(vma->vm_end, HPAGE_PMD_SIZE);
>> -		if (khugepaged_scan.address > hend)
>> -			goto skip;
>> +		if (khugepaged_scan.address > hend) {
>> +			progress++;
>> +			continue;
>> +		}
> 
> Hi, Shivank
> 
> The change here looks good, while I come up with an question.
> 
> The @progress here seems record two things:
> 
>   * number of pages scaned
>   * number of vma skipped
> 
Three things: number of mm. It's incremented 1 for whole khugepaged_scan_mm_slot().


> While in very rare case, we may miss to count the second case.
> 
> For example, we have following vmas in a process:
> 
>      vma1             vma2
>     +----------------+------------+
>     |2M              |1M          |
>     +----------------+------------+
> 
> Let's assume vma1 is exactly HPAGE_PMD_SIZE and also HPAGE_PMD_SIZE aligned.
> But vma2 is only half of HPAGE_PMD_SIZE.
> 
> When scan finish vma1 and start on vma2, we would have hstart = hend =
> address. So we continue here but would not do real scan, since address == hend.
> 
> I am thinking whether this could handle it:
> 
> 		if (khugepaged_scan.address > hend || hend <= hstart) {
> 			progress++;
> 			continue;
> 		}
> 
> Do you thinks I am correct on this?

I think you're correct.
IIUC, @progress acts as rate limiter here.

It is increasing +1 for whole, and then increases by +1 per VMA (if skipped),
or by +HPAGE_PMD_NR (if actually scanned).

So, progress ensuring the hugepaged_do_scan run only until (progress >= pages)
at which point it yields and sleeps (wait_event_freezable).

Without your suggested fix, if a process contains a large number of small VMAs (where
round_up hstart >= round_down(hend), it will unfairly consume more CPU cycles before
yielding compared to a process with fewer or aligned VMAs.

I think your suggestion is ensuring fairness by charging 'progress' count correctly.

Thanks,
Shivank

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

[Garg, Shivank]

On 12/18/2025 9:24 PM, Zi Yan wrote:
> On 18 Dec 2025, at 4:17, David Hildenbrand (Red Hat) wrote:
> 
>> On 12/18/25 06:40, Garg, Shivank wrote:
>>>
>>>
>>> On 12/16/2025 9:08 PM, Zi Yan wrote:
>>>> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
>>>>
>>>
>>>>
>>>>>   int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>>>>>   			    bool install_pmd)
>>>>>   {
>>>>
>>>> And here.
>>> Since this function is declared in khugepaged.h, I need to
>>> move the enum definition to that header. I see two options for handling
>>> the CONFIG_TRANSPARENT_HUGEPAGE check:
>>>
>>> 1. Define enum OUTSIDE the ifdef: This allows the static inline stub
>>>     to also return enum scan_result, keeping the API consistent.
>>> 2. Define enum INSIDE the ifdef: The enum is hidden when THP is disabled,
>>>     forcing the stub to return int 0 instead.
>>>
>>> The only external caller (uprobes.c) of collapse_pte_mapped_thp currently
>>> ignores the return value.
>>
>> Probably best to not expose that enum (especially when nobody cares ...) and instead expose a new void function for uprobe purposes.
>>
>> Maybe
>>
>> void collapse_pte_mapped_thp(...)
>> {
>> 	try_collapse_pte_mapped_thp();
>> }
>>
>> Maybe something like that?
> 
> Sounds good to me.

Thanks, this makes sense to me.
I'll do this.

Best Regards,
Shivank

Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label

[Garg, Shivank]

On 12/16/2025 8:57 PM, Zi Yan wrote:
> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
> 
>> Replace 'goto skip' with actual logic for better code readability.
>>
>> No functional change.
>>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>>  mm/khugepaged.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
> LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>
> 

Hi Zi,

I noticed that b4 didn't pick up your Reviewed-by tag automatically (possibly because
it was on the same line as "LGTM."). Just thought I'd mention it in case it's helpful,
Since maintainers may be using b4 or similar tools and your tags might occasionally
get missed.

Of course, I'll make sure to add your tag manually when sending v2.

Thanks,
Shivank

Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label

[Konstantin Ryabitsev]

On Fri, Dec 19, 2025 at 05:50:39PM +0530, Garg, Shivank wrote:
> I noticed that b4 didn't pick up your Reviewed-by tag automatically (possibly because
> it was on the same line as "LGTM.").

That is correct -- code review trailers must always be on a separate line for
any tooling to properly recognize them.

Best regards,
-K

Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label

[Zi Yan]

On 19 Dec 2025, at 11:00, Konstantin Ryabitsev wrote:

> On Fri, Dec 19, 2025 at 05:50:39PM +0530, Garg, Shivank wrote:
>> I noticed that b4 didn't pick up your Reviewed-by tag automatically (possibly because
>> it was on the same line as "LGTM.").
>
> That is correct -- code review trailers must always be on a separate line for
> any tooling to properly recognize them.

Got it. Will do that from now on. Thank you both for pointing this out.

Best Regards,
Yan, Zi

Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label

[Wei Yang]

On Fri, Dec 19, 2025 at 02:54:40PM +0530, Garg, Shivank wrote:
>
>
>On 12/17/2025 8:18 AM, Wei Yang wrote:
>> On Tue, Dec 16, 2025 at 11:11:38AM +0000, Shivank Garg wrote:
>>> Replace 'goto skip' with actual logic for better code readability.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>>> ---
>>> mm/khugepaged.c | 7 ++++---
>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index 6c8c35d3e0c9..107146f012b1 100644
>>> --- a/mm/khugepaged.c
>>> +++ b/mm/khugepaged.c
>>> @@ -2442,14 +2442,15 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>>> 			break;
>>> 		}
>>> 		if (!thp_vma_allowable_order(vma, vma->vm_flags, TVA_KHUGEPAGED, PMD_ORDER)) {
>>> -skip:
>>> 			progress++;
>>> 			continue;
>>> 		}
>>> 		hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE);
>>> 		hend = round_down(vma->vm_end, HPAGE_PMD_SIZE);
>>> -		if (khugepaged_scan.address > hend)
>>> -			goto skip;
>>> +		if (khugepaged_scan.address > hend) {
>>> +			progress++;
>>> +			continue;
>>> +		}
>> 
>> Hi, Shivank
>> 
>> The change here looks good, while I come up with an question.
>> 
>> The @progress here seems record two things:
>> 
>>   * number of pages scaned
>>   * number of vma skipped
>> 
>Three things: number of mm. It's incremented 1 for whole khugepaged_scan_mm_slot().
>

Agree.

>
>> While in very rare case, we may miss to count the second case.
>> 
>> For example, we have following vmas in a process:
>> 
>>      vma1             vma2
>>     +----------------+------------+
>>     |2M              |1M          |
>>     +----------------+------------+
>> 
>> Let's assume vma1 is exactly HPAGE_PMD_SIZE and also HPAGE_PMD_SIZE aligned.
>> But vma2 is only half of HPAGE_PMD_SIZE.
>> 
>> When scan finish vma1 and start on vma2, we would have hstart = hend =
>> address. So we continue here but would not do real scan, since address == hend.
>> 
>> I am thinking whether this could handle it:
>> 
>> 		if (khugepaged_scan.address > hend || hend <= hstart) {
>> 			progress++;
>> 			continue;
>> 		}
>> 
>> Do you thinks I am correct on this?
>
>I think you're correct.
>IIUC, @progress acts as rate limiter here.
>
>It is increasing +1 for whole, and then increases by +1 per VMA (if skipped),
>or by +HPAGE_PMD_NR (if actually scanned).
>
>So, progress ensuring the hugepaged_do_scan run only until (progress >= pages)
>at which point it yields and sleeps (wait_event_freezable).
>
>Without your suggested fix, if a process contains a large number of small VMAs (where
>round_up hstart >= round_down(hend), it will unfairly consume more CPU cycles before
>yielding compared to a process with fewer or aligned VMAs.

You are right. While I am not sure it exists in reality, but in theory it
could be.

>
>I think your suggestion is ensuring fairness by charging 'progress' count correctly.
>

Thanks for your confirmation. Would you mind add a cleanup in next version, or
you prefer me to send it :-)

>Thanks,
>Shivank

-- 
Wei Yang
Help you, Help me

Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label

[Garg, Shivank]

On 12/20/2025 6:08 AM, Wei Yang wrote:
> On Fri, Dec 19, 2025 at 02:54:40PM +0530, Garg, Shivank wrote:
>>
>>
>> On 12/17/2025 8:18 AM, Wei Yang wrote:
>>> On Tue, Dec 16, 2025 at 11:11:38AM +0000, Shivank Garg wrote:
>>>> Replace 'goto skip' with actual logic for better code readability.
>>>>
>>>> No functional change.
>>>>
>>>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>>>> ---
>>>> mm/khugepaged.c | 7 ++++---
>>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>> index 6c8c35d3e0c9..107146f012b1 100644
>>>> --- a/mm/khugepaged.c
>>>> +++ b/mm/khugepaged.c
>>>> @@ -2442,14 +2442,15 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>>>> 			break;
>>>> 		}
>>>> 		if (!thp_vma_allowable_order(vma, vma->vm_flags, TVA_KHUGEPAGED, PMD_ORDER)) {
>>>> -skip:
>>>> 			progress++;
>>>> 			continue;
>>>> 		}
>>>> 		hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE);
>>>> 		hend = round_down(vma->vm_end, HPAGE_PMD_SIZE);
>>>> -		if (khugepaged_scan.address > hend)
>>>> -			goto skip;
>>>> +		if (khugepaged_scan.address > hend) {
>>>> +			progress++;
>>>> +			continue;
>>>> +		}
>>>
>>> Hi, Shivank
>>>
>>> The change here looks good, while I come up with an question.
>>>
>>> The @progress here seems record two things:
>>>
>>>   * number of pages scaned
>>>   * number of vma skipped
>>>
>> Three things: number of mm. It's incremented 1 for whole khugepaged_scan_mm_slot().
>>
> 
> Agree.
> 
>>
>>> While in very rare case, we may miss to count the second case.
>>>
>>> For example, we have following vmas in a process:
>>>
>>>      vma1             vma2
>>>     +----------------+------------+
>>>     |2M              |1M          |
>>>     +----------------+------------+
>>>
>>> Let's assume vma1 is exactly HPAGE_PMD_SIZE and also HPAGE_PMD_SIZE aligned.
>>> But vma2 is only half of HPAGE_PMD_SIZE.
>>>
>>> When scan finish vma1 and start on vma2, we would have hstart = hend =
>>> address. So we continue here but would not do real scan, since address == hend.
>>>
>>> I am thinking whether this could handle it:
>>>
>>> 		if (khugepaged_scan.address > hend || hend <= hstart) {
>>> 			progress++;
>>> 			continue;
>>> 		}
>>>
>>> Do you thinks I am correct on this?
>>
>> I think you're correct.
>> IIUC, @progress acts as rate limiter here.
>>
>> It is increasing +1 for whole, and then increases by +1 per VMA (if skipped),
>> or by +HPAGE_PMD_NR (if actually scanned).
>>
>> So, progress ensuring the hugepaged_do_scan run only until (progress >= pages)
>> at which point it yields and sleeps (wait_event_freezable).
>>
>> Without your suggested fix, if a process contains a large number of small VMAs (where
>> round_up hstart >= round_down(hend), it will unfairly consume more CPU cycles before
>> yielding compared to a process with fewer or aligned VMAs.
> 
> You are right. While I am not sure it exists in reality, but in theory it
> could be.
> 
>>
>> I think your suggestion is ensuring fairness by charging 'progress' count correctly.
>>
> 
> Thanks for your confirmation. Would you mind add a cleanup in next version, or
> you prefer me to send it :-)

Sure, I'll add this fix patch in the next version.

Thanks,
Shivank

Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label

[Wei Yang]

On Sun, Dec 21, 2025 at 12:02:11AM +0530, Garg, Shivank wrote:
>
>
>On 12/20/2025 6:08 AM, Wei Yang wrote:
>> On Fri, Dec 19, 2025 at 02:54:40PM +0530, Garg, Shivank wrote:
>>>
>>>
>>> On 12/17/2025 8:18 AM, Wei Yang wrote:
>>>> On Tue, Dec 16, 2025 at 11:11:38AM +0000, Shivank Garg wrote:
>>>>> Replace 'goto skip' with actual logic for better code readability.
>>>>>
>>>>> No functional change.
>>>>>
>>>>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>>>>> ---
>>>>> mm/khugepaged.c | 7 ++++---
>>>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>>> index 6c8c35d3e0c9..107146f012b1 100644
>>>>> --- a/mm/khugepaged.c
>>>>> +++ b/mm/khugepaged.c
>>>>> @@ -2442,14 +2442,15 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
>>>>> 			break;
>>>>> 		}
>>>>> 		if (!thp_vma_allowable_order(vma, vma->vm_flags, TVA_KHUGEPAGED, PMD_ORDER)) {
>>>>> -skip:
>>>>> 			progress++;
>>>>> 			continue;
>>>>> 		}
>>>>> 		hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE);
>>>>> 		hend = round_down(vma->vm_end, HPAGE_PMD_SIZE);
>>>>> -		if (khugepaged_scan.address > hend)
>>>>> -			goto skip;
>>>>> +		if (khugepaged_scan.address > hend) {
>>>>> +			progress++;
>>>>> +			continue;
>>>>> +		}
>>>>
>>>> Hi, Shivank
>>>>
>>>> The change here looks good, while I come up with an question.
>>>>
>>>> The @progress here seems record two things:
>>>>
>>>>   * number of pages scaned
>>>>   * number of vma skipped
>>>>
>>> Three things: number of mm. It's incremented 1 for whole khugepaged_scan_mm_slot().
>>>
>> 
>> Agree.
>> 
>>>
>>>> While in very rare case, we may miss to count the second case.
>>>>
>>>> For example, we have following vmas in a process:
>>>>
>>>>      vma1             vma2
>>>>     +----------------+------------+
>>>>     |2M              |1M          |
>>>>     +----------------+------------+
>>>>
>>>> Let's assume vma1 is exactly HPAGE_PMD_SIZE and also HPAGE_PMD_SIZE aligned.
>>>> But vma2 is only half of HPAGE_PMD_SIZE.
>>>>
>>>> When scan finish vma1 and start on vma2, we would have hstart = hend =
>>>> address. So we continue here but would not do real scan, since address == hend.
>>>>
>>>> I am thinking whether this could handle it:
>>>>
>>>> 		if (khugepaged_scan.address > hend || hend <= hstart) {
>>>> 			progress++;
>>>> 			continue;
>>>> 		}
>>>>
>>>> Do you thinks I am correct on this?
>>>
>>> I think you're correct.
>>> IIUC, @progress acts as rate limiter here.
>>>
>>> It is increasing +1 for whole, and then increases by +1 per VMA (if skipped),
>>> or by +HPAGE_PMD_NR (if actually scanned).
>>>
>>> So, progress ensuring the hugepaged_do_scan run only until (progress >= pages)
>>> at which point it yields and sleeps (wait_event_freezable).
>>>
>>> Without your suggested fix, if a process contains a large number of small VMAs (where
>>> round_up hstart >= round_down(hend), it will unfairly consume more CPU cycles before
>>> yielding compared to a process with fewer or aligned VMAs.
>> 
>> You are right. While I am not sure it exists in reality, but in theory it
>> could be.
>> 
>>>
>>> I think your suggestion is ensuring fairness by charging 'progress' count correctly.
>>>
>> 
>> Thanks for your confirmation. Would you mind add a cleanup in next version, or
>> you prefer me to send it :-)
>
>Sure, I'll add this fix patch in the next version.
>

Thanks

>Thanks,
>Shivank

-- 
Wei Yang
Help you, Help me