* [PATCH] mm: synchronize with PTE table reclaim for all folio sizes
@ 2026-11-05 2:56 Baolin Wang
2026-11-05 3:22 ` Qi Zheng
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Baolin Wang @ 2026-11-05 2:56 UTC (permalink / raw)
To: akpm, david, ljs
Cc: riel, liam, vbabka, harry, jannh, lance.yang, qi.zheng, pfalcato,
ayushr, baolin.wang, linux-mm, linux-kernel
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().
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@vger.kernel.org
Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
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)
sync_with_folio_pmd_zap(mm, pvmw->pmd);
goto next_pmd;
--
2.47.3
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-05 2:56 [PATCH] mm: synchronize with PTE table reclaim for all folio sizes Baolin Wang @ 2026-11-05 3:22 ` Qi Zheng 2026-11-05 4:54 ` Lance Yang 2026-11-05 11:37 ` Pedro Falcato 2 siblings, 0 replies; 20+ messages in thread From: Qi Zheng @ 2026-11-05 3:22 UTC (permalink / raw) To: Baolin Wang, akpm, david, ljs Cc: riel, liam, vbabka, harry, jannh, lance.yang, pfalcato, ayushr, linux-mm, linux-kernel On 11/5/26 10:56 AM, 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(). > > 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@vger.kernel.org > Suggested-by: David Hildenbrand (Arm) <david@kernel.org> > Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> > --- > 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) > sync_with_folio_pmd_zap(mm, pvmw->pmd); > > goto next_pmd; Acked-by: Qi Zheng <qi.zheng@linux.dev> Thanks, Qi ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-05 2:56 [PATCH] mm: synchronize with PTE table reclaim for all folio sizes Baolin Wang 2026-11-05 3:22 ` Qi Zheng @ 2026-11-05 4:54 ` Lance Yang 2026-11-05 5:48 ` Lance Yang 2026-11-05 11:37 ` Pedro Falcato 2 siblings, 1 reply; 20+ messages in thread From: Lance Yang @ 2026-11-05 4:54 UTC (permalink / raw) To: baolin.wang Cc: akpm, david, ljs, riel, liam, vbabka, harry, jannh, lance.yang, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel 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@vger.kernel.org >Suggested-by: David Hildenbrand (Arm) <david@kernel.org> >Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >--- > 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 ;) LGTM, thanks! Reviewed-by: Lance Yang <lance.yang@linux.dev> > sync_with_folio_pmd_zap(mm, pvmw->pmd); > > goto next_pmd; >-- >2.47.3 > > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-05 4:54 ` Lance Yang @ 2026-11-05 5:48 ` Lance Yang 2026-11-05 9:00 ` Baolin Wang 0 siblings, 1 reply; 20+ messages in thread From: Lance Yang @ 2026-11-05 5:48 UTC (permalink / raw) To: baolin.wang Cc: akpm, david, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel 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@vger.kernel.org >> Suggested-by: David Hildenbrand (Arm) <david@kernel.org> >> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >> --- >> 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? Cheers, Lance > LGTM, thanks! > > Reviewed-by: Lance Yang <lance.yang@linux.dev> > >> sync_with_folio_pmd_zap(mm, pvmw->pmd); >> >> goto next_pmd; >> -- >> 2.47.3 >> >> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-05 5:48 ` Lance Yang @ 2026-11-05 9:00 ` Baolin Wang 2026-11-05 10:06 ` David Hildenbrand (Arm) 2026-11-06 6:24 ` Barry Song 0 siblings, 2 replies; 20+ messages in thread From: Baolin Wang @ 2026-11-05 9:00 UTC (permalink / raw) To: Lance Yang, Barry Song Cc: akpm, david, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel 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@vger.kernel.org >>> Suggested-by: David Hildenbrand (Arm) <david@kernel.org> >>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >>> --- >>> 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? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-05 9:00 ` Baolin Wang @ 2026-11-05 10:06 ` David Hildenbrand (Arm) 2026-11-05 11:18 ` Baolin Wang 2026-11-05 13:22 ` Lance Yang 2026-11-06 6:24 ` Barry Song 1 sibling, 2 replies; 20+ messages in thread From: David Hildenbrand (Arm) @ 2026-11-05 10:06 UTC (permalink / raw) To: Baolin Wang, Lance Yang, Barry Song Cc: akpm, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On 11/5/26 10:00, Baolin Wang 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: >>> >>> >>> Good catch! >>> >>> >>> 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? That makes sense. I hope this doesn't throw a big wrench into our reclaim performance numbers. Can someone benchmark this? But maybe we should really follow-up with the folio-size based optimization in sync_with_folio_pmd_zap(), where we only need a memory barrier (and sufficient documentation why something that cannot be PMD-mapped can get away cheaper). -- Cheers, David ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-05 10:06 ` David Hildenbrand (Arm) @ 2026-11-05 11:18 ` Baolin Wang 2026-11-05 13:22 ` Lance Yang 1 sibling, 0 replies; 20+ messages in thread From: Baolin Wang @ 2026-11-05 11:18 UTC (permalink / raw) To: David Hildenbrand (Arm), Lance Yang, Barry Song Cc: akpm, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On 11/5/26 6:06 PM, David Hildenbrand (Arm) wrote: > On 11/5/26 10:00, Baolin Wang 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: >>>> >>>> >>>> Good catch! >>>> >>>> >>>> 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? > > That makes sense. I hope this doesn't throw a big wrench into our reclaim > performance numbers. > > Can someone benchmark this? Sure. I'll measure it if no one objects to this change. > But maybe we should really follow-up with the folio-size based optimization in > sync_with_folio_pmd_zap(), where we only need a memory barrier (and sufficient > documentation why something that cannot be PMD-mapped can get away cheaper). OK. I'll do this later. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-05 10:06 ` David Hildenbrand (Arm) 2026-11-05 11:18 ` Baolin Wang @ 2026-11-05 13:22 ` Lance Yang 2026-11-05 19:35 ` David Hildenbrand (Arm) 2026-11-06 1:57 ` Baolin Wang 1 sibling, 2 replies; 20+ messages in thread From: Lance Yang @ 2026-11-05 13:22 UTC (permalink / raw) To: david, baolin.wang Cc: lance.yang, baohua, akpm, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On Thu, Nov 05, 2026 at 11:06:29AM +0100, David Hildenbrand (Arm) wrote: >On 11/5/26 10:00, Baolin Wang 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: >>>> >>>> >>>> Good catch! >>>> >>>> >>>> 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? > >That makes sense. I hope this doesn't throw a big wrench into our reclaim >performance numbers. > >Can someone benchmark this? > >But maybe we should really follow-up with the folio-size based optimization in >sync_with_folio_pmd_zap(), where we only need a memory barrier (and sufficient >documentation why something that cannot be PMD-mapped can get away cheaper). Dug into the history a bit :D TTU_SYNC goes back to Hugh's THP splitting fix [1]. A racing zap could clear the PTE/PMD before updating mapcount, so the walker could skip the lock and return before the accounting caught up. Barry later enabled it for large-folio reclaim [2], to avoid skipping temporarily cleared PTEs and ending up with a partially unmapped folio. He mentioned that small folios had the same race, but with just one PTE, they couldn't be partially unmapped. That's where the large-folio check in shrink_folio_list() came from. Then PTE table reclaim [3] added another way to end up with an empty PMD, even for a folio that can't be PMD-mapped. The truncation fix [4] moved the rmap flush before the PMD clear and extended synchronization in zap_pmd_range() to cover those folios too. Looking back at the truncation discussion [5], you also pointed out that page_vma_mapped_walk() needed checking, and suggested a possible folio-size based optimization in the helper. [1] https://lore.kernel.org/all/c1e95853-8bcd-d8fd-55fa-e7f2488e78f@google.com/ [2] https://lore.kernel.org/all/20240306095219.71086-1-21cnbao@gmail.com/ [3] https://lore.kernel.org/all/92aba2b319a734913f18ba41e7d86a265f0b84e2.1733305182.git.zhengqi.arch@bytedance.com/ [4] https://lore.kernel.org/all/14ebbd556cc4941f0ebc43da59fa6a2fdca25dc8.1793148609.git.baolin.wang@linux.alibaba.com/ [5] https://lore.kernel.org/all/08a0907f-8e3a-4071-a2e4-fd1411c446f3@kernel.org/ So, how about starting with order-0 folios? ---8<--- diff --git a/include/linux/rmap.h b/include/linux/rmap.h index 74cca0e3c726..d01fe8282417 100644 --- a/include/linux/rmap.h +++ b/include/linux/rmap.h @@ -872,6 +872,8 @@ struct page_vma_mapped_walk { spinlock_t *ptl; unsigned int flags; bool pgoff_is_anon : 1; + /* Set from the folio, not the number of pages being walked. */ + bool folio_is_order0 : 1; }; #define DEFINE_FOLIO_VMA_WALK(name, _folio, _vma, _address, _flags) \ @@ -883,6 +885,7 @@ struct page_vma_mapped_walk { .address = _address, \ .flags = _flags, \ .pgoff_is_anon = folio_test_anon(_folio), \ + .folio_is_order0 = !folio_test_large(_folio), \ } static inline void page_vma_mapped_walk_done(struct page_vma_mapped_walk *pvmw) diff --git a/mm/internal.h b/mm/internal.h index 9b8864a8540a..e70869b38292 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -592,21 +592,29 @@ void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte); * sync_with_folio_pmd_zap - sync with concurrent zapping of folios * @mm: The mm_struct. * @pmdp: Pointer to the pmd that was found to be pmd_none(). + * @order0: True if the target is known to be an order-0 folio. * - * When we find a pmd_none() while unmapping a folio without holding the PTL, - * zap_huge_pmd() may have cleared the PMD but not yet updated the folio - * mapcount, and zap_empty_pte_table() clears the PMD only after the rmaps - * for the table have been removed, but nothing orders the pmd_none() read - * against later mapcount reads. Skipping the PMD without synchronization - * could make folio unmapping code assume that unmapping failed. + * An empty PMD does not guarantee that a later mapcount read sees the + * completed rmap updates. For PMD-mapped folios, zap_huge_pmd() clears + * the PMD before updating mapcount, so we must wait for the PMD lock. * - * Wait for concurrent zapping to complete by grabbing the PTL. + * Order-0 folios cannot be PMD-mapped. Their fully ordered + * atomic_add_negative() updates mapcount before PTE table reclaim clears + * the PMD. The PTE lock preserves this ordering if another CPU reclaims + * the table. A read barrier therefore suffices for order-0 folios. + * Keep taking the PMD lock for all other folios. */ -static inline void sync_with_folio_pmd_zap(struct mm_struct *mm, pmd_t *pmdp) +static inline void sync_with_folio_pmd_zap(struct mm_struct *mm, pmd_t *pmdp, + bool order0) { - spinlock_t *ptl = pmd_lock(mm, pmdp); + if (order0) { + /* Order the PMD read before subsequent mapcount reads. */ + smp_rmb(); + } else { + spinlock_t *ptl = pmd_lock(mm, pmdp); - spin_unlock(ptl); + spin_unlock(ptl); + } } struct zap_details; diff --git a/mm/memory.c b/mm/memory.c index 44376321180b..8955d1cd31b0 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -2112,7 +2112,8 @@ static inline unsigned long zap_pmd_range(struct mmu_gather *tlb, /* fall through */ } else if (pmd_none(*pmd)) { if (details && details->single_folio) - sync_with_folio_pmd_zap(tlb->mm, pmd); + sync_with_folio_pmd_zap(tlb->mm, pmd, + !folio_test_large(details->single_folio)); addr = next; continue; } diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c index e6b6170c2f4f..36881f8fd45e 100644 --- a/mm/page_vma_mapped.c +++ b/mm/page_vma_mapped.c @@ -293,11 +293,9 @@ 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)) - sync_with_folio_pmd_zap(mm, pvmw->pmd); + if (pvmw->flags & PVMW_SYNC) + sync_with_folio_pmd_zap(mm, pvmw->pmd, + pvmw->folio_is_order0); goto next_pmd; } @@ -379,6 +377,7 @@ unsigned long page_mapped_in_vma(const struct page *page, .nr_pages = 1, .vma = vma, .flags = PVMW_SYNC, + .folio_is_order0 = !folio_test_large(folio), }; if (folio_test_anon(folio)) diff --git a/mm/vmscan.c b/mm/vmscan.c index 7db6be53f3a9..79cfa9bc475b 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1434,26 +1434,17 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, * processes. Try to unmap it here. */ if (folio_mapped(folio)) { - enum ttu_flags flags = TTU_BATCH_FLUSH; + enum ttu_flags flags = TTU_BATCH_FLUSH | TTU_SYNC; bool was_swapbacked = folio_test_swapbacked(folio); if (folio_test_pmd_mappable(folio)) flags |= TTU_SPLIT_HUGE_PMD; + /* - * Without TTU_SYNC, try_to_unmap will only begin to - * hold PTL from the first present PTE within a large - * folio. Some initial PTEs might be skipped due to - * races with parallel PTE writes in which PTEs can be - * cleared temporarily before being written new present - * values. This will lead to a large folio is still - * mapped while some subpages have been partially - * unmapped after try_to_unmap; TTU_SYNC helps - * try_to_unmap acquire PTL from the first PTE, - * eliminating the influence of temporary PTE values. + * Synchronize with concurrent rmap updates before checking + * folio_mapped(). Also avoid skipping temporarily cleared + * PTEs, which could leave a large folio partially mapped. */ - if (folio_test_large(folio)) - flags |= TTU_SYNC; - try_to_unmap(folio, flags); if (folio_mapped(folio)) { stat->nr_unmap_fail += nr_pages; -- atomic_add_negative() provides full ordering for mapcount updates. With 243cf1f67b36, rmap removal completes before clearing PMD, including when another CPU reclaims an empty table under PTE lock. So smp_rmb() is enough here, without an extra writer-side barrier. nr_pages == 1 isn't enough to identify an order-0 folio, since page_mapped_in_vma() also checks individual pages of large folios. We can then enable TTU_SYNC for order-0 reclaim and use smp_rmb() on an empty PMD, while keeping PMD locking for large folios. (Haven't benchmarked it yet) ... Cheers, Lance ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-05 13:22 ` Lance Yang @ 2026-11-05 19:35 ` David Hildenbrand (Arm) 2026-11-06 1:57 ` Baolin Wang 1 sibling, 0 replies; 20+ messages in thread From: David Hildenbrand (Arm) @ 2026-11-05 19:35 UTC (permalink / raw) To: Lance Yang, baolin.wang Cc: baohua, akpm, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On 11/5/26 14:22, Lance Yang wrote: > > On Thu, Nov 05, 2026 at 11:06:29AM +0100, David Hildenbrand (Arm) wrote: >> On 11/5/26 10:00, Baolin Wang wrote: >>> CC Barry, who added the TTU_SYNC in shrink_folio_list(). >>> >>> >>> 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? >> >> That makes sense. I hope this doesn't throw a big wrench into our reclaim >> performance numbers. >> >> Can someone benchmark this? >> >> But maybe we should really follow-up with the folio-size based optimization in >> sync_with_folio_pmd_zap(), where we only need a memory barrier (and sufficient >> documentation why something that cannot be PMD-mapped can get away cheaper). > > Dug into the history a bit :D > > TTU_SYNC goes back to Hugh's THP splitting fix [1]. A racing zap could > clear the PTE/PMD before updating mapcount, so the walker could skip the > lock and return before the accounting caught up. > > Barry later enabled it for large-folio reclaim [2], to avoid skipping > temporarily cleared PTEs and ending up with a partially unmapped folio. > He mentioned that small folios had the same race, but with just one PTE, > they couldn't be partially unmapped. That's where the large-folio check > in shrink_folio_list() came from. > > Then PTE table reclaim [3] added another way to end up with an empty PMD, > even for a folio that can't be PMD-mapped. The truncation fix [4] moved > the rmap flush before the PMD clear and extended synchronization in > zap_pmd_range() to cover those folios too. > > Looking back at the truncation discussion [5], you also pointed out that > page_vma_mapped_walk() needed checking, and suggested a possible > folio-size based optimization in the helper. > > [1] https://lore.kernel.org/all/c1e95853-8bcd-d8fd-55fa-e7f2488e78f@google.com/ > [2] https://lore.kernel.org/all/20240306095219.71086-1-21cnbao@gmail.com/ > [3] https://lore.kernel.org/all/92aba2b319a734913f18ba41e7d86a265f0b84e2.1733305182.git.zhengqi.arch@bytedance.com/ > [4] https://lore.kernel.org/all/14ebbd556cc4941f0ebc43da59fa6a2fdca25dc8.1793148609.git.baolin.wang@linux.alibaba.com/ > [5] https://lore.kernel.org/all/08a0907f-8e3a-4071-a2e4-fd1411c446f3@kernel.org/ > > So, how about starting with order-0 folios? > > ---8<--- > diff --git a/include/linux/rmap.h b/include/linux/rmap.h > index 74cca0e3c726..d01fe8282417 100644 > --- a/include/linux/rmap.h > +++ b/include/linux/rmap.h > @@ -872,6 +872,8 @@ struct page_vma_mapped_walk { > spinlock_t *ptl; > unsigned int flags; > bool pgoff_is_anon : 1; > + /* Set from the folio, not the number of pages being walked. */ > + bool folio_is_order0 : 1; > }; > > #define DEFINE_FOLIO_VMA_WALK(name, _folio, _vma, _address, _flags) \ > @@ -883,6 +885,7 @@ struct page_vma_mapped_walk { > .address = _address, \ > .flags = _flags, \ > .pgoff_is_anon = folio_test_anon(_folio), \ > + .folio_is_order0 = !folio_test_large(_folio), \ > } We should pass the actual folio ;) But yes, this goes into the direction I had in mind. But I do wonder whether we need a separate fix first before we improve it? -- Cheers, David ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-05 13:22 ` Lance Yang 2026-11-05 19:35 ` David Hildenbrand (Arm) @ 2026-11-06 1:57 ` Baolin Wang 2026-11-06 2:33 ` Lance Yang 1 sibling, 1 reply; 20+ messages in thread From: Baolin Wang @ 2026-11-06 1:57 UTC (permalink / raw) To: Lance Yang, david Cc: baohua, akpm, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On 11/5/26 9:22 PM, Lance Yang wrote: > > On Thu, Nov 05, 2026 at 11:06:29AM +0100, David Hildenbrand (Arm) wrote: >> On 11/5/26 10:00, Baolin Wang 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: >>>>> >>>>> >>>>> Good catch! >>>>> >>>>> >>>>> 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? >> >> That makes sense. I hope this doesn't throw a big wrench into our reclaim >> performance numbers. >> >> Can someone benchmark this? >> >> But maybe we should really follow-up with the folio-size based optimization in >> sync_with_folio_pmd_zap(), where we only need a memory barrier (and sufficient >> documentation why something that cannot be PMD-mapped can get away cheaper). > > Dug into the history a bit :D > > TTU_SYNC goes back to Hugh's THP splitting fix [1]. A racing zap could > clear the PTE/PMD before updating mapcount, so the walker could skip the > lock and return before the accounting caught up. > > Barry later enabled it for large-folio reclaim [2], to avoid skipping > temporarily cleared PTEs and ending up with a partially unmapped folio. > He mentioned that small folios had the same race, but with just one PTE, > they couldn't be partially unmapped. That's where the large-folio check > in shrink_folio_list() came from. > > Then PTE table reclaim [3] added another way to end up with an empty PMD, > even for a folio that can't be PMD-mapped. The truncation fix [4] moved > the rmap flush before the PMD clear and extended synchronization in > zap_pmd_range() to cover those folios too. > > Looking back at the truncation discussion [5], you also pointed out that > page_vma_mapped_walk() needed checking, and suggested a possible > folio-size based optimization in the helper. Thanks. > [1] https://lore.kernel.org/all/c1e95853-8bcd-d8fd-55fa-e7f2488e78f@google.com/ > [2] https://lore.kernel.org/all/20240306095219.71086-1-21cnbao@gmail.com/ > [3] https://lore.kernel.org/all/92aba2b319a734913f18ba41e7d86a265f0b84e2.1733305182.git.zhengqi.arch@bytedance.com/ > [4] https://lore.kernel.org/all/14ebbd556cc4941f0ebc43da59fa6a2fdca25dc8.1793148609.git.baolin.wang@linux.alibaba.com/ > [5] https://lore.kernel.org/all/08a0907f-8e3a-4071-a2e4-fd1411c446f3@kernel.org/ > > So, how about starting with order-0 folios? > > ---8<--- > diff --git a/include/linux/rmap.h b/include/linux/rmap.h > index 74cca0e3c726..d01fe8282417 100644 > --- a/include/linux/rmap.h > +++ b/include/linux/rmap.h > @@ -872,6 +872,8 @@ struct page_vma_mapped_walk { > spinlock_t *ptl; > unsigned int flags; > bool pgoff_is_anon : 1; > + /* Set from the folio, not the number of pages being walked. */ > + bool folio_is_order0 : 1; > }; > > #define DEFINE_FOLIO_VMA_WALK(name, _folio, _vma, _address, _flags) \ > @@ -883,6 +885,7 @@ struct page_vma_mapped_walk { > .address = _address, \ > .flags = _flags, \ > .pgoff_is_anon = folio_test_anon(_folio), \ > + .folio_is_order0 = !folio_test_large(_folio), \ > } > > static inline void page_vma_mapped_walk_done(struct page_vma_mapped_walk *pvmw) > diff --git a/mm/internal.h b/mm/internal.h > index 9b8864a8540a..e70869b38292 100644 > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -592,21 +592,29 @@ void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte); > * sync_with_folio_pmd_zap - sync with concurrent zapping of folios > * @mm: The mm_struct. > * @pmdp: Pointer to the pmd that was found to be pmd_none(). > + * @order0: True if the target is known to be an order-0 folio. > * > - * When we find a pmd_none() while unmapping a folio without holding the PTL, > - * zap_huge_pmd() may have cleared the PMD but not yet updated the folio > - * mapcount, and zap_empty_pte_table() clears the PMD only after the rmaps > - * for the table have been removed, but nothing orders the pmd_none() read > - * against later mapcount reads. Skipping the PMD without synchronization > - * could make folio unmapping code assume that unmapping failed. > + * An empty PMD does not guarantee that a later mapcount read sees the > + * completed rmap updates. For PMD-mapped folios, zap_huge_pmd() clears > + * the PMD before updating mapcount, so we must wait for the PMD lock. > * > - * Wait for concurrent zapping to complete by grabbing the PTL. > + * Order-0 folios cannot be PMD-mapped. Their fully ordered > + * atomic_add_negative() updates mapcount before PTE table reclaim clears > + * the PMD. The PTE lock preserves this ordering if another CPU reclaims > + * the table. A read barrier therefore suffices for order-0 folios. > + * Keep taking the PMD lock for all other folios. > */ > -static inline void sync_with_folio_pmd_zap(struct mm_struct *mm, pmd_t *pmdp) > +static inline void sync_with_folio_pmd_zap(struct mm_struct *mm, pmd_t *pmdp, > + bool order0) > { > - spinlock_t *ptl = pmd_lock(mm, pmdp); > + if (order0) { > + /* Order the PMD read before subsequent mapcount reads. */ > + smp_rmb(); > + } else { > + spinlock_t *ptl = pmd_lock(mm, pmdp); > > - spin_unlock(ptl); > + spin_unlock(ptl); > + } > } > > struct zap_details; > diff --git a/mm/memory.c b/mm/memory.c > index 44376321180b..8955d1cd31b0 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -2112,7 +2112,8 @@ static inline unsigned long zap_pmd_range(struct mmu_gather *tlb, > /* fall through */ > } else if (pmd_none(*pmd)) { > if (details && details->single_folio) > - sync_with_folio_pmd_zap(tlb->mm, pmd); > + sync_with_folio_pmd_zap(tlb->mm, pmd, > + !folio_test_large(details->single_folio)); > addr = next; > continue; > } > diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c > index e6b6170c2f4f..36881f8fd45e 100644 > --- a/mm/page_vma_mapped.c > +++ b/mm/page_vma_mapped.c > @@ -293,11 +293,9 @@ 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)) > - sync_with_folio_pmd_zap(mm, pvmw->pmd); > + if (pvmw->flags & PVMW_SYNC) > + sync_with_folio_pmd_zap(mm, pvmw->pmd, > + pvmw->folio_is_order0); > > goto next_pmd; > } > @@ -379,6 +377,7 @@ unsigned long page_mapped_in_vma(const struct page *page, > .nr_pages = 1, > .vma = vma, > .flags = PVMW_SYNC, > + .folio_is_order0 = !folio_test_large(folio), > }; > > if (folio_test_anon(folio)) > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 7db6be53f3a9..79cfa9bc475b 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -1434,26 +1434,17 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, > * processes. Try to unmap it here. > */ > if (folio_mapped(folio)) { > - enum ttu_flags flags = TTU_BATCH_FLUSH; > + enum ttu_flags flags = TTU_BATCH_FLUSH | TTU_SYNC; > bool was_swapbacked = folio_test_swapbacked(folio); > > if (folio_test_pmd_mappable(folio)) > flags |= TTU_SPLIT_HUGE_PMD; > + > /* > - * Without TTU_SYNC, try_to_unmap will only begin to > - * hold PTL from the first present PTE within a large > - * folio. Some initial PTEs might be skipped due to > - * races with parallel PTE writes in which PTEs can be > - * cleared temporarily before being written new present > - * values. This will lead to a large folio is still > - * mapped while some subpages have been partially > - * unmapped after try_to_unmap; TTU_SYNC helps > - * try_to_unmap acquire PTL from the first PTE, > - * eliminating the influence of temporary PTE values. > + * Synchronize with concurrent rmap updates before checking > + * folio_mapped(). Also avoid skipping temporarily cleared > + * PTEs, which could leave a large folio partially mapped. > */ > - if (folio_test_large(folio)) > - flags |= TTU_SYNC; > - > try_to_unmap(folio, flags); > if (folio_mapped(folio)) { > stat->nr_unmap_fail += nr_pages; > -- > > atomic_add_negative() provides full ordering for mapcount updates. With > 243cf1f67b36, rmap removal completes before clearing PMD, including when > another CPU reclaims an empty table under PTE lock. So smp_rmb() is enough > here, without an extra writer-side barrier. I'm afraid the memory barrier here is subtle. __folio_remove_rmap() is complex, and in the CONFIG_NO_PAGE_MAPCOUNT branch, folio_sub_return_large_mapcount() doesn't provide full ordering, so we may still need smp_wmb(). Anyway, as David said, let's fix this race first. Memory barrier optimization requires careful analysis and clear documentation. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-06 1:57 ` Baolin Wang @ 2026-11-06 2:33 ` Lance Yang 2026-11-06 2:54 ` Baolin Wang 0 siblings, 1 reply; 20+ messages in thread From: Lance Yang @ 2026-11-06 2:33 UTC (permalink / raw) To: baolin.wang, david Cc: lance.yang, baohua, akpm, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On Fri, Nov 06, 2026 at 09:57:47AM +0800, Baolin Wang wrote: > > >On 11/5/26 9:22 PM, Lance Yang wrote: >> >> On Thu, Nov 05, 2026 at 11:06:29AM +0100, David Hildenbrand (Arm) wrote: >>> On 11/5/26 10:00, Baolin Wang 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: >>>>>> >>>>>> >>>>>> Good catch! >>>>>> >>>>>> >>>>>> 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? >>> >>> That makes sense. I hope this doesn't throw a big wrench into our reclaim >>> performance numbers. >>> >>> Can someone benchmark this? >>> >>> But maybe we should really follow-up with the folio-size based optimization in >>> sync_with_folio_pmd_zap(), where we only need a memory barrier (and sufficient >>> documentation why something that cannot be PMD-mapped can get away cheaper). >> >> Dug into the history a bit :D >> >> TTU_SYNC goes back to Hugh's THP splitting fix [1]. A racing zap could >> clear the PTE/PMD before updating mapcount, so the walker could skip the >> lock and return before the accounting caught up. >> >> Barry later enabled it for large-folio reclaim [2], to avoid skipping >> temporarily cleared PTEs and ending up with a partially unmapped folio. >> He mentioned that small folios had the same race, but with just one PTE, >> they couldn't be partially unmapped. That's where the large-folio check >> in shrink_folio_list() came from. >> >> Then PTE table reclaim [3] added another way to end up with an empty PMD, >> even for a folio that can't be PMD-mapped. The truncation fix [4] moved >> the rmap flush before the PMD clear and extended synchronization in >> zap_pmd_range() to cover those folios too. >> >> Looking back at the truncation discussion [5], you also pointed out that >> page_vma_mapped_walk() needed checking, and suggested a possible >> folio-size based optimization in the helper. > >Thanks. Cheers! >> [1] https://lore.kernel.org/all/c1e95853-8bcd-d8fd-55fa-e7f2488e78f@google.com/ >> [2] https://lore.kernel.org/all/20240306095219.71086-1-21cnbao@gmail.com/ >> [3] https://lore.kernel.org/all/92aba2b319a734913f18ba41e7d86a265f0b84e2.1733305182.git.zhengqi.arch@bytedance.com/ >> [4] https://lore.kernel.org/all/14ebbd556cc4941f0ebc43da59fa6a2fdca25dc8.1793148609.git.baolin.wang@linux.alibaba.com/ >> [5] https://lore.kernel.org/all/08a0907f-8e3a-4071-a2e4-fd1411c446f3@kernel.org/ >> >> So, how about starting with order-0 folios? >> >> ---8<--- >> diff --git a/include/linux/rmap.h b/include/linux/rmap.h >> index 74cca0e3c726..d01fe8282417 100644 >> --- a/include/linux/rmap.h >> +++ b/include/linux/rmap.h >> @@ -872,6 +872,8 @@ struct page_vma_mapped_walk { >> spinlock_t *ptl; >> unsigned int flags; >> bool pgoff_is_anon : 1; >> + /* Set from the folio, not the number of pages being walked. */ >> + bool folio_is_order0 : 1; >> }; >> >> #define DEFINE_FOLIO_VMA_WALK(name, _folio, _vma, _address, _flags) \ >> @@ -883,6 +885,7 @@ struct page_vma_mapped_walk { >> .address = _address, \ >> .flags = _flags, \ >> .pgoff_is_anon = folio_test_anon(_folio), \ >> + .folio_is_order0 = !folio_test_large(_folio), \ >> } >> >> static inline void page_vma_mapped_walk_done(struct page_vma_mapped_walk *pvmw) >> diff --git a/mm/internal.h b/mm/internal.h >> index 9b8864a8540a..e70869b38292 100644 >> --- a/mm/internal.h >> +++ b/mm/internal.h >> @@ -592,21 +592,29 @@ void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte); >> * sync_with_folio_pmd_zap - sync with concurrent zapping of folios >> * @mm: The mm_struct. >> * @pmdp: Pointer to the pmd that was found to be pmd_none(). >> + * @order0: True if the target is known to be an order-0 folio. >> * >> - * When we find a pmd_none() while unmapping a folio without holding the PTL, >> - * zap_huge_pmd() may have cleared the PMD but not yet updated the folio >> - * mapcount, and zap_empty_pte_table() clears the PMD only after the rmaps >> - * for the table have been removed, but nothing orders the pmd_none() read >> - * against later mapcount reads. Skipping the PMD without synchronization >> - * could make folio unmapping code assume that unmapping failed. >> + * An empty PMD does not guarantee that a later mapcount read sees the >> + * completed rmap updates. For PMD-mapped folios, zap_huge_pmd() clears >> + * the PMD before updating mapcount, so we must wait for the PMD lock. >> * >> - * Wait for concurrent zapping to complete by grabbing the PTL. >> + * Order-0 folios cannot be PMD-mapped. Their fully ordered >> + * atomic_add_negative() updates mapcount before PTE table reclaim clears >> + * the PMD. The PTE lock preserves this ordering if another CPU reclaims >> + * the table. A read barrier therefore suffices for order-0 folios. >> + * Keep taking the PMD lock for all other folios. >> */ >> -static inline void sync_with_folio_pmd_zap(struct mm_struct *mm, pmd_t *pmdp) >> +static inline void sync_with_folio_pmd_zap(struct mm_struct *mm, pmd_t *pmdp, >> + bool order0) >> { >> - spinlock_t *ptl = pmd_lock(mm, pmdp); >> + if (order0) { >> + /* Order the PMD read before subsequent mapcount reads. */ >> + smp_rmb(); >> + } else { >> + spinlock_t *ptl = pmd_lock(mm, pmdp); >> >> - spin_unlock(ptl); >> + spin_unlock(ptl); >> + } >> } >> >> struct zap_details; >> diff --git a/mm/memory.c b/mm/memory.c >> index 44376321180b..8955d1cd31b0 100644 >> --- a/mm/memory.c >> +++ b/mm/memory.c >> @@ -2112,7 +2112,8 @@ static inline unsigned long zap_pmd_range(struct mmu_gather *tlb, >> /* fall through */ >> } else if (pmd_none(*pmd)) { >> if (details && details->single_folio) >> - sync_with_folio_pmd_zap(tlb->mm, pmd); >> + sync_with_folio_pmd_zap(tlb->mm, pmd, >> + !folio_test_large(details->single_folio)); >> addr = next; >> continue; >> } >> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c >> index e6b6170c2f4f..36881f8fd45e 100644 >> --- a/mm/page_vma_mapped.c >> +++ b/mm/page_vma_mapped.c >> @@ -293,11 +293,9 @@ 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)) >> - sync_with_folio_pmd_zap(mm, pvmw->pmd); >> + if (pvmw->flags & PVMW_SYNC) >> + sync_with_folio_pmd_zap(mm, pvmw->pmd, >> + pvmw->folio_is_order0); >> >> goto next_pmd; >> } >> @@ -379,6 +377,7 @@ unsigned long page_mapped_in_vma(const struct page *page, >> .nr_pages = 1, >> .vma = vma, >> .flags = PVMW_SYNC, >> + .folio_is_order0 = !folio_test_large(folio), >> }; >> >> if (folio_test_anon(folio)) >> diff --git a/mm/vmscan.c b/mm/vmscan.c >> index 7db6be53f3a9..79cfa9bc475b 100644 >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -1434,26 +1434,17 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, >> * processes. Try to unmap it here. >> */ >> if (folio_mapped(folio)) { >> - enum ttu_flags flags = TTU_BATCH_FLUSH; >> + enum ttu_flags flags = TTU_BATCH_FLUSH | TTU_SYNC; >> bool was_swapbacked = folio_test_swapbacked(folio); >> >> if (folio_test_pmd_mappable(folio)) >> flags |= TTU_SPLIT_HUGE_PMD; >> + >> /* >> - * Without TTU_SYNC, try_to_unmap will only begin to >> - * hold PTL from the first present PTE within a large >> - * folio. Some initial PTEs might be skipped due to >> - * races with parallel PTE writes in which PTEs can be >> - * cleared temporarily before being written new present >> - * values. This will lead to a large folio is still >> - * mapped while some subpages have been partially >> - * unmapped after try_to_unmap; TTU_SYNC helps >> - * try_to_unmap acquire PTL from the first PTE, >> - * eliminating the influence of temporary PTE values. >> + * Synchronize with concurrent rmap updates before checking >> + * folio_mapped(). Also avoid skipping temporarily cleared >> + * PTEs, which could leave a large folio partially mapped. >> */ >> - if (folio_test_large(folio)) >> - flags |= TTU_SYNC; >> - >> try_to_unmap(folio, flags); >> if (folio_mapped(folio)) { >> stat->nr_unmap_fail += nr_pages; >> -- >> >> atomic_add_negative() provides full ordering for mapcount updates. With >> 243cf1f67b36, rmap removal completes before clearing PMD, including when >> another CPU reclaims an empty table under PTE lock. So smp_rmb() is enough >> here, without an extra writer-side barrier. > >I'm afraid the memory barrier here is subtle. __folio_remove_rmap() is >complex, and in the CONFIG_NO_PAGE_MAPCOUNT branch, >folio_sub_return_large_mapcount() doesn't provide full ordering, so we >may still need smp_wmb(). Yeah, agreed for large folios. In the diff above, only order-0 folios use smp_rmb(). Note that order-0 folios take the fully ordered atomic_add_negative() branch and skip the CONFIG_NO_PAGE_MAPCOUNT branch entirely. static __always_inline void __folio_remove_rmap(struct folio *folio, struct page *page, int nr_pages, struct vm_area_struct *vma, enum pgtable_level level) { ... switch (level) { case PGTABLE_LEVEL_PTE: if (!folio_test_large(folio)) { nr = atomic_add_negative(-1, &folio->_mapcount); break; } if (IS_ENABLED(CONFIG_NO_PAGE_MAPCOUNT)) { nr = folio_sub_return_large_mapcount(folio, nr_pages, vma); ... } ... } ... } And large folios still take the PMD lock :D >Anyway, as David said, let's fix this race first. Memory barrier >optimization requires careful analysis and clear documentation. > Yep, fine by me :D The fix goes first, of course. Thanks! Cheers, Lance ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-06 2:33 ` Lance Yang @ 2026-11-06 2:54 ` Baolin Wang 0 siblings, 0 replies; 20+ messages in thread From: Baolin Wang @ 2026-11-06 2:54 UTC (permalink / raw) To: Lance Yang, david Cc: baohua, akpm, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On 11/6/26 10:33 AM, Lance Yang wrote: > > On Fri, Nov 06, 2026 at 09:57:47AM +0800, Baolin Wang wrote: >> >> >> On 11/5/26 9:22 PM, Lance Yang wrote: >>> >>> On Thu, Nov 05, 2026 at 11:06:29AM +0100, David Hildenbrand (Arm) wrote: >>>> On 11/5/26 10:00, Baolin Wang 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: >>>>>>> >>>>>>> >>>>>>> Good catch! >>>>>>> >>>>>>> >>>>>>> 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? >>>> >>>> That makes sense. I hope this doesn't throw a big wrench into our reclaim >>>> performance numbers. >>>> >>>> Can someone benchmark this? >>>> >>>> But maybe we should really follow-up with the folio-size based optimization in >>>> sync_with_folio_pmd_zap(), where we only need a memory barrier (and sufficient >>>> documentation why something that cannot be PMD-mapped can get away cheaper). >>> >>> Dug into the history a bit :D >>> >>> TTU_SYNC goes back to Hugh's THP splitting fix [1]. A racing zap could >>> clear the PTE/PMD before updating mapcount, so the walker could skip the >>> lock and return before the accounting caught up. >>> >>> Barry later enabled it for large-folio reclaim [2], to avoid skipping >>> temporarily cleared PTEs and ending up with a partially unmapped folio. >>> He mentioned that small folios had the same race, but with just one PTE, >>> they couldn't be partially unmapped. That's where the large-folio check >>> in shrink_folio_list() came from. >>> >>> Then PTE table reclaim [3] added another way to end up with an empty PMD, >>> even for a folio that can't be PMD-mapped. The truncation fix [4] moved >>> the rmap flush before the PMD clear and extended synchronization in >>> zap_pmd_range() to cover those folios too. >>> >>> Looking back at the truncation discussion [5], you also pointed out that >>> page_vma_mapped_walk() needed checking, and suggested a possible >>> folio-size based optimization in the helper. >> >> Thanks. > > Cheers! > >>> [1] https://lore.kernel.org/all/c1e95853-8bcd-d8fd-55fa-e7f2488e78f@google.com/ >>> [2] https://lore.kernel.org/all/20240306095219.71086-1-21cnbao@gmail.com/ >>> [3] https://lore.kernel.org/all/92aba2b319a734913f18ba41e7d86a265f0b84e2.1733305182.git.zhengqi.arch@bytedance.com/ >>> [4] https://lore.kernel.org/all/14ebbd556cc4941f0ebc43da59fa6a2fdca25dc8.1793148609.git.baolin.wang@linux.alibaba.com/ >>> [5] https://lore.kernel.org/all/08a0907f-8e3a-4071-a2e4-fd1411c446f3@kernel.org/ >>> >>> So, how about starting with order-0 folios? ....... >>> >>> atomic_add_negative() provides full ordering for mapcount updates. With >>> 243cf1f67b36, rmap removal completes before clearing PMD, including when >>> another CPU reclaims an empty table under PTE lock. So smp_rmb() is enough >>> here, without an extra writer-side barrier. >> >> I'm afraid the memory barrier here is subtle. __folio_remove_rmap() is >> complex, and in the CONFIG_NO_PAGE_MAPCOUNT branch, >> folio_sub_return_large_mapcount() doesn't provide full ordering, so we >> may still need smp_wmb(). > > Yeah, agreed for large folios. In the diff above, only order-0 folios > use smp_rmb(). > > Note that order-0 folios take the fully ordered atomic_add_negative() > branch and skip the CONFIG_NO_PAGE_MAPCOUNT branch entirely. Yes. > static __always_inline void __folio_remove_rmap(struct folio *folio, > struct page *page, int nr_pages, struct vm_area_struct *vma, > enum pgtable_level level) > { > ... > switch (level) { > case PGTABLE_LEVEL_PTE: > if (!folio_test_large(folio)) { > nr = atomic_add_negative(-1, &folio->_mapcount); > break; > } > > if (IS_ENABLED(CONFIG_NO_PAGE_MAPCOUNT)) { > nr = folio_sub_return_large_mapcount(folio, nr_pages, vma); > ... > } > ... > } > ... > } > > And large folios still take the PMD lock :D IIUC, the memory barrier optimization targets all non-PMD-sized folios (including large folios). That is, for PMD-sized large folios we still use the original PMD lock, while for other non-PMD-sized folios we can use memory barriers to sync. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-05 9:00 ` Baolin Wang 2026-11-05 10:06 ` David Hildenbrand (Arm) @ 2026-11-06 6:24 ` Barry Song 2026-11-06 8:54 ` Baolin Wang 1 sibling, 1 reply; 20+ messages in thread From: Barry Song @ 2026-11-06 6:24 UTC (permalink / raw) To: Baolin Wang Cc: Lance Yang, akpm, david, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On Thu, Nov 5, 2026 at 5:00 PM Baolin Wang <baolin.wang@linux.alibaba.com> 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@vger.kernel.org > >>> Suggested-by: David Hildenbrand (Arm) <david@kernel.org> > >>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> > >>> --- > >>> 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-06 6:24 ` Barry Song @ 2026-11-06 8:54 ` Baolin Wang 2026-11-06 9:15 ` Lance Yang 2026-11-06 9:56 ` Barry Song 0 siblings, 2 replies; 20+ messages in thread From: Baolin Wang @ 2026-11-06 8:54 UTC (permalink / raw) To: Barry Song Cc: Lance Yang, akpm, david, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On 11/6/26 2:24 PM, Barry Song wrote: > On Thu, Nov 5, 2026 at 5:00 PM Baolin Wang > <baolin.wang@linux.alibaba.com> 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@vger.kernel.org >>>>> Suggested-by: David Hildenbrand (Arm) <david@kernel.org> >>>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >>>>> --- >>>>> 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. Thanks for the information. > 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. Agree. > If the concern is reclaim failure, I agree that there is no > fundamental difference between large folios and small folios. OK. And I did a quick measurement to reclaim anon and file folios via the memory.reclaim interface. From test results, reclaiming 1G small anonymous folios (using zram as swap) shows no significant performance change. However, reclaiming 1G mapped clean small file folios does show an observable regression if the TTU_SYNC flag is set. Without TTU_SYNC for small folios: time echo 1G > /sys/fs/cgroup/test/memory.reclaim real 0m0.178s user 0m0.000s sys 0m0.178s With TTU_SYNC for small folios: real 0m0.315s user 0m0.000s sys 0m0.314s So for the current patch, I'd prefer to keep things as is: only sync when TTU_SYNC is set. Without setting the TTU_SYNC flag, keep the original logic, meaning the caller can tolerate some racy checks. Any other thoughts? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-06 8:54 ` Baolin Wang @ 2026-11-06 9:15 ` Lance Yang 2026-11-06 9:29 ` Lance Yang 2026-11-06 9:56 ` Barry Song 1 sibling, 1 reply; 20+ messages in thread From: Lance Yang @ 2026-11-06 9:15 UTC (permalink / raw) To: baolin.wang Cc: baohua, lance.yang, akpm, david, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On Fri, Nov 06, 2026 at 04:54:44PM +0800, Baolin Wang wrote: > > >On 11/6/26 2:24 PM, Barry Song wrote: >> On Thu, Nov 5, 2026 at 5:00 PM Baolin Wang >> <baolin.wang@linux.alibaba.com> 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@vger.kernel.org >>>>>> Suggested-by: David Hildenbrand (Arm) <david@kernel.org> >>>>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >>>>>> --- >>>>>> 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. > >Thanks for the information. > >> 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. > >Agree. > >> If the concern is reclaim failure, I agree that there is no >> fundamental difference between large folios and small folios. > >OK. And I did a quick measurement to reclaim anon and file folios via >the memory.reclaim interface. > > From test results, reclaiming 1G small anonymous folios (using zram as >swap) shows no significant performance change. However, reclaiming 1G >mapped clean small file folios does show an observable regression if the >TTU_SYNC flag is set. > >Without TTU_SYNC for small folios: >time echo 1G > /sys/fs/cgroup/test/memory.reclaim >real 0m0.178s >user 0m0.000s >sys 0m0.178s > >With TTU_SYNC for small folios: >real 0m0.315s >user 0m0.000s >sys 0m0.314s Nice numbers, cool! >So for the current patch, I'd prefer to keep things as is: only sync >when TTU_SYNC is set. Without setting the TTU_SYNC flag, keep the >original logic, meaning the caller can tolerate some racy checks. > >Any other thoughts? One thing I want to pin down first, for the order-0 folios, was that run with TTU_SYNC going through the pmd lock, or did it already go the smp_rmb() way? My guess is the first one, right? If so, please keep the fix as is and only sync when the caller asks for it (at least, order-0 folios should not go the pmd lock way) :D Cheers, Lance ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-06 9:15 ` Lance Yang @ 2026-11-06 9:29 ` Lance Yang 2026-11-06 9:50 ` Baolin Wang 0 siblings, 1 reply; 20+ messages in thread From: Lance Yang @ 2026-11-06 9:29 UTC (permalink / raw) To: baolin.wang Cc: baohua, akpm, david, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On 2026/11/6 17:15, Lance Yang wrote: > > On Fri, Nov 06, 2026 at 04:54:44PM +0800, Baolin Wang wrote: >> >> >> On 11/6/26 2:24 PM, Barry Song wrote: >>> On Thu, Nov 5, 2026 at 5:00 PM Baolin Wang >>> <baolin.wang@linux.alibaba.com> 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@vger.kernel.org >>>>>>> Suggested-by: David Hildenbrand (Arm) <david@kernel.org> >>>>>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >>>>>>> --- >>>>>>> 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. >> >> Thanks for the information. >> >>> 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. >> >> Agree. >> >>> If the concern is reclaim failure, I agree that there is no >>> fundamental difference between large folios and small folios. >> >> OK. And I did a quick measurement to reclaim anon and file folios via >> the memory.reclaim interface. >> >> From test results, reclaiming 1G small anonymous folios (using zram as >> swap) shows no significant performance change. However, reclaiming 1G >> mapped clean small file folios does show an observable regression if the >> TTU_SYNC flag is set. >> >> Without TTU_SYNC for small folios: >> time echo 1G > /sys/fs/cgroup/test/memory.reclaim >> real 0m0.178s >> user 0m0.000s >> sys 0m0.178s >> >> With TTU_SYNC for small folios: >> real 0m0.315s >> user 0m0.000s >> sys 0m0.314s > > Nice numbers, cool! > >> So for the current patch, I'd prefer to keep things as is: only sync >> when TTU_SYNC is set. Without setting the TTU_SYNC flag, keep the >> original logic, meaning the caller can tolerate some racy checks. >> >> Any other thoughts? > > One thing I want to pin down first, for the order-0 folios, was that run > with TTU_SYNC going through the pmd lock, or did it already go the > smp_rmb() way? > > My guess is the first one, right? > > If so, please keep the fix as is and only sync when the caller asks for > it (at least, order-0 folios should not go the pmd lock way) :D In addition: For order-0 folios, we'd miss reclaim and reactivate a cold folio. That doesn't sound too bad, especially for such a rare race. so keeping the current behavior seems reasonable given the sync overhead ;) > Cheers, Lance ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-06 9:29 ` Lance Yang @ 2026-11-06 9:50 ` Baolin Wang 0 siblings, 0 replies; 20+ messages in thread From: Baolin Wang @ 2026-11-06 9:50 UTC (permalink / raw) To: Lance Yang Cc: baohua, akpm, david, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On 11/6/26 5:29 PM, Lance Yang wrote: > > > On 2026/11/6 17:15, Lance Yang wrote: >> >> On Fri, Nov 06, 2026 at 04:54:44PM +0800, Baolin Wang wrote: >>> >>> >>> On 11/6/26 2:24 PM, Barry Song wrote: >>>> On Thu, Nov 5, 2026 at 5:00 PM Baolin Wang >>>> <baolin.wang@linux.alibaba.com> 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@vger.kernel.org >>>>>>>> Suggested-by: David Hildenbrand (Arm) <david@kernel.org> >>>>>>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >>>>>>>> --- >>>>>>>> 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. >>> >>> Thanks for the information. >>> >>>> 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. >>> >>> Agree. >>> >>>> If the concern is reclaim failure, I agree that there is no >>>> fundamental difference between large folios and small folios. >>> >>> OK. And I did a quick measurement to reclaim anon and file folios via >>> the memory.reclaim interface. >>> >>> From test results, reclaiming 1G small anonymous folios (using zram as >>> swap) shows no significant performance change. However, reclaiming 1G >>> mapped clean small file folios does show an observable regression if the >>> TTU_SYNC flag is set. >>> >>> Without TTU_SYNC for small folios: >>> time echo 1G > /sys/fs/cgroup/test/memory.reclaim >>> real 0m0.178s >>> user 0m0.000s >>> sys 0m0.178s >>> >>> With TTU_SYNC for small folios: >>> real 0m0.315s >>> user 0m0.000s >>> sys 0m0.314s >> >> Nice numbers, cool! >> >>> So for the current patch, I'd prefer to keep things as is: only sync >>> when TTU_SYNC is set. Without setting the TTU_SYNC flag, keep the >>> original logic, meaning the caller can tolerate some racy checks. >>> >>> Any other thoughts? >> >> One thing I want to pin down first, for the order-0 folios, was that run >> with TTU_SYNC going through the pmd lock, or did it already go the >> smp_rmb() way? >> >> My guess is the first one, right? Right. I didn't add the memory barriers optimization (will do in a follow-up). The test changes are as follows: diff --git a/mm/vmscan.c b/mm/vmscan.c index daa7e37c2236..6a95a4fdc64c 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1434,11 +1434,6 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, * processes. Try to unmap it here. */ if (folio_mapped(folio)) { - enum ttu_flags flags = TTU_BATCH_FLUSH; - bool was_swapbacked = folio_test_swapbacked(folio); - - if (folio_test_pmd_mappable(folio)) - flags |= TTU_SPLIT_HUGE_PMD; /* * Without TTU_SYNC, try_to_unmap will only begin to * hold PTL from the first present PTE within a large @@ -1451,8 +1446,11 @@ static unsigned int shrink_folio_list(struct list_head *folio_list, * try_to_unmap acquire PTL from the first PTE, * eliminating the influence of temporary PTE values. */ - if (folio_test_large(folio)) - flags |= TTU_SYNC; + enum ttu_flags flags = TTU_BATCH_FLUSH | TTU_SYNC; + bool was_swapbacked = folio_test_swapbacked(folio); + + if (folio_test_pmd_mappable(folio)) + flags |= TTU_SPLIT_HUGE_PMD; try_to_unmap(folio, flags); if (folio_mapped(folio)) { >> If so, please keep the fix as is and only sync when the caller asks for >> it (at least, order-0 folios should not go the pmd lock way) :D > > In addition: > > For order-0 folios, we'd miss reclaim and reactivate a cold folio. > > That doesn't sound too bad, especially for such a rare race. > > so keeping the current behavior seems reasonable given the sync > overhead ;) That's also my thought:) ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-06 8:54 ` Baolin Wang 2026-11-06 9:15 ` Lance Yang @ 2026-11-06 9:56 ` Barry Song 1 sibling, 0 replies; 20+ messages in thread From: Barry Song @ 2026-11-06 9:56 UTC (permalink / raw) To: Baolin Wang Cc: Lance Yang, akpm, david, ljs, riel, liam, vbabka, harry, jannh, qi.zheng, pfalcato, ayushr, linux-mm, linux-kernel On Fri, Nov 6, 2026 at 4:55 PM Baolin Wang <baolin.wang@linux.alibaba.com> wrote: [...] > >>> 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. > > Thanks for the information. > > > 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. > > Agree. > > > If the concern is reclaim failure, I agree that there is no > > fundamental difference between large folios and small folios. > > OK. And I did a quick measurement to reclaim anon and file folios via > the memory.reclaim interface. > > From test results, reclaiming 1G small anonymous folios (using zram as > swap) shows no significant performance change. However, reclaiming 1G > mapped clean small file folios does show an observable regression if the > TTU_SYNC flag is set. > > Without TTU_SYNC for small folios: > time echo 1G > /sys/fs/cgroup/test/memory.reclaim > real 0m0.178s > user 0m0.000s > sys 0m0.178s > > With TTU_SYNC for small folios: > real 0m0.315s > user 0m0.000s > sys 0m0.314s > > So for the current patch, I'd prefer to keep things as is: only sync > when TTU_SYNC is set. Without setting the TTU_SYNC flag, keep the > original logic, meaning the caller can tolerate some racy checks. > > Any other thoughts? I'm still struggling to understand where the regression comes from. You're doing echo 1G > /sys/fs/cgroup/test/memory.reclaim without the concurrent CPU 1 (MADV_DONTNEED) operation that causes the race. so the regression actually comes from other places, maybe here? static bool map_pte(struct page_vma_mapped_walk *pvmw, pmd_t *pmdvalp, spinlock_t **ptlp) { bool is_migration; pte_t ptent; if (pvmw->flags & PVMW_SYNC) { /* Use the stricter lookup */ pvmw->pte = pte_offset_map_lock(pvmw->vma->vm_mm, pvmw->pmd, pvmw->address, &pvmw->ptl); *ptlp = pvmw->ptl; return !!pvmw->pte; } ... } Am I missing something? Best, Barry ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-05 2:56 [PATCH] mm: synchronize with PTE table reclaim for all folio sizes Baolin Wang 2026-11-05 3:22 ` Qi Zheng 2026-11-05 4:54 ` Lance Yang @ 2026-11-05 11:37 ` Pedro Falcato 2026-11-06 2:12 ` Baolin Wang 2 siblings, 1 reply; 20+ messages in thread From: Pedro Falcato @ 2026-11-05 11:37 UTC (permalink / raw) To: Baolin Wang Cc: akpm, david, ljs, riel, liam, vbabka, harry, jannh, lance.yang, qi.zheng, ayushr, linux-mm, linux-kernel 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(). > > 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. This is very handwavy. Do you have numbers? Does this actually make sense? -- Pedro ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] mm: synchronize with PTE table reclaim for all folio sizes 2026-11-05 11:37 ` Pedro Falcato @ 2026-11-06 2:12 ` Baolin Wang 0 siblings, 0 replies; 20+ messages in thread From: Baolin Wang @ 2026-11-06 2:12 UTC (permalink / raw) To: Pedro Falcato Cc: akpm, david, ljs, riel, liam, vbabka, harry, jannh, lance.yang, qi.zheng, ayushr, linux-mm, linux-kernel On 11/5/26 7:37 PM, Pedro Falcato 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(). >> >> 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. > > This is very handwavy. Do you have numbers? Does this actually make sense? This is hard to measure in practice. But in theory, if we can't reclaim cold large folios that are actually unmapped due to the racy checks, it could cause cold/hot inversion (e.g., in MGLRU) and increase refaults. In extreme cases, failing to reclaim unmapped large folios could even cause OOM. ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-11-06 9:56 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-11-05 2:56 [PATCH] mm: synchronize with PTE table reclaim for all folio sizes Baolin Wang 2026-11-05 3:22 ` Qi Zheng 2026-11-05 4:54 ` Lance Yang 2026-11-05 5:48 ` Lance Yang 2026-11-05 9:00 ` Baolin Wang 2026-11-05 10:06 ` David Hildenbrand (Arm) 2026-11-05 11:18 ` Baolin Wang 2026-11-05 13:22 ` Lance Yang 2026-11-05 19:35 ` David Hildenbrand (Arm) 2026-11-06 1:57 ` Baolin Wang 2026-11-06 2:33 ` Lance Yang 2026-11-06 2:54 ` Baolin Wang 2026-11-06 6:24 ` Barry Song 2026-11-06 8:54 ` Baolin Wang 2026-11-06 9:15 ` Lance Yang 2026-11-06 9:29 ` Lance Yang 2026-11-06 9:50 ` Baolin Wang 2026-11-06 9:56 ` Barry Song 2026-11-05 11:37 ` Pedro Falcato 2026-11-06 2:12 ` Baolin Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®