From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Vlastimil Babka <vbabka@suse.cz>, Jann Horn <jannh@google.com>,
"Liam R . Howlett" <Liam.Howlett@oracle.com>,
Suren Baghdasaryan <surenb@google.com>,
David Hildenbrand <david@redhat.com>,
Matthew Wilcox <willy@infradead.org>,
Rik van Riel <riel@surriel.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: [PATCH 0/3] fix incorrectly disallowed anonymous VMA merges
Date: Mon, 17 Mar 2025 21:15:02 +0000 [thread overview]
Message-ID: <cover.1742245056.git.lorenzo.stoakes@oracle.com> (raw)
It appears that we have been incorrectly rejecting merge cases for 15
years, apparently by mistake.
Imagine a range of anonymous mapped momemory divided into two VMAs like
this, with incompatible protection bits:
RW RWX
unfaulted faulted
|-----------|-----------|
| prev | vma |
|-----------|-----------|
mprotect(RW)
Now imagine mprotect()'ing vma so it is RW. This appears as if it should
merge, it does not.
Neither does this case, again mprotect()'ing vma RW:
RWX RW
faulted unfaulted
|-----------|-----------|
| vma | next |
|-----------|-----------|
mprotect(RW)
Nor:
RW RWX RW
unfaulted faulted unfaulted
|-----------|-----------|-----------|
| prev | vma | next |
|-----------|-----------|-----------|
mprotect(RW)
What's going on here?
In commit 5beb49305251 ("mm: change anon_vma linking to fix multi-process
server scalability issue"), from 2010, Rik von Riel took careful care to
account for these cases - commenting that '[this is] easily overlooked:
when mprotect shifts the boundary, make sure the expanding vma has anon_vma
set if the shrinking vma had, to cover any anon pages imported.'
However, commit 965f55dea0e3 ("mmap: avoid merging cloned VMAs") introduced
a little over a year later, appears to have accidentally disallowed this.
By adjusting the is_mergeable_anon_vma() function to avoid lock contention
across large trees of forked anon_vma's, this commit wrongly assumed the
VMA being checked (the ostensible merge 'target') should be faulted, that
is, have an anon_vma, and thus an anon_vma_chain list established, but only
of length 1.
This appears to have been unintentional, as disallowing empty target VMAs
like this across the board makes no sense.
We already have logic that accounts for this case, the same logic Rik
introduced in 2010, now via dup_anon_vma() (and ultimately
anon_vma_clone()), so there is no problem permitting this.
This series fixes this mistake and also ensures that scalability concerns
remain addressed by explicitly checking that whatever VMA is being merged
has not been forked.
A full set of self tests which reproduce the issue are provided, as well as
updating userland VMA tests to assert this behaviour.
The self tests additionally assert scalability concerns are addressed.
Lorenzo Stoakes (3):
mm/vma: fix incorrectly disallowed anonymous VMA merges
tools/testing: add PROCMAP_QUERY helper functions in mm self tests
tools/testing/selftests: assert that anon merge cases behave as
expected
mm/vma.c | 81 ++--
tools/testing/selftests/mm/.gitignore | 1 +
tools/testing/selftests/mm/Makefile | 1 +
tools/testing/selftests/mm/merge.c | 454 ++++++++++++++++++++++
tools/testing/selftests/mm/run_vmtests.sh | 2 +
tools/testing/selftests/mm/vm_util.c | 62 +++
tools/testing/selftests/mm/vm_util.h | 21 +
tools/testing/vma/vma.c | 100 ++---
8 files changed, 652 insertions(+), 70 deletions(-)
create mode 100644 tools/testing/selftests/mm/merge.c
--
2.48.1
next reply other threads:[~2025-03-17 21:15 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-17 21:15 Lorenzo Stoakes [this message]
2025-03-17 21:15 ` [PATCH 1/3] mm/vma: fix incorrectly disallowed anonymous VMA merges Lorenzo Stoakes
2025-04-04 12:53 ` Wei Yang
2025-04-04 13:04 ` Lorenzo Stoakes
2025-04-04 23:32 ` Wei Yang
2025-04-07 10:24 ` Lorenzo Stoakes
2025-04-07 16:44 ` Lorenzo Stoakes
2025-03-17 21:15 ` [PATCH 2/3] tools/testing: add PROCMAP_QUERY helper functions in mm self tests Lorenzo Stoakes
2025-03-18 15:18 ` Lorenzo Stoakes
2025-04-07 2:42 ` Wei Yang
2025-03-17 21:15 ` [PATCH 3/3] tools/testing/selftests: assert that anon merge cases behave as expected Lorenzo Stoakes
2025-04-07 2:54 ` Wei Yang
2025-04-07 11:02 ` Lorenzo Stoakes
2025-04-07 12:09 ` Wei Yang
2025-03-20 13:33 ` [PATCH 0/3] fix incorrectly disallowed anonymous VMA merges Yeoreum Yun
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1742245056.git.lorenzo.stoakes@oracle.com \
--to=lorenzo.stoakes@oracle.com \
--cc=Liam.Howlett@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=david@redhat.com \
--cc=jannh@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=riel@surriel.com \
--cc=surenb@google.com \
--cc=vbabka@suse.cz \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).