Re: [PATCH 1/1] mm/khugepaged: map anonymous pte-mapped THPs by pmds

[David Hildenbrand <david@redhat.com>]

On 13.11.23 10:33, Xu Yu wrote:
> On 11/13/23 5:26 PM, David Hildenbrand wrote:
>> On 13.11.23 10:05, Xu Yu wrote:
>>> In the anonymous collapse path, khugepaged collapses pte-mapped
>>> hugepages by allocating and copying to a new hugepage, which is
>>> suboptimally.
>>>
>>> In fact, we only need to update the mapping page tables for anonymous
>>> pte-mapped THPs, in the same way as file/shmem-backed pte-mapped THPs,
>>> as shown in commit 58ac9a8993a1 ("mm/khugepaged: attempt to map
>>> file/shmem-backed pte-mapped THPs by pmds").
>>
>> Somewhere along that patch set discussion, there was a discussion about
>> how to deal with PageAnonExclusive properly.
>>
>> Quite a lot of information is documented in:
>>
>> commit dee2ad120571f38433211098cd6b95a59bdfc8c7
>> Author: David Hildenbrand <david@redhat.com>
>> Date:   Wed Jan 4 15:49:05 2023 +0100
>>
>>       selftests/vm: cow: add COW tests for collapsing of PTE-mapped anon THP
>>       Currently, anonymous PTE-mapped THPs cannot be collapsed in-place:
>>       collapsing (e.g., via MADV_COLLAPSE) implies allocating a fresh THP and
>>       mapping that new THP via a PMD: as it's a fresh anon THP, it will get the
>>       exclusive flag set on the head page and everybody is happy.
>> ...
>>
>>
>>
>> I purely speculate that you didn't consider it and got it wrong, sorry :P
> 
> As you said, I did not consider it. Actually, I didn't even know that. :(

I think the simplest two cases to handle that we should implement:

no subpages are PageAnonExclusive (PTEs must be R/O) -> we can collapse 
into a R/O PMD without further action.

all subpages are PageAnonExclusive (PTEs may be either R/O or R/W) -> 
clear PageAnonExclusive on all tail pages but the first (head) page and 
collapse to a R/W PMD with VM_WRITE or a R/O PMD without VM_WRITE.

We must check PageAnonExclusive with the page table lock held, and 
recheck in case we dropped it (IOW, what existing collapse code already 
does).

softdirty tracking and uffd-wp checks should likely be the same as for 
existing collapse handling.

-- 
Cheers,

David / dhildenb