Specify which operation is being performed to anon_vma_clone(), which
allows us to do checks specific to each operation type, as well as to
separate out and make clear that the anon_vma reuse logic is absolutely
specific to fork only.

This opens the door to further refactorings and refinements later as we
have more information to work with.

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
 mm/internal.h                    | 11 ++++-
 mm/rmap.c                        | 74 ++++++++++++++++++++++----------
 mm/vma.c                         |  6 +--
 tools/testing/vma/vma_internal.h | 11 ++++-
 4 files changed, 74 insertions(+), 28 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index 4ba784023a9f..8baa7bd2b8f7 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -244,7 +244,16 @@ static inline void anon_vma_unlock_read(struct anon_vma *anon_vma)
 
 struct anon_vma *folio_get_anon_vma(const struct folio *folio);
 
-int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src);
+/* Operations which modify VMAs. */
+enum vma_operation {
+	VMA_OP_SPLIT,
+	VMA_OP_MERGE_UNFAULTED,
+	VMA_OP_REMAP,
+	VMA_OP_FORK,
+};
+
+int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,
+	enum vma_operation operation);
 int anon_vma_fork(struct vm_area_struct *vma, struct vm_area_struct *pvma);
 int  __anon_vma_prepare(struct vm_area_struct *vma);
 void unlink_anon_vmas(struct vm_area_struct *vma);
diff --git a/mm/rmap.c b/mm/rmap.c
index 8f4393546bce..336b27e00238 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -233,12 +233,13 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
 }
 
 static void check_anon_vma_clone(struct vm_area_struct *dst,
-				 struct vm_area_struct *src)
+				 struct vm_area_struct *src,
+				 enum vma_operation operation)
 {
 	/* The write lock must be held. */
 	mmap_assert_write_locked(src->vm_mm);
-	/* If not a fork (implied by dst->anon_vma) then must be on same mm. */
-	VM_WARN_ON_ONCE(dst->anon_vma && dst->vm_mm != src->vm_mm);
+	/* If not a fork then must be on same mm. */
+	VM_WARN_ON_ONCE(operation != VMA_OP_FORK && dst->vm_mm != src->vm_mm);
 
 	/* If we have anything to do src->anon_vma must be provided. */
 	VM_WARN_ON_ONCE(!src->anon_vma && !list_empty(&src->anon_vma_chain));
@@ -250,6 +251,40 @@ static void check_anon_vma_clone(struct vm_area_struct *dst,
 	 * must be the same across dst and src.
 	 */
 	VM_WARN_ON_ONCE(dst->anon_vma && dst->anon_vma != src->anon_vma);
+	/*
+	 * Essentially equivalent to above - if not a no-op, we should expect
+	 * dst->anon_vma to be set for everything except a fork.
+	 */
+	VM_WARN_ON_ONCE(operation != VMA_OP_FORK && src->anon_vma &&
+			!dst->anon_vma);
+	/* For the anon_vma to be compatible, it can only be singular. */
+	VM_WARN_ON_ONCE(operation == VMA_OP_MERGE_UNFAULTED &&
+			!list_is_singular(&src->anon_vma_chain));
+#ifdef CONFIG_PER_VMA_LOCK
+	/* Only merging an unfaulted VMA leaves the destination attached. */
+	VM_WARN_ON_ONCE(operation != VMA_OP_MERGE_UNFAULTED &&
+			vma_is_attached(dst));
+#endif
+}
+
+static void try_to_reuse_anon_vma(struct vm_area_struct *dst,
+				  struct anon_vma *anon_vma)
+{
+	/* If already populated, nothing to do.*/
+	if (dst->anon_vma)
+		return;
+
+	/*
+	 * We reuse an anon_vma if any linking VMAs were unmapped and it has
+	 * only a single child at most.
+	 */
+	if (anon_vma->num_active_vmas > 0)
+		return;
+	if (anon_vma->num_children > 1)
+		return;
+
+	dst->anon_vma = anon_vma;
+	anon_vma->num_active_vmas++;
 }
 
 static void cleanup_partial_anon_vmas(struct vm_area_struct *vma);
@@ -259,6 +294,7 @@ static void cleanup_partial_anon_vmas(struct vm_area_struct *vma);
  * all of the anon_vma objects contained within @src anon_vma_chain's.
  * @dst: The destination VMA with an empty anon_vma_chain.
  * @src: The source VMA we wish to duplicate.
+ * @operation: The type of operation which resulted in the clone.
  *
  * This is the heart of the VMA side of the anon_vma implementation - we invoke
  * this function whenever we need to set up a new VMA's anon_vma state.
@@ -281,17 +317,17 @@ static void cleanup_partial_anon_vmas(struct vm_area_struct *vma);
  *
  * Returns: 0 on success, -ENOMEM on failure.
  */
-int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
+int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,
+		   enum vma_operation operation)
 {
 	struct anon_vma_chain *avc, *pavc;
+	struct anon_vma *active_anon_vma = src->anon_vma;
 
-	check_anon_vma_clone(dst, src);
+	check_anon_vma_clone(dst, src, operation);
 
-	if (!src->anon_vma)
+	if (!active_anon_vma)
 		return 0;
 
-	check_anon_vma_clone(dst, src);
-
 	/*
 	 * Allocate AVCs. We don't need an anon_vma lock for this as we
 	 * are not updating the anon_vma rbtree nor are we changing
@@ -317,22 +353,14 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
 		struct anon_vma *anon_vma = avc->anon_vma;
 
 		anon_vma_interval_tree_insert(avc, &anon_vma->rb_root);
-
-		/*
-		 * Reuse existing anon_vma if it has no vma and only one
-		 * anon_vma child.
-		 *
-		 * Root anon_vma is never reused:
-		 * it has self-parent reference and at least one child.
-		 */
-		if (!dst->anon_vma && src->anon_vma &&
-		    anon_vma->num_children < 2 &&
-		    anon_vma->num_active_vmas == 0)
-			dst->anon_vma = anon_vma;
+		if (operation == VMA_OP_FORK)
+			try_to_reuse_anon_vma(dst, anon_vma);
 	}
-	if (dst->anon_vma)
+
+	if (operation != VMA_OP_FORK)
 		dst->anon_vma->num_active_vmas++;
-	anon_vma_unlock_write(src->anon_vma);
+
+	anon_vma_unlock_write(active_anon_vma);
 	return 0;
 
  enomem_failure:
@@ -362,7 +390,7 @@ int anon_vma_fork(struct vm_area_struct *vma, struct vm_area_struct *pvma)
 	 * 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);
+	error = anon_vma_clone(vma, pvma, VMA_OP_FORK);
 	if (error)
 		return error;
 
diff --git a/mm/vma.c b/mm/vma.c
index 4294ecdc23a5..2a063d6568d9 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -528,7 +528,7 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
 	if (err)
 		goto out_free_vmi;
 
-	err = anon_vma_clone(new, vma);
+	err = anon_vma_clone(new, vma, VMA_OP_SPLIT);
 	if (err)
 		goto out_free_mpol;
 
@@ -626,7 +626,7 @@ static int dup_anon_vma(struct vm_area_struct *dst,
 
 		vma_assert_write_locked(dst);
 		dst->anon_vma = src->anon_vma;
-		ret = anon_vma_clone(dst, src);
+		ret = anon_vma_clone(dst, src, VMA_OP_MERGE_UNFAULTED);
 		if (ret)
 			return ret;
 
@@ -1899,7 +1899,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
 		vma_set_range(new_vma, addr, addr + len, pgoff);
 		if (vma_dup_policy(vma, new_vma))
 			goto out_free_vma;
-		if (anon_vma_clone(new_vma, vma))
+		if (anon_vma_clone(new_vma, vma, VMA_OP_REMAP))
 			goto out_free_mempol;
 		if (new_vma->vm_file)
 			get_file(new_vma->vm_file);
diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
index 93e5792306d9..7fa56dcc53a6 100644
--- a/tools/testing/vma/vma_internal.h
+++ b/tools/testing/vma/vma_internal.h
@@ -600,6 +600,14 @@ struct mmap_action {
 	bool hide_from_rmap_until_complete :1;
 };
 
+/* Operations which modify VMAs. */
+enum vma_operation {
+	VMA_OP_SPLIT,
+	VMA_OP_MERGE_UNFAULTED,
+	VMA_OP_REMAP,
+	VMA_OP_FORK,
+};
+
 /*
  * Describes a VMA that is about to be mmap()'ed. Drivers may choose to
  * manipulate mutable fields which will cause those fields to be updated in the
@@ -1157,7 +1165,8 @@ static inline int vma_dup_policy(struct vm_area_struct *src, struct vm_area_stru
 	return 0;
 }
 
-static inline int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
+static inline int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,
+				 enum vma_operation operation)
 {
 	/* For testing purposes. We indicate that an anon_vma has been cloned. */
 	if (src->anon_vma != NULL) {
-- 
2.52.0

Hi Andrew,

Please apply the below fix-patch to rename try_to_reuse_anon_vma() to
maybe_reuse_anon_vma() to address Liam's concern about this naming.

Thanks, Lorenzo

----8<----
From c2bd360718da23c62a4aaa0addfa48955b2007f9 Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Date: Thu, 8 Jan 2026 18:49:44 +0000
Subject: [PATCH] fix rename try_to_reuse_anon_vma()

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
 mm/rmap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index f66f98416a7c..5da3b47b1251 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -267,8 +267,8 @@ static void check_anon_vma_clone(struct vm_area_struct *dst,
 #endif
 }

-static void try_to_reuse_anon_vma(struct vm_area_struct *dst,
-				 struct anon_vma *anon_vma)
+static void maybe_reuse_anon_vma(struct vm_area_struct *dst,
+		struct anon_vma *anon_vma)
 {
 	/* If already populated, nothing to do.*/
 	if (dst->anon_vma)
@@ -356,7 +356,7 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,

 		anon_vma_interval_tree_insert(avc, &anon_vma->rb_root);
 		if (operation == VMA_OP_FORK)
-			try_to_reuse_anon_vma(dst, anon_vma);
+			maybe_reuse_anon_vma(dst, anon_vma);
 	}

 	if (operation != VMA_OP_FORK)
--
2.52.0

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [260106 10:05]:
> Specify which operation is being performed to anon_vma_clone(), which
> allows us to do checks specific to each operation type, as well as to
> separate out and make clear that the anon_vma reuse logic is absolutely
> specific to fork only.
> 
> This opens the door to further refactorings and refinements later as we
> have more information to work with.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

A few minor things, but this looks correct.

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  mm/internal.h                    | 11 ++++-
>  mm/rmap.c                        | 74 ++++++++++++++++++++++----------
>  mm/vma.c                         |  6 +--
>  tools/testing/vma/vma_internal.h | 11 ++++-
>  4 files changed, 74 insertions(+), 28 deletions(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 4ba784023a9f..8baa7bd2b8f7 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -244,7 +244,16 @@ static inline void anon_vma_unlock_read(struct anon_vma *anon_vma)
>  
>  struct anon_vma *folio_get_anon_vma(const struct folio *folio);
>  
> -int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src);
> +/* Operations which modify VMAs. */
> +enum vma_operation {
> +	VMA_OP_SPLIT,
> +	VMA_OP_MERGE_UNFAULTED,
> +	VMA_OP_REMAP,
> +	VMA_OP_FORK,
> +};
> +
> +int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,
> +	enum vma_operation operation);
>  int anon_vma_fork(struct vm_area_struct *vma, struct vm_area_struct *pvma);
>  int  __anon_vma_prepare(struct vm_area_struct *vma);
>  void unlink_anon_vmas(struct vm_area_struct *vma);
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 8f4393546bce..336b27e00238 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -233,12 +233,13 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
>  }
>  
>  static void check_anon_vma_clone(struct vm_area_struct *dst,
> -				 struct vm_area_struct *src)
> +				 struct vm_area_struct *src,
> +				 enum vma_operation operation)

You could save a line here by putting src and operation on the same line
and tabbing only twice, but sure.  This is true in earlier patches as
well.

>  {
>  	/* The write lock must be held. */
>  	mmap_assert_write_locked(src->vm_mm);
> -	/* If not a fork (implied by dst->anon_vma) then must be on same mm. */
> -	VM_WARN_ON_ONCE(dst->anon_vma && dst->vm_mm != src->vm_mm);
> +	/* If not a fork then must be on same mm. */
> +	VM_WARN_ON_ONCE(operation != VMA_OP_FORK && dst->vm_mm != src->vm_mm);
>  
>  	/* If we have anything to do src->anon_vma must be provided. */
>  	VM_WARN_ON_ONCE(!src->anon_vma && !list_empty(&src->anon_vma_chain));
> @@ -250,6 +251,40 @@ static void check_anon_vma_clone(struct vm_area_struct *dst,
>  	 * must be the same across dst and src.
>  	 */
>  	VM_WARN_ON_ONCE(dst->anon_vma && dst->anon_vma != src->anon_vma);
> +	/*
> +	 * Essentially equivalent to above - if not a no-op, we should expect
> +	 * dst->anon_vma to be set for everything except a fork.
> +	 */
> +	VM_WARN_ON_ONCE(operation != VMA_OP_FORK && src->anon_vma &&
> +			!dst->anon_vma);
> +	/* For the anon_vma to be compatible, it can only be singular. */
> +	VM_WARN_ON_ONCE(operation == VMA_OP_MERGE_UNFAULTED &&
> +			!list_is_singular(&src->anon_vma_chain));
> +#ifdef CONFIG_PER_VMA_LOCK
> +	/* Only merging an unfaulted VMA leaves the destination attached. */
> +	VM_WARN_ON_ONCE(operation != VMA_OP_MERGE_UNFAULTED &&
> +			vma_is_attached(dst));
> +#endif
> +}
> +

try seems to imply we can return something saying it didn't work out,
but this is void.  Naming is hard.  reuse_anon_vma_if_necessary seems
insane, so I don't really have anything better.

> +static void try_to_reuse_anon_vma(struct vm_area_struct *dst,
> +				  struct anon_vma *anon_vma)
> +{
> +	/* If already populated, nothing to do.*/
> +	if (dst->anon_vma)
> +		return;

This is only used on VMA_OP_FORK, how is it populated?

I assume this is a later refinement?

> +
> +	/*
> +	 * We reuse an anon_vma if any linking VMAs were unmapped and it has
> +	 * only a single child at most.
> +	 */
> +	if (anon_vma->num_active_vmas > 0)
> +		return;
> +	if (anon_vma->num_children > 1)
> +		return;
> +
> +	dst->anon_vma = anon_vma;
> +	anon_vma->num_active_vmas++;
>  }
>  
>  static void cleanup_partial_anon_vmas(struct vm_area_struct *vma);
> @@ -259,6 +294,7 @@ static void cleanup_partial_anon_vmas(struct vm_area_struct *vma);
>   * all of the anon_vma objects contained within @src anon_vma_chain's.
>   * @dst: The destination VMA with an empty anon_vma_chain.
>   * @src: The source VMA we wish to duplicate.
> + * @operation: The type of operation which resulted in the clone.
>   *
>   * This is the heart of the VMA side of the anon_vma implementation - we invoke
>   * this function whenever we need to set up a new VMA's anon_vma state.
> @@ -281,17 +317,17 @@ static void cleanup_partial_anon_vmas(struct vm_area_struct *vma);
>   *
>   * Returns: 0 on success, -ENOMEM on failure.
>   */
> -int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> +int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,
> +		   enum vma_operation operation)
>  {
>  	struct anon_vma_chain *avc, *pavc;
> +	struct anon_vma *active_anon_vma = src->anon_vma;
>  
> -	check_anon_vma_clone(dst, src);
> +	check_anon_vma_clone(dst, src, operation);
>  
> -	if (!src->anon_vma)
> +	if (!active_anon_vma)
>  		return 0;
>  
> -	check_anon_vma_clone(dst, src);
> -
>  	/*
>  	 * Allocate AVCs. We don't need an anon_vma lock for this as we
>  	 * are not updating the anon_vma rbtree nor are we changing
> @@ -317,22 +353,14 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
>  		struct anon_vma *anon_vma = avc->anon_vma;
>  
>  		anon_vma_interval_tree_insert(avc, &anon_vma->rb_root);
> -
> -		/*
> -		 * Reuse existing anon_vma if it has no vma and only one
> -		 * anon_vma child.
> -		 *
> -		 * Root anon_vma is never reused:
> -		 * it has self-parent reference and at least one child.
> -		 */
> -		if (!dst->anon_vma && src->anon_vma &&
> -		    anon_vma->num_children < 2 &&
> -		    anon_vma->num_active_vmas == 0)
> -			dst->anon_vma = anon_vma;
> +		if (operation == VMA_OP_FORK)
> +			try_to_reuse_anon_vma(dst, anon_vma);
>  	}
> -	if (dst->anon_vma)
> +
> +	if (operation != VMA_OP_FORK)
>  		dst->anon_vma->num_active_vmas++;
> -	anon_vma_unlock_write(src->anon_vma);
> +
> +	anon_vma_unlock_write(active_anon_vma);
>  	return 0;
>  
>   enomem_failure:
> @@ -362,7 +390,7 @@ int anon_vma_fork(struct vm_area_struct *vma, struct vm_area_struct *pvma)
>  	 * 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);
> +	error = anon_vma_clone(vma, pvma, VMA_OP_FORK);
>  	if (error)
>  		return error;
>  
> diff --git a/mm/vma.c b/mm/vma.c
> index 4294ecdc23a5..2a063d6568d9 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -528,7 +528,7 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
>  	if (err)
>  		goto out_free_vmi;
>  
> -	err = anon_vma_clone(new, vma);
> +	err = anon_vma_clone(new, vma, VMA_OP_SPLIT);
>  	if (err)
>  		goto out_free_mpol;
>  
> @@ -626,7 +626,7 @@ static int dup_anon_vma(struct vm_area_struct *dst,
>  
>  		vma_assert_write_locked(dst);
>  		dst->anon_vma = src->anon_vma;
> -		ret = anon_vma_clone(dst, src);
> +		ret = anon_vma_clone(dst, src, VMA_OP_MERGE_UNFAULTED);
>  		if (ret)
>  			return ret;
>  
> @@ -1899,7 +1899,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
>  		vma_set_range(new_vma, addr, addr + len, pgoff);
>  		if (vma_dup_policy(vma, new_vma))
>  			goto out_free_vma;
> -		if (anon_vma_clone(new_vma, vma))
> +		if (anon_vma_clone(new_vma, vma, VMA_OP_REMAP))
>  			goto out_free_mempol;
>  		if (new_vma->vm_file)
>  			get_file(new_vma->vm_file);
> diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
> index 93e5792306d9..7fa56dcc53a6 100644
> --- a/tools/testing/vma/vma_internal.h
> +++ b/tools/testing/vma/vma_internal.h
> @@ -600,6 +600,14 @@ struct mmap_action {
>  	bool hide_from_rmap_until_complete :1;
>  };
>  
> +/* Operations which modify VMAs. */
> +enum vma_operation {
> +	VMA_OP_SPLIT,
> +	VMA_OP_MERGE_UNFAULTED,
> +	VMA_OP_REMAP,
> +	VMA_OP_FORK,
> +};
> +
>  /*
>   * Describes a VMA that is about to be mmap()'ed. Drivers may choose to
>   * manipulate mutable fields which will cause those fields to be updated in the
> @@ -1157,7 +1165,8 @@ static inline int vma_dup_policy(struct vm_area_struct *src, struct vm_area_stru
>  	return 0;
>  }
>  
> -static inline int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> +static inline int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,
> +				 enum vma_operation operation)
>  {
>  	/* For testing purposes. We indicate that an anon_vma has been cloned. */
>  	if (src->anon_vma != NULL) {
> -- 
> 2.52.0
>

On Tue, Jan 6, 2026 at 11:27 AM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
>
> * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [260106 10:05]:
> > Specify which operation is being performed to anon_vma_clone(), which
> > allows us to do checks specific to each operation type, as well as to
> > separate out and make clear that the anon_vma reuse logic is absolutely
> > specific to fork only.
> >
> > This opens the door to further refactorings and refinements later as we
> > have more information to work with.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>
> A few minor things, but this looks correct.
>
> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

Reviewed-by: Suren Baghdasaryan <surenb@google.com>

>
> > ---
> >  mm/internal.h                    | 11 ++++-
> >  mm/rmap.c                        | 74 ++++++++++++++++++++++----------
> >  mm/vma.c                         |  6 +--
> >  tools/testing/vma/vma_internal.h | 11 ++++-
> >  4 files changed, 74 insertions(+), 28 deletions(-)
> >
> > diff --git a/mm/internal.h b/mm/internal.h
> > index 4ba784023a9f..8baa7bd2b8f7 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -244,7 +244,16 @@ static inline void anon_vma_unlock_read(struct anon_vma *anon_vma)
> >
> >  struct anon_vma *folio_get_anon_vma(const struct folio *folio);
> >
> > -int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src);
> > +/* Operations which modify VMAs. */
> > +enum vma_operation {
> > +     VMA_OP_SPLIT,
> > +     VMA_OP_MERGE_UNFAULTED,
> > +     VMA_OP_REMAP,
> > +     VMA_OP_FORK,
> > +};
> > +
> > +int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,
> > +     enum vma_operation operation);
> >  int anon_vma_fork(struct vm_area_struct *vma, struct vm_area_struct *pvma);
> >  int  __anon_vma_prepare(struct vm_area_struct *vma);
> >  void unlink_anon_vmas(struct vm_area_struct *vma);
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index 8f4393546bce..336b27e00238 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -233,12 +233,13 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
> >  }
> >
> >  static void check_anon_vma_clone(struct vm_area_struct *dst,
> > -                              struct vm_area_struct *src)
> > +                              struct vm_area_struct *src,
> > +                              enum vma_operation operation)
>
> You could save a line here by putting src and operation on the same line
> and tabbing only twice, but sure.  This is true in earlier patches as
> well.
>
> >  {
> >       /* The write lock must be held. */
> >       mmap_assert_write_locked(src->vm_mm);
> > -     /* If not a fork (implied by dst->anon_vma) then must be on same mm. */
> > -     VM_WARN_ON_ONCE(dst->anon_vma && dst->vm_mm != src->vm_mm);
> > +     /* If not a fork then must be on same mm. */
> > +     VM_WARN_ON_ONCE(operation != VMA_OP_FORK && dst->vm_mm != src->vm_mm);
> >
> >       /* If we have anything to do src->anon_vma must be provided. */
> >       VM_WARN_ON_ONCE(!src->anon_vma && !list_empty(&src->anon_vma_chain));
> > @@ -250,6 +251,40 @@ static void check_anon_vma_clone(struct vm_area_struct *dst,
> >        * must be the same across dst and src.
> >        */
> >       VM_WARN_ON_ONCE(dst->anon_vma && dst->anon_vma != src->anon_vma);
> > +     /*
> > +      * Essentially equivalent to above - if not a no-op, we should expect
> > +      * dst->anon_vma to be set for everything except a fork.
> > +      */
> > +     VM_WARN_ON_ONCE(operation != VMA_OP_FORK && src->anon_vma &&
> > +                     !dst->anon_vma);
> > +     /* For the anon_vma to be compatible, it can only be singular. */
> > +     VM_WARN_ON_ONCE(operation == VMA_OP_MERGE_UNFAULTED &&
> > +                     !list_is_singular(&src->anon_vma_chain));
> > +#ifdef CONFIG_PER_VMA_LOCK
> > +     /* Only merging an unfaulted VMA leaves the destination attached. */
> > +     VM_WARN_ON_ONCE(operation != VMA_OP_MERGE_UNFAULTED &&
> > +                     vma_is_attached(dst));
> > +#endif
> > +}
> > +
>
> try seems to imply we can return something saying it didn't work out,
> but this is void.  Naming is hard.  reuse_anon_vma_if_necessary seems
> insane, so I don't really have anything better.
>
> > +static void try_to_reuse_anon_vma(struct vm_area_struct *dst,
> > +                               struct anon_vma *anon_vma)
> > +{
> > +     /* If already populated, nothing to do.*/
> > +     if (dst->anon_vma)
> > +             return;
>
> This is only used on VMA_OP_FORK, how is it populated?
>
> I assume this is a later refinement?
>
> > +
> > +     /*
> > +      * We reuse an anon_vma if any linking VMAs were unmapped and it has
> > +      * only a single child at most.
> > +      */
> > +     if (anon_vma->num_active_vmas > 0)
> > +             return;
> > +     if (anon_vma->num_children > 1)
> > +             return;
> > +
> > +     dst->anon_vma = anon_vma;
> > +     anon_vma->num_active_vmas++;
> >  }
> >
> >  static void cleanup_partial_anon_vmas(struct vm_area_struct *vma);
> > @@ -259,6 +294,7 @@ static void cleanup_partial_anon_vmas(struct vm_area_struct *vma);
> >   * all of the anon_vma objects contained within @src anon_vma_chain's.
> >   * @dst: The destination VMA with an empty anon_vma_chain.
> >   * @src: The source VMA we wish to duplicate.
> > + * @operation: The type of operation which resulted in the clone.
> >   *
> >   * This is the heart of the VMA side of the anon_vma implementation - we invoke
> >   * this function whenever we need to set up a new VMA's anon_vma state.
> > @@ -281,17 +317,17 @@ static void cleanup_partial_anon_vmas(struct vm_area_struct *vma);
> >   *
> >   * Returns: 0 on success, -ENOMEM on failure.
> >   */
> > -int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> > +int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,
> > +                enum vma_operation operation)
> >  {
> >       struct anon_vma_chain *avc, *pavc;
> > +     struct anon_vma *active_anon_vma = src->anon_vma;
> >
> > -     check_anon_vma_clone(dst, src);
> > +     check_anon_vma_clone(dst, src, operation);
> >
> > -     if (!src->anon_vma)
> > +     if (!active_anon_vma)
> >               return 0;
> >
> > -     check_anon_vma_clone(dst, src);
> > -
> >       /*
> >        * Allocate AVCs. We don't need an anon_vma lock for this as we
> >        * are not updating the anon_vma rbtree nor are we changing
> > @@ -317,22 +353,14 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> >               struct anon_vma *anon_vma = avc->anon_vma;
> >
> >               anon_vma_interval_tree_insert(avc, &anon_vma->rb_root);
> > -
> > -             /*
> > -              * Reuse existing anon_vma if it has no vma and only one
> > -              * anon_vma child.
> > -              *
> > -              * Root anon_vma is never reused:
> > -              * it has self-parent reference and at least one child.
> > -              */
> > -             if (!dst->anon_vma && src->anon_vma &&
> > -                 anon_vma->num_children < 2 &&
> > -                 anon_vma->num_active_vmas == 0)
> > -                     dst->anon_vma = anon_vma;
> > +             if (operation == VMA_OP_FORK)
> > +                     try_to_reuse_anon_vma(dst, anon_vma);
> >       }
> > -     if (dst->anon_vma)
> > +
> > +     if (operation != VMA_OP_FORK)
> >               dst->anon_vma->num_active_vmas++;
> > -     anon_vma_unlock_write(src->anon_vma);
> > +
> > +     anon_vma_unlock_write(active_anon_vma);
> >       return 0;
> >
> >   enomem_failure:
> > @@ -362,7 +390,7 @@ int anon_vma_fork(struct vm_area_struct *vma, struct vm_area_struct *pvma)
> >        * 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);
> > +     error = anon_vma_clone(vma, pvma, VMA_OP_FORK);
> >       if (error)
> >               return error;
> >
> > diff --git a/mm/vma.c b/mm/vma.c
> > index 4294ecdc23a5..2a063d6568d9 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -528,7 +528,7 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
> >       if (err)
> >               goto out_free_vmi;
> >
> > -     err = anon_vma_clone(new, vma);
> > +     err = anon_vma_clone(new, vma, VMA_OP_SPLIT);
> >       if (err)
> >               goto out_free_mpol;
> >
> > @@ -626,7 +626,7 @@ static int dup_anon_vma(struct vm_area_struct *dst,
> >
> >               vma_assert_write_locked(dst);
> >               dst->anon_vma = src->anon_vma;
> > -             ret = anon_vma_clone(dst, src);
> > +             ret = anon_vma_clone(dst, src, VMA_OP_MERGE_UNFAULTED);
> >               if (ret)
> >                       return ret;
> >
> > @@ -1899,7 +1899,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
> >               vma_set_range(new_vma, addr, addr + len, pgoff);
> >               if (vma_dup_policy(vma, new_vma))
> >                       goto out_free_vma;
> > -             if (anon_vma_clone(new_vma, vma))
> > +             if (anon_vma_clone(new_vma, vma, VMA_OP_REMAP))
> >                       goto out_free_mempol;
> >               if (new_vma->vm_file)
> >                       get_file(new_vma->vm_file);
> > diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
> > index 93e5792306d9..7fa56dcc53a6 100644
> > --- a/tools/testing/vma/vma_internal.h
> > +++ b/tools/testing/vma/vma_internal.h
> > @@ -600,6 +600,14 @@ struct mmap_action {
> >       bool hide_from_rmap_until_complete :1;
> >  };
> >
> > +/* Operations which modify VMAs. */
> > +enum vma_operation {
> > +     VMA_OP_SPLIT,
> > +     VMA_OP_MERGE_UNFAULTED,
> > +     VMA_OP_REMAP,
> > +     VMA_OP_FORK,
> > +};
> > +
> >  /*
> >   * Describes a VMA that is about to be mmap()'ed. Drivers may choose to
> >   * manipulate mutable fields which will cause those fields to be updated in the
> > @@ -1157,7 +1165,8 @@ static inline int vma_dup_policy(struct vm_area_struct *src, struct vm_area_stru
> >       return 0;
> >  }
> >
> > -static inline int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> > +static inline int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,
> > +                              enum vma_operation operation)
> >  {
> >       /* For testing purposes. We indicate that an anon_vma has been cloned. */
> >       if (src->anon_vma != NULL) {
> > --
> > 2.52.0
> >

On Tue, Jan 06, 2026 at 02:27:37PM -0500, Liam R. Howlett wrote:
> * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [260106 10:05]:
> > Specify which operation is being performed to anon_vma_clone(), which
> > allows us to do checks specific to each operation type, as well as to
> > separate out and make clear that the anon_vma reuse logic is absolutely
> > specific to fork only.
> >
> > This opens the door to further refactorings and refinements later as we
> > have more information to work with.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>
> A few minor things, but this looks correct.
>
> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
>
> > ---
> >  mm/internal.h                    | 11 ++++-
> >  mm/rmap.c                        | 74 ++++++++++++++++++++++----------
> >  mm/vma.c                         |  6 +--
> >  tools/testing/vma/vma_internal.h | 11 ++++-
> >  4 files changed, 74 insertions(+), 28 deletions(-)
> >
> > diff --git a/mm/internal.h b/mm/internal.h
> > index 4ba784023a9f..8baa7bd2b8f7 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -244,7 +244,16 @@ static inline void anon_vma_unlock_read(struct anon_vma *anon_vma)
> >
> >  struct anon_vma *folio_get_anon_vma(const struct folio *folio);
> >
> > -int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src);
> > +/* Operations which modify VMAs. */
> > +enum vma_operation {
> > +	VMA_OP_SPLIT,
> > +	VMA_OP_MERGE_UNFAULTED,
> > +	VMA_OP_REMAP,
> > +	VMA_OP_FORK,
> > +};
> > +
> > +int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,
> > +	enum vma_operation operation);
> >  int anon_vma_fork(struct vm_area_struct *vma, struct vm_area_struct *pvma);
> >  int  __anon_vma_prepare(struct vm_area_struct *vma);
> >  void unlink_anon_vmas(struct vm_area_struct *vma);
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index 8f4393546bce..336b27e00238 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -233,12 +233,13 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
> >  }
> >
> >  static void check_anon_vma_clone(struct vm_area_struct *dst,
> > -				 struct vm_area_struct *src)
> > +				 struct vm_area_struct *src,
> > +				 enum vma_operation operation)
>
> You could save a line here by putting src and operation on the same line
> and tabbing only twice, but sure.  This is true in earlier patches as
> well.
>
> >  {
> >  	/* The write lock must be held. */
> >  	mmap_assert_write_locked(src->vm_mm);
> > -	/* If not a fork (implied by dst->anon_vma) then must be on same mm. */
> > -	VM_WARN_ON_ONCE(dst->anon_vma && dst->vm_mm != src->vm_mm);
> > +	/* If not a fork then must be on same mm. */
> > +	VM_WARN_ON_ONCE(operation != VMA_OP_FORK && dst->vm_mm != src->vm_mm);
> >
> >  	/* If we have anything to do src->anon_vma must be provided. */
> >  	VM_WARN_ON_ONCE(!src->anon_vma && !list_empty(&src->anon_vma_chain));
> > @@ -250,6 +251,40 @@ static void check_anon_vma_clone(struct vm_area_struct *dst,
> >  	 * must be the same across dst and src.
> >  	 */
> >  	VM_WARN_ON_ONCE(dst->anon_vma && dst->anon_vma != src->anon_vma);
> > +	/*
> > +	 * Essentially equivalent to above - if not a no-op, we should expect
> > +	 * dst->anon_vma to be set for everything except a fork.
> > +	 */
> > +	VM_WARN_ON_ONCE(operation != VMA_OP_FORK && src->anon_vma &&
> > +			!dst->anon_vma);
> > +	/* For the anon_vma to be compatible, it can only be singular. */
> > +	VM_WARN_ON_ONCE(operation == VMA_OP_MERGE_UNFAULTED &&
> > +			!list_is_singular(&src->anon_vma_chain));
> > +#ifdef CONFIG_PER_VMA_LOCK
> > +	/* Only merging an unfaulted VMA leaves the destination attached. */
> > +	VM_WARN_ON_ONCE(operation != VMA_OP_MERGE_UNFAULTED &&
> > +			vma_is_attached(dst));
> > +#endif
> > +}
> > +
>
> try seems to imply we can return something saying it didn't work out,
> but this is void.  Naming is hard.  reuse_anon_vma_if_necessary seems
> insane, so I don't really have anything better.

As discussed on irc, this is probably better as maybe_reuse_anon_vma().

Will send a fix-patch.

>
> > +static void try_to_reuse_anon_vma(struct vm_area_struct *dst,
> > +				  struct anon_vma *anon_vma)
> > +{
> > +	/* If already populated, nothing to do.*/
> > +	if (dst->anon_vma)
> > +		return;
>
> This is only used on VMA_OP_FORK, how is it populated?

Below,

See:

+	dst->anon_vma = anon_vma;

So this just means we use the first anon_vma we encounter in the hierarchy and
don't waste time trying to find another.

>
> I assume this is a later refinement?
>
> > +
> > +	/*
> > +	 * We reuse an anon_vma if any linking VMAs were unmapped and it has
> > +	 * only a single child at most.
> > +	 */
> > +	if (anon_vma->num_active_vmas > 0)
> > +		return;
> > +	if (anon_vma->num_children > 1)
> > +		return;
> > +
> > +	dst->anon_vma = anon_vma;
> > +	anon_vma->num_active_vmas++;
> >  }
> >
> >  static void cleanup_partial_anon_vmas(struct vm_area_struct *vma);
> > @@ -259,6 +294,7 @@ static void cleanup_partial_anon_vmas(struct vm_area_struct *vma);
> >   * all of the anon_vma objects contained within @src anon_vma_chain's.
> >   * @dst: The destination VMA with an empty anon_vma_chain.
> >   * @src: The source VMA we wish to duplicate.
> > + * @operation: The type of operation which resulted in the clone.
> >   *
> >   * This is the heart of the VMA side of the anon_vma implementation - we invoke
> >   * this function whenever we need to set up a new VMA's anon_vma state.
> > @@ -281,17 +317,17 @@ static void cleanup_partial_anon_vmas(struct vm_area_struct *vma);
> >   *
> >   * Returns: 0 on success, -ENOMEM on failure.
> >   */
> > -int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> > +int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,
> > +		   enum vma_operation operation)
> >  {
> >  	struct anon_vma_chain *avc, *pavc;
> > +	struct anon_vma *active_anon_vma = src->anon_vma;
> >
> > -	check_anon_vma_clone(dst, src);
> > +	check_anon_vma_clone(dst, src, operation);
> >
> > -	if (!src->anon_vma)
> > +	if (!active_anon_vma)
> >  		return 0;
> >
> > -	check_anon_vma_clone(dst, src);
> > -
> >  	/*
> >  	 * Allocate AVCs. We don't need an anon_vma lock for this as we
> >  	 * are not updating the anon_vma rbtree nor are we changing
> > @@ -317,22 +353,14 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> >  		struct anon_vma *anon_vma = avc->anon_vma;
> >
> >  		anon_vma_interval_tree_insert(avc, &anon_vma->rb_root);
> > -
> > -		/*
> > -		 * Reuse existing anon_vma if it has no vma and only one
> > -		 * anon_vma child.
> > -		 *
> > -		 * Root anon_vma is never reused:
> > -		 * it has self-parent reference and at least one child.
> > -		 */
> > -		if (!dst->anon_vma && src->anon_vma &&
> > -		    anon_vma->num_children < 2 &&
> > -		    anon_vma->num_active_vmas == 0)
> > -			dst->anon_vma = anon_vma;
> > +		if (operation == VMA_OP_FORK)
> > +			try_to_reuse_anon_vma(dst, anon_vma);
> >  	}
> > -	if (dst->anon_vma)
> > +
> > +	if (operation != VMA_OP_FORK)
> >  		dst->anon_vma->num_active_vmas++;
> > -	anon_vma_unlock_write(src->anon_vma);
> > +
> > +	anon_vma_unlock_write(active_anon_vma);
> >  	return 0;
> >
> >   enomem_failure:
> > @@ -362,7 +390,7 @@ int anon_vma_fork(struct vm_area_struct *vma, struct vm_area_struct *pvma)
> >  	 * 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);
> > +	error = anon_vma_clone(vma, pvma, VMA_OP_FORK);
> >  	if (error)
> >  		return error;
> >
> > diff --git a/mm/vma.c b/mm/vma.c
> > index 4294ecdc23a5..2a063d6568d9 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -528,7 +528,7 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
> >  	if (err)
> >  		goto out_free_vmi;
> >
> > -	err = anon_vma_clone(new, vma);
> > +	err = anon_vma_clone(new, vma, VMA_OP_SPLIT);
> >  	if (err)
> >  		goto out_free_mpol;
> >
> > @@ -626,7 +626,7 @@ static int dup_anon_vma(struct vm_area_struct *dst,
> >
> >  		vma_assert_write_locked(dst);
> >  		dst->anon_vma = src->anon_vma;
> > -		ret = anon_vma_clone(dst, src);
> > +		ret = anon_vma_clone(dst, src, VMA_OP_MERGE_UNFAULTED);
> >  		if (ret)
> >  			return ret;
> >
> > @@ -1899,7 +1899,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
> >  		vma_set_range(new_vma, addr, addr + len, pgoff);
> >  		if (vma_dup_policy(vma, new_vma))
> >  			goto out_free_vma;
> > -		if (anon_vma_clone(new_vma, vma))
> > +		if (anon_vma_clone(new_vma, vma, VMA_OP_REMAP))
> >  			goto out_free_mempol;
> >  		if (new_vma->vm_file)
> >  			get_file(new_vma->vm_file);
> > diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
> > index 93e5792306d9..7fa56dcc53a6 100644
> > --- a/tools/testing/vma/vma_internal.h
> > +++ b/tools/testing/vma/vma_internal.h
> > @@ -600,6 +600,14 @@ struct mmap_action {
> >  	bool hide_from_rmap_until_complete :1;
> >  };
> >
> > +/* Operations which modify VMAs. */
> > +enum vma_operation {
> > +	VMA_OP_SPLIT,
> > +	VMA_OP_MERGE_UNFAULTED,
> > +	VMA_OP_REMAP,
> > +	VMA_OP_FORK,
> > +};
> > +
> >  /*
> >   * Describes a VMA that is about to be mmap()'ed. Drivers may choose to
> >   * manipulate mutable fields which will cause those fields to be updated in the
> > @@ -1157,7 +1165,8 @@ static inline int vma_dup_policy(struct vm_area_struct *src, struct vm_area_stru
> >  	return 0;
> >  }
> >
> > -static inline int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> > +static inline int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src,
> > +				 enum vma_operation operation)
> >  {
> >  	/* For testing purposes. We indicate that an anon_vma has been cloned. */
> >  	if (src->anon_vma != NULL) {
> > --
> > 2.52.0
> >