Re: [RFC PATCH 18/57] mm/collapse: install the destinations at PTE level

[Lance Yang]

On Sun, Aug 16, 2026 at 11:45:30PM +0100, Kiryl Shutsemau wrote:
>From: "Kiryl Shutsemau (Meta)" <kas@xxxxxxxxxx>
>
[...]
> /* Publish each destination folio in place of the sources it replaces */
> static void collapse_install(struct vm_area_struct *vma,
> 			     struct collapse_control *cc, pmd_t *pmd)
> {
>+	struct mm_struct *mm = vma->vm_mm;
>+	pte_t *pte, *table;
>+	spinlock_t *ptl;
>+	unsigned int i;
>+
>+	if (is_pmd_order(cc->candidates[0].order)) {
>+		/* A PMD candidate fills the slot pool: always alone */
>+		VM_WARN_ON_ONCE(cc->nr_candidates != 1);
>+		collapse_install_pmd(vma, cc, pmd);
>+		return;
>+	}
>+
>+	pte = pte_offset_map_lock(mm, pmd, cc->candidates[0].addr, &ptl);
>+	if (!pte) {
>+		/*
>+		 * Table gone under us (see collapse_abort_candidate() on @pte).
>+		 * Tear down every frozen candidate -- stranding them would leak
>+		 * frozen, locked sources.
>+		 */
>+		for (i = 0; i < cc->nr_candidates; i++) {
>+			struct collapse_candidate *cand = &cc->candidates[i];
>+
>+			if (cand->state != CAND_FROZEN)
>+				continue;
>+
>+			cand->result = SCAN_NO_PTE_TABLE;
>+			collapse_abort_candidate(vma, cand, NULL);
>+		}
>+		return;
>+	}
>+	table = pte - pte_index(cc->candidates[0].addr);
>+
>+	for (i = 0; i < cc->nr_candidates; i++) {
>+		struct collapse_candidate *cand = &cc->candidates[i];
>+		pte_t *cand_pte = table + pte_index(cand->addr);
>+		unsigned int nr_populated;
>+
>+		if (cand->state != CAND_FROZEN)
>+			continue;
>+
>+		if (cand->result != SCAN_SUCCEED) {
>+			/* Machine check during the copy */
>+			collapse_abort_candidate(vma, cand, cand_pte);
>+			continue;
>+		}
>+
>+		/* No destination: the provision pass could not spare one */
>+		if (!cand->new_folio) {
>+			collapse_abort_candidate(vma, cand, cand_pte);
>+			continue;
>+		}
>+
>+		if (!collapse_verify_candidate(cand, cand_pte, &nr_populated)) {
>+			cand->result = SCAN_PTE_NON_PRESENT;
>+			collapse_abort_candidate(vma, cand, cand_pte);
>+			continue;
>+		}
>+
>+		/*
>+		 * The smp_wmb() in __folio_mark_uptodate() orders the copied
>+		 * data before the set_ptes() that publishes it.
>+		 */
>+		__folio_mark_uptodate(cand->new_folio);
>+		map_anon_folio_pte_nopf(cand->new_folio, cand_pte, vma,
>+					cand->addr, /*uffd_wp=*/ false);
>+
>+		/* Slots with no source gain anon memory that no zap accounted */
>+		if (nr_populated)
>+			add_mm_counter(mm, MM_ANONPAGES, nr_populated);

Well ... KSM zero-page accounting gets out of sync here.

Say a saved zero PTE came from KSM. ksm_map_zero_page() has already
bumped global + per-mm counters. ksm_might_unmap_zero_page() uses its|
dirty bit to drop them again later:

#define is_ksm_zero_pte(pte)	(is_zero_pfn(pte_pfn(pte)) && pte_dirty(pte))

static inline void ksm_map_zero_page(struct mm_struct *mm)
{
	atomic_long_inc(&ksm_zero_pages);
	atomic_long_inc(&mm->ksm_zero_pages);
}

static inline void ksm_might_unmap_zero_page(struct mm_struct *mm, pte_t pte)
{
	if (is_ksm_zero_pte(pte)) {
		atomic_long_dec(&ksm_zero_pages);
		atomic_long_dec(&mm->ksm_zero_pages);
	}
}

Right, once a candidate reaches install, those counters stay valid only
if collapse_abort_candidate() restores the saved PTE. A successful PTE
install doesn't restore it. Neither does an abort after the slot was
refilled or the PTE table disappeared. Patch #19 has the same
successful-install case in collapse_install_pmd() ...

I was thinking of something like this:

---8<---
diff --git a/mm/collapse.c b/mm/collapse.c
index 7c10888031f7..68b37e706341 100644
--- a/mm/collapse.c
+++ b/mm/collapse.c
@@ -8,6 +8,7 @@
 #include <linux/highmem.h>
 #include <linux/huge_mm.h>
 #include <linux/hugetlb.h>	/* x86 flush_tlb_range() uses hstate_vma() */
+#include <linux/ksm.h>
 #include <linux/leafops.h>
 #include <linux/math64.h>
 #include <linux/memcontrol.h>
@@ -1430,6 +1431,8 @@ static void collapse_abort_candidate(struct vm_area_struct *vma,
 		if (is_zero_pfn(pte_pfn(saved))) {
 			if (pte && pte_none(ptep_get(pte + i)))
 				set_pte_at(mm, addr, pte + i, saved);
+			else
+				ksm_might_unmap_zero_page(mm, saved);
 			continue;
 		}

@@ -1499,6 +1502,16 @@ static bool collapse_verify_candidate(struct collapse_candidate *cand,
 	return true;
 }

+static void collapse_unmap_zero_pages(struct mm_struct *mm,
+				      struct collapse_candidate *cand)
+{
+	const unsigned int nr_pages = candidate_nr_pages(cand);
+	unsigned int i;
+
+	for (i = 0; i < nr_pages; i++)
+		ksm_might_unmap_zero_page(mm, cand->saved_ptes[i]);
+}
+
 /*
  * The PMD terminal layer: verify, detach the table, deposit a fresh one and
  * install the leaf, as one atomic section under the pmd lock.  A pmd_none window
@@ -1616,6 +1629,7 @@ static void collapse_install_pmd(struct vm_area_struct *vma,
 	/* Slots with no source gain anon memory that no zap accounted */
 	if (nr_populated)
 		add_mm_counter(mm, MM_ANONPAGES, nr_populated);
+	collapse_unmap_zero_pages(mm, cand);
 	cand->deposit = NULL;
 	cand->new_folio = NULL;	/* ownership: the mapping */
 	cand->state = CAND_INSTALLED;
@@ -1708,6 +1722,7 @@ static void collapse_install(struct vm_area_struct *vma,
 		/* Slots with no source gain anon memory that no zap accounted */
 		if (nr_populated)
 			add_mm_counter(mm, MM_ANONPAGES, nr_populated);
+		collapse_unmap_zero_pages(mm, cand);
 		cand->new_folio = NULL;	/* ownership: the mappings */
 		cand->state = CAND_INSTALLED;
 	}
---

Cheers, Lance

>+		cand->new_folio = NULL;	/* ownership: the mappings */
>+		cand->state = CAND_INSTALLED;
>+	}
>+
>+	pte_unmap_unlock(pte, ptl);
> }
> 
> /*
>-- 
>2.54.0
>
>