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.
