Re: [PATCH v4 01/12] mm/khugepaged: deposit a newly allocated page table on collapse

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Wed, Sep 23, 2026 at 10:24:36AM +0800, Lance Yang wrote:
> > mm/khugepaged.c | 32 ++++++++++++++++++++++++++++++--
> > 1 file changed, 30 insertions(+), 2 deletions(-)
> >
> >diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> >index f49a6710933b..dab421f8233e 100644
> >--- a/mm/khugepaged.c
> >+++ b/mm/khugepaged.c
> >@@ -1278,6 +1278,23 @@ static enum scan_result alloc_charge_folio(struct folio **foliop, struct mm_stru
> > 	return SCAN_SUCCEED;
> > }
> >
> >+static pgtable_t alloc_deposit_pte(struct mm_struct *mm)
> >+{
> >+	/*
> >+	 * khugepaged is run from a kernel thread, so need to manually set the
> >+	 * correct memcg so the allocation gets charged correctly.
> >+	 */
> >+	struct mem_cgroup *memcg = get_mem_cgroup_from_mm(mm);
> >+	struct mem_cgroup *old_memcg = set_active_memcg(memcg);
> >+	pgtable_t pgtable;
> >+
> >+	pgtable = pte_alloc_one(mm);
> >+
> >+	set_active_memcg(old_memcg);
> >+	mem_cgroup_put(memcg);
> >+	return pgtable;
> >+}
> >+
> > /*
> >  * collapse_huge_page() expects the mmap_lock to be unlocked before entering and
> >  * will always return with the lock unlocked, to avoid holding the mmap_lock
> >@@ -1293,7 +1310,7 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long s
> > 	LIST_HEAD(compound_pagelist);
> > 	pmd_t *pmd, _pmd;
> > 	pte_t *pte = NULL;
> >-	pgtable_t pgtable;
> >+	pgtable_t pgtable = NULL;
> > 	struct folio *folio;
> > 	spinlock_t *pmd_ptl, *pte_ptl;
> > 	enum scan_result result = SCAN_FAIL;
> >@@ -1310,6 +1327,12 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long s
> > 		goto out_nolock;
> > 	}
> >
> >+	if (is_pmd_order(order)) {
> >+		pgtable = alloc_deposit_pte(mm);
> >+		if (!pgtable)
> >+			goto out_nolock;
>
> One small thing: result is still SCAN_SUCCEED after alloc_charge_folio(),
>
> 	result = alloc_charge_folio(&folio, mm, cc, order);
> 	if (result != SCAN_SUCCEED)
> 		goto out_nolock;
>
> 	if (folio_memcg_alloc_deferred(folio)) {
> 		result = SCAN_ALLOC_HUGE_PAGE_FAIL;
> 		goto out_nolock;
> 	}
>
> 	if (is_pmd_order(order)) {
> 		pgtable = alloc_deposit_pte(mm);
> 		if (!pgtable)
> 			goto out_nolock;
> 	}
>
> so if alloc_deposit_pte() fails, collapse_huge_page() returns success
> without installing a PMD.
>
> I see sashiko pointed that out too :)
>
> Cheers, Lance

Sigh ack yeah oops :) I think SCAN_ALLOC_HUGE_PAGE_FAIL is the closest though
it's not allocating a huge page so much as the page table, but gets the correct
MADV_COLLAPSE -ENOMEM back.

(Another case of local AI review not catchign something btw! Sashiko does lean
towards over-reporting/hallucinating but catches more)

Will ask Andrew to update in reply to this patch as a quick thing rather than
respin so quick, and fix up on any respin.

--
Cheers, Lorenzo