Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes

[Barry Song]

On Thu, Nov 5, 2026 at 5:00 PM Baolin Wang
<baolin.wang@xxxxxxxxxxxxxxxxx> wrote:
>
> CC Barry, who added the TTU_SYNC in shrink_folio_list().
>
> On 11/5/26 1:48 PM, Lance Yang wrote:
> >
> >
> > On 2026/11/5 12:54, Lance Yang wrote:
> >>
> >> On Thu, Nov 05, 2026 at 10:56:28AM +0800, Baolin Wang wrote:
> >>> Similar to commit 243cf1f67b36 ("mm: memory: fix truncation removing
> >>> mapped
> >>> folios"), the '!pmd_present()' check in page_vma_mapped_walk() can
> >>> also race
> >>> with concurrent page table reclamation via zap_empty_pte_table().
> >>
> >> Good catch!
> >>
> >>> page_vma_mapped_walk() is used by many functions including
> >>> try_to_unmap(),
> >>> try_to_migrate(), folio_mkclean(), pfn_mkclean_range(),
> >>> remove_migration_ptes(),
> >>> page_mapped_in_vma(), and mapping_wrprotect_range(). Except for
> >>> try_to_unmap()
> >>> and try_to_migrate(), the other callers do not perform post-walk
> >>> accounting
> >>> checks, so the race has no observable impact on them.
> >>>
> >>> For migration, the race can cause unnecessary retries. For reclaim via
> >>> try_to_unmap(), the race can cause failure to reclaim a folio that is in
> >>> fact already unmapped (as shown below), which may lead to unexpected OOM
> >>> or increased refaults.
> >>>
> >>> CPU 0 (reclaim)                      CPU 1 (MADV_DONTNEED)
> >>> --------------------------------    --------------------------------
> >>> folio_lock(F)
> >>> try_to_unmap(F, TTU_SYNC)
> >>>                                     zap_pte_range(full PMD range)
> >>>                                       spin_lock(ptl)
> >>>                                       clear PTEs
> >>>                                       zap_empty_pte_table()
> >>>                                         spin_trylock(pml)
> >>>                                         pmd_clear()
> >>>                                         spin_unlock(pml)
> >>>
> >>>   page_vma_mapped_walk()
> >>>     observe !pmd_present(pmde)
> >>>     nr_pages < HPAGE_PMD_NR
> >>>     skip synchronization
> >>>     return false
> >>>
> >>> try_to_unmap() returns
> >>> folio_mapped(F) == true
> >>> nr_unmap_fail += folio_nr_pages(F)
> >>>                                     tlb_flush_mmu_tlbonly()
> >>>                                     tlb_flush_rmaps()
> >>>                                       remove F's rmaps
> >>>                                     spin_unlock(ptl)
> >>>
> >>> Commit 243cf1f67b36 ("mm: memory: fix truncation removing mapped
> >>> folios")
> >>> has already moved the PMD clear operation (via zap_empty_pte_table()) to
> >>> after tlb_flush_rmaps(). Similarly, page_vma_mapped_walk() also needs to
> >>> use sync_with_folio_pmd_zap() to synchronize and avoid memory ordering
> >>> issues on weakly ordered CPUs.
> >>>
> >>> Fixes: 6375e95f381e ("mm: pgtable: reclaim empty PTE page in
> >>> madvise(MADV_DONTNEED)")
> >>> Cc: stable@xxxxxxxxxxxxxxx
> >>> Suggested-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> >>> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
> >>> ---
> >>> mm/page_vma_mapped.c | 5 +----
> >>> 1 file changed, 1 insertion(+), 4 deletions(-)
> >>>
> >>> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> >>> index e6b6170c2f4f..8797467b6f0c 100644
> >>> --- a/mm/page_vma_mapped.c
> >>> +++ b/mm/page_vma_mapped.c
> >>> @@ -293,10 +293,7 @@ bool page_vma_mapped_walk(struct
> >>> page_vma_mapped_walk *pvmw)
> >>>             spin_unlock(pvmw->ptl);
> >>>             pvmw->ptl = NULL;
> >>>         } else if (!pmd_present(pmde)) {
> >>> -            if ((pvmw->flags & PVMW_SYNC) &&
> >>> -                thp_vma_suitable_order(vma, pvmw->address,
> >>> -                           PMD_ORDER) &&
> >>> -                (pvmw->nr_pages >= HPAGE_PMD_NR))
> >>> +            if (pvmw->flags & PVMW_SYNC)
> >>
> >> Right. Whether the folio can be PMD-mapped doesn't tell us whether its
> >> PTE table can be reclaimed, so those extra checks can skip
> >> synchronization
> >> that the caller explicitly asked for ;)
> >
> > Ouch ... one thing I missed ... what about order-0 reclaim?
> >
> > shrink_folio_list() only sets TTU_SYNC for large folio, so can't we still
> > hit the same race there?
>
> Thanks Lance. But this matches the purpose of the TTU_SYNC flag, which
> is used to avoid racy checks with PVMW_SYNC. If no TTU_SYNC flag is set,
> it means the caller can tolerate some racy checks.
>
> For this case, I think we should set TTU_SYNC unconditionally in
> shrink_folio_list(). Barry, what do you think?

Baolin, thanks for reaching out to me. I added that flag
mainly for large folios.

I observed cases where PTEs for large folios are partially
cleared during certain transition states, for example, when
PTEs are cleared before being modified:

pte = ptep_get_and_clear();
pte modification;
set_pte_at();

In such cases, page_vma_mapped_walk() skips the empty
entries and starts from the next valid one.

This may result in partially unmapping PTEs for large folios
and trigger unexpected folio splits. At the time, I was
particularly concerned about unnecessary folio splitting,
so my focus was mainly on large folios.

However, I also noticed that a similar race can occur with
small folios. In that case, the folio may simply remain
unreclaimed by the reclamation path.

However, my main concern at the time was avoiding
unnecessary folio splits.

Your new race also seems to amplify the same situation.
However, I don't think this problem necessarily depends on
the commit you mentioned. It has been around for a long time.

If the concern is reclaim failure, I agree that there is no
fundamental difference between large folios and small folios.

Best,
Barry