Re: [PATCH mm-new v2 2/2] mm/khugepaged: abort collapse scan on guard PTEs
From: David Hildenbrand
Date: Fri Sep 19 2025 - 03:57:33 EST
On 19.09.25 04:41, Lance Yang wrote:
On 2025/9/19 02:47, David Hildenbrand wrote:
On 18.09.25 07:04, Lance Yang wrote:
From: Lance Yang <lance.yang@xxxxxxxxx>
Guard PTE markers are installed via MADV_GUARD_INSTALL to create
lightweight guard regions.
Currently, any collapse path (khugepaged or MADV_COLLAPSE) will fail when
encountering such a range.
MADV_COLLAPSE fails deep inside the collapse logic when trying to swap-in
the special marker in __collapse_huge_page_swapin().
hpage_collapse_scan_pmd()
`- collapse_huge_page()
`- __collapse_huge_page_swapin() -> fails!
khugepaged's behavior is slightly different due to its max_ptes_swap
limit
(default 64). It won't fail as deep, but it will still needlessly scan up
to 64 swap entries before bailing out.
IMHO, we can and should detect this much earlier.
This patch adds a check directly inside the PTE scan loop. If a guard
marker is found, the scan is aborted immediately with
SCAN_PTE_NON_PRESENT,
avoiding wasted work.
Suggested-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
Signed-off-by: Lance Yang <lance.yang@xxxxxxxxx>
---
mm/khugepaged.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 9ed1af2b5c38..70ebfc7c1f3e 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1306,6 +1306,16 @@ static int hpage_collapse_scan_pmd(struct
mm_struct *mm,
result = SCAN_PTE_UFFD_WP;
goto out_unmap;
}
+ /*
+ * Guard PTE markers are installed by
+ * MADV_GUARD_INSTALL. Any collapse path must
+ * not touch them, so abort the scan immediately
+ * if one is found.
+ */
+ if (is_guard_pte_marker(pteval)) {
+ result = SCAN_PTE_NON_PRESENT;
+ goto out_unmap;
+ }
Thinking about it, this is interesting.
Essentially we track any non-swap swap entries towards
khugepaged_max_ptes_swap, which is rather weird.
I think we might also run into migration entries here and hwpoison entries?
So what about just generalizing this:
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index af5f5c80fe4ed..28f1f4bf0e0a8 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1293,7 +1293,24 @@ static int hpage_collapse_scan_pmd(struct
mm_struct *mm,
for (_address = address, _pte = pte; _pte < pte + HPAGE_PMD_NR;
_pte++, _address += PAGE_SIZE) {
pte_t pteval = ptep_get(_pte);
- if (is_swap_pte(pteval)) {
+
+ if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) {
+ ++none_or_zero;
+ if (!userfaultfd_armed(vma) &&
+ (!cc->is_khugepaged ||
+ none_or_zero <= khugepaged_max_ptes_none)) {
+ continue;
+ } else {
+ result = SCAN_EXCEED_NONE_PTE;
+ count_vm_event(THP_SCAN_EXCEED_NONE_PTE);
+ goto out_unmap;
+ }
+ } else if (!pte_present(pteval)) {
+ if (non_swap_entry(pte_to_swp_entry(pteval))) {
+ result = SCAN_PTE_NON_PRESENT;
+ goto out_unmap;
+ }
+
++unmapped;
if (!cc->is_khugepaged ||
unmapped <= khugepaged_max_ptes_swap) {
@@ -1313,18 +1330,7 @@ static int hpage_collapse_scan_pmd(struct
mm_struct *mm,
goto out_unmap;
}
}
- if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) {
- ++none_or_zero;
- if (!userfaultfd_armed(vma) &&
- (!cc->is_khugepaged ||
- none_or_zero <= khugepaged_max_ptes_none)) {
- continue;
- } else {
- result = SCAN_EXCEED_NONE_PTE;
- count_vm_event(THP_SCAN_EXCEED_NONE_PTE);
- goto out_unmap;
- }
- }
+
if (pte_uffd_wp(pteval)) {
/*
* Don't collapse the page if any of the small
With that, the function flow looks more similar to
__collapse_huge_page_isolate(),
except that we handle swap entries in there now.
Ah, indeed. I like this crazy idea ;p
And with that in place, couldn't we factor out a huge chunk of both
scanning
functions into some helper (passing whether swap entries are allowed or
not?).
Yes. Factoring out the common scanning logic into a new helper is a
good suggestion. It would clean things up ;)
Yes, I know, refactoring khugepaged, crazy idea.
I'll look into that. But let's do this separately :)
Right, but let's just skip any non-swap entries early in this patch instead of special-casing only guard ptes.
--
Cheers
David / dhildenb