Re: [RFC] Trylock the mmap_lock in vmf_anon_prepare()

[Suren Baghdasaryan <surenb@google.com>]

On Mon, Apr 15, 2024 at 11:18 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Apr 15, 2024 at 09:14:10AM -0700, Suren Baghdasaryan wrote:
> > >         if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
> > > -               vma_end_read(vma);
> > > -               return VM_FAULT_RETRY;
> > > +               if (!mmap_read_trylock(vma->vm_mm)) {
> > > +                       vma_end_read(vma);
> > > +                       return VM_FAULT_RETRY;
> > > +               }
> > >         }
> > >         if (__anon_vma_prepare(vma))
> > >                 return VM_FAULT_OOM;
> >
> > You should drop mmap_lock when returning VM_FAULT_OOM as well.
> >
> > > +       if (vmf->flags & FAULT_FLAG_VMA_LOCK)
> > > +               mmap_read_unlock(vma->vm_mm);
> > >         return 0;
> > >  }
>
> Thanks.  Fixed and pushed to
> git://git.infradead.org/users/willy/pagecache.git vma-lock

That looks correct now.
Reviewed-by: Suren Baghdasaryan <surenb@google.com>

>
> +++ b/mm/memory.c
> @@ -3224,16 +3224,21 @@ static inline vm_fault_t vmf_can_call_fault(const struct vm_fault *vmf)
>  vm_fault_t vmf_anon_prepare(struct vm_fault *vmf)
>  {
>         struct vm_area_struct *vma = vmf->vma;
> +       vm_fault_t ret = 0;
>
>         if (likely(vma->anon_vma))
>                 return 0;
>         if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
> -               vma_end_read(vma);
> -               return VM_FAULT_RETRY;
> +               if (!mmap_read_trylock(vma->vm_mm)) {
> +                       vma_end_read(vma);
> +                       return VM_FAULT_RETRY;
> +               }
>         }
>         if (__anon_vma_prepare(vma))
> -               return VM_FAULT_OOM;
> -       return 0;
> +               ret = VM_FAULT_OOM;
> +       if (vmf->flags & FAULT_FLAG_VMA_LOCK)
> +               mmap_read_unlock(vma->vm_mm);
> +       return ret;
>  }
>
>  /*
>