On Wed, Sep 16, 2026 at 04:42:04PM +0200, David Hildenbrand (Arm) wrote:
> On 9/11/26 21:36, Lorenzo Stoakes (ARM) wrote:
> > When an anonymous mapping is collapsed for THP, a PTE page table is
> > 'deposited' with the installed PMD entry.
>
> Right. Or when we allocate an anon THP.
That case doesn't interact with page walking though, as they're
just-allocated right?
Am focusing on collapse case for that reason rather than listing all
possible places deposit can happen.
>
> >
> > This is done in order that a split can be performed without needing to
> > allocate additional memory.
> >
> > The freeing occurs in zap_deposited_table() and is done directly without
> > any delay via pte_free().
> >
> > This is currently not a problem as existing page table walks are protected
> > by the mmap or anon rmap lock.
>
> Or VMA lock?
Yup indeed, can update on respin/ask Andrew to add VMA lock here.
>
> >
> > However this becomes problematic in a future where RCU-only page table
> > walkers exist, as there is nothing to prevent a page table walker that
> > started the walk prior to collapse having its PTE table freed underneath
> > it.
>
>
> Wait, but wouldn't it be really problematic to punch a page table that is
> still
Punch? You mean deposit?
> being walked into the deposited list where it can just be allocated from
> another
> PMD->PTE split?
In general, RCU-only page table walkers are _only_ guaranteed that the page
tables are not freed from underneath them, as per the cover letter:
"As a result, page table walks can now be performed safely under RCU
without
any risk of page tables being freed underneath a walker.
However, this is the only guarantee that this work provides - page table
walkers must still ensure that page table entries are as expected
throughout."
So this series doesn't actually have to answer that :)
But a PTE PTL -> pmd_same() check will flag anything like that,
a.k.a. pte_offset_map_lock().
The RCU lock replaces stablisation on stuff other than VMA/mmap or rmap
lock.
If the walker needs to be sure the PTE is actually valid and belongs to the
expected walk then further is required.
Alternatively (like GUP-fast) if you wanted to avoid locking at all, IRQs
off would be required paired with the tlb_remove_table_sync_one() in
collapse_huge_page().
Another strategy could be used I guess with looped checks but it gets a bit
sketchy with timing etc.
But actually maybe we can avoid all that...
>
> Note that pgtable_trans_huge_withdraw() just dequeues *some* PTE page table in
> the list attached to the PMD table.
>
> Something is odd here.
...Since this code path already allocates (the huge folio), so allocation
here isn't an issue (as long as done outside of lcosk), why not have it
also allocate a new PTE to deposit and RCU-free the existing PTE instead?
That eliminates the one place in the kernel (afaik) where a
page-table-walker-visible table can just get yoinked over somewhere else.
Then a lockless PMD check works.
E.g. something like the below?
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index f49a6710933b..d9cb6b406ebb 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1293,7 +1293,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 +1310,12 @@ static enum scan_result collapse_huge_page(struct
mm_struct *mm, unsigned long s
goto out_nolock;
}
+ if (is_pmd_order(order)) {
+ pgtable = pte_alloc_one(mm);
+ if (!pgtable)
+ goto out_nolock;
+ }
+
mmap_read_lock(mm);
result = hugepage_vma_revalidate(mm, pmd_addr, /*expect_anon=*/ true,
&vma, cc, order);
@@ -1433,8 +1439,8 @@ static enum scan_result collapse_huge_page(struct
mm_struct *mm, unsigned long s
spin_lock(pmd_ptl);
VM_WARN_ON_ONCE(!pmd_none(*pmd));
if (is_pmd_order(order)) {
- pgtable = pmd_pgtable(_pmd);
pgtable_trans_huge_deposit(mm, pmd, pgtable);
+ pgtable = NULL;
map_anon_folio_pmd_nopf(folio, pmd, vma, pmd_addr);
} else {
/*
@@ -1453,6 +1459,9 @@ static enum scan_result collapse_huge_page(struct
mm_struct *mm, unsigned long s
}
spin_unlock(pmd_ptl);
+ if (is_pmd_order(order))
+ pte_free_defer(mm, pmd_pgtable(_pmd));
+
folio = NULL;
result = SCAN_SUCCEED;
@@ -1463,6 +1472,8 @@ static enum scan_result collapse_huge_page(struct
mm_struct *mm, unsigned long s
anon_vma_unlock_write(vma->anon_vma);
mmap_write_unlock(mm);
out_nolock:
+ if (pgtable)
+ pte_free(mm, pgtable);
if (folio)
folio_put(folio);
trace_mm_collapse_huge_page(mm, result == SCAN_SUCCEED, result, order);
>
> --
> Cheers,
>
> David
--
Cheers, Lorenzo
