On Wed, 28 Mar 2018, Laurent Dufour wrote:
> >> @@ -326,7 +336,10 @@ static unsigned long move_vma(struct vm_area_struct
> >> *vma,
> >> mremap_userfaultfd_prep(new_vma, uf);
> >> arch_remap(mm, old_addr, old_addr + old_len,
> >> new_addr, new_addr + new_len);
> >> + if (vma != new_vma)
> >> + vm_raw_write_end(vma);
> >> }
> >> + vm_raw_write_end(new_vma);
> >
> > Just do
> >
> > vm_raw_write_end(vma);
> > vm_raw_write_end(new_vma);
> >
> > here.
>
> Are you sure ? we can have vma = new_vma done if (unlikely(err))
>
Sorry, what I meant was do
if (vma != new_vma)
vm_raw_write_end(vma);
vm_raw_write_end(new_vma);
after the conditional. Having the locking unnecessarily embedded in the
conditional has been an issue in the past with other areas of core code,
unless you have a strong reason for it.
