[RFC PATCH RESEND 19/28] mm: disallow do_swap_page to handle page faults under VMA lock

Suren Baghdasaryan posted 28 patches 3 years, 7 months ago
[RFC PATCH RESEND 19/28] mm: disallow do_swap_page to handle page faults under VMA lock
Posted by Suren Baghdasaryan 3 years, 7 months ago
Due to the possibility of do_swap_page dropping mmap_lock, abort fault
handling under VMA lock and retry holding mmap_lock. This can be handled
more gracefully in the future.

Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
 mm/memory.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index 9ac9944e8c62..29d2f49f922a 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3738,6 +3738,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 	vm_fault_t ret = 0;
 	void *shadow = NULL;
 
+	if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
+		ret = VM_FAULT_RETRY;
+		goto out;
+	}
+
 	if (!pte_unmap_same(vmf))
 		goto out;
 
-- 
2.37.2.789.g6183377224-goog
Re: [RFC PATCH RESEND 19/28] mm: disallow do_swap_page to handle page faults under VMA lock
Posted by Laurent Dufour 3 years, 6 months ago
Le 01/09/2022 à 19:35, Suren Baghdasaryan a écrit :
> Due to the possibility of do_swap_page dropping mmap_lock, abort fault
> handling under VMA lock and retry holding mmap_lock. This can be handled
> more gracefully in the future.
> 
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>

Reviewed-by: Laurent Dufour <laurent.dufour@fr.ibm.com>

> ---
>  mm/memory.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 9ac9944e8c62..29d2f49f922a 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3738,6 +3738,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>  	vm_fault_t ret = 0;
>  	void *shadow = NULL;
>  
> +	if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
> +		ret = VM_FAULT_RETRY;
> +		goto out;
> +	}
> +
>  	if (!pte_unmap_same(vmf))
>  		goto out;
>  

Re: [RFC PATCH RESEND 19/28] mm: disallow do_swap_page to handle page faults under VMA lock
Posted by Peter Xu 3 years, 7 months ago
On Thu, Sep 01, 2022 at 10:35:07AM -0700, Suren Baghdasaryan wrote:
> Due to the possibility of do_swap_page dropping mmap_lock, abort fault
> handling under VMA lock and retry holding mmap_lock. This can be handled
> more gracefully in the future.
> 
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> ---
>  mm/memory.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 9ac9944e8c62..29d2f49f922a 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3738,6 +3738,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>  	vm_fault_t ret = 0;
>  	void *shadow = NULL;
>  
> +	if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
> +		ret = VM_FAULT_RETRY;
> +		goto out;
> +	}
> +

May want to fail early similarly for handle_userfault() too for similar
reason.  Thanks,

-- 
Peter Xu
Re: [RFC PATCH RESEND 19/28] mm: disallow do_swap_page to handle page faults under VMA lock
Posted by Suren Baghdasaryan 3 years, 7 months ago
On Tue, Sep 6, 2022 at 12:39 PM Peter Xu <peterx@redhat.com> wrote:
>
> On Thu, Sep 01, 2022 at 10:35:07AM -0700, Suren Baghdasaryan wrote:
> > Due to the possibility of do_swap_page dropping mmap_lock, abort fault
> > handling under VMA lock and retry holding mmap_lock. This can be handled
> > more gracefully in the future.
> >
> > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > ---
> >  mm/memory.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 9ac9944e8c62..29d2f49f922a 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -3738,6 +3738,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> >       vm_fault_t ret = 0;
> >       void *shadow = NULL;
> >
> > +     if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
> > +             ret = VM_FAULT_RETRY;
> > +             goto out;
> > +     }
> > +
>
> May want to fail early similarly for handle_userfault() too for similar
> reason.  Thanks,

I wasn't aware of a similar issue there. Will have a closer look. Thanks!

>
> --
> Peter Xu
>
Re: [RFC PATCH RESEND 19/28] mm: disallow do_swap_page to handle page faults under VMA lock
Posted by Peter Xu 3 years, 7 months ago
On Tue, Sep 06, 2022 at 01:08:10PM -0700, Suren Baghdasaryan wrote:
> On Tue, Sep 6, 2022 at 12:39 PM Peter Xu <peterx@redhat.com> wrote:
> >
> > On Thu, Sep 01, 2022 at 10:35:07AM -0700, Suren Baghdasaryan wrote:
> > > Due to the possibility of do_swap_page dropping mmap_lock, abort fault
> > > handling under VMA lock and retry holding mmap_lock. This can be handled
> > > more gracefully in the future.
> > >
> > > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > > ---
> > >  mm/memory.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index 9ac9944e8c62..29d2f49f922a 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -3738,6 +3738,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> > >       vm_fault_t ret = 0;
> > >       void *shadow = NULL;
> > >
> > > +     if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
> > > +             ret = VM_FAULT_RETRY;
> > > +             goto out;
> > > +     }
> > > +
> >
> > May want to fail early similarly for handle_userfault() too for similar
> > reason.  Thanks,
> 
> I wasn't aware of a similar issue there. Will have a closer look. Thanks!

Sure.

Just in case this would be anything helpful - handle_userfault() will both
assert at the entry (mmap_assert_locked) and will in most cases release
read lock along the way when waiting for page fault resolutions.

And userfaultfd should work on anonymous memory for either missing mode or
write protect mode.

Thanks,

-- 
Peter Xu
Re: [RFC PATCH RESEND 19/28] mm: disallow do_swap_page to handle page faults under VMA lock
Posted by Suren Baghdasaryan 3 years, 7 months ago
On Tue, Sep 6, 2022 at 1:22 PM Peter Xu <peterx@redhat.com> wrote:
>
> On Tue, Sep 06, 2022 at 01:08:10PM -0700, Suren Baghdasaryan wrote:
> > On Tue, Sep 6, 2022 at 12:39 PM Peter Xu <peterx@redhat.com> wrote:
> > >
> > > On Thu, Sep 01, 2022 at 10:35:07AM -0700, Suren Baghdasaryan wrote:
> > > > Due to the possibility of do_swap_page dropping mmap_lock, abort fault
> > > > handling under VMA lock and retry holding mmap_lock. This can be handled
> > > > more gracefully in the future.
> > > >
> > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > > > ---
> > > >  mm/memory.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/mm/memory.c b/mm/memory.c
> > > > index 9ac9944e8c62..29d2f49f922a 100644
> > > > --- a/mm/memory.c
> > > > +++ b/mm/memory.c
> > > > @@ -3738,6 +3738,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> > > >       vm_fault_t ret = 0;
> > > >       void *shadow = NULL;
> > > >
> > > > +     if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
> > > > +             ret = VM_FAULT_RETRY;
> > > > +             goto out;
> > > > +     }
> > > > +
> > >
> > > May want to fail early similarly for handle_userfault() too for similar
> > > reason.  Thanks,
> >
> > I wasn't aware of a similar issue there. Will have a closer look. Thanks!
>
> Sure.
>
> Just in case this would be anything helpful - handle_userfault() will both
> assert at the entry (mmap_assert_locked) and will in most cases release
> read lock along the way when waiting for page fault resolutions.
>
> And userfaultfd should work on anonymous memory for either missing mode or
> write protect mode.

Got it. Thanks for the explanation. It definitely helps!

>
> Thanks,
>
> --
> Peter Xu
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>