For both anon_vma_clone() and unlink_anon_vmas(), if the source VMA or the
VMA to be linked are unfaulted (e.g. !vma->anon_vma), then the functions do
nothing. Simply exit early in these cases.

In the unlink_anon_vmas() case we can also remove a conditional that checks
whether vma->anon_vma is set.

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

diff --git a/mm/rmap.c b/mm/rmap.c
index 0e34c0a69fbc..9332d1cbc643 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -309,6 +309,9 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
 	struct anon_vma_chain *avc, *pavc;
 	struct anon_vma *root = NULL;
 
+	if (!src->anon_vma)
+		return 0;
+
 	check_anon_vma_clone(dst, src);
 
 	list_for_each_entry_reverse(pavc, &src->anon_vma_chain, same_vma) {
@@ -441,7 +444,8 @@ void unlink_anon_vmas(struct vm_area_struct *vma)
 	mmap_assert_locked(vma->vm_mm);
 
 	/* Unfaulted is a no-op. */
-	VM_WARN_ON_ONCE(!vma->anon_vma && !list_empty(&vma->anon_vma_chain));
+	if (!vma->anon_vma)
+		return;
 
 	/*
 	 * Unlink each anon_vma chained to the VMA.  This list is ordered
@@ -465,15 +469,13 @@ void unlink_anon_vmas(struct vm_area_struct *vma)
 		list_del(&avc->same_vma);
 		anon_vma_chain_free(avc);
 	}
-	if (vma->anon_vma) {
-		vma->anon_vma->num_active_vmas--;
 
-		/*
-		 * vma would still be needed after unlink, and anon_vma will be prepared
-		 * when handle fault.
-		 */
-		vma->anon_vma = NULL;
-	}
+	vma->anon_vma->num_active_vmas--;
+	/*
+	 * vma would still be needed after unlink, and anon_vma will be prepared
+	 * when handle fault.
+	 */
+	vma->anon_vma = NULL;
 	unlock_anon_vma_root(root);
 
 	/*
-- 
2.52.0

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [251217 07:27]:
> For both anon_vma_clone() and unlink_anon_vmas(), if the source VMA or the
> VMA to be linked are unfaulted (e.g. !vma->anon_vma), then the functions do
> nothing. Simply exit early in these cases.
> 
> In the unlink_anon_vmas() case we can also remove a conditional that checks
> whether vma->anon_vma is set.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
>  mm/rmap.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 0e34c0a69fbc..9332d1cbc643 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -309,6 +309,9 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
>  	struct anon_vma_chain *avc, *pavc;
>  	struct anon_vma *root = NULL;
>  
> +	if (!src->anon_vma)
> +		return 0;
> +
>  	check_anon_vma_clone(dst, src);
>  
>  	list_for_each_entry_reverse(pavc, &src->anon_vma_chain, same_vma) {
> @@ -441,7 +444,8 @@ void unlink_anon_vmas(struct vm_area_struct *vma)
>  	mmap_assert_locked(vma->vm_mm);
>  
>  	/* Unfaulted is a no-op. */
> -	VM_WARN_ON_ONCE(!vma->anon_vma && !list_empty(&vma->anon_vma_chain));
> +	if (!vma->anon_vma)
> +		return;

I guess it doesn't matter because you just added the !list_empty()
check, but did you mean to drop that part?

>  
>  	/*
>  	 * Unlink each anon_vma chained to the VMA.  This list is ordered
> @@ -465,15 +469,13 @@ void unlink_anon_vmas(struct vm_area_struct *vma)
>  		list_del(&avc->same_vma);
>  		anon_vma_chain_free(avc);
>  	}
> -	if (vma->anon_vma) {
> -		vma->anon_vma->num_active_vmas--;
>  
> -		/*
> -		 * vma would still be needed after unlink, and anon_vma will be prepared
> -		 * when handle fault.
> -		 */
> -		vma->anon_vma = NULL;
> -	}
> +	vma->anon_vma->num_active_vmas--;
> +	/*
> +	 * vma would still be needed after unlink, and anon_vma will be prepared
> +	 * when handle fault.
> +	 */
> +	vma->anon_vma = NULL;
>  	unlock_anon_vma_root(root);
>  
>  	/*
> -- 
> 2.52.0
>

On Fri, Dec 19, 2025 at 01:28:03PM -0500, Liam R. Howlett wrote:
> * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [251217 07:27]:
> > For both anon_vma_clone() and unlink_anon_vmas(), if the source VMA or the
> > VMA to be linked are unfaulted (e.g. !vma->anon_vma), then the functions do
> > nothing. Simply exit early in these cases.
> >
> > In the unlink_anon_vmas() case we can also remove a conditional that checks
> > whether vma->anon_vma is set.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > ---
> >  mm/rmap.c | 20 +++++++++++---------
> >  1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index 0e34c0a69fbc..9332d1cbc643 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -309,6 +309,9 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> >  	struct anon_vma_chain *avc, *pavc;
> >  	struct anon_vma *root = NULL;
> >
> > +	if (!src->anon_vma)
> > +		return 0;
> > +
> >  	check_anon_vma_clone(dst, src);
> >
> >  	list_for_each_entry_reverse(pavc, &src->anon_vma_chain, same_vma) {
> > @@ -441,7 +444,8 @@ void unlink_anon_vmas(struct vm_area_struct *vma)
> >  	mmap_assert_locked(vma->vm_mm);
> >
> >  	/* Unfaulted is a no-op. */
> > -	VM_WARN_ON_ONCE(!vma->anon_vma && !list_empty(&vma->anon_vma_chain));
> > +	if (!vma->anon_vma)
> > +		return;
>
> I guess it doesn't matter because you just added the !list_empty()
> check, but did you mean to drop that part?

I did mean to. Really this doesn't happen in reality, the assert was more of a
place holder I suppose.

I don't think we should be falling over ourselves to assert impossible things,
really the debug-only asserts are intended to essentially document what's going
on.

Anyway it's moot, as I've had to drop both the assert and the condition here
sadly, because of the fact we (of course) use unlink_anon_vmas() to clean up
incompletely set up anon_vma's on a destination VMA.

When has doing things on incompletely setup up VMAs ever gone wrong :)

As ever with anon_vma, there are always deeper depths of horror to find.

>
> >
> >  	/*
> >  	 * Unlink each anon_vma chained to the VMA.  This list is ordered
> > @@ -465,15 +469,13 @@ void unlink_anon_vmas(struct vm_area_struct *vma)
> >  		list_del(&avc->same_vma);
> >  		anon_vma_chain_free(avc);
> >  	}
> > -	if (vma->anon_vma) {
> > -		vma->anon_vma->num_active_vmas--;
> >
> > -		/*
> > -		 * vma would still be needed after unlink, and anon_vma will be prepared
> > -		 * when handle fault.
> > -		 */
> > -		vma->anon_vma = NULL;
> > -	}
> > +	vma->anon_vma->num_active_vmas--;
> > +	/*
> > +	 * vma would still be needed after unlink, and anon_vma will be prepared
> > +	 * when handle fault.
> > +	 */
> > +	vma->anon_vma = NULL;
> >  	unlock_anon_vma_root(root);
> >
> >  	/*
> > --
> > 2.52.0
> >

On Tue, Jan 06, 2026 at 01:14:10PM +0000, Lorenzo Stoakes wrote:
> On Fri, Dec 19, 2025 at 01:28:03PM -0500, Liam R. Howlett wrote:
> > * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [251217 07:27]:
> > > For both anon_vma_clone() and unlink_anon_vmas(), if the source VMA or the
> > > VMA to be linked are unfaulted (e.g. !vma->anon_vma), then the functions do
> > > nothing. Simply exit early in these cases.
> > >
> > > In the unlink_anon_vmas() case we can also remove a conditional that checks
> > > whether vma->anon_vma is set.
> > >
> > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > ---
> > >  mm/rmap.c | 20 +++++++++++---------
> > >  1 file changed, 11 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/mm/rmap.c b/mm/rmap.c
> > > index 0e34c0a69fbc..9332d1cbc643 100644
> > > --- a/mm/rmap.c
> > > +++ b/mm/rmap.c
> > > @@ -309,6 +309,9 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> > >  	struct anon_vma_chain *avc, *pavc;
> > >  	struct anon_vma *root = NULL;
> > >
> > > +	if (!src->anon_vma)
> > > +		return 0;
> > > +
> > >  	check_anon_vma_clone(dst, src);
> > >
> > >  	list_for_each_entry_reverse(pavc, &src->anon_vma_chain, same_vma) {
> > > @@ -441,7 +444,8 @@ void unlink_anon_vmas(struct vm_area_struct *vma)
> > >  	mmap_assert_locked(vma->vm_mm);
> > >
> > >  	/* Unfaulted is a no-op. */
> > > -	VM_WARN_ON_ONCE(!vma->anon_vma && !list_empty(&vma->anon_vma_chain));
> > > +	if (!vma->anon_vma)
> > > +		return;
> >
> > I guess it doesn't matter because you just added the !list_empty()
> > check, but did you mean to drop that part?
>
> I did mean to. Really this doesn't happen in reality, the assert was more of a
> place holder I suppose.
>
> I don't think we should be falling over ourselves to assert impossible things,
> really the debug-only asserts are intended to essentially document what's going
> on.
>
> Anyway it's moot, as I've had to drop both the assert and the condition here
> sadly, because of the fact we (of course) use unlink_anon_vmas() to clean up
> incompletely set up anon_vma's on a destination VMA.
>
> When has doing things on incompletely setup up VMAs ever gone wrong :)
>
> As ever with anon_vma, there are always deeper depths of horror to find.
>

OK scratch that see 1/8 thread... I will add a specific partial cleanup
path to deal with the anon_vma_clone() horror show and then we get to keep
all this.

In which case I guess I'll reinstate the check just to be super careful :)

On Fri, Dec 19, 2025 at 10:28 AM Liam R. Howlett
<Liam.Howlett@oracle.com> wrote:
>
> * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [251217 07:27]:
> > For both anon_vma_clone() and unlink_anon_vmas(), if the source VMA or the
> > VMA to be linked are unfaulted (e.g. !vma->anon_vma), then the functions do
> > nothing. Simply exit early in these cases.
> >
> > In the unlink_anon_vmas() case we can also remove a conditional that checks
> > whether vma->anon_vma is set.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > ---
> >  mm/rmap.c | 20 +++++++++++---------
> >  1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index 0e34c0a69fbc..9332d1cbc643 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -309,6 +309,9 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> >       struct anon_vma_chain *avc, *pavc;
> >       struct anon_vma *root = NULL;
> >
> > +     if (!src->anon_vma)
> > +             return 0;
> > +
> >       check_anon_vma_clone(dst, src);

check_anon_vma_clone() is used only here and contains a couple of
warnings with "!src->anon_vma && ..." conditions, which now will never
be triggered even if the second part of those conditions was true.
This seems like a regression (you are not checking conditions you were
checking before this change). To avoid that, you can place this early
exit after the call to check_anon_vma_clone(dst, src).

> >
> >       list_for_each_entry_reverse(pavc, &src->anon_vma_chain, same_vma) {
> > @@ -441,7 +444,8 @@ void unlink_anon_vmas(struct vm_area_struct *vma)
> >       mmap_assert_locked(vma->vm_mm);
> >
> >       /* Unfaulted is a no-op. */
> > -     VM_WARN_ON_ONCE(!vma->anon_vma && !list_empty(&vma->anon_vma_chain));
> > +     if (!vma->anon_vma)
> > +             return;
>
> I guess it doesn't matter because you just added the !list_empty()
> check, but did you mean to drop that part?
>
> >
> >       /*
> >        * Unlink each anon_vma chained to the VMA.  This list is ordered
> > @@ -465,15 +469,13 @@ void unlink_anon_vmas(struct vm_area_struct *vma)
> >               list_del(&avc->same_vma);
> >               anon_vma_chain_free(avc);
> >       }
> > -     if (vma->anon_vma) {
> > -             vma->anon_vma->num_active_vmas--;
> >
> > -             /*
> > -              * vma would still be needed after unlink, and anon_vma will be prepared
> > -              * when handle fault.
> > -              */
> > -             vma->anon_vma = NULL;
> > -     }
> > +     vma->anon_vma->num_active_vmas--;
> > +     /*
> > +      * vma would still be needed after unlink, and anon_vma will be prepared
> > +      * when handle fault.
> > +      */
> > +     vma->anon_vma = NULL;
> >       unlock_anon_vma_root(root);
> >
> >       /*
> > --
> > 2.52.0
> >

On Mon, Dec 29, 2025 at 01:41:10PM -0800, Suren Baghdasaryan wrote:
> On Fri, Dec 19, 2025 at 10:28 AM Liam R. Howlett
> <Liam.Howlett@oracle.com> wrote:
> >
> > * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [251217 07:27]:
> > > For both anon_vma_clone() and unlink_anon_vmas(), if the source VMA or the
> > > VMA to be linked are unfaulted (e.g. !vma->anon_vma), then the functions do
> > > nothing. Simply exit early in these cases.
> > >
> > > In the unlink_anon_vmas() case we can also remove a conditional that checks
> > > whether vma->anon_vma is set.
> > >
> > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > ---
> > >  mm/rmap.c | 20 +++++++++++---------
> > >  1 file changed, 11 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/mm/rmap.c b/mm/rmap.c
> > > index 0e34c0a69fbc..9332d1cbc643 100644
> > > --- a/mm/rmap.c
> > > +++ b/mm/rmap.c
> > > @@ -309,6 +309,9 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
> > >       struct anon_vma_chain *avc, *pavc;
> > >       struct anon_vma *root = NULL;
> > >
> > > +     if (!src->anon_vma)
> > > +             return 0;
> > > +
> > >       check_anon_vma_clone(dst, src);
>
> check_anon_vma_clone() is used only here and contains a couple of
> warnings with "!src->anon_vma && ..." conditions, which now will never
> be triggered even if the second part of those conditions was true.
> This seems like a regression (you are not checking conditions you were
> checking before this change). To avoid that, you can place this early
> exit after the call to check_anon_vma_clone(dst, src).

Yup am aware. Was because later on we do stuff that assumes src.

I guess I can move it and then change the later commit.

>
> > >
> > >       list_for_each_entry_reverse(pavc, &src->anon_vma_chain, same_vma) {
> > > @@ -441,7 +444,8 @@ void unlink_anon_vmas(struct vm_area_struct *vma)
> > >       mmap_assert_locked(vma->vm_mm);
> > >
> > >       /* Unfaulted is a no-op. */
> > > -     VM_WARN_ON_ONCE(!vma->anon_vma && !list_empty(&vma->anon_vma_chain));
> > > +     if (!vma->anon_vma)
> > > +             return;
> >
> > I guess it doesn't matter because you just added the !list_empty()
> > check, but did you mean to drop that part?
> >
> > >
> > >       /*
> > >        * Unlink each anon_vma chained to the VMA.  This list is ordered
> > > @@ -465,15 +469,13 @@ void unlink_anon_vmas(struct vm_area_struct *vma)
> > >               list_del(&avc->same_vma);
> > >               anon_vma_chain_free(avc);
> > >       }
> > > -     if (vma->anon_vma) {
> > > -             vma->anon_vma->num_active_vmas--;
> > >
> > > -             /*
> > > -              * vma would still be needed after unlink, and anon_vma will be prepared
> > > -              * when handle fault.
> > > -              */
> > > -             vma->anon_vma = NULL;
> > > -     }
> > > +     vma->anon_vma->num_active_vmas--;
> > > +     /*
> > > +      * vma would still be needed after unlink, and anon_vma will be prepared
> > > +      * when handle fault.
> > > +      */
> > > +     vma->anon_vma = NULL;
> > >       unlock_anon_vma_root(root);
> > >
> > >       /*
> > > --
> > > 2.52.0
> > >