Re: [PATCH v4 2/2] mm: Optimize mremap() by PTE batching

[David Hildenbrand <david@redhat.com>]

>> -               pte = get_and_clear_full_ptes(mm, old_addr, old_ptep,
>> nr_ptes, 0);
>> -               pte = move_pte(pte, old_addr, new_addr);
>> -               pte = move_soft_dirty_pte(pte);
>> -
>> -               if (need_clear_uffd_wp && pte_marker_uffd_wp(pte))
>> -                       pte_clear(mm, new_addr, new_ptep);
>> -               else {
>> -                       if (need_clear_uffd_wp) {
>> -                               if (pte_present(pte))
>> -                                       pte = pte_clear_uffd_wp(pte);
>> -                               else if (is_swap_pte(pte))
>> +
>> +                       pte = get_and_clear_full_ptes(mm, old_addr,
>> old_ptep,
>> +                                                     nr_ptes, 0);
>> +                       /*
>> +                        * Moving present PTEs requires special care
>> on some
>> +                        * archs.
>> +                        */
>> +                       pte = move_pte(pte, old_addr, new_addr);
>> +                       /* make userspace aware that this pte moved. */
>> +                       pte = pte_mksoft_dirty(pte);
>> +                       if (need_clear_uffd_wp)
>> +                               pte = pte_clear_uffd_wp(pte);
>> +                       set_ptes(mm, new_addr, new_ptep, pte, nr_ptes);
>> +               } else if (need_clear_uffd_wp &&
>> pte_marker_uffd_wp(pte)) {
>> +                       pte_clear(mm, old_addr, old_ptep);
>> +               } else {
>> +                       pte_clear(mm, old_addr, old_ptep);
> 
> Should pte_clear be included here? It is currently being done only for
> the case
> 
> need_clear_uffd_wp && pte_marker_uffd_wp().

We always cleared the old pte (using get_and_clear_*, which is not 
required if we already know that it is !present).

The case you mean was what I describe here:

>>
>>
>>
>> Note that I don't know why we had the existing
>>
>> -               if (need_clear_uffd_wp && pte_marker_uffd_wp(pte))
>> -                       pte_clear(mm, new_addr, new_ptep);
>>
>>
>> I thought we would always expect that the destination pte is already
>> pte_none() ?

So clearing the destination PTE can be dropped if we didn't move in the 
first place.

-- 
Cheers,

David / dhildenb