On Mon, 2022-10-03 at 18:56 -0700, Nadav Amit wrote:
> Hopefully I will not waste your time again… If it has been discussed
> in the
> last 26 iterations, just tell me and ignore.
>
> On Sep 29, 2022, at 3:29 PM, Rick Edgecombe <
> rick.p.edgecombe@xxxxxxxxx> wrote:
>
> > --- a/mm/migrate_device.c
> > +++ b/mm/migrate_device.c
> > @@ -606,8 +606,7 @@ static void migrate_vma_insert_page(struct
> > migrate_vma *migrate,
> > goto abort;
> > }
> > entry = mk_pte(page, vma->vm_page_prot);
> > - if (vma->vm_flags & VM_WRITE)
> > - entry = pte_mkwrite(pte_mkdirty(entry));
> > + entry = maybe_mkwrite(pte_mkdirty(entry), vma);
> > }
>
> This is not exactly the same logic. You might dirty read-only pages
> since
> you call pte_mkdirty() unconditionally. It has been known not to be
> very
> robust (e.g., dirty-COW and friends). Perhaps it is not dangerous
> following
> some recent enhancements, but why do you want to take the risk?
Yea those changes let me drop a patch. But, it's a good point.
>
> Instead, although it might seem redundant, the compiler will
> hopefully would
> make it efficient:
>
> if (vma->vm_flags & VM_WRITE) {
> entry = pte_mkdirty(entry);
> entry = maybe_mkwrite(entry, vma);
> }
>
Thanks Nadav. I think you're right, it should have the open coded logic
here and in the do_anonymous_page() chunk that got moved to the
previous patch on accident.
