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

[David Hildenbrand]

> > -               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