[PATCH v3 0/3] mm: khugepaged: fix tracepoint UAF

[PATCH v3 0/3] mm: khugepaged: fix tracepoint UAF

[Vernon Yang]

From: Vernon Yang <yanglincheng@kylinos.cn>

The khugepaged tracepoints take a folio pointer and call folio_pfn(),
but by then the folio may no longer be valid: freed after folio_put(),
folio_unlock() or pte_unmap_unlock(), or not a folio at all but an
xarray-encoded swap entry. On classic SPARSEMEM, dereferencing it oopses
khugepaged as soon as the trace event is enabled; on other memory models
it merely prints a bogus pfn.

Pass the pfn to the tracepoints directly, captured while the folio is
still pinned, closing the use-after-free windows in
mm_khugepaged_scan_file(), mm_khugepaged_scan_pmd() and
mm_khugepaged_collapse_file().

This series is based on mm-new.

V2 -> V3:
- Place folio_pfn() inside the xas_for_each() loop in PATCH#1.
- Already defaulted the pfn value to -1, to simple it in PATCH#2.

V1 -> V2:
- Instead of passing the folio, just pass the pfn directly.
- Using the folio_pfn() before dropping the reference or the page table
  lock.

V2 : https://lore.kernel.org/linux-mm/20260815051924.194810-1-vernon2gm@gmail.com/
V1 : https://lore.kernel.org/linux-mm/20260811133655.267739-1-vernon2gm@gmail.com/

Vernon Yang (3):
  mm: khugepaged: fix swap entry value to folio_pfn()
  mm: khugepaged: fix folio is used after pte_unmap_unlock()
  mm: khugepaged: fix folio is used after folio_put/unlock()

 include/trace/events/huge_memory.h | 18 +++++++++---------
 mm/khugepaged.c                    | 15 ++++++++++++---
 2 files changed, 21 insertions(+), 12 deletions(-)


base-commit: a032d41a86cb82a747bc14d9c82b3e153a9a9ab7
--
2.53.0

[PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[Vernon Yang]

From: Vernon Yang <yanglincheng@kylinos.cn>

When the swap entries found exceed max_ptes_swap, the loop is left via
break with folio still holding the xarray value that encodes the swap
entry, not valid folio pointer.

That value is passed to trace_mm_khugepaged_scan_file(), which feeds it
to folio_pfn(). On FLATMEM and SPARSEMEM_VMEMMAP, the page_to_pfn() is
plain pointer arithmetic, so the trace event merely prints bogus
scan_pfn. On classic SPARSEMEM, the page_to_pfn() reads page->flags,
dereferencing the tiny encoded integer and oopsing khugepaged whenever
the trace event is enabled.

So when folio is the swap entry value, simply set pfn to -1, just like
exhausted scan naturally.

And the folio_put() has maybe dropped the last reference of folio. The
trace_mm_khugepaged_scan_file() is left with a dangling folio pointer.
so using the folio_pfn() before dropping the reference, closing
use-after-free window.

Fixes: d41fd2016ed0 ("mm/khugepaged: add tracepoint to hpage_collapse_scan_file()")
Cc: stable@vger.kernel.org
Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
---
 include/trace/events/huge_memory.h | 6 +++---
 mm/khugepaged.c                    | 5 ++++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
index 5a48c5406cce..7b526528f85b 100644
--- a/include/trace/events/huge_memory.h
+++ b/include/trace/events/huge_memory.h
@@ -178,10 +178,10 @@ TRACE_EVENT(mm_collapse_huge_page_swapin,
 
 TRACE_EVENT(mm_khugepaged_scan_file,
 
-	TP_PROTO(struct mm_struct *mm, struct folio *folio, struct file *file,
+	TP_PROTO(struct mm_struct *mm, unsigned long pfn, struct file *file,
 		 int present, int swap, int result),
 
-	TP_ARGS(mm, folio, file, present, swap, result),
+	TP_ARGS(mm, pfn, file, present, swap, result),
 
 	TP_STRUCT__entry(
 		__field(struct mm_struct *, mm)
@@ -194,7 +194,7 @@ TRACE_EVENT(mm_khugepaged_scan_file,
 
 	TP_fast_assign(
 		__entry->mm = mm;
-		__entry->pfn = folio ? folio_pfn(folio) : -1;
+		__entry->pfn = pfn;
 		__assign_str(filename);
 		__entry->present = present;
 		__entry->swap = swap;
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 79effd3f3da4..00337405c0e0 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2689,6 +2689,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
 	int present, swap;
 	int node = NUMA_NO_NODE;
 	enum scan_result result = SCAN_SUCCEED;
+	unsigned long pfn;
 
 	present = 0;
 	swap = 0;
@@ -2719,6 +2720,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
 			continue;
 		}
 
+		pfn = folio_pfn(folio);
 		if (is_pmd_order(folio_order(folio))) {
 			result = SCAN_PTE_MAPPED_HUGEPAGE;
 			/*
@@ -2779,7 +2781,8 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
 		}
 	}
 
-	trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
+	trace_mm_khugepaged_scan_file(mm, (!folio || xa_is_value(folio)) ? -1 : pfn,
+				      file, present, swap, result);
 	return result;
 }
 
-- 
2.53.0

[PATCH v3 2/3] mm: khugepaged: fix folio is used after pte_unmap_unlock()

[Vernon Yang]

From: Vernon Yang <yanglincheng@kylinos.cn>

After the page table lock has dropped, the folio can be freed
concurrently. The trace_mm_khugepaged_scan_pmd() is left with
a dangling folio pointer.

So using the folio_pfn() before dropping the page table lock,
closing use-after-free window.

Fixes: 7d2eba0557c1 ("mm: add tracepoint for scanning pages")
Cc: stable@vger.kernel.org
Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
---
 include/trace/events/huge_memory.h | 6 +++---
 mm/khugepaged.c                    | 5 ++++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
index 7b526528f85b..fa828967e1fb 100644
--- a/include/trace/events/huge_memory.h
+++ b/include/trace/events/huge_memory.h
@@ -55,10 +55,10 @@ SCAN_STATUS
 
 TRACE_EVENT(mm_khugepaged_scan_pmd,
 
-	TP_PROTO(struct mm_struct *mm, struct folio *folio,
+	TP_PROTO(struct mm_struct *mm, unsigned long pfn,
 		 int referenced, int none_or_zero, int status, int unmapped),
 
-	TP_ARGS(mm, folio, referenced, none_or_zero, status, unmapped),
+	TP_ARGS(mm, pfn, referenced, none_or_zero, status, unmapped),
 
 	TP_STRUCT__entry(
 		__field(struct mm_struct *, mm)
@@ -71,7 +71,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
 
 	TP_fast_assign(
 		__entry->mm = mm;
-		__entry->pfn = folio ? folio_pfn(folio) : -1;
+		__entry->pfn = pfn;
 		__entry->referenced = referenced;
 		__entry->none_or_zero = none_or_zero;
 		__entry->status = status;
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 00337405c0e0..4e0fca5942dd 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1618,6 +1618,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
 	enum scan_result result = SCAN_FAIL;
 	struct page *page = NULL;
 	struct folio *folio = NULL;
+	unsigned long pfn = -1;
 	unsigned long addr;
 	unsigned long enabled_orders;
 	spinlock_t *ptl;
@@ -1780,6 +1781,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
 		result = SCAN_SUCCEED;
 	}
 out_unmap:
+	if (folio)
+		pfn = folio_pfn(folio);
 	pte_unmap_unlock(pte, ptl);
 	if (result == SCAN_SUCCEED) {
 		/* collapse_huge_page() expects the lock to be dropped before calling */
@@ -1790,7 +1793,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
 		*lock_dropped = true;
 	}
 out:
-	trace_mm_khugepaged_scan_pmd(mm, folio, referenced,
+	trace_mm_khugepaged_scan_pmd(mm, pfn, referenced,
 				     none_or_zero, result, unmapped);
 	return result;
 }
-- 
2.53.0

[PATCH v3 3/3] mm: khugepaged: fix folio is used after folio_put/unlock()

[Vernon Yang]

From: Vernon Yang <yanglincheng@kylinos.cn>

On the rollback path, folio_put() has already dropped the last reference
of new_folio. On the success path, new_folio is already unlocked and can
be freed concurrently. The trace_mm_khugepaged_collapse_file() is left
with a dangling folio pointer.

So using the folio_pfn() before dropping the reference, closing
use-after-free window.

Fixes: 4c9473e87e75 ("mm/khugepaged: add tracepoint to collapse_file()")
Cc: stable@vger.kernel.org
Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
---
 include/trace/events/huge_memory.h | 6 +++---
 mm/khugepaged.c                    | 5 ++++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
index fa828967e1fb..5fb4d92cfd84 100644
--- a/include/trace/events/huge_memory.h
+++ b/include/trace/events/huge_memory.h
@@ -211,10 +211,10 @@ TRACE_EVENT(mm_khugepaged_scan_file,
 );
 
 TRACE_EVENT(mm_khugepaged_collapse_file,
-	TP_PROTO(struct mm_struct *mm, struct folio *new_folio, pgoff_t index,
+	TP_PROTO(struct mm_struct *mm, unsigned long new_pfn, pgoff_t index,
 			unsigned long addr, bool is_shmem, struct file *file,
 			int nr, int result),
-	TP_ARGS(mm, new_folio, index, addr, is_shmem, file, nr, result),
+	TP_ARGS(mm, new_pfn, index, addr, is_shmem, file, nr, result),
 	TP_STRUCT__entry(
 		__field(struct mm_struct *, mm)
 		__field(unsigned long, hpfn)
@@ -228,7 +228,7 @@ TRACE_EVENT(mm_khugepaged_collapse_file,
 
 	TP_fast_assign(
 		__entry->mm = mm;
-		__entry->hpfn = new_folio ? folio_pfn(new_folio) : -1;
+		__entry->hpfn = new_pfn;
 		__entry->index = index;
 		__entry->addr = addr;
 		__entry->is_shmem = is_shmem;
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 4e0fca5942dd..24347f1a94ae 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2254,6 +2254,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
 	struct address_space *mapping = file->f_mapping;
 	struct page *dst;
 	struct folio *folio, *tmp, *new_folio;
+	unsigned long new_pfn = -1;
 	pgoff_t index = 0, end = start + HPAGE_PMD_NR;
 	LIST_HEAD(pagelist);
 	XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
@@ -2633,6 +2634,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
 	retract_page_tables(mapping, start);
 	if (cc && !cc->is_khugepaged)
 		result = SCAN_PTE_MAPPED_HUGEPAGE;
+	new_pfn = folio_pfn(new_folio);
 	folio_unlock(new_folio);
 
 	/*
@@ -2671,12 +2673,13 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
 	}
 
 	new_folio->mapping = NULL;
+	new_pfn = folio_pfn(new_folio);
 
 	folio_unlock(new_folio);
 	folio_put(new_folio);
 out:
 	VM_BUG_ON(!list_empty(&pagelist));
-	trace_mm_khugepaged_collapse_file(mm, new_folio, index, addr, is_shmem, file, HPAGE_PMD_NR, result);
+	trace_mm_khugepaged_collapse_file(mm, new_pfn, index, addr, is_shmem, file, HPAGE_PMD_NR, result);
 	return result;
 }
 
-- 
2.53.0

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[David Hildenbrand (Arm)]

On 8/24/26 11:29, Vernon Yang wrote:
> From: Vernon Yang <yanglincheng@kylinos.cn>
> 
> When the swap entries found exceed max_ptes_swap, the loop is left via
> break with folio still holding the xarray value that encodes the swap
> entry, not valid folio pointer.
> 
> That value is passed to trace_mm_khugepaged_scan_file(), which feeds it
> to folio_pfn(). On FLATMEM and SPARSEMEM_VMEMMAP, the page_to_pfn() is
> plain pointer arithmetic, so the trace event merely prints bogus
> scan_pfn. On classic SPARSEMEM, the page_to_pfn() reads page->flags,
> dereferencing the tiny encoded integer and oopsing khugepaged whenever
> the trace event is enabled.
> 
> So when folio is the swap entry value, simply set pfn to -1, just like
> exhausted scan naturally.
> 
> And the folio_put() has maybe dropped the last reference of folio. The
> trace_mm_khugepaged_scan_file() is left with a dangling folio pointer.
> so using the folio_pfn() before dropping the reference, closing
> use-after-free window.
> 
> Fixes: d41fd2016ed0 ("mm/khugepaged: add tracepoint to hpage_collapse_scan_file()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> ---
>  include/trace/events/huge_memory.h | 6 +++---
>  mm/khugepaged.c                    | 5 ++++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> index 5a48c5406cce..7b526528f85b 100644
> --- a/include/trace/events/huge_memory.h
> +++ b/include/trace/events/huge_memory.h
> @@ -178,10 +178,10 @@ TRACE_EVENT(mm_collapse_huge_page_swapin,
>  
>  TRACE_EVENT(mm_khugepaged_scan_file,
>  
> -	TP_PROTO(struct mm_struct *mm, struct folio *folio, struct file *file,
> +	TP_PROTO(struct mm_struct *mm, unsigned long pfn, struct file *file,
>  		 int present, int swap, int result),
>  
> -	TP_ARGS(mm, folio, file, present, swap, result),
> +	TP_ARGS(mm, pfn, file, present, swap, result),
>  
>  	TP_STRUCT__entry(
>  		__field(struct mm_struct *, mm)
> @@ -194,7 +194,7 @@ TRACE_EVENT(mm_khugepaged_scan_file,
>  
>  	TP_fast_assign(
>  		__entry->mm = mm;
> -		__entry->pfn = folio ? folio_pfn(folio) : -1;
> +		__entry->pfn = pfn;
>  		__assign_str(filename);
>  		__entry->present = present;
>  		__entry->swap = swap;
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 79effd3f3da4..00337405c0e0 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2689,6 +2689,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
>  	int present, swap;
>  	int node = NUMA_NO_NODE;
>  	enum scan_result result = SCAN_SUCCEED;
> +	unsigned long pfn;
>  
>  	present = 0;
>  	swap = 0;
> @@ -2719,6 +2720,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
>  			continue;
>  		}
>  
> +		pfn = folio_pfn(folio);
>  		if (is_pmd_order(folio_order(folio))) {
>  			result = SCAN_PTE_MAPPED_HUGEPAGE;
>  			/*
> @@ -2779,7 +2781,8 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
>  		}
>  	}
>  
> -	trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
> +	trace_mm_khugepaged_scan_file(mm, (!folio || xa_is_value(folio)) ? -1 : pfn,
> +				      file, present, swap, result);
>  	return result;
>  }
>  

Shouldn't we just reset PFN to -1 at the beginning of the loop (and set it
initially)?

-- 
Cheers,

David

Re: [PATCH v3 2/3] mm: khugepaged: fix folio is used after pte_unmap_unlock()

[David Hildenbrand (Arm)]

On 8/24/26 11:29, Vernon Yang wrote:
> From: Vernon Yang <yanglincheng@kylinos.cn>
> 
> After the page table lock has dropped, the folio can be freed
> concurrently. The trace_mm_khugepaged_scan_pmd() is left with
> a dangling folio pointer.
> 
> So using the folio_pfn() before dropping the page table lock,
> closing use-after-free window.
> 
> Fixes: 7d2eba0557c1 ("mm: add tracepoint for scanning pages")
> Cc: stable@vger.kernel.org
> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> ---
>  include/trace/events/huge_memory.h | 6 +++---
>  mm/khugepaged.c                    | 5 ++++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> index 7b526528f85b..fa828967e1fb 100644
> --- a/include/trace/events/huge_memory.h
> +++ b/include/trace/events/huge_memory.h
> @@ -55,10 +55,10 @@ SCAN_STATUS
>  
>  TRACE_EVENT(mm_khugepaged_scan_pmd,
>  
> -	TP_PROTO(struct mm_struct *mm, struct folio *folio,
> +	TP_PROTO(struct mm_struct *mm, unsigned long pfn,
>  		 int referenced, int none_or_zero, int status, int unmapped),
>  
> -	TP_ARGS(mm, folio, referenced, none_or_zero, status, unmapped),
> +	TP_ARGS(mm, pfn, referenced, none_or_zero, status, unmapped),
>  
>  	TP_STRUCT__entry(
>  		__field(struct mm_struct *, mm)
> @@ -71,7 +71,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
>  
>  	TP_fast_assign(
>  		__entry->mm = mm;
> -		__entry->pfn = folio ? folio_pfn(folio) : -1;
> +		__entry->pfn = pfn;
>  		__entry->referenced = referenced;
>  		__entry->none_or_zero = none_or_zero;
>  		__entry->status = status;
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 00337405c0e0..4e0fca5942dd 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -1618,6 +1618,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>  	enum scan_result result = SCAN_FAIL;
>  	struct page *page = NULL;
>  	struct folio *folio = NULL;
> +	unsigned long pfn = -1;
>  	unsigned long addr;
>  	unsigned long enabled_orders;
>  	spinlock_t *ptl;
> @@ -1780,6 +1781,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>  		result = SCAN_SUCCEED;
>  	}
>  out_unmap:
> +	if (folio)
> +		pfn = folio_pfn(folio);

Should we reset the folio to NULL at the beginning of the loop? Then we really
only trace the PFN if it really was problematic.

-- 
Cheers,

David

Re: [PATCH v3 3/3] mm: khugepaged: fix folio is used after folio_put/unlock()

[David Hildenbrand (Arm)]

On 8/24/26 11:29, Vernon Yang wrote:
> From: Vernon Yang <yanglincheng@kylinos.cn>
> 
> On the rollback path, folio_put() has already dropped the last reference
> of new_folio. On the success path, new_folio is already unlocked and can
> be freed concurrently. The trace_mm_khugepaged_collapse_file() is left
> with a dangling folio pointer.
> 
> So using the folio_pfn() before dropping the reference, closing
> use-after-free window.
> 
> Fixes: 4c9473e87e75 ("mm/khugepaged: add tracepoint to collapse_file()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> ---
>  include/trace/events/huge_memory.h | 6 +++---
>  mm/khugepaged.c                    | 5 ++++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> index fa828967e1fb..5fb4d92cfd84 100644
> --- a/include/trace/events/huge_memory.h
> +++ b/include/trace/events/huge_memory.h
> @@ -211,10 +211,10 @@ TRACE_EVENT(mm_khugepaged_scan_file,
>  );
>  
>  TRACE_EVENT(mm_khugepaged_collapse_file,
> -	TP_PROTO(struct mm_struct *mm, struct folio *new_folio, pgoff_t index,
> +	TP_PROTO(struct mm_struct *mm, unsigned long new_pfn, pgoff_t index,
>  			unsigned long addr, bool is_shmem, struct file *file,
>  			int nr, int result),
> -	TP_ARGS(mm, new_folio, index, addr, is_shmem, file, nr, result),
> +	TP_ARGS(mm, new_pfn, index, addr, is_shmem, file, nr, result),
>  	TP_STRUCT__entry(
>  		__field(struct mm_struct *, mm)
>  		__field(unsigned long, hpfn)
> @@ -228,7 +228,7 @@ TRACE_EVENT(mm_khugepaged_collapse_file,
>  
>  	TP_fast_assign(
>  		__entry->mm = mm;
> -		__entry->hpfn = new_folio ? folio_pfn(new_folio) : -1;
> +		__entry->hpfn = new_pfn;
>  		__entry->index = index;
>  		__entry->addr = addr;
>  		__entry->is_shmem = is_shmem;
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 4e0fca5942dd..24347f1a94ae 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2254,6 +2254,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>  	struct address_space *mapping = file->f_mapping;
>  	struct page *dst;
>  	struct folio *folio, *tmp, *new_folio;
> +	unsigned long new_pfn = -1;
>  	pgoff_t index = 0, end = start + HPAGE_PMD_NR;
>  	LIST_HEAD(pagelist);
>  	XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
> @@ -2633,6 +2634,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>  	retract_page_tables(mapping, start);
>  	if (cc && !cc->is_khugepaged)
>  		result = SCAN_PTE_MAPPED_HUGEPAGE;
> +	new_pfn = folio_pfn(new_folio);

Why not set new_pfn once after successful alloc_charge_folio()?




-- 
Cheers,

David

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[Vernon Yang]

On Mon, Aug 24, 2026 at 01:54:18PM +0200, David Hildenbrand (Arm) wrote:
> On 8/24/26 11:29, Vernon Yang wrote:
> > From: Vernon Yang <yanglincheng@kylinos.cn>
> >
> > When the swap entries found exceed max_ptes_swap, the loop is left via
> > break with folio still holding the xarray value that encodes the swap
> > entry, not valid folio pointer.
> >
> > That value is passed to trace_mm_khugepaged_scan_file(), which feeds it
> > to folio_pfn(). On FLATMEM and SPARSEMEM_VMEMMAP, the page_to_pfn() is
> > plain pointer arithmetic, so the trace event merely prints bogus
> > scan_pfn. On classic SPARSEMEM, the page_to_pfn() reads page->flags,
> > dereferencing the tiny encoded integer and oopsing khugepaged whenever
> > the trace event is enabled.
> >
> > So when folio is the swap entry value, simply set pfn to -1, just like
> > exhausted scan naturally.
> >
> > And the folio_put() has maybe dropped the last reference of folio. The
> > trace_mm_khugepaged_scan_file() is left with a dangling folio pointer.
> > so using the folio_pfn() before dropping the reference, closing
> > use-after-free window.
> >
> > Fixes: d41fd2016ed0 ("mm/khugepaged: add tracepoint to hpage_collapse_scan_file()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> > ---
> >  include/trace/events/huge_memory.h | 6 +++---
> >  mm/khugepaged.c                    | 5 ++++-
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> > index 5a48c5406cce..7b526528f85b 100644
> > --- a/include/trace/events/huge_memory.h
> > +++ b/include/trace/events/huge_memory.h
> > @@ -178,10 +178,10 @@ TRACE_EVENT(mm_collapse_huge_page_swapin,
> >
> >  TRACE_EVENT(mm_khugepaged_scan_file,
> >
> > -	TP_PROTO(struct mm_struct *mm, struct folio *folio, struct file *file,
> > +	TP_PROTO(struct mm_struct *mm, unsigned long pfn, struct file *file,
> >  		 int present, int swap, int result),
> >
> > -	TP_ARGS(mm, folio, file, present, swap, result),
> > +	TP_ARGS(mm, pfn, file, present, swap, result),
> >
> >  	TP_STRUCT__entry(
> >  		__field(struct mm_struct *, mm)
> > @@ -194,7 +194,7 @@ TRACE_EVENT(mm_khugepaged_scan_file,
> >
> >  	TP_fast_assign(
> >  		__entry->mm = mm;
> > -		__entry->pfn = folio ? folio_pfn(folio) : -1;
> > +		__entry->pfn = pfn;
> >  		__assign_str(filename);
> >  		__entry->present = present;
> >  		__entry->swap = swap;
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 79effd3f3da4..00337405c0e0 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -2689,6 +2689,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >  	int present, swap;
> >  	int node = NUMA_NO_NODE;
> >  	enum scan_result result = SCAN_SUCCEED;
> > +	unsigned long pfn;
> >
> >  	present = 0;
> >  	swap = 0;
> > @@ -2719,6 +2720,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >  			continue;
> >  		}
> >
> > +		pfn = folio_pfn(folio);
> >  		if (is_pmd_order(folio_order(folio))) {
> >  			result = SCAN_PTE_MAPPED_HUGEPAGE;
> >  			/*
> > @@ -2779,7 +2781,8 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >  		}
> >  	}
> >
> > -	trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
> > +	trace_mm_khugepaged_scan_file(mm, (!folio || xa_is_value(folio)) ? -1 : pfn,
> > +				      file, present, swap, result);
> >  	return result;
> >  }
> >
>
> Shouldn't we just reset PFN to -1 at the beginning of the loop (and set it
> initially)?

When the `xas_for_each()` iteration to terminate and the folio operation
preceding is normal, but pfn will be incorrect.

If we just reset PFN to -1 at the __ending__ of the loop (and set it
initially), then the above situation won't occur. If so, I will do it in
the next version.

--
Cheers,
Vernon

Re: [PATCH v3 2/3] mm: khugepaged: fix folio is used after pte_unmap_unlock()

[Vernon Yang]

On Mon, Aug 24, 2026 at 01:57:15PM +0200, David Hildenbrand (Arm) wrote:
> On 8/24/26 11:29, Vernon Yang wrote:
> > From: Vernon Yang <yanglincheng@kylinos.cn>
> >
> > After the page table lock has dropped, the folio can be freed
> > concurrently. The trace_mm_khugepaged_scan_pmd() is left with
> > a dangling folio pointer.
> >
> > So using the folio_pfn() before dropping the page table lock,
> > closing use-after-free window.
> >
> > Fixes: 7d2eba0557c1 ("mm: add tracepoint for scanning pages")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> > ---
> >  include/trace/events/huge_memory.h | 6 +++---
> >  mm/khugepaged.c                    | 5 ++++-
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> > index 7b526528f85b..fa828967e1fb 100644
> > --- a/include/trace/events/huge_memory.h
> > +++ b/include/trace/events/huge_memory.h
> > @@ -55,10 +55,10 @@ SCAN_STATUS
> >
> >  TRACE_EVENT(mm_khugepaged_scan_pmd,
> >
> > -	TP_PROTO(struct mm_struct *mm, struct folio *folio,
> > +	TP_PROTO(struct mm_struct *mm, unsigned long pfn,
> >  		 int referenced, int none_or_zero, int status, int unmapped),
> >
> > -	TP_ARGS(mm, folio, referenced, none_or_zero, status, unmapped),
> > +	TP_ARGS(mm, pfn, referenced, none_or_zero, status, unmapped),
> >
> >  	TP_STRUCT__entry(
> >  		__field(struct mm_struct *, mm)
> > @@ -71,7 +71,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
> >
> >  	TP_fast_assign(
> >  		__entry->mm = mm;
> > -		__entry->pfn = folio ? folio_pfn(folio) : -1;
> > +		__entry->pfn = pfn;
> >  		__entry->referenced = referenced;
> >  		__entry->none_or_zero = none_or_zero;
> >  		__entry->status = status;
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 00337405c0e0..4e0fca5942dd 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -1618,6 +1618,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> >  	enum scan_result result = SCAN_FAIL;
> >  	struct page *page = NULL;
> >  	struct folio *folio = NULL;
> > +	unsigned long pfn = -1;
> >  	unsigned long addr;
> >  	unsigned long enabled_orders;
> >  	spinlock_t *ptl;
> > @@ -1780,6 +1781,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> >  		result = SCAN_SUCCEED;
> >  	}
> >  out_unmap:
> > +	if (folio)
> > +		pfn = folio_pfn(folio);
>
> Should we reset the folio to NULL at the beginning of the loop? Then we really
> only trace the PFN if it really was problematic.

Yes, this is a pre-existing bug, and I'll fix it together. Thanks!

But it is not at the beginning of the loop, it is at the __ending__ of
the loop, for the same reason as PATCH#1.

--
Cheers,
Vernon

Re: [PATCH v3 3/3] mm: khugepaged: fix folio is used after folio_put/unlock()

[Vernon Yang]

On Mon, Aug 24, 2026 at 01:59:53PM +0200, David Hildenbrand (Arm) wrote:
> On 8/24/26 11:29, Vernon Yang wrote:
> > From: Vernon Yang <yanglincheng@kylinos.cn>
> >
> > On the rollback path, folio_put() has already dropped the last reference
> > of new_folio. On the success path, new_folio is already unlocked and can
> > be freed concurrently. The trace_mm_khugepaged_collapse_file() is left
> > with a dangling folio pointer.
> >
> > So using the folio_pfn() before dropping the reference, closing
> > use-after-free window.
> >
> > Fixes: 4c9473e87e75 ("mm/khugepaged: add tracepoint to collapse_file()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> > ---
> >  include/trace/events/huge_memory.h | 6 +++---
> >  mm/khugepaged.c                    | 5 ++++-
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> > index fa828967e1fb..5fb4d92cfd84 100644
> > --- a/include/trace/events/huge_memory.h
> > +++ b/include/trace/events/huge_memory.h
> > @@ -211,10 +211,10 @@ TRACE_EVENT(mm_khugepaged_scan_file,
> >  );
> >
> >  TRACE_EVENT(mm_khugepaged_collapse_file,
> > -	TP_PROTO(struct mm_struct *mm, struct folio *new_folio, pgoff_t index,
> > +	TP_PROTO(struct mm_struct *mm, unsigned long new_pfn, pgoff_t index,
> >  			unsigned long addr, bool is_shmem, struct file *file,
> >  			int nr, int result),
> > -	TP_ARGS(mm, new_folio, index, addr, is_shmem, file, nr, result),
> > +	TP_ARGS(mm, new_pfn, index, addr, is_shmem, file, nr, result),
> >  	TP_STRUCT__entry(
> >  		__field(struct mm_struct *, mm)
> >  		__field(unsigned long, hpfn)
> > @@ -228,7 +228,7 @@ TRACE_EVENT(mm_khugepaged_collapse_file,
> >
> >  	TP_fast_assign(
> >  		__entry->mm = mm;
> > -		__entry->hpfn = new_folio ? folio_pfn(new_folio) : -1;
> > +		__entry->hpfn = new_pfn;
> >  		__entry->index = index;
> >  		__entry->addr = addr;
> >  		__entry->is_shmem = is_shmem;
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 4e0fca5942dd..24347f1a94ae 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -2254,6 +2254,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
> >  	struct address_space *mapping = file->f_mapping;
> >  	struct page *dst;
> >  	struct folio *folio, *tmp, *new_folio;
> > +	unsigned long new_pfn = -1;
> >  	pgoff_t index = 0, end = start + HPAGE_PMD_NR;
> >  	LIST_HEAD(pagelist);
> >  	XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
> > @@ -2633,6 +2634,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
> >  	retract_page_tables(mapping, start);
> >  	if (cc && !cc->is_khugepaged)
> >  		result = SCAN_PTE_MAPPED_HUGEPAGE;
> > +	new_pfn = folio_pfn(new_folio);
>
> Why not set new_pfn once after successful alloc_charge_folio()?

LGTM, I'll do it in the next version. Thanks!

--
Cheers,
Vernon

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[David Hildenbrand (Arm)]

On 8/26/26 04:44, Vernon Yang wrote:
> On Mon, Aug 24, 2026 at 01:54:18PM +0200, David Hildenbrand (Arm) wrote:
>> On 8/24/26 11:29, Vernon Yang wrote:
>>> From: Vernon Yang <yanglincheng@kylinos.cn>
>>>
>>> When the swap entries found exceed max_ptes_swap, the loop is left via
>>> break with folio still holding the xarray value that encodes the swap
>>> entry, not valid folio pointer.
>>>
>>> That value is passed to trace_mm_khugepaged_scan_file(), which feeds it
>>> to folio_pfn(). On FLATMEM and SPARSEMEM_VMEMMAP, the page_to_pfn() is
>>> plain pointer arithmetic, so the trace event merely prints bogus
>>> scan_pfn. On classic SPARSEMEM, the page_to_pfn() reads page->flags,
>>> dereferencing the tiny encoded integer and oopsing khugepaged whenever
>>> the trace event is enabled.
>>>
>>> So when folio is the swap entry value, simply set pfn to -1, just like
>>> exhausted scan naturally.
>>>
>>> And the folio_put() has maybe dropped the last reference of folio. The
>>> trace_mm_khugepaged_scan_file() is left with a dangling folio pointer.
>>> so using the folio_pfn() before dropping the reference, closing
>>> use-after-free window.
>>>
>>> Fixes: d41fd2016ed0 ("mm/khugepaged: add tracepoint to hpage_collapse_scan_file()")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
>>> ---
>>>  include/trace/events/huge_memory.h | 6 +++---
>>>  mm/khugepaged.c                    | 5 ++++-
>>>  2 files changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
>>> index 5a48c5406cce..7b526528f85b 100644
>>> --- a/include/trace/events/huge_memory.h
>>> +++ b/include/trace/events/huge_memory.h
>>> @@ -178,10 +178,10 @@ TRACE_EVENT(mm_collapse_huge_page_swapin,
>>>
>>>  TRACE_EVENT(mm_khugepaged_scan_file,
>>>
>>> -	TP_PROTO(struct mm_struct *mm, struct folio *folio, struct file *file,
>>> +	TP_PROTO(struct mm_struct *mm, unsigned long pfn, struct file *file,
>>>  		 int present, int swap, int result),
>>>
>>> -	TP_ARGS(mm, folio, file, present, swap, result),
>>> +	TP_ARGS(mm, pfn, file, present, swap, result),
>>>
>>>  	TP_STRUCT__entry(
>>>  		__field(struct mm_struct *, mm)
>>> @@ -194,7 +194,7 @@ TRACE_EVENT(mm_khugepaged_scan_file,
>>>
>>>  	TP_fast_assign(
>>>  		__entry->mm = mm;
>>> -		__entry->pfn = folio ? folio_pfn(folio) : -1;
>>> +		__entry->pfn = pfn;
>>>  		__assign_str(filename);
>>>  		__entry->present = present;
>>>  		__entry->swap = swap;
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index 79effd3f3da4..00337405c0e0 100644
>>> --- a/mm/khugepaged.c
>>> +++ b/mm/khugepaged.c
>>> @@ -2689,6 +2689,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
>>>  	int present, swap;
>>>  	int node = NUMA_NO_NODE;
>>>  	enum scan_result result = SCAN_SUCCEED;
>>> +	unsigned long pfn;
>>>
>>>  	present = 0;
>>>  	swap = 0;
>>> @@ -2719,6 +2720,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
>>>  			continue;
>>>  		}
>>>
>>> +		pfn = folio_pfn(folio);
>>>  		if (is_pmd_order(folio_order(folio))) {
>>>  			result = SCAN_PTE_MAPPED_HUGEPAGE;
>>>  			/*
>>> @@ -2779,7 +2781,8 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
>>>  		}
>>>  	}
>>>
>>> -	trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
>>> +	trace_mm_khugepaged_scan_file(mm, (!folio || xa_is_value(folio)) ? -1 : pfn,
>>> +				      file, present, swap, result);
>>>  	return result;
>>>  }
>>>
>>
>> Shouldn't we just reset PFN to -1 at the beginning of the loop (and set it
>> initially)?
> 
> When the `xas_for_each()` iteration to terminate and the folio operation
> preceding is normal, but pfn will be incorrect.

The PFN is only relevant when a folio participated in the failure. Maybe the
following would be cleanest?

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 75639298efc27..371ee0b16d10c 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2683,6 +2683,7 @@ static enum scan_result collapse_scan_file(struct
mm_struct *mm,
        int present, swap;
        int node = NUMA_NO_NODE;
        enum scan_result result = SCAN_SUCCEED;
+       unsigned long problematic_pfn = -1;

        present = 0;
        swap = 0;
@@ -2714,6 +2715,7 @@ static enum scan_result collapse_scan_file(struct
mm_struct *mm,
                }

                if (is_pmd_order(folio_order(folio))) {
+                       problematic_pfn = folio_pfn(folio);
                        result = SCAN_PTE_MAPPED_HUGEPAGE;
                        /*
                         * PMD-sized THP implies that we can only try
@@ -2725,6 +2727,7 @@ static enum scan_result collapse_scan_file(struct
mm_struct *mm,

                node = folio_nid(folio);
                if (collapse_scan_abort(node, cc)) {
+                       problematic_pfn = folio_pfn(folio);
                        result = SCAN_SCAN_ABORT;
                        folio_put(folio);
                        break;
@@ -2732,12 +2735,14 @@ static enum scan_result collapse_scan_file(struct
mm_struct *mm,
                cc->node_load[node]++;

                if (!folio_test_lru(folio)) {
+                       problematic_pfn = folio_pfn(folio);
                        result = SCAN_PAGE_LRU;
                        folio_put(folio);
                        break;
                }

                if (folio_expected_ref_count(folio) + 1 != folio_ref_count(folio)) {
+                       problematic_pfn = folio_pfn(folio);
                        result = SCAN_PAGE_COUNT;
                        folio_put(folio);
                        break;
@@ -2773,7 +2778,7 @@ static enum scan_result collapse_scan_file(struct
mm_struct *mm,
                }
        }

-       trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
+       trace_mm_khugepaged_scan_file(mm, problematic_pfn, file, present, swap,
result);
        return result;
 }



-- 
Cheers,

David

Re: [PATCH v3 2/3] mm: khugepaged: fix folio is used after pte_unmap_unlock()

[David Hildenbrand (Arm)]

On 8/26/26 04:46, Vernon Yang wrote:
> On Mon, Aug 24, 2026 at 01:57:15PM +0200, David Hildenbrand (Arm) wrote:
>> On 8/24/26 11:29, Vernon Yang wrote:
>>> From: Vernon Yang <yanglincheng@kylinos.cn>
>>>
>>> After the page table lock has dropped, the folio can be freed
>>> concurrently. The trace_mm_khugepaged_scan_pmd() is left with
>>> a dangling folio pointer.
>>>
>>> So using the folio_pfn() before dropping the page table lock,
>>> closing use-after-free window.
>>>
>>> Fixes: 7d2eba0557c1 ("mm: add tracepoint for scanning pages")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
>>> ---
>>>  include/trace/events/huge_memory.h | 6 +++---
>>>  mm/khugepaged.c                    | 5 ++++-
>>>  2 files changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
>>> index 7b526528f85b..fa828967e1fb 100644
>>> --- a/include/trace/events/huge_memory.h
>>> +++ b/include/trace/events/huge_memory.h
>>> @@ -55,10 +55,10 @@ SCAN_STATUS
>>>
>>>  TRACE_EVENT(mm_khugepaged_scan_pmd,
>>>
>>> -	TP_PROTO(struct mm_struct *mm, struct folio *folio,
>>> +	TP_PROTO(struct mm_struct *mm, unsigned long pfn,
>>>  		 int referenced, int none_or_zero, int status, int unmapped),
>>>
>>> -	TP_ARGS(mm, folio, referenced, none_or_zero, status, unmapped),
>>> +	TP_ARGS(mm, pfn, referenced, none_or_zero, status, unmapped),
>>>
>>>  	TP_STRUCT__entry(
>>>  		__field(struct mm_struct *, mm)
>>> @@ -71,7 +71,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
>>>
>>>  	TP_fast_assign(
>>>  		__entry->mm = mm;
>>> -		__entry->pfn = folio ? folio_pfn(folio) : -1;
>>> +		__entry->pfn = pfn;
>>>  		__entry->referenced = referenced;
>>>  		__entry->none_or_zero = none_or_zero;
>>>  		__entry->status = status;
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index 00337405c0e0..4e0fca5942dd 100644
>>> --- a/mm/khugepaged.c
>>> +++ b/mm/khugepaged.c
>>> @@ -1618,6 +1618,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>>>  	enum scan_result result = SCAN_FAIL;
>>>  	struct page *page = NULL;
>>>  	struct folio *folio = NULL;
>>> +	unsigned long pfn = -1;
>>>  	unsigned long addr;
>>>  	unsigned long enabled_orders;
>>>  	spinlock_t *ptl;
>>> @@ -1780,6 +1781,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>>>  		result = SCAN_SUCCEED;
>>>  	}
>>>  out_unmap:
>>> +	if (folio)
>>> +		pfn = folio_pfn(folio);
>>
>> Should we reset the folio to NULL at the beginning of the loop? Then we really
>> only trace the PFN if it really was problematic.
> 
> Yes, this is a pre-existing bug, and I'll fix it together. Thanks!
> 
> But it is not at the beginning of the loop, it is at the __ending__ of
> the loop, for the same reason as PATCH#1.

See my reply to patch #1, hth.

-- 
Cheers,

David

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[Lorenzo Stoakes (ARM)]

On Wed, Aug 26, 2026 at 09:57:05AM +0200, David Hildenbrand (Arm) wrote:
> On 8/26/26 04:44, Vernon Yang wrote:
> > On Mon, Aug 24, 2026 at 01:54:18PM +0200, David Hildenbrand (Arm) wrote:
> >> On 8/24/26 11:29, Vernon Yang wrote:
> >>> From: Vernon Yang <yanglincheng@kylinos.cn>
> >>>
> >>> When the swap entries found exceed max_ptes_swap, the loop is left via
> >>> break with folio still holding the xarray value that encodes the swap
> >>> entry, not valid folio pointer.
> >>>
> >>> That value is passed to trace_mm_khugepaged_scan_file(), which feeds it
> >>> to folio_pfn(). On FLATMEM and SPARSEMEM_VMEMMAP, the page_to_pfn() is
> >>> plain pointer arithmetic, so the trace event merely prints bogus
> >>> scan_pfn. On classic SPARSEMEM, the page_to_pfn() reads page->flags,
> >>> dereferencing the tiny encoded integer and oopsing khugepaged whenever
> >>> the trace event is enabled.
> >>>
> >>> So when folio is the swap entry value, simply set pfn to -1, just like
> >>> exhausted scan naturally.
> >>>
> >>> And the folio_put() has maybe dropped the last reference of folio. The
> >>> trace_mm_khugepaged_scan_file() is left with a dangling folio pointer.
> >>> so using the folio_pfn() before dropping the reference, closing
> >>> use-after-free window.
> >>>
> >>> Fixes: d41fd2016ed0 ("mm/khugepaged: add tracepoint to hpage_collapse_scan_file()")
> >>> Cc: stable@vger.kernel.org
> >>> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> >>> ---
> >>>  include/trace/events/huge_memory.h | 6 +++---
> >>>  mm/khugepaged.c                    | 5 ++++-
> >>>  2 files changed, 7 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> >>> index 5a48c5406cce..7b526528f85b 100644
> >>> --- a/include/trace/events/huge_memory.h
> >>> +++ b/include/trace/events/huge_memory.h
> >>> @@ -178,10 +178,10 @@ TRACE_EVENT(mm_collapse_huge_page_swapin,
> >>>
> >>>  TRACE_EVENT(mm_khugepaged_scan_file,
> >>>
> >>> -	TP_PROTO(struct mm_struct *mm, struct folio *folio, struct file *file,
> >>> +	TP_PROTO(struct mm_struct *mm, unsigned long pfn, struct file *file,
> >>>  		 int present, int swap, int result),
> >>>
> >>> -	TP_ARGS(mm, folio, file, present, swap, result),
> >>> +	TP_ARGS(mm, pfn, file, present, swap, result),
> >>>
> >>>  	TP_STRUCT__entry(
> >>>  		__field(struct mm_struct *, mm)
> >>> @@ -194,7 +194,7 @@ TRACE_EVENT(mm_khugepaged_scan_file,
> >>>
> >>>  	TP_fast_assign(
> >>>  		__entry->mm = mm;
> >>> -		__entry->pfn = folio ? folio_pfn(folio) : -1;
> >>> +		__entry->pfn = pfn;
> >>>  		__assign_str(filename);
> >>>  		__entry->present = present;
> >>>  		__entry->swap = swap;
> >>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> >>> index 79effd3f3da4..00337405c0e0 100644
> >>> --- a/mm/khugepaged.c
> >>> +++ b/mm/khugepaged.c
> >>> @@ -2689,6 +2689,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >>>  	int present, swap;
> >>>  	int node = NUMA_NO_NODE;
> >>>  	enum scan_result result = SCAN_SUCCEED;
> >>> +	unsigned long pfn;
> >>>
> >>>  	present = 0;
> >>>  	swap = 0;
> >>> @@ -2719,6 +2720,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >>>  			continue;
> >>>  		}
> >>>
> >>> +		pfn = folio_pfn(folio);
> >>>  		if (is_pmd_order(folio_order(folio))) {
> >>>  			result = SCAN_PTE_MAPPED_HUGEPAGE;
> >>>  			/*
> >>> @@ -2779,7 +2781,8 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >>>  		}
> >>>  	}
> >>>
> >>> -	trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
> >>> +	trace_mm_khugepaged_scan_file(mm, (!folio || xa_is_value(folio)) ? -1 : pfn,
> >>> +				      file, present, swap, result);
> >>>  	return result;
> >>>  }
> >>>
> >>
> >> Shouldn't we just reset PFN to -1 at the beginning of the loop (and set it
> >> initially)?
> >
> > When the `xas_for_each()` iteration to terminate and the folio operation
> > preceding is normal, but pfn will be incorrect.
>
> The PFN is only relevant when a folio participated in the failure. Maybe the
> following would be cleanest?
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 75639298efc27..371ee0b16d10c 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2683,6 +2683,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>         int present, swap;
>         int node = NUMA_NO_NODE;
>         enum scan_result result = SCAN_SUCCEED;
> +       unsigned long problematic_pfn = -1;

I find this name... problematic :)

What about:

	pfn_t pfn = -1; /* Assign on failure before dropping ref */

Then:

	if (result == SCAN_SUCCEED) {
		...
		trace_mm_khugepaged_scan_file(mm, -1, file, present, swap, result);
	} else {
		trace_mm_khugepaged_scan_file(mm, pfn, file, present, swap, result);
	}

?

Other than that I do think your approach of assigning it on failure is the
right one.

>
>         present = 0;
>         swap = 0;
> @@ -2714,6 +2715,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>                 }
>
>                 if (is_pmd_order(folio_order(folio))) {
> +                       problematic_pfn = folio_pfn(folio);
>                         result = SCAN_PTE_MAPPED_HUGEPAGE;
>                         /*
>                          * PMD-sized THP implies that we can only try
> @@ -2725,6 +2727,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>
>                 node = folio_nid(folio);
>                 if (collapse_scan_abort(node, cc)) {
> +                       problematic_pfn = folio_pfn(folio);
>                         result = SCAN_SCAN_ABORT;
>                         folio_put(folio);
>                         break;
> @@ -2732,12 +2735,14 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>                 cc->node_load[node]++;
>
>                 if (!folio_test_lru(folio)) {
> +                       problematic_pfn = folio_pfn(folio);
>                         result = SCAN_PAGE_LRU;
>                         folio_put(folio);
>                         break;
>                 }
>
>                 if (folio_expected_ref_count(folio) + 1 != folio_ref_count(folio)) {
> +                       problematic_pfn = folio_pfn(folio);
>                         result = SCAN_PAGE_COUNT;
>                         folio_put(folio);
>                         break;
> @@ -2773,7 +2778,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>                 }
>         }
>
> -       trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
> +       trace_mm_khugepaged_scan_file(mm, problematic_pfn, file, present, swap,
> result);
>         return result;
>  }
>
>
>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[David Hildenbrand (Arm)]

On 8/26/26 10:07, Lorenzo Stoakes (ARM) wrote:
> On Wed, Aug 26, 2026 at 09:57:05AM +0200, David Hildenbrand (Arm) wrote:
>> On 8/26/26 04:44, Vernon Yang wrote:
>>>
>>> When the `xas_for_each()` iteration to terminate and the folio operation
>>> preceding is normal, but pfn will be incorrect.
>>
>> The PFN is only relevant when a folio participated in the failure. Maybe the
>> following would be cleanest?
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 75639298efc27..371ee0b16d10c 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -2683,6 +2683,7 @@ static enum scan_result collapse_scan_file(struct
>> mm_struct *mm,
>>         int present, swap;
>>         int node = NUMA_NO_NODE;
>>         enum scan_result result = SCAN_SUCCEED;
>> +       unsigned long problematic_pfn = -1;
> 
> I find this name... problematic :)

Elaborate.

-- 
Cheers,

David

Re: [PATCH v3 3/3] mm: khugepaged: fix folio is used after folio_put/unlock()

[Lorenzo Stoakes (ARM)]

On Mon, Aug 24, 2026 at 05:29:35PM +0800, Vernon Yang wrote:
> From: Vernon Yang <yanglincheng@kylinos.cn>
>
> On the rollback path, folio_put() has already dropped the last reference
> of new_folio. On the success path, new_folio is already unlocked and can
> be freed concurrently. The trace_mm_khugepaged_collapse_file() is left
> with a dangling folio pointer.
>
> So using the folio_pfn() before dropping the reference, closing
> use-after-free window.
>
> Fixes: 4c9473e87e75 ("mm/khugepaged: add tracepoint to collapse_file()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> ---
>  include/trace/events/huge_memory.h | 6 +++---
>  mm/khugepaged.c                    | 5 ++++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> index fa828967e1fb..5fb4d92cfd84 100644
> --- a/include/trace/events/huge_memory.h
> +++ b/include/trace/events/huge_memory.h
> @@ -211,10 +211,10 @@ TRACE_EVENT(mm_khugepaged_scan_file,
>  );
>
>  TRACE_EVENT(mm_khugepaged_collapse_file,
> -	TP_PROTO(struct mm_struct *mm, struct folio *new_folio, pgoff_t index,
> +	TP_PROTO(struct mm_struct *mm, unsigned long new_pfn, pgoff_t index,
>  			unsigned long addr, bool is_shmem, struct file *file,
>  			int nr, int result),
> -	TP_ARGS(mm, new_folio, index, addr, is_shmem, file, nr, result),
> +	TP_ARGS(mm, new_pfn, index, addr, is_shmem, file, nr, result),
>  	TP_STRUCT__entry(
>  		__field(struct mm_struct *, mm)
>  		__field(unsigned long, hpfn)
> @@ -228,7 +228,7 @@ TRACE_EVENT(mm_khugepaged_collapse_file,
>
>  	TP_fast_assign(
>  		__entry->mm = mm;
> -		__entry->hpfn = new_folio ? folio_pfn(new_folio) : -1;
> +		__entry->hpfn = new_pfn;
>  		__entry->index = index;
>  		__entry->addr = addr;
>  		__entry->is_shmem = is_shmem;
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 4e0fca5942dd..24347f1a94ae 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2254,6 +2254,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>  	struct address_space *mapping = file->f_mapping;
>  	struct page *dst;
>  	struct folio *folio, *tmp, *new_folio;
> +	unsigned long new_pfn = -1;

Nitty but:

Could we use pfn_t? I don't love that we are inconsistent with that.

Also the general pattern for pfn names is pfn_xxx so pfn_new instead?

>  	pgoff_t index = 0, end = start + HPAGE_PMD_NR;
>  	LIST_HEAD(pagelist);
>  	XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
> @@ -2633,6 +2634,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>  	retract_page_tables(mapping, start);
>  	if (cc && !cc->is_khugepaged)
>  		result = SCAN_PTE_MAPPED_HUGEPAGE;
> +	new_pfn = folio_pfn(new_folio);
>  	folio_unlock(new_folio);
>
>  	/*
> @@ -2671,12 +2673,13 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>  	}
>
>  	new_folio->mapping = NULL;
> +	new_pfn = folio_pfn(new_folio);
>
>  	folio_unlock(new_folio);
>  	folio_put(new_folio);
>  out:
>  	VM_BUG_ON(!list_empty(&pagelist));
> -	trace_mm_khugepaged_collapse_file(mm, new_folio, index, addr, is_shmem, file, HPAGE_PMD_NR, result);
> +	trace_mm_khugepaged_collapse_file(mm, new_pfn, index, addr, is_shmem, file, HPAGE_PMD_NR, result);
>  	return result;
>  }
>
> --
> 2.53.0
>

--
Cheers, Lorenzo

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[Lorenzo Stoakes (ARM)]

On Wed, Aug 26, 2026 at 10:08:58AM +0200, David Hildenbrand (Arm) wrote:
> On 8/26/26 10:07, Lorenzo Stoakes (ARM) wrote:
> > On Wed, Aug 26, 2026 at 09:57:05AM +0200, David Hildenbrand (Arm) wrote:
> >> On 8/26/26 04:44, Vernon Yang wrote:
> >>>
> >>> When the `xas_for_each()` iteration to terminate and the folio operation
> >>> preceding is normal, but pfn will be incorrect.
> >>
> >> The PFN is only relevant when a folio participated in the failure. Maybe the
> >> following would be cleanest?
> >>
> >> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> >> index 75639298efc27..371ee0b16d10c 100644
> >> --- a/mm/khugepaged.c
> >> +++ b/mm/khugepaged.c
> >> @@ -2683,6 +2683,7 @@ static enum scan_result collapse_scan_file(struct
> >> mm_struct *mm,
> >>         int present, swap;
> >>         int node = NUMA_NO_NODE;
> >>         enum scan_result result = SCAN_SUCCEED;
> >> +       unsigned long problematic_pfn = -1;
> >
> > I find this name... problematic :)
>
> Elaborate.

It's overly long, I read it and am confused as to what is 'problematic' or not,
it reads weirdly in English and pfn_xxx is the usual convention for naming of
pfn's anyway.

I made a suggestion in the reply as to how to refactor this to avoid the need
for assigning the name like that anyway, if you branch the result ==
SCAN_SUCCEED then it's implied.

I did wonder about pfn_folio but then that's confusing wrt the function, maybe
pfn_fail if you really want the name not to be pfn.

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH v3 2/3] mm: khugepaged: fix folio is used after pte_unmap_unlock()

[Lorenzo Stoakes (ARM)]

On Mon, Aug 24, 2026 at 05:29:34PM +0800, Vernon Yang wrote:
> From: Vernon Yang <yanglincheng@kylinos.cn>
>
> After the page table lock has dropped, the folio can be freed
> concurrently. The trace_mm_khugepaged_scan_pmd() is left with
> a dangling folio pointer.
>
> So using the folio_pfn() before dropping the page table lock,
> closing use-after-free window.
>
> Fixes: 7d2eba0557c1 ("mm: add tracepoint for scanning pages")
> Cc: stable@vger.kernel.org
> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> ---
>  include/trace/events/huge_memory.h | 6 +++---
>  mm/khugepaged.c                    | 5 ++++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> index 7b526528f85b..fa828967e1fb 100644
> --- a/include/trace/events/huge_memory.h
> +++ b/include/trace/events/huge_memory.h
> @@ -55,10 +55,10 @@ SCAN_STATUS
>
>  TRACE_EVENT(mm_khugepaged_scan_pmd,
>
> -	TP_PROTO(struct mm_struct *mm, struct folio *folio,
> +	TP_PROTO(struct mm_struct *mm, unsigned long pfn,
>  		 int referenced, int none_or_zero, int status, int unmapped),
>
> -	TP_ARGS(mm, folio, referenced, none_or_zero, status, unmapped),
> +	TP_ARGS(mm, pfn, referenced, none_or_zero, status, unmapped),
>
>  	TP_STRUCT__entry(
>  		__field(struct mm_struct *, mm)
> @@ -71,7 +71,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
>
>  	TP_fast_assign(
>  		__entry->mm = mm;
> -		__entry->pfn = folio ? folio_pfn(folio) : -1;
> +		__entry->pfn = pfn;
>  		__entry->referenced = referenced;
>  		__entry->none_or_zero = none_or_zero;
>  		__entry->status = status;
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 00337405c0e0..4e0fca5942dd 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -1618,6 +1618,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>  	enum scan_result result = SCAN_FAIL;
>  	struct page *page = NULL;
>  	struct folio *folio = NULL;
> +	unsigned long pfn = -1;

pfn_t please.

>  	unsigned long addr;
>  	unsigned long enabled_orders;
>  	spinlock_t *ptl;
> @@ -1780,6 +1781,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>  		result = SCAN_SUCCEED;
>  	}
>  out_unmap:
> +	if (folio)
> +		pfn = folio_pfn(folio);

And +1 to David on ensuring this is reset to NULL appropriately.

>  	pte_unmap_unlock(pte, ptl);
>  	if (result == SCAN_SUCCEED) {
>  		/* collapse_huge_page() expects the lock to be dropped before calling */
> @@ -1790,7 +1793,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>  		*lock_dropped = true;
>  	}
>  out:
> -	trace_mm_khugepaged_scan_pmd(mm, folio, referenced,
> +	trace_mm_khugepaged_scan_pmd(mm, pfn, referenced,
>  				     none_or_zero, result, unmapped);
>  	return result;
>  }
> --
> 2.53.0
>

--
Cheers, Lorenzo

Re: [PATCH v3 3/3] mm: khugepaged: fix folio is used after folio_put/unlock()

[David Hildenbrand (Arm)]

On 8/26/26 10:09, Lorenzo Stoakes (ARM) wrote:
> On Mon, Aug 24, 2026 at 05:29:35PM +0800, Vernon Yang wrote:
>> From: Vernon Yang <yanglincheng@kylinos.cn>
>>
>> On the rollback path, folio_put() has already dropped the last reference
>> of new_folio. On the success path, new_folio is already unlocked and can
>> be freed concurrently. The trace_mm_khugepaged_collapse_file() is left
>> with a dangling folio pointer.
>>
>> So using the folio_pfn() before dropping the reference, closing
>> use-after-free window.
>>
>> Fixes: 4c9473e87e75 ("mm/khugepaged: add tracepoint to collapse_file()")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
>> ---
>>  include/trace/events/huge_memory.h | 6 +++---
>>  mm/khugepaged.c                    | 5 ++++-
>>  2 files changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
>> index fa828967e1fb..5fb4d92cfd84 100644
>> --- a/include/trace/events/huge_memory.h
>> +++ b/include/trace/events/huge_memory.h
>> @@ -211,10 +211,10 @@ TRACE_EVENT(mm_khugepaged_scan_file,
>>  );
>>
>>  TRACE_EVENT(mm_khugepaged_collapse_file,
>> -	TP_PROTO(struct mm_struct *mm, struct folio *new_folio, pgoff_t index,
>> +	TP_PROTO(struct mm_struct *mm, unsigned long new_pfn, pgoff_t index,
>>  			unsigned long addr, bool is_shmem, struct file *file,
>>  			int nr, int result),
>> -	TP_ARGS(mm, new_folio, index, addr, is_shmem, file, nr, result),
>> +	TP_ARGS(mm, new_pfn, index, addr, is_shmem, file, nr, result),
>>  	TP_STRUCT__entry(
>>  		__field(struct mm_struct *, mm)
>>  		__field(unsigned long, hpfn)
>> @@ -228,7 +228,7 @@ TRACE_EVENT(mm_khugepaged_collapse_file,
>>
>>  	TP_fast_assign(
>>  		__entry->mm = mm;
>> -		__entry->hpfn = new_folio ? folio_pfn(new_folio) : -1;
>> +		__entry->hpfn = new_pfn;
>>  		__entry->index = index;
>>  		__entry->addr = addr;
>>  		__entry->is_shmem = is_shmem;
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 4e0fca5942dd..24347f1a94ae 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -2254,6 +2254,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>>  	struct address_space *mapping = file->f_mapping;
>>  	struct page *dst;
>>  	struct folio *folio, *tmp, *new_folio;
>> +	unsigned long new_pfn = -1;
> 
> Nitty but:
> 
> Could we use pfn_t? I don't love that we are inconsistent with that.

Note that the monstrosity we called pfn_t no longer exits. :)

-- 
Cheers,

David

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[David Hildenbrand (Arm)]

On 8/26/26 10:11, Lorenzo Stoakes (ARM) wrote:
> On Wed, Aug 26, 2026 at 10:08:58AM +0200, David Hildenbrand (Arm) wrote:
>> On 8/26/26 10:07, Lorenzo Stoakes (ARM) wrote:
>>>
>>> I find this name... problematic :)
>>
>> Elaborate.
> 
> It's overly long, I read it and am confused as to what is 'problematic' or not,
> it reads weirdly in English and pfn_xxx is the usual convention for naming of
> pfn's anyway.

Excuse me, what? Are you now just making up arguments?

> 
> I made a suggestion in the reply as to how to refactor this to avoid the need
> for assigning the name like that anyway, if you branch the result ==
> SCAN_SUCCEED then it's implied.

How is something that doesn't state the purpose make it any clearer?

-- 
Cheers,

David

Re: [PATCH v3 3/3] mm: khugepaged: fix folio is used after folio_put/unlock()

[Lorenzo Stoakes (ARM)]

On Wed, Aug 26, 2026 at 10:14:43AM +0200, David Hildenbrand (Arm) wrote:
> Note that the monstrosity we called pfn_t no longer exits. :)

Oops! :) Fair enough. I mistook it for one of the various benign typedef
unsigned long xxx_t decls and forgot (repressed?) the horror show elements of
struct { ... } with flags and what not.

Vernon - please disregard the pfn_t feedback, obviously :)

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[David Hildenbrand (Arm)]

On 8/26/26 10:16, David Hildenbrand (Arm) wrote:
> On 8/26/26 10:11, Lorenzo Stoakes (ARM) wrote:
>> On Wed, Aug 26, 2026 at 10:08:58AM +0200, David Hildenbrand (Arm) wrote:
>>>
>>> Elaborate.
>>
>> It's overly long, I read it and am confused as to what is 'problematic' or not,
>> it reads weirdly in English and pfn_xxx is the usual convention for naming of
>> pfn's anyway.
> 
> Excuse me, what? Are you now just making up arguments?
To clarify, we have various users of "xxx_pfn" in the tree and I fail to see how
"this is a problematic pfn" -> "problematic_pfn" is odd and why
"pfn_problematic" would be any clearer.

I do agree with the "problematic" aspect. "failed" might indeed be nicer.

-- 
Cheers,

David

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[Lorenzo Stoakes (ARM)]

On Wed, Aug 26, 2026 at 10:24:56AM +0200, David Hildenbrand (Arm) wrote:
> On 8/26/26 10:16, David Hildenbrand (Arm) wrote:
> > On 8/26/26 10:11, Lorenzo Stoakes (ARM) wrote:
> >> On Wed, Aug 26, 2026 at 10:08:58AM +0200, David Hildenbrand (Arm) wrote:
> >>>
> >>> Elaborate.
> >>
> >> It's overly long, I read it and am confused as to what is 'problematic' or not,
> >> it reads weirdly in English and pfn_xxx is the usual convention for naming of
> >> pfn's anyway.
> >
> > Excuse me, what? Are you now just making up arguments?
> To clarify, we have various users of "xxx_pfn" in the tree and I fail to see how
> "this is a problematic pfn" -> "problematic_pfn" is odd and why
> "pfn_problematic" would be any clearer.
>
> I do agree with the "problematic" aspect. "failed" might indeed be nicer.

Right yeah. Mostly the push back is on the word being a bit confusing. Fair
enough on the pfn thing, failed_pfn is actually the nicest name suggested so far
:)

I still think:

       	if (result == SCAN_SUCCEED) {
               	...
               	trace_mm_khugepaged_scan_file(mm, -1, file, present, swap, result);
       	} else {
               	trace_mm_khugepaged_scan_file(mm, failed_pfn, file, present,
					      swap, result);
       	}

Is a little neater as then it's only on the failure path that we trace the
failed pfn, and otherwise we explicitly -1.

But it's not exactly a show stopper this :)

Very rough edit of your patch - if you're happy then let's go with this, if not
then edit it + post so Vernon has a clear direction. I'm not feeling super
strongly on this so don't want to block anything:

----8<----
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 75639298efc27..371ee0b16d10c 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2683,6 +2683,7 @@ static enum scan_result collapse_scan_file(struct
mm_struct *mm,
       	int present, swap;
       	int node = NUMA_NO_NODE;
       	enum scan_result result = SCAN_SUCCEED;
+       unsigned long failed_pfn = -1;

       	present = 0;
       	swap = 0;
@@ -2714,6 +2715,7 @@ static enum scan_result collapse_scan_file(struct
mm_struct *mm,
               	}

               	if (is_pmd_order(folio_order(folio))) {
+                       failed_pfn = folio_pfn(folio);
                       	result = SCAN_PTE_MAPPED_HUGEPAGE;
                       	/*
                         * PMD-sized THP implies that we can only try
@@ -2725,6 +2727,7 @@ static enum scan_result collapse_scan_file(struct
mm_struct *mm,

               	node = folio_nid(folio);
               	if (collapse_scan_abort(node, cc)) {
+                       failed_pfn = folio_pfn(folio);
                       	result = SCAN_SCAN_ABORT;
                       	folio_put(folio);
                       	break;
@@ -2732,12 +2735,14 @@ static enum scan_result collapse_scan_file(struct
mm_struct *mm,
               	cc->node_load[node]++;

               	if (!folio_test_lru(folio)) {
+                       failed_pfn = folio_pfn(folio);
                       	result = SCAN_PAGE_LRU;
                       	folio_put(folio);
                       	break;
               	}

                if (folio_expected_ref_count(folio) + 1 != folio_ref_count(folio)) {
+                       failed_pfn = folio_pfn(folio);
                        result = SCAN_PAGE_COUNT;
                        folio_put(folio);
                        break;
@@ -2773,7 +2778,7 @@ static enum scan_result collapse_scan_file(struct
mm_struct *mm,
               	}
-      	}
-       trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
+       	trace_mm_khugepaged_scan_file(mm, -1, file, present, swap,
+				SCAN_SUCCEED);
+	} else {
+	       trace_mm_khugepaged_scan_file(mm, failed_pfn, file, present,
+				swap, result);
+	}
+
        return result;
 }

--
Cheers, Lorenzo

Re: [PATCH v3 3/3] mm: khugepaged: fix folio is used after folio_put/unlock()

[Lorenzo Stoakes (ARM)]

On Wed, Aug 26, 2026 at 09:09:16AM +0100, Lorenzo Stoakes (ARM) wrote:
> On Mon, Aug 24, 2026 at 05:29:35PM +0800, Vernon Yang wrote:
> > From: Vernon Yang <yanglincheng@kylinos.cn>
> >
> > On the rollback path, folio_put() has already dropped the last reference
> > of new_folio. On the success path, new_folio is already unlocked and can
> > be freed concurrently. The trace_mm_khugepaged_collapse_file() is left
> > with a dangling folio pointer.
> >
> > So using the folio_pfn() before dropping the reference, closing
> > use-after-free window.
> >
> > Fixes: 4c9473e87e75 ("mm/khugepaged: add tracepoint to collapse_file()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> > ---
> >  include/trace/events/huge_memory.h | 6 +++---
> >  mm/khugepaged.c                    | 5 ++++-
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> > index fa828967e1fb..5fb4d92cfd84 100644
> > --- a/include/trace/events/huge_memory.h
> > +++ b/include/trace/events/huge_memory.h
> > @@ -211,10 +211,10 @@ TRACE_EVENT(mm_khugepaged_scan_file,
> >  );
> >
> >  TRACE_EVENT(mm_khugepaged_collapse_file,
> > -	TP_PROTO(struct mm_struct *mm, struct folio *new_folio, pgoff_t index,
> > +	TP_PROTO(struct mm_struct *mm, unsigned long new_pfn, pgoff_t index,
> >  			unsigned long addr, bool is_shmem, struct file *file,
> >  			int nr, int result),
> > -	TP_ARGS(mm, new_folio, index, addr, is_shmem, file, nr, result),
> > +	TP_ARGS(mm, new_pfn, index, addr, is_shmem, file, nr, result),
> >  	TP_STRUCT__entry(
> >  		__field(struct mm_struct *, mm)
> >  		__field(unsigned long, hpfn)
> > @@ -228,7 +228,7 @@ TRACE_EVENT(mm_khugepaged_collapse_file,
> >
> >  	TP_fast_assign(
> >  		__entry->mm = mm;
> > -		__entry->hpfn = new_folio ? folio_pfn(new_folio) : -1;
> > +		__entry->hpfn = new_pfn;
> >  		__entry->index = index;
> >  		__entry->addr = addr;
> >  		__entry->is_shmem = is_shmem;
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 4e0fca5942dd..24347f1a94ae 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -2254,6 +2254,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
> >  	struct address_space *mapping = file->f_mapping;
> >  	struct page *dst;
> >  	struct folio *folio, *tmp, *new_folio;
> > +	unsigned long new_pfn = -1;
>
> Nitty but:
>
> Could we use pfn_t? I don't love that we are inconsistent with that.

(As before disregard, I was somehow behind on the times + pre-morning-coffee
confused :)

>
> Also the general pattern for pfn names is pfn_xxx so pfn_new instead?

Also since we're using xxx_pfn elsewhere here new_pfn is fine.

Therefore, with David's suggestion applied:

Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

>
> >  	pgoff_t index = 0, end = start + HPAGE_PMD_NR;
> >  	LIST_HEAD(pagelist);
> >  	XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
> > @@ -2633,6 +2634,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
> >  	retract_page_tables(mapping, start);
> >  	if (cc && !cc->is_khugepaged)
> >  		result = SCAN_PTE_MAPPED_HUGEPAGE;
> > +	new_pfn = folio_pfn(new_folio);
> >  	folio_unlock(new_folio);
> >
> >  	/*
> > @@ -2671,12 +2673,13 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
> >  	}
> >
> >  	new_folio->mapping = NULL;
> > +	new_pfn = folio_pfn(new_folio);
> >
> >  	folio_unlock(new_folio);
> >  	folio_put(new_folio);
> >  out:
> >  	VM_BUG_ON(!list_empty(&pagelist));
> > -	trace_mm_khugepaged_collapse_file(mm, new_folio, index, addr, is_shmem, file, HPAGE_PMD_NR, result);
> > +	trace_mm_khugepaged_collapse_file(mm, new_pfn, index, addr, is_shmem, file, HPAGE_PMD_NR, result);
> >  	return result;
> >  }
> >
> > --
> > 2.53.0
> >
>
> --
> Cheers, Lorenzo

--
Cheers, Lorenzo

Re: [PATCH v3 2/3] mm: khugepaged: fix folio is used after pte_unmap_unlock()

[Lorenzo Stoakes (ARM)]

On Wed, Aug 26, 2026 at 09:12:52AM +0100, Lorenzo Stoakes (ARM) wrote:
> On Mon, Aug 24, 2026 at 05:29:34PM +0800, Vernon Yang wrote:
> > From: Vernon Yang <yanglincheng@kylinos.cn>
> >
> > After the page table lock has dropped, the folio can be freed
> > concurrently. The trace_mm_khugepaged_scan_pmd() is left with
> > a dangling folio pointer.
> >
> > So using the folio_pfn() before dropping the page table lock,
> > closing use-after-free window.
> >
> > Fixes: 7d2eba0557c1 ("mm: add tracepoint for scanning pages")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> > ---
> >  include/trace/events/huge_memory.h | 6 +++---
> >  mm/khugepaged.c                    | 5 ++++-
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> > index 7b526528f85b..fa828967e1fb 100644
> > --- a/include/trace/events/huge_memory.h
> > +++ b/include/trace/events/huge_memory.h
> > @@ -55,10 +55,10 @@ SCAN_STATUS
> >
> >  TRACE_EVENT(mm_khugepaged_scan_pmd,
> >
> > -	TP_PROTO(struct mm_struct *mm, struct folio *folio,
> > +	TP_PROTO(struct mm_struct *mm, unsigned long pfn,
> >  		 int referenced, int none_or_zero, int status, int unmapped),
> >
> > -	TP_ARGS(mm, folio, referenced, none_or_zero, status, unmapped),
> > +	TP_ARGS(mm, pfn, referenced, none_or_zero, status, unmapped),
> >
> >  	TP_STRUCT__entry(
> >  		__field(struct mm_struct *, mm)
> > @@ -71,7 +71,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
> >
> >  	TP_fast_assign(
> >  		__entry->mm = mm;
> > -		__entry->pfn = folio ? folio_pfn(folio) : -1;
> > +		__entry->pfn = pfn;
> >  		__entry->referenced = referenced;
> >  		__entry->none_or_zero = none_or_zero;
> >  		__entry->status = status;
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 00337405c0e0..4e0fca5942dd 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -1618,6 +1618,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> >  	enum scan_result result = SCAN_FAIL;
> >  	struct page *page = NULL;
> >  	struct folio *folio = NULL;
> > +	unsigned long pfn = -1;
>
> pfn_t please.

Disregard :)

With David's suggestion applied:

Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

>
> >  	unsigned long addr;
> >  	unsigned long enabled_orders;
> >  	spinlock_t *ptl;
> > @@ -1780,6 +1781,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> >  		result = SCAN_SUCCEED;
> >  	}
> >  out_unmap:
> > +	if (folio)
> > +		pfn = folio_pfn(folio);
>
> And +1 to David on ensuring this is reset to NULL appropriately.
>
> >  	pte_unmap_unlock(pte, ptl);
> >  	if (result == SCAN_SUCCEED) {
> >  		/* collapse_huge_page() expects the lock to be dropped before calling */
> > @@ -1790,7 +1793,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> >  		*lock_dropped = true;
> >  	}
> >  out:
> > -	trace_mm_khugepaged_scan_pmd(mm, folio, referenced,
> > +	trace_mm_khugepaged_scan_pmd(mm, pfn, referenced,
> >  				     none_or_zero, result, unmapped);
> >  	return result;
> >  }
> > --
> > 2.53.0
> >
>
> --
> Cheers, Lorenzo

--
Cheers, Lorenzo

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[Vernon Yang]

On Wed, Aug 26, 2026 at 09:57:05AM +0200, David Hildenbrand (Arm) wrote:
> On 8/26/26 04:44, Vernon Yang wrote:
> > On Mon, Aug 24, 2026 at 01:54:18PM +0200, David Hildenbrand (Arm) wrote:
> >> On 8/24/26 11:29, Vernon Yang wrote:
> >>> From: Vernon Yang <yanglincheng@kylinos.cn>
> >>>
> >>> When the swap entries found exceed max_ptes_swap, the loop is left via
> >>> break with folio still holding the xarray value that encodes the swap
> >>> entry, not valid folio pointer.
> >>>
> >>> That value is passed to trace_mm_khugepaged_scan_file(), which feeds it
> >>> to folio_pfn(). On FLATMEM and SPARSEMEM_VMEMMAP, the page_to_pfn() is
> >>> plain pointer arithmetic, so the trace event merely prints bogus
> >>> scan_pfn. On classic SPARSEMEM, the page_to_pfn() reads page->flags,
> >>> dereferencing the tiny encoded integer and oopsing khugepaged whenever
> >>> the trace event is enabled.
> >>>
> >>> So when folio is the swap entry value, simply set pfn to -1, just like
> >>> exhausted scan naturally.
> >>>
> >>> And the folio_put() has maybe dropped the last reference of folio. The
> >>> trace_mm_khugepaged_scan_file() is left with a dangling folio pointer.
> >>> so using the folio_pfn() before dropping the reference, closing
> >>> use-after-free window.
> >>>
> >>> Fixes: d41fd2016ed0 ("mm/khugepaged: add tracepoint to hpage_collapse_scan_file()")
> >>> Cc: stable@vger.kernel.org
> >>> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> >>> ---
> >>>  include/trace/events/huge_memory.h | 6 +++---
> >>>  mm/khugepaged.c                    | 5 ++++-
> >>>  2 files changed, 7 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> >>> index 5a48c5406cce..7b526528f85b 100644
> >>> --- a/include/trace/events/huge_memory.h
> >>> +++ b/include/trace/events/huge_memory.h
> >>> @@ -178,10 +178,10 @@ TRACE_EVENT(mm_collapse_huge_page_swapin,
> >>>
> >>>  TRACE_EVENT(mm_khugepaged_scan_file,
> >>>
> >>> -	TP_PROTO(struct mm_struct *mm, struct folio *folio, struct file *file,
> >>> +	TP_PROTO(struct mm_struct *mm, unsigned long pfn, struct file *file,
> >>>  		 int present, int swap, int result),
> >>>
> >>> -	TP_ARGS(mm, folio, file, present, swap, result),
> >>> +	TP_ARGS(mm, pfn, file, present, swap, result),
> >>>
> >>>  	TP_STRUCT__entry(
> >>>  		__field(struct mm_struct *, mm)
> >>> @@ -194,7 +194,7 @@ TRACE_EVENT(mm_khugepaged_scan_file,
> >>>
> >>>  	TP_fast_assign(
> >>>  		__entry->mm = mm;
> >>> -		__entry->pfn = folio ? folio_pfn(folio) : -1;
> >>> +		__entry->pfn = pfn;
> >>>  		__assign_str(filename);
> >>>  		__entry->present = present;
> >>>  		__entry->swap = swap;
> >>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> >>> index 79effd3f3da4..00337405c0e0 100644
> >>> --- a/mm/khugepaged.c
> >>> +++ b/mm/khugepaged.c
> >>> @@ -2689,6 +2689,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >>>  	int present, swap;
> >>>  	int node = NUMA_NO_NODE;
> >>>  	enum scan_result result = SCAN_SUCCEED;
> >>> +	unsigned long pfn;
> >>>
> >>>  	present = 0;
> >>>  	swap = 0;
> >>> @@ -2719,6 +2720,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >>>  			continue;
> >>>  		}
> >>>
> >>> +		pfn = folio_pfn(folio);
> >>>  		if (is_pmd_order(folio_order(folio))) {
> >>>  			result = SCAN_PTE_MAPPED_HUGEPAGE;
> >>>  			/*
> >>> @@ -2779,7 +2781,8 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >>>  		}
> >>>  	}
> >>>
> >>> -	trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
> >>> +	trace_mm_khugepaged_scan_file(mm, (!folio || xa_is_value(folio)) ? -1 : pfn,
> >>> +				      file, present, swap, result);
> >>>  	return result;
> >>>  }
> >>>
> >>
> >> Shouldn't we just reset PFN to -1 at the beginning of the loop (and set it
> >> initially)?
> >
> > When the `xas_for_each()` iteration to terminate and the folio operation
> > preceding is normal, but pfn will be incorrect.
>
> The PFN is only relevant when a folio participated in the failure. Maybe the
> following would be cleanest?

Yes, this is clearer. I considered this approach before, but it would be
quite verbose.

LGTM, I will use this in the next version. Thanks!

> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 75639298efc27..371ee0b16d10c 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2683,6 +2683,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>         int present, swap;
>         int node = NUMA_NO_NODE;
>         enum scan_result result = SCAN_SUCCEED;
> +       unsigned long problematic_pfn = -1;
>
>         present = 0;
>         swap = 0;
> @@ -2714,6 +2715,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>                 }
>
>                 if (is_pmd_order(folio_order(folio))) {
> +                       problematic_pfn = folio_pfn(folio);
>                         result = SCAN_PTE_MAPPED_HUGEPAGE;
>                         /*
>                          * PMD-sized THP implies that we can only try
> @@ -2725,6 +2727,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>
>                 node = folio_nid(folio);
>                 if (collapse_scan_abort(node, cc)) {
> +                       problematic_pfn = folio_pfn(folio);
>                         result = SCAN_SCAN_ABORT;
>                         folio_put(folio);
>                         break;
> @@ -2732,12 +2735,14 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>                 cc->node_load[node]++;
>
>                 if (!folio_test_lru(folio)) {
> +                       problematic_pfn = folio_pfn(folio);
>                         result = SCAN_PAGE_LRU;
>                         folio_put(folio);
>                         break;
>                 }
>
>                 if (folio_expected_ref_count(folio) + 1 != folio_ref_count(folio)) {
> +                       problematic_pfn = folio_pfn(folio);
>                         result = SCAN_PAGE_COUNT;
>                         folio_put(folio);
>                         break;
> @@ -2773,7 +2778,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>                 }
>         }
>
> -       trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
> +       trace_mm_khugepaged_scan_file(mm, problematic_pfn, file, present, swap,
> result);
>         return result;
>  }
>
>
>
> --
> Cheers,
>
> David
>

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[David Hildenbrand (Arm)]

On 8/26/26 10:35, Lorenzo Stoakes (ARM) wrote:
> On Wed, Aug 26, 2026 at 10:24:56AM +0200, David Hildenbrand (Arm) wrote:
>> On 8/26/26 10:16, David Hildenbrand (Arm) wrote:
>>>
>>> Excuse me, what? Are you now just making up arguments?
>> To clarify, we have various users of "xxx_pfn" in the tree and I fail to see how
>> "this is a problematic pfn" -> "problematic_pfn" is odd and why
>> "pfn_problematic" would be any clearer.
>>
>> I do agree with the "problematic" aspect. "failed" might indeed be nicer.
> 
> Right yeah. Mostly the push back is on the word being a bit confusing. Fair
> enough on the pfn thing, failed_pfn is actually the nicest name suggested so far
> :)
> 
> I still think:
> 
>        	if (result == SCAN_SUCCEED) {
>                	...
>                	trace_mm_khugepaged_scan_file(mm, -1, file, present, swap, result);
>        	} else {
>                	trace_mm_khugepaged_scan_file(mm, failed_pfn, file, present,
> 					      swap, result);
>        	}
> 
> Is a little neater as then it's only on the failure path that we trace the
> failed pfn, and otherwise we explicitly -1.
> 
> But it's not exactly a show stopper this :)
> 
> Very rough edit of your patch - if you're happy then let's go with this, if not
> then edit it + post so Vernon has a clear direction. I'm not feeling super
> strongly on this so don't want to block anything:
> 
> ----8<----
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 75639298efc27..371ee0b16d10c 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2683,6 +2683,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>        	int present, swap;
>        	int node = NUMA_NO_NODE;
>        	enum scan_result result = SCAN_SUCCEED;
> +       unsigned long failed_pfn = -1;
> 
>        	present = 0;
>        	swap = 0;
> @@ -2714,6 +2715,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>                	}
> 
>                	if (is_pmd_order(folio_order(folio))) {
> +                       failed_pfn = folio_pfn(folio);
>                        	result = SCAN_PTE_MAPPED_HUGEPAGE;
>                        	/*
>                          * PMD-sized THP implies that we can only try
> @@ -2725,6 +2727,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
> 
>                	node = folio_nid(folio);
>                	if (collapse_scan_abort(node, cc)) {
> +                       failed_pfn = folio_pfn(folio);
>                        	result = SCAN_SCAN_ABORT;
>                        	folio_put(folio);
>                        	break;
> @@ -2732,12 +2735,14 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>                	cc->node_load[node]++;
> 
>                	if (!folio_test_lru(folio)) {
> +                       failed_pfn = folio_pfn(folio);
>                        	result = SCAN_PAGE_LRU;
>                        	folio_put(folio);
>                        	break;
>                	}
> 
>                 if (folio_expected_ref_count(folio) + 1 != folio_ref_count(folio)) {
> +                       failed_pfn = folio_pfn(folio);
>                         result = SCAN_PAGE_COUNT;
>                         folio_put(folio);
>                         break;
> @@ -2773,7 +2778,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>                	}
> -      	}
> -       trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
> +       	trace_mm_khugepaged_scan_file(mm, -1, file, present, swap,
> +				SCAN_SUCCEED);
> +	} else {
> +	       trace_mm_khugepaged_scan_file(mm, failed_pfn, file, present,
> +				swap, result);
> +	}
> +
>         return result;
>  }

That looks good to me, thanks.

My memory is bad this week, so I have to ask with the fear of having asked
before: is tracing the PFN of any value?

What can someone tracing this on userspace reasonably do with the PFN? Inspect
kpageflags? I'd assume someone actually doing that is quite ... rare.

-- 
Cheers,

David

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[Vernon Yang]

On Wed, Aug 26, 2026 at 09:35:38AM +0100, Lorenzo Stoakes (ARM) wrote:
> On Wed, Aug 26, 2026 at 10:24:56AM +0200, David Hildenbrand (Arm) wrote:
> > On 8/26/26 10:16, David Hildenbrand (Arm) wrote:
> > > On 8/26/26 10:11, Lorenzo Stoakes (ARM) wrote:
> > >> On Wed, Aug 26, 2026 at 10:08:58AM +0200, David Hildenbrand (Arm) wrote:
> > >>>
> > >>> Elaborate.
> > >>
> > >> It's overly long, I read it and am confused as to what is 'problematic' or not,
> > >> it reads weirdly in English and pfn_xxx is the usual convention for naming of
> > >> pfn's anyway.
> > >
> > > Excuse me, what? Are you now just making up arguments?
> > To clarify, we have various users of "xxx_pfn" in the tree and I fail to see how
> > "this is a problematic pfn" -> "problematic_pfn" is odd and why
> > "pfn_problematic" would be any clearer.
> >
> > I do agree with the "problematic" aspect. "failed" might indeed be nicer.
>
> Right yeah. Mostly the push back is on the word being a bit confusing. Fair
> enough on the pfn thing, failed_pfn is actually the nicest name suggested so far
> :)

failed_pfn is good to me.

> I still think:
>
>        	if (result == SCAN_SUCCEED) {
>                	...
>                	trace_mm_khugepaged_scan_file(mm, -1, file, present, swap, result);
>        	} else {
>                	trace_mm_khugepaged_scan_file(mm, failed_pfn, file, present,
> 					      swap, result);
>        	}
>
> Is a little neater as then it's only on the failure path that we trace the
> failed pfn, and otherwise we explicitly -1.

I understand what you're trying to say, but personally it isn't necessary,
because failed_pfn defaults to -1, and one
trace_mm_khugepaged_scan_file() already covers it.

If everyone clearly expresses that they want two
trace_mm_khugepaged_scan_file(), please let me know explicitly. Thanks!

> But it's not exactly a show stopper this :)
>
> Very rough edit of your patch - if you're happy then let's go with this, if not
> then edit it + post so Vernon has a clear direction. I'm not feeling super
> strongly on this so don't want to block anything:
>
> ----8<----
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 75639298efc27..371ee0b16d10c 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2683,6 +2683,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>        	int present, swap;
>        	int node = NUMA_NO_NODE;
>        	enum scan_result result = SCAN_SUCCEED;
> +       unsigned long failed_pfn = -1;
>
>        	present = 0;
>        	swap = 0;
> @@ -2714,6 +2715,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>                	}
>
>                	if (is_pmd_order(folio_order(folio))) {
> +                       failed_pfn = folio_pfn(folio);
>                        	result = SCAN_PTE_MAPPED_HUGEPAGE;
>                        	/*
>                          * PMD-sized THP implies that we can only try
> @@ -2725,6 +2727,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>
>                	node = folio_nid(folio);
>                	if (collapse_scan_abort(node, cc)) {
> +                       failed_pfn = folio_pfn(folio);
>                        	result = SCAN_SCAN_ABORT;
>                        	folio_put(folio);
>                        	break;
> @@ -2732,12 +2735,14 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>                	cc->node_load[node]++;
>
>                	if (!folio_test_lru(folio)) {
> +                       failed_pfn = folio_pfn(folio);
>                        	result = SCAN_PAGE_LRU;
>                        	folio_put(folio);
>                        	break;
>                	}
>
>                 if (folio_expected_ref_count(folio) + 1 != folio_ref_count(folio)) {
> +                       failed_pfn = folio_pfn(folio);
>                         result = SCAN_PAGE_COUNT;
>                         folio_put(folio);
>                         break;
> @@ -2773,7 +2778,7 @@ static enum scan_result collapse_scan_file(struct
> mm_struct *mm,
>                	}
> -      	}
> -       trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
> +       	trace_mm_khugepaged_scan_file(mm, -1, file, present, swap,
> +				SCAN_SUCCEED);
> +	} else {
> +	       trace_mm_khugepaged_scan_file(mm, failed_pfn, file, present,
> +				swap, result);
> +	}
> +
>         return result;
>  }
>
> --
> Cheers, Lorenzo
>

Re: [PATCH v3 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[Lorenzo Stoakes (ARM)]

On Wed, Aug 26, 2026 at 05:21:51PM +0800, Vernon Yang wrote:
> On Wed, Aug 26, 2026 at 09:35:38AM +0100, Lorenzo Stoakes (ARM) wrote:
> > On Wed, Aug 26, 2026 at 10:24:56AM +0200, David Hildenbrand (Arm) wrote:
> > > On 8/26/26 10:16, David Hildenbrand (Arm) wrote:
> > > > On 8/26/26 10:11, Lorenzo Stoakes (ARM) wrote:
> > > >> On Wed, Aug 26, 2026 at 10:08:58AM +0200, David Hildenbrand (Arm) wrote:
> > > >>>
> > > >>> Elaborate.
> > > >>
> > > >> It's overly long, I read it and am confused as to what is 'problematic' or not,
> > > >> it reads weirdly in English and pfn_xxx is the usual convention for naming of
> > > >> pfn's anyway.
> > > >
> > > > Excuse me, what? Are you now just making up arguments?
> > > To clarify, we have various users of "xxx_pfn" in the tree and I fail to see how
> > > "this is a problematic pfn" -> "problematic_pfn" is odd and why
> > > "pfn_problematic" would be any clearer.
> > >
> > > I do agree with the "problematic" aspect. "failed" might indeed be nicer.
> >
> > Right yeah. Mostly the push back is on the word being a bit confusing. Fair
> > enough on the pfn thing, failed_pfn is actually the nicest name suggested so far
> > :)
>
> failed_pfn is good to me.

Thanks.

>
> > I still think:
> >
> >        	if (result == SCAN_SUCCEED) {
> >                	...
> >                	trace_mm_khugepaged_scan_file(mm, -1, file, present, swap, result);
> >        	} else {
> >                	trace_mm_khugepaged_scan_file(mm, failed_pfn, file, present,
> > 					      swap, result);
> >        	}
> >
> > Is a little neater as then it's only on the failure path that we trace the
> > failed pfn, and otherwise we explicitly -1.
>
> I understand what you're trying to say, but personally it isn't necessary,
> because failed_pfn defaults to -1, and one
> trace_mm_khugepaged_scan_file() already covers it.
>
> If everyone clearly expresses that they want two
> trace_mm_khugepaged_scan_file(), please let me know explicitly. Thanks!

See David's reply, we'd like to go with this thanks.

--
Cheers, Lorenzo