[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20260118120658.8da21fc257774feb4e753969@linux-foundation.org>
Date: Sun, 18 Jan 2026 12:06:58 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Suren Baghdasaryan <surenb@...gle.com>, "Liam R . Howlett"
<Liam.Howlett@...cle.com>, Vlastimil Babka <vbabka@...e.cz>, Shakeel Butt
<shakeel.butt@...ux.dev>, David Hildenbrand <david@...nel.org>, Rik van
Riel <riel@...riel.com>, Harry Yoo <harry.yoo@...cle.com>, Jann Horn
<jannh@...gle.com>, Mike Rapoport <rppt@...nel.org>, Michal Hocko
<mhocko@...e.com>, Pedro Falcato <pfalcato@...e.de>, Chris Li
<chriscli@...gle.com>, Barry Song <v-songbaohua@...o.com>,
linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 0/9] mm: clean up anon_vma implementation
On Sun, 18 Jan 2026 14:50:36 +0000 Lorenzo Stoakes <lorenzo.stoakes@...cle.com> wrote:
> The anon_vma logic is hugely confusing and, much like a bundle of wires
> entangled with one another, pulling on one thread seems only to lead to
> more entanglement elsewhere.
>
> There is a mish-mash of the core implementation, how that implementation is
> invoked, how helper functions are invoked and concepts such as adjacent
> anon_vma merge and anon_vma object reuse.
>
> This series tries to improve the situation somewhat.
Updated, thanks.
> v3:
> * Propagate tags (thanks everyone!)
> * Fold fix-patches into series.
> * Add fix for syzbot report about an accursed partially-initialised VMA
> fault injection error path.
> * Fixed a typo, a comment whitespace error I noticed and add some comments
> to anon_vma_fork(), set anon_vma->num_active_vmas = 1 to make it clear
> that we're setting this on a newly allocated anon_vma.
Below is how this update altered mm.git:
--- a/mm/rmap.c~b
+++ a/mm/rmap.c
@@ -333,10 +333,10 @@ int anon_vma_clone(struct vm_area_struct
* are not updating the anon_vma rbtree nor are we changing
* anon_vma statistics.
*
- * Either src, dst have the same mm for which we hold an exclusive mmap
- * write lock, or we are forking and we hold it on src->vm_mm and dst is
- * not yet accessible to other threads so there's no possibliity of the
- * unlinked AVC's being observed yet.
+ * Either src, dst have the same mm for which we hold an exclusive mmap
+ * write lock, or we are forking and we hold it on src->vm_mm and dst is
+ * not yet accessible to other threads so there's no possibliity of the
+ * unlinked AVC's being observed yet.
*/
list_for_each_entry(pavc, &src->anon_vma_chain, same_vma) {
avc = anon_vma_chain_alloc(GFP_KERNEL);
@@ -379,7 +379,7 @@ int anon_vma_fork(struct vm_area_struct
{
struct anon_vma_chain *avc;
struct anon_vma *anon_vma;
- int error;
+ int rc;
/* Don't bother if the parent process has no anon_vma here. */
if (!pvma->anon_vma)
@@ -388,27 +388,35 @@ int anon_vma_fork(struct vm_area_struct
/* Drop inherited anon_vma, we'll reuse existing or allocate new. */
vma->anon_vma = NULL;
+ anon_vma = anon_vma_alloc();
+ if (!anon_vma)
+ return -ENOMEM;
+ avc = anon_vma_chain_alloc(GFP_KERNEL);
+ if (!avc) {
+ put_anon_vma(anon_vma);
+ return -ENOMEM;
+ }
+
/*
* First, attach the new VMA to the parent VMA's anon_vmas,
* so rmap can find non-COWed pages in child processes.
*/
- error = anon_vma_clone(vma, pvma, VMA_OP_FORK);
- if (error)
- return error;
-
- /* An existing anon_vma has been reused, all done then. */
- if (vma->anon_vma)
- return 0;
+ rc = anon_vma_clone(vma, pvma, VMA_OP_FORK);
+ /* An error arose or an existing anon_vma was reused, all done then. */
+ if (rc || vma->anon_vma) {
+ put_anon_vma(anon_vma);
+ anon_vma_chain_free(avc);
+ return rc;
+ }
- /* Then add our own anon_vma. */
- anon_vma = anon_vma_alloc();
- if (!anon_vma)
- goto out_error;
- anon_vma->num_active_vmas++;
- avc = anon_vma_chain_alloc(GFP_KERNEL);
- if (!avc)
- goto out_error_free_anon_vma;
+ /*
+ * OK no reuse, so add our own anon_vma.
+ *
+ * Since it is not linked anywhere we can safely manipulate anon_vma
+ * fields without a lock.
+ */
+ anon_vma->num_active_vmas = 1;
/*
* The root anon_vma's rwsem is the lock actually used when we
* lock any of the anon_vmas in this anon_vma tree.
@@ -431,12 +439,6 @@ int anon_vma_fork(struct vm_area_struct
anon_vma_unlock_write(anon_vma);
return 0;
-
- out_error_free_anon_vma:
- put_anon_vma(anon_vma);
- out_error:
- unlink_anon_vmas(vma);
- return -ENOMEM;
}
/*
_
Powered by blists - more mailing lists