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

[Lance Yang <lance.yang@linux.dev>]

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