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

[Baolin Wang <baolin.wang@linux.alibaba.com>]

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?