linux-next: manual merge of the mm tree with Linus' tree

  Hi all,

Today's linux-next merge of the mm tree got a conflict in:

  mm/memory.c

between commit:

  657b5146955e ("mm: lock_vma_under_rcu() must check vma->anon_vma under vma lock")

from Linus' tree and commits:

  69f6bbd1317f ("mm: handle userfaults under VMA lock")
  a3bdf38e85aa ("mm: allow per-VMA locks on file-backed VMAs")

from the mm tree.

I fixed it up (I think, please check - see below) and can carry the fix
as necessary. This is now fixed as far as linux-next is concerned, but
any non trivial conflicts should be mentioned to your upstream
maintainer when your tree is submitted for merging.  You may also want
to consider cooperating with the maintainer of the conflicting tree to
minimise any particularly complex conflicts.
  

Hi Matthew,

On Fri, 28 Jul 2023 00:49:50 +0100 Matthew Wilcox <willy@infradead.org> wrote:
>
> On Fri, Jul 28, 2023 at 09:18:49AM +1000, Stephen Rothwell wrote:
> > diff --cc mm/memory.c
> > index ca632b58f792,271982fab2b8..000000000000
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@@ -5392,32 -5597,18 +5597,21 @@@ retry
> >   	if (!vma)
> >   		goto inval;
> >   
> > - 	/* Only anonymous and tcp vmas are supported for now */
> > - 	if (!vma_is_anonymous(vma) && !vma_is_tcp(vma))
> >  -	/* find_mergeable_anon_vma uses adjacent vmas which are not locked */
> >  -	if (vma_is_anonymous(vma) && !vma->anon_vma)
> > --		goto inval;
> > --
> >   	if (!vma_start_read(vma))
> >   		goto inval;
> >   
> >  +	/*
> >  +	 * find_mergeable_anon_vma uses adjacent vmas which are not locked.
> >  +	 * This check must happen after vma_start_read(); otherwise, a
> >  +	 * concurrent mremap() with MREMAP_DONTUNMAP could dissociate the VMA
> >  +	 * from its anon_vma.
> >  +	 */
> > - 	if (unlikely(!vma->anon_vma && !vma_is_tcp(vma)))
> > - 		goto inval_end_read;
> > - 
> > - 	/*
> > - 	 * Due to the possibility of userfault handler dropping mmap_lock, avoid
> > - 	 * it for now and fall back to page fault handling under mmap_lock.
> > - 	 */
> > - 	if (userfaultfd_armed(vma))
> > ++	if (unlikely(vma_is_anonymous(vma) && !vma_is_tcp(vma)))
> >  +		goto inval_end_read;
> >  +  
> 
> No, this isn't right.  It should be:
> 
> 	if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma))
> 		goto inval_end_read;

Yeah, see my second attempt.

> I'm not sure about the userfaultfd_armed() clause.  My patch wasn't
> intended to affect that.

That was removed by commit

  69f6bbd1317f ("mm: handle userfaults under VMA lock")

in the mm branch.
  

On Thu, Jul 27, 2023 at 5:21 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Matthew,
>
> On Fri, 28 Jul 2023 00:49:50 +0100 Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Fri, Jul 28, 2023 at 09:18:49AM +1000, Stephen Rothwell wrote:
> > > diff --cc mm/memory.c
> > > index ca632b58f792,271982fab2b8..000000000000
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@@ -5392,32 -5597,18 +5597,21 @@@ retry
> > >     if (!vma)
> > >             goto inval;
> > >
> > > -   /* Only anonymous and tcp vmas are supported for now */
> > > -   if (!vma_is_anonymous(vma) && !vma_is_tcp(vma))
> > >  -  /* find_mergeable_anon_vma uses adjacent vmas which are not locked */
> > >  -  if (vma_is_anonymous(vma) && !vma->anon_vma)
> > > --          goto inval;
> > > --
> > >     if (!vma_start_read(vma))
> > >             goto inval;
> > >
> > >  +  /*
> > >  +   * find_mergeable_anon_vma uses adjacent vmas which are not locked.
> > >  +   * This check must happen after vma_start_read(); otherwise, a
> > >  +   * concurrent mremap() with MREMAP_DONTUNMAP could dissociate the VMA
> > >  +   * from its anon_vma.
> > >  +   */
> > > -   if (unlikely(!vma->anon_vma && !vma_is_tcp(vma)))
> > > -           goto inval_end_read;
> > > -
> > > -   /*
> > > -    * Due to the possibility of userfault handler dropping mmap_lock, avoid
> > > -    * it for now and fall back to page fault handling under mmap_lock.
> > > -    */
> > > -   if (userfaultfd_armed(vma))
> > > ++  if (unlikely(vma_is_anonymous(vma) && !vma_is_tcp(vma)))
> > >  +          goto inval_end_read;
> > >  +
> >
> > No, this isn't right.  It should be:
> >
> >       if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma))
> >               goto inval_end_read;
>
> Yeah, see my second attempt.
>
> > I'm not sure about the userfaultfd_armed() clause.  My patch wasn't
> > intended to affect that.
>
> That was removed by commit
>
>   69f6bbd1317f ("mm: handle userfaults under VMA lock")
>
> in the mm branch.


Hmm. 657b5146955e ("mm: lock_vma_under_rcu() must check vma->anon_vma
under vma lock") should be adding a "inval_end_read" label. At least I
see it in https://lore.kernel.org/all/20230726214103.3261108-3-jannh@google.com/
and will check Linus' tree in a min. I don't see that label in your
patch...
I also misspoke in my previous message. The patches in mm tree remove
some code from that function, so it's easier to apply them first and
then the one from Linus' tree.

>
> --
> Cheers,
> Stephen Rothwell
  

Hi Suren,

On Thu, 27 Jul 2023 17:23:45 -0700 Suren Baghdasaryan <surenb@google.com> wrote:
>
 Hmm. 657b5146955e ("mm: lock_vma_under_rcu() must check vma->anon_vma
> under vma lock") should be adding a "inval_end_read" label. At least I
> see it in https://lore.kernel.org/all/20230726214103.3261108-3-jannh@google.com/
> and will check Linus' tree in a min. I don't see that label in your
> patch...

It's there in the file, but did not conflict with anything.  What I
published is not a "patch" as such, but a diff showing the conflict
resolution.
  

On Thu, Jul 27, 2023 at 5:29 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Suren,
>
> On Thu, 27 Jul 2023 17:23:45 -0700 Suren Baghdasaryan <surenb@google.com> wrote:
> >
>  Hmm. 657b5146955e ("mm: lock_vma_under_rcu() must check vma->anon_vma
> > under vma lock") should be adding a "inval_end_read" label. At least I
> > see it in https://lore.kernel.org/all/20230726214103.3261108-3-jannh@google.com/
> > and will check Linus' tree in a min. I don't see that label in your
> > patch...
>
> It's there in the file, but did not conflict with anything.  What I
> published is not a "patch" as such, but a diff showing the conflict
> resolution.

Ah, sorry for the noise then.

>
> --
> Cheers,
> Stephen Rothwell
  

On Fri, Jul 28, 2023 at 10:00:47AM +1000, Stephen Rothwell wrote:
> I have gone with below.  Again, please check.

LGTM

> diff --cc mm/memory.c
> index ca632b58f792,271982fab2b8..000000000000
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@@ -5392,32 -5597,18 +5597,21 @@@ retry
>   	if (!vma)
>   		goto inval;
>   
> - 	/* Only anonymous and tcp vmas are supported for now */
> - 	if (!vma_is_anonymous(vma) && !vma_is_tcp(vma))
>  -	/* find_mergeable_anon_vma uses adjacent vmas which are not locked */
>  -	if (vma_is_anonymous(vma) && !vma->anon_vma)
> --		goto inval;
> --
>   	if (!vma_start_read(vma))
>   		goto inval;
>   
>  +	/*
>  +	 * find_mergeable_anon_vma uses adjacent vmas which are not locked.
>  +	 * This check must happen after vma_start_read(); otherwise, a
>  +	 * concurrent mremap() with MREMAP_DONTUNMAP could dissociate the VMA
>  +	 * from its anon_vma.
>  +	 */
> - 	if (unlikely(!vma->anon_vma && !vma_is_tcp(vma)))
> - 		goto inval_end_read;
> - 
> - 	/*
> - 	 * Due to the possibility of userfault handler dropping mmap_lock, avoid
> - 	 * it for now and fall back to page fault handling under mmap_lock.
> - 	 */
> - 	if (userfaultfd_armed(vma))
> ++	if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma))
>  +		goto inval_end_read;
>  +
>   	/* Check since vm_start/vm_end might change before we lock the VMA */
>  -	if (unlikely(address < vma->vm_start || address >= vma->vm_end)) {
>  -		vma_end_read(vma);
>  -		goto inval;
>  -	}
>  +	if (unlikely(address < vma->vm_start || address >= vma->vm_end))
>  +		goto inval_end_read;
>   
>   	/* Check if the VMA got isolated after we found it */
>   	if (vma->detached) {

If Andrew wants to rebase on Linus' tree, I'll be happy to respin
on top of that.