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

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

On 11/9/26 12:27 PM, Barry Song wrote:
> On Mon, Nov 9, 2026 at 11:42 AM Baolin Wang
> <baolin.wang@linux.alibaba.com> wrote:
>>
>>
>>
>> On 11/8/26 3:58 AM, David Hildenbrand (Arm) wrote:
>>> On 11/6/26 03:12, Baolin Wang wrote:
>>>>
>>>>
>>>> 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.
>>>
>>> Yeah, but the question is if it really matters in practice.
>>>
>>> I agree that page_mapped_in_vma() should sync properly, but the current impact
>>> might not be as severe.
>>
>> Yes, what I meant is the theoretical impact. In practice, this race is
>> not easy to trigger with no real reports so far, or the impact is tolerable.
>>
>> But I still think the fix is needed, as it matches the purpose of the
>> TTU_SYNC flag.
> 
> I assume the impact comes from LRU over-protecting folios
> that remain mapped after try_to_unmap().
>                          try_to_unmap(folio, flags);
>                          if (folio_mapped(folio)) {
>                                  stat->nr_unmap_fail += nr_pages;
>                                  if (!was_swapbacked &&
>                                      folio_test_swapbacked(folio))
>                                          stat->nr_lazyfree_fail += nr_pages;
>                                  goto activate_locked;
>                          }
> 
> folios are activated -  over protected in LRU.
> 
> The racy window should be quite small in theory, so the
> impact might not be that severe. There is no evidence
> that it could lead to OOM.
> 
> For large folios, the impact could be greater because
> we are over-protecting more memory, potentially
> increasing with the folio order. For small folios, we
> are only over-protecting a single 4KB page.

Agree.

> So perhaps the current TTU_SYNC flag for large folios
> is reasonable. Baolin's patch to respect the flag,
> which currently applies to large folios, also seems
> reasonable, as long as we don't extend it to small
> folios and introduce regressions, possibly in
> map_pte()?
> 
> I'm also wondering whether we should downgrade
> ACTIVATE to KEEP, which would allow racy folios to be
> reclaimed sooner.
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 071a6eaef1a1..726266e49533 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1353,7 +1353,7 @@ static unsigned int shrink_folio_list(struct
> list_head *folio_list,
>                                  if (!was_swapbacked &&
>                                      folio_test_swapbacked(folio))
>                                          stat->nr_lazyfree_fail += nr_pages;
> -                               goto activate_locked;
> +                               goto keep_locked;
>                          }
>                  }

Sounds reasonable to me. It's also not easy to measure the impact. As 
mentioned earlier, this kind of racy check scenario probably doesn't 
have much impact anyway.