Re: [PATCH 7.2 v2 02/12] mm/khugepaged: add folio dirty check after try_to_unmap_flush()

[David Hildenbrand (Arm)]

On 4/17/26 04:09, Zi Yan wrote:
> On 14 Apr 2026, at 11:55, Zi Yan wrote:
> 
>> On 14 Apr 2026, at 6:38, David Hildenbrand (Arm) wrote:
>>
>>>
>>> maybe simplify to "folios, since that would require taking the folio lock first."
>>
>> Sure.
>>
>>>
>>>
>>>
>>> IIRC, after successful try_to_unmap() the PTE dirty bit would be synced to the
>>> folio. That's what you care about, not about any stale TLB entries.
>>
>> Right. I missed the PTE dirty bit to folio dirtiness part.
>>
>>>
>>> The important part is that the
>>>
>>> So can't we simply test for dirty folios after the refcount check (where
>>> we made sure the folio is no longer mapped)?
>>>
>>
>> I think so.
> 
> After more thoughts, this still works but the reasoning is more complicated.
> 
> After try_to_unmap(), PTE dirty bit is transferred to folio dirty flag if exists.
> The code bails out if the folio is dirty. A clean folio means the removed
> PTEs were clean, thus TLB entries caching these PTEs have dirty bit not set.

Right.

> Any write to the folio after try_to_unmap() causes a page table walk and
> a page fault due to invalid PTEs, and they cannot proceed since collapse_file()
> is holding the folio lock. 

Right.

> As a result, we can skip marking folio dirty
> after the collapse.

Exactly.

> 
> If we allow a dirty folio after try_to_unmap(), we need to mark the folio dirty
> after the collapse like shmem. And a potential optimization is that we only
> mark the after-collapse folio dirty if any old folio is dirty after try_to_unmap().

That was my thinking as well, but it would be something we'd try to
handle separately.

Folios that are under writeback cannot be collapsed IIUC.

-- 
Cheers,

David