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.
--
Cheers,
Stephen Rothwell
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;
+
/* 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) {
Hi all,
On Fri, 28 Jul 2023 09:18:49 +1000 Stephen Rothwell <[email protected]> wrote:
>
> 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.
>
> --
> Cheers,
> Stephen Rothwell
>
> 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;
> +
> /* 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) {
Sorry, doesn't even build ... let me try that again.
--
Cheers,
Stephen Rothwell
On Thu, Jul 27, 2023 at 04:40:20PM -0700, Suren Baghdasaryan wrote:
> On Thu, Jul 27, 2023 at 4:29 PM Stephen Rothwell <[email protected]> wrote:
> >
> > Hi all,
> >
> > On Fri, 28 Jul 2023 09:18:49 +1000 Stephen Rothwell <[email protected]> wrote:
> > >
> > > 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.
> > >
> > > --
> > > Cheers,
> > > Stephen Rothwell
> > >
> > > 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)))
>
> Is the above extra '+' what compiler complains about?
> Patches from Linus' tree remove some code from that function, so
> applying them first should simplify the merge.
I see you're unfamiliar with the output of git diff --cc ...
Hi all,
On Fri, 28 Jul 2023 09:29:15 +1000 Stephen Rothwell <[email protected]> wrote:
>
> On Fri, 28 Jul 2023 09:18:49 +1000 Stephen Rothwell <[email protected]> wrote:
> >
> > 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.
> >
> > --
> > Cheers,
> > Stephen Rothwell
> >
> > 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;
> > +
> > /* 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) {
>
> Sorry, doesn't even build ... let me try that again.
I have gone with below. Again, please check.
--
Cheers,
Stephen Rothwell
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) {
On Thu, Jul 27, 2023 at 5:29 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi Suren,
>
> On Thu, 27 Jul 2023 17:23:45 -0700 Suren Baghdasaryan <[email protected]> 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/[email protected]/
> > 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 Thu, Jul 27, 2023 at 5:21 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi Matthew,
>
> On Fri, 28 Jul 2023 00:49:50 +0100 Matthew Wilcox <[email protected]> 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/[email protected]/
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 Matthew,
On Fri, 28 Jul 2023 00:49:50 +0100 Matthew Wilcox <[email protected]> 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.
--
Cheers,
Stephen Rothwell
On Thu, Jul 27, 2023 at 4:50 PM Matthew Wilcox <[email protected]> wrote:
>
> On Thu, Jul 27, 2023 at 04:40:20PM -0700, Suren Baghdasaryan wrote:
> > On Thu, Jul 27, 2023 at 4:29 PM Stephen Rothwell <[email protected]> wrote:
> > >
> > > Hi all,
> > >
> > > On Fri, 28 Jul 2023 09:18:49 +1000 Stephen Rothwell <[email protected]> wrote:
> > > >
> > > > 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.
> > > >
> > > > --
> > > > Cheers,
> > > > Stephen Rothwell
> > > >
> > > > 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)))
> >
> > Is the above extra '+' what compiler complains about?
> > Patches from Linus' tree remove some code from that function, so
> > applying them first should simplify the merge.
>
> I see you're unfamiliar with the output of git diff --cc ...
guilty as charged.
>
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;
I'm not sure about the userfaultfd_armed() clause. My patch wasn't
intended to affect that.
> /* 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) {
On Thu, Jul 27, 2023 at 4:29 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi all,
>
> On Fri, 28 Jul 2023 09:18:49 +1000 Stephen Rothwell <[email protected]> wrote:
> >
> > 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.
> >
> > --
> > Cheers,
> > Stephen Rothwell
> >
> > 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)))
Is the above extra '+' what compiler complains about?
Patches from Linus' tree remove some code from that function, so
applying them first should simplify the merge.
> > 657b5146955e ("mm: lock_vma_under_rcu() must check vma->anon_vma under vma lock")
> > + 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) {
>
> Sorry, doesn't even build ... let me try that again.
>
> --
> Cheers,
> Stephen Rothwell
Hi Suren,
On Thu, 27 Jul 2023 17:23:45 -0700 Suren Baghdasaryan <[email protected]> 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/[email protected]/
> 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.
--
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.