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:54:07AM +0200, David Hildenbrand (Arm) wrote:
> On 9/22/26 17:35, Lorenzo Stoakes (ARM) wrote:
> > collapse_huge_page() deposits a PTE page table on PMD collapse in order
> > that it can be utilised for subsequent split operations, meaning that those
> > operations do not need to perform an allocation (as they are in a context
> > where it might be unwise).
> >
> > However the PTE page table which is deposited is the one which is currently
> > mapped by the PMD entry that is in the process of being collapsed.
> >
> > Once deposited, the PTE page table may be used in a split of any other
> > unrelated PMD entry.
> >
> > This is currently not an issue as this operation is performed with VMA/mmap
> > write lock + anon rmap locks held, so ordinary page table walkers will
> > never accidentally end up walking the wrong thing, and GUP-fast is
> > protected by an IPI via tlb_remove_table_sync_one().
> >
> > However, the series to which this commit belongs implements RCU-safe page
> > table traversal, at which point this becomes problematic.
> >
> > This can be resolved by using pte_offset_map_lock() which gates on a PTE
> > PTL and a pmd_same() check, but lockless walks are unsafe as things stand.
> >
> > Resolve this by simply allocating a new, zeroed, PTE page table to deposit
> > at the point of collapse.
> >
> > This path is already costly and an allocation has already been performed
> > for the huge folio, so this allocation is statistical noise in terms of
> > performance and memory usage at this point.
> >
> > With this PTE page table deposited, RCU-free the existing PTE page table
> > so it is safe for page table walkers to traverse within a grace period.
> >
> > This also brings this deposit case in line with all other page table
> > deposit logic which deposit a fresh page table.
> >
> > Additionally, this was the only place in the kernel that displaced a page
> > table like this, so eliminating it also helps consistency.
>
> ack
>
> >
> > An edge case for deposit exists for powerpc and its hash-based MMU - it
> > stores hash slot data in deposited page tables and zeroes them on withdraw,
> > so a zeroed deposited page table works correctly for it.
>
> I think we would have zeroed out all page table entries either way already right
> now before depositing. See clear_ptes() in
> __collapse_huge_page_copy_succeeded(). So this paragraph might just be dropped.

Yup I know, but it was there just in case people wondered about it.

But sure will drop.

>
> >
> > Since khugepaged runs as a kernel thread, do a little dance in
> > alloc_deposit_pte() to correctly charge the allocation.
> >
> > This is already done for the folio allocation via alloc_charge_folio() but
> > no such wrapper exists for a page table allocation.
> >
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > ---
> >  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)
>
> alloc_deposit_pte_table or alloc_deposit_pgtable
>
> should be clearer.

The inevitable naming nit :)

Yup agreed, I have tried to be consistent in saying 'pte page table' in commit
msg etc. to differentiate an entry so that's fair, will fix up.

>
> > +{
> > +	/*
> > +	 * 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);
>
> I'm no memcg expert but that looks good to me.

Yeah, it mirrors the pattern elsewhere.

>
> > +	mem_cgroup_put(memcg);
> > +	return pgtable;
> > +}
> > +
>
> With the fixup
>
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>

Thanks!

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo