On Sun, Apr 12, 2020 at 08:54:08PM +0800, Hillf Danton wrote:
>
> On Fri, 10 Apr 2020 11:32:34 -0400 Peter Xu wrote:
> >
> > I'm not sure this is correct. As I mentioned, the commit wanted to
> > apply the uffd-wp bit even for the swap entries so that even the swap
> > entries got swapped in, the page will still be write protected. So
> > IIUC think we can't remove that.
>
> Yes you are right.
>
> Now both CONFIG_MIGRATION and swap entry are restored after making uffd_wq
> survive migrate the same way as soft_dirty.
>
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -236,6 +236,8 @@ static bool remove_migration_pte(struct
> pte = pte_mkold(mk_pte(new, READ_ONCE(vma->vm_page_prot)));
> if (pte_swp_soft_dirty(*pvmw.pte))
> pte = pte_mksoft_dirty(pte);
> + if (pte_swp_uffd_wp(*pvmw.pte))
> + pte = pte_mkuffd_wp(pte);
>
> /*
> * Recheck VMA as permissions can change since migration started
> @@ -243,15 +245,11 @@ static bool remove_migration_pte(struct
> entry = pte_to_swp_entry(*pvmw.pte);
> if (is_write_migration_entry(entry))
> pte = maybe_mkwrite(pte, vma);
> - else if (pte_swp_uffd_wp(*pvmw.pte))
> - pte = pte_mkuffd_wp(pte);
>
> if (unlikely(is_zone_device_page(new))) {
> if (is_device_private_page(new)) {
> entry = make_device_private_entry(new, pte_write(pte));
> pte = swp_entry_to_pte(entry);
> - if (pte_swp_uffd_wp(*pvmw.pte))
> - pte = pte_mkuffd_wp(pte);
> }
> }
>
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -139,11 +139,13 @@ static unsigned long change_pte_range(st
> }
> ptep_modify_prot_commit(vma, addr, pte, oldpte, ptent);
> pages++;
> - } else if (is_swap_pte(oldpte)) {
> + } else if (IS_ENABLED(CONFIG_MIGRATION)) {
> swp_entry_t entry = pte_to_swp_entry(oldpte);
> pte_t newpte;
>
> - if (is_write_migration_entry(entry)) {
> + if (!non_swap_entry(entry)) {
> + newpte = oldpte;
> + } else if (is_write_migration_entry(entry)) {
> /*
> * A protection check is difficult so
> * just be safe and disable write
> @@ -164,7 +166,7 @@ static unsigned long change_pte_range(st
> if (pte_swp_uffd_wp(oldpte))
> newpte = pte_swp_mkuffd_wp(newpte);
> } else {
> - newpte = oldpte;
> + continue;
> }
>
> if (uffd_wp)
>
>
Hi, Hillf,
Feel free to have a look at another report, which I think is the same
issue of this:
https://lore.kernel.org/lkml/CA+G9fYsRGvkqtpdGv_aVr+Hn17KgYq04Q=EE=pB774qVxRqOeg@xxxxxxxxxxxxxx/
IMHO this bisected commit is correct itself, it's just that we
shouldn't enable uffd-wp on 32bit system.
Thanks,
--
Peter Xu
