[PATCH] mm/vma: predicate vma_assert_stabilised() on CONFIG_DEBUG_VM

[PATCH] mm/vma: predicate vma_assert_stabilised() on CONFIG_DEBUG_VM

[Lorenzo Stoakes (ARM)]

I introduced commit 17fd82c3abe0 ("mm/vma: add and use
vma_assert_stabilised()") with the intent of picking up programmatic bugs
when debugging mm code, but kept the general practice of asserting even
when CONFIG_DEBUG_VM is not set in line with existing code.

However, it turns out doing so can cause performance regressions,
especially when asserted in vma_has_anon_rmap() which is referenced in a
number of hotpaths.

Since the intent was always a debug-only assertion, explicitly check for
CONFIG_DEBUG_VM and do not assert if it isn't set.

Really all of the assert paths need to be predicated on this, but start
with this one only, out of caution.

Additionally, since a detached VMA is trivially stabilised (nothing to
stabilise against), check for this case at the outset, and update
vma_has_anon_rmap() to not explicitly need to check for this.

This also, importantly, avoids reading any VMA fields if CONFIG_DEBUG_VM is
not set.

Target the commit which introduced vma_has_anon_rmap() for the fix, as this
is where the issue started manifesting in practice.

Fixes: 38564710f443 ("mm: implement and use vma_has_anon_rmap(), silence KCSAN")
Reported-by: Seunghyun An <san55@wisc.edu>
Closes: https://lore.kernel.org/linux-mm/20261110000002.1834454-1-san55@wisc.edu/
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
Note that vma_is_anon_rmap() was introduced in 7.4, so no need for a cc: stable,
just needs to be sent to Linus as a hotfix. Thanks!
---
 include/linux/mmap_lock.h | 19 ++++++++++++++-----
 mm/vma.h                  |  3 +--
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
index 9ccbbcbd7aca..9c3a9c6525c1 100644
--- a/include/linux/mmap_lock.h
+++ b/include/linux/mmap_lock.h
@@ -408,6 +408,11 @@ static inline void vma_assert_locked(const struct vm_area_struct *vma)
 	vma_assert_write_locked(vma);
 }
 
+static inline bool vma_is_attached(const struct vm_area_struct *vma)
+{
+	return refcount_read(&vma->vm_refcnt);
+}
+
 /**
  * vma_assert_stabilised() - assert that this VMA cannot be changed from
  * underneath us either by having a VMA or mmap lock held.
@@ -418,9 +423,18 @@ static inline void vma_assert_locked(const struct vm_area_struct *vma)
  *
  * With lockdep disabled we may sometimes race with other threads acquiring the
  * mmap read lock simultaneous with our VMA read lock.
+ *
+ * Assert only if CONFIG_DEBUG_VM is set.
  */
 static inline void vma_assert_stabilised(const struct vm_area_struct *vma)
 {
+	if (!IS_ENABLED(CONFIG_DEBUG_VM))
+		return;
+
+	/* Detached VMAs are trivially stabilised. */
+	if (!vma_is_attached(vma))
+		return;
+
 	/*
 	 * If another thread owns an mmap lock, it may go away at any time, and
 	 * thus is no guarantee of stability.
@@ -454,11 +468,6 @@ static inline void vma_assert_stabilised(const struct vm_area_struct *vma)
 	vma_assert_locked(vma);
 }
 
-static inline bool vma_is_attached(const struct vm_area_struct *vma)
-{
-	return refcount_read(&vma->vm_refcnt);
-}
-
 /*
  * WARNING: to avoid racing with vma_mark_attached()/vma_mark_detached(), these
  * assertions should be made either under mmap_write_lock or when the object
diff --git a/mm/vma.h b/mm/vma.h
index 3fc3b47a954b..05300badd70b 100644
--- a/mm/vma.h
+++ b/mm/vma.h
@@ -278,8 +278,7 @@ static inline pgoff_t vmg_end_pgoff(const struct vma_merge_struct *vmg)
  */
 static inline bool vma_has_anon_rmap(const struct vm_area_struct *vma)
 {
-	if (vma_is_attached(vma))
-		vma_assert_stabilised(vma);
+	vma_assert_stabilised(vma);
 	/* KCSAN gets confused about the optimistic check. Silence it. */
 	return data_race(vma->anon_vma);
 }

---
base-commit: 93d7a816a2df3e907e2719bd12b0073f1285fc5c
change-id: 20261110-vma-fixup-debug-asserts-2d327fe27f8c

Cheers,
-- 
Lorenzo Stoakes (ARM) <ljs@kernel.org>

Re: [PATCH] mm/vma: predicate vma_assert_stabilised() on CONFIG_DEBUG_VM

[Suren Baghdasaryan]

On Tue, Nov 10, 2026 at 7:10 AM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
>
> I introduced commit 17fd82c3abe0 ("mm/vma: add and use
> vma_assert_stabilised()") with the intent of picking up programmatic bugs
> when debugging mm code, but kept the general practice of asserting even
> when CONFIG_DEBUG_VM is not set in line with existing code.
>
> However, it turns out doing so can cause performance regressions,
> especially when asserted in vma_has_anon_rmap() which is referenced in a
> number of hotpaths.
>
> Since the intent was always a debug-only assertion, explicitly check for
> CONFIG_DEBUG_VM and do not assert if it isn't set.
>
> Really all of the assert paths need to be predicated on this, but start
> with this one only, out of caution.
>
> Additionally, since a detached VMA is trivially stabilised (nothing to
> stabilise against), check for this case at the outset, and update
> vma_has_anon_rmap() to not explicitly need to check for this.
>
> This also, importantly, avoids reading any VMA fields if CONFIG_DEBUG_VM is
> not set.
>
> Target the commit which introduced vma_has_anon_rmap() for the fix, as this
> is where the issue started manifesting in practice.
>
> Fixes: 38564710f443 ("mm: implement and use vma_has_anon_rmap(), silence KCSAN")
> Reported-by: Seunghyun An <san55@wisc.edu>
> Closes: https://lore.kernel.org/linux-mm/20261110000002.1834454-1-san55@wisc.edu/
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

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

> ---
> Note that vma_is_anon_rmap() was introduced in 7.4, so no need for a cc: stable,
> just needs to be sent to Linus as a hotfix. Thanks!
> ---
>  include/linux/mmap_lock.h | 19 ++++++++++++++-----
>  mm/vma.h                  |  3 +--
>  2 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> index 9ccbbcbd7aca..9c3a9c6525c1 100644
> --- a/include/linux/mmap_lock.h
> +++ b/include/linux/mmap_lock.h
> @@ -408,6 +408,11 @@ static inline void vma_assert_locked(const struct vm_area_struct *vma)
>         vma_assert_write_locked(vma);
>  }
>
> +static inline bool vma_is_attached(const struct vm_area_struct *vma)
> +{
> +       return refcount_read(&vma->vm_refcnt);
> +}
> +
>  /**
>   * vma_assert_stabilised() - assert that this VMA cannot be changed from
>   * underneath us either by having a VMA or mmap lock held.
> @@ -418,9 +423,18 @@ static inline void vma_assert_locked(const struct vm_area_struct *vma)
>   *
>   * With lockdep disabled we may sometimes race with other threads acquiring the
>   * mmap read lock simultaneous with our VMA read lock.
> + *
> + * Assert only if CONFIG_DEBUG_VM is set.
>   */
>  static inline void vma_assert_stabilised(const struct vm_area_struct *vma)
>  {
> +       if (!IS_ENABLED(CONFIG_DEBUG_VM))
> +               return;
> +
> +       /* Detached VMAs are trivially stabilised. */
> +       if (!vma_is_attached(vma))
> +               return;
> +
>         /*
>          * If another thread owns an mmap lock, it may go away at any time, and
>          * thus is no guarantee of stability.
> @@ -454,11 +468,6 @@ static inline void vma_assert_stabilised(const struct vm_area_struct *vma)
>         vma_assert_locked(vma);
>  }
>
> -static inline bool vma_is_attached(const struct vm_area_struct *vma)
> -{
> -       return refcount_read(&vma->vm_refcnt);
> -}
> -
>  /*
>   * WARNING: to avoid racing with vma_mark_attached()/vma_mark_detached(), these
>   * assertions should be made either under mmap_write_lock or when the object
> diff --git a/mm/vma.h b/mm/vma.h
> index 3fc3b47a954b..05300badd70b 100644
> --- a/mm/vma.h
> +++ b/mm/vma.h
> @@ -278,8 +278,7 @@ static inline pgoff_t vmg_end_pgoff(const struct vma_merge_struct *vmg)
>   */
>  static inline bool vma_has_anon_rmap(const struct vm_area_struct *vma)
>  {
> -       if (vma_is_attached(vma))
> -               vma_assert_stabilised(vma);
> +       vma_assert_stabilised(vma);
>         /* KCSAN gets confused about the optimistic check. Silence it. */
>         return data_race(vma->anon_vma);
>  }
>
> ---
> base-commit: 93d7a816a2df3e907e2719bd12b0073f1285fc5c
> change-id: 20261110-vma-fixup-debug-asserts-2d327fe27f8c
>
> Cheers,
> --
> Lorenzo Stoakes (ARM) <ljs@kernel.org>
>

Re: [PATCH] mm/vma: predicate vma_assert_stabilised() on CONFIG_DEBUG_VM

[Liam R. Howlett]

On 26/11/10 03:10PM, Lorenzo Stoakes (ARM) wrote:
> I introduced commit 17fd82c3abe0 ("mm/vma: add and use
> vma_assert_stabilised()") with the intent of picking up programmatic bugs
> when debugging mm code, but kept the general practice of asserting even
> when CONFIG_DEBUG_VM is not set in line with existing code.
> 
> However, it turns out doing so can cause performance regressions,
> especially when asserted in vma_has_anon_rmap() which is referenced in a
> number of hotpaths.
> 
> Since the intent was always a debug-only assertion, explicitly check for
> CONFIG_DEBUG_VM and do not assert if it isn't set.
> 
> Really all of the assert paths need to be predicated on this, but start
> with this one only, out of caution.
> 
> Additionally, since a detached VMA is trivially stabilised (nothing to
> stabilise against), check for this case at the outset, and update
> vma_has_anon_rmap() to not explicitly need to check for this.
> 
> This also, importantly, avoids reading any VMA fields if CONFIG_DEBUG_VM is
> not set.
> 
> Target the commit which introduced vma_has_anon_rmap() for the fix, as this
> is where the issue started manifesting in practice.
> 
> Fixes: 38564710f443 ("mm: implement and use vma_has_anon_rmap(), silence KCSAN")
> Reported-by: Seunghyun An <san55@wisc.edu>
> Closes: https://lore.kernel.org/linux-mm/20261110000002.1834454-1-san55@wisc.edu/
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Reviewed-by: Liam R. Howlett (Google) <liam@infradead.org>

> ---
> Note that vma_is_anon_rmap() was introduced in 7.4, so no need for a cc: stable,
> just needs to be sent to Linus as a hotfix. Thanks!
> ---
>  include/linux/mmap_lock.h | 19 ++++++++++++++-----
>  mm/vma.h                  |  3 +--
>  2 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> index 9ccbbcbd7aca..9c3a9c6525c1 100644
> --- a/include/linux/mmap_lock.h
> +++ b/include/linux/mmap_lock.h
> @@ -408,6 +408,11 @@ static inline void vma_assert_locked(const struct vm_area_struct *vma)
>  	vma_assert_write_locked(vma);
>  }
>  
> +static inline bool vma_is_attached(const struct vm_area_struct *vma)
> +{
> +	return refcount_read(&vma->vm_refcnt);
> +}
> +
>  /**
>   * vma_assert_stabilised() - assert that this VMA cannot be changed from
>   * underneath us either by having a VMA or mmap lock held.
> @@ -418,9 +423,18 @@ static inline void vma_assert_locked(const struct vm_area_struct *vma)
>   *
>   * With lockdep disabled we may sometimes race with other threads acquiring the
>   * mmap read lock simultaneous with our VMA read lock.
> + *
> + * Assert only if CONFIG_DEBUG_VM is set.
>   */
>  static inline void vma_assert_stabilised(const struct vm_area_struct *vma)
>  {
> +	if (!IS_ENABLED(CONFIG_DEBUG_VM))
> +		return;
> +
> +	/* Detached VMAs are trivially stabilised. */
> +	if (!vma_is_attached(vma))
> +		return;
> +
>  	/*
>  	 * If another thread owns an mmap lock, it may go away at any time, and
>  	 * thus is no guarantee of stability.
> @@ -454,11 +468,6 @@ static inline void vma_assert_stabilised(const struct vm_area_struct *vma)
>  	vma_assert_locked(vma);
>  }
>  
> -static inline bool vma_is_attached(const struct vm_area_struct *vma)
> -{
> -	return refcount_read(&vma->vm_refcnt);
> -}
> -
>  /*
>   * WARNING: to avoid racing with vma_mark_attached()/vma_mark_detached(), these
>   * assertions should be made either under mmap_write_lock or when the object
> diff --git a/mm/vma.h b/mm/vma.h
> index 3fc3b47a954b..05300badd70b 100644
> --- a/mm/vma.h
> +++ b/mm/vma.h
> @@ -278,8 +278,7 @@ static inline pgoff_t vmg_end_pgoff(const struct vma_merge_struct *vmg)
>   */
>  static inline bool vma_has_anon_rmap(const struct vm_area_struct *vma)
>  {
> -	if (vma_is_attached(vma))
> -		vma_assert_stabilised(vma);
> +	vma_assert_stabilised(vma);
>  	/* KCSAN gets confused about the optimistic check. Silence it. */
>  	return data_race(vma->anon_vma);
>  }
> 
> ---
> base-commit: 93d7a816a2df3e907e2719bd12b0073f1285fc5c
> change-id: 20261110-vma-fixup-debug-asserts-2d327fe27f8c
> 
> Cheers,
> -- 
> Lorenzo Stoakes (ARM) <ljs@kernel.org>
> 

Re: [PATCH] mm/vma: predicate vma_assert_stabilised() on CONFIG_DEBUG_VM

[Pedro Falcato]

On Tue, Nov 10, 2026 at 03:10:40PM +0000, Lorenzo Stoakes (ARM) wrote:
> I introduced commit 17fd82c3abe0 ("mm/vma: add and use
> vma_assert_stabilised()") with the intent of picking up programmatic bugs
> when debugging mm code, but kept the general practice of asserting even
> when CONFIG_DEBUG_VM is not set in line with existing code.
> 
> However, it turns out doing so can cause performance regressions,
> especially when asserted in vma_has_anon_rmap() which is referenced in a
> number of hotpaths.
> 
> Since the intent was always a debug-only assertion, explicitly check for
> CONFIG_DEBUG_VM and do not assert if it isn't set.
> 
> Really all of the assert paths need to be predicated on this, but start
> with this one only, out of caution.
> 
> Additionally, since a detached VMA is trivially stabilised (nothing to
> stabilise against), check for this case at the outset, and update
> vma_has_anon_rmap() to not explicitly need to check for this.
> 
> This also, importantly, avoids reading any VMA fields if CONFIG_DEBUG_VM is
> not set.
> 
> Target the commit which introduced vma_has_anon_rmap() for the fix, as this
> is where the issue started manifesting in practice.
> 
> Fixes: 38564710f443 ("mm: implement and use vma_has_anon_rmap(), silence KCSAN")
> Reported-by: Seunghyun An <san55@wisc.edu>
> Closes: https://lore.kernel.org/linux-mm/20261110000002.1834454-1-san55@wisc.edu/
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Reviewed-by: Pedro Falcato <pfalcato@suse.de>

-- 
Pedro

Re: [PATCH] mm/vma: predicate vma_assert_stabilised() on CONFIG_DEBUG_VM

[Lorenzo Stoakes (ARM)]

Andrew - could you update as below?

Thanks!

On Tue, Nov 10, 2026 at 03:10:40PM +0000, Lorenzo Stoakes (ARM) wrote:
[ ... ]
> diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> index 9ccbbcbd7aca..9c3a9c6525c1 100644
> --- a/include/linux/mmap_lock.h
> +++ b/include/linux/mmap_lock.h
[ ... ]
>  /**
>   * vma_assert_stabilised() - assert that this VMA cannot be changed from
>   * underneath us either by having a VMA or mmap lock held.
> @@ -418,9 +423,18 @@ static inline void vma_assert_locked(const struct vm_area_struct *vma)
>   *
>   * With lockdep disabled we may sometimes race with other threads acquiring the
>   * mmap read lock simultaneous with our VMA read lock.
> + *
> + * Assert only if CONFIG_DEBUG_VM is set.
>   */
>  static inline void vma_assert_stabilised(const struct vm_area_struct *vma)
>  {
> +	if (!IS_ENABLED(CONFIG_DEBUG_VM))

Actually I think this should be:

	if (!IS_ENABLED(CONFIG_DEBUG_VM) || !IS_ENABLED(CONFIG_LOCKDEP))

So we keep the lockdep asserts going regardless if lockdep is enabled (you'd
expect most users to enable both together but still :)

> +		return;

--
Cheers, Lorenzo

Re: [PATCH] mm/vma: predicate vma_assert_stabilised() on CONFIG_DEBUG_VM

[Lorenzo Stoakes (ARM)]

On Tue, Nov 10, 2026 at 04:07:48PM +0000, Lorenzo Stoakes (ARM) wrote:
> Andrew - could you update as below?
>
> Thanks!
>
> On Tue, Nov 10, 2026 at 03:10:40PM +0000, Lorenzo Stoakes (ARM) wrote:
> [ ... ]
> > diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> > index 9ccbbcbd7aca..9c3a9c6525c1 100644
> > --- a/include/linux/mmap_lock.h
> > +++ b/include/linux/mmap_lock.h
> [ ... ]
> >  /**
> >   * vma_assert_stabilised() - assert that this VMA cannot be changed from
> >   * underneath us either by having a VMA or mmap lock held.
> > @@ -418,9 +423,18 @@ static inline void vma_assert_locked(const struct vm_area_struct *vma)
> >   *
> >   * With lockdep disabled we may sometimes race with other threads acquiring the
> >   * mmap read lock simultaneous with our VMA read lock.
> > + *
> > + * Assert only if CONFIG_DEBUG_VM is set.

Also this should say:

'Assert only if CONFIG_DEBUG_VM or CONFIG_LOCKDEP is set.'

> >   */
> >  static inline void vma_assert_stabilised(const struct vm_area_struct *vma)
> >  {
> > +	if (!IS_ENABLED(CONFIG_DEBUG_VM))
>
> Actually I think this should be:
>
> 	if (!IS_ENABLED(CONFIG_DEBUG_VM) || !IS_ENABLED(CONFIG_LOCKDEP))

Sorry this should be...

	if (!IS_ENABLED(CONFIG_DEBUG_VM) && !IS_ENABLED(CONFIG_LOCKDEP))

(I have a bit of a fever today, can you tell? :P)

Thanks!

--
Cheers, Lorenzo

Re: [PATCH] mm/vma: predicate vma_assert_stabilised() on CONFIG_DEBUG_VM

[Lorenzo Stoakes (ARM)]

On Tue, Nov 10, 2026 at 04:18:35PM +0000, Lorenzo Stoakes (ARM) wrote:
> On Tue, Nov 10, 2026 at 04:07:48PM +0000, Lorenzo Stoakes (ARM) wrote:
> > Andrew - could you update as below?
> >
> > Thanks!
> >
> > On Tue, Nov 10, 2026 at 03:10:40PM +0000, Lorenzo Stoakes (ARM) wrote:
> > [ ... ]
> > > diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> > > index 9ccbbcbd7aca..9c3a9c6525c1 100644
> > > --- a/include/linux/mmap_lock.h
> > > +++ b/include/linux/mmap_lock.h
> > [ ... ]
> > >  /**
> > >   * vma_assert_stabilised() - assert that this VMA cannot be changed from
> > >   * underneath us either by having a VMA or mmap lock held.
> > > @@ -418,9 +423,18 @@ static inline void vma_assert_locked(const struct vm_area_struct *vma)
> > >   *
> > >   * With lockdep disabled we may sometimes race with other threads acquiring the
> > >   * mmap read lock simultaneous with our VMA read lock.
> > > + *
> > > + * Assert only if CONFIG_DEBUG_VM is set.
>
> Also this should say:
>
> 'Assert only if CONFIG_DEBUG_VM or CONFIG_LOCKDEP is set.'
>
> > >   */
> > >  static inline void vma_assert_stabilised(const struct vm_area_struct *vma)
> > >  {
> > > +	if (!IS_ENABLED(CONFIG_DEBUG_VM))
> >
> > Actually I think this should be:
> >
> > 	if (!IS_ENABLED(CONFIG_DEBUG_VM) || !IS_ENABLED(CONFIG_LOCKDEP))
>
> Sorry this should be...
>
> 	if (!IS_ENABLED(CONFIG_DEBUG_VM) && !IS_ENABLED(CONFIG_LOCKDEP))
>
> (I have a bit of a fever today, can you tell? :P)
>
> Thanks!

OK OK actually. Sorry. The change isn't required, it's fine :P

Please leave the patch as it is.

If !CONFIG_DEBUG_VM but CONFIG_LOCKDEP we don't actually assert anything.

Since this:

	if (IS_ENABLED(CONFIG_LOCKDEP)) {
		if (lockdep_is_held(&vma->vm_mm->mmap_lock))
			return;
	}

Is the only thing that would run in the functino and early-exit if the condition
is hit.

Then it defers to vma_assert_locked() which then reaches:

	if (IS_ENABLED(CONFIG_LOCKDEP)) {
		if (!lock_is_held(__vma_lockdep_map(vma)))
			vma_assert_write_locked(vma);
		return;
	}

And vma_assert_write_locked() uses VM_WARN_ON_ONCE_VMA().

(flu vaccine on sunday, seems to have impacted my brain :P)

--
Cheers, Lorenzo

RE: [PATCH] mm/vma: predicate vma_assert_stabilised() on CONFIG_DEBUG_VM

[Seunghyun An]

On Tue, Nov 10, 2026 at 03:10:40PM +0000, Lorenzo Stoakes (ARM) wrote:
> I introduced commit 17fd82c3abe0 ("mm/vma: add and use
> vma_assert_stabilised()") with the intent of picking up programmatic bugs

Tested on an AMD EPYC 9634 (84 cores, 12 CCDs), CONFIG_DEBUG_VM=n.
Applied on d48723711bae; base and patched were measured on the same day.
Throughput figures are in faults/s; JVM pretouch time is in ms.

                              base      patched
  shared-VMA faults, 7 CCDs   1.776M    1.917M   +7.9%
  shared-VMA faults, 12 CCDs  2.303M    2.431M   +5.6%
  pft, 7 CCDs                 1.910M    2.126M  +11.3%
  pft, 84 cores               4.201M    5.028M  +19.7%
  JVM G1 pretouch (32G)       1877 ms   1672 ms -10.9%

Tested-by: Seunghyun An <san55@wisc.edu>

As a follow-up, I also checked whether avoiding the __vm_anon_pgoff_hi
read in __folio_set_anon() provides any further benefit. For comparison,
I tested my earlier patch 1 on top, which computes the index from
vm_pgoff for anonymous VMAs and avoids that read:

                              this patch  + patch 1
  shared-VMA faults, 12 CCDs  2.431M      2.941M   +21%
  pft, 12 CCDs                2.434M      3.474M   +43%

With pft on 12 CCDs, this patch alone is essentially unchanged from
base (2.442M faults/s). The additional improvement with patch 1 suggests
that avoiding the __vm_anon_pgoff_hi read may be worth investigating on
CPUs with high coherency costs between L3 cache domains.

Happy to run anything you'd like to try.

Thanks,
Seunghyun

Re: [PATCH] mm/vma: predicate vma_assert_stabilised() on CONFIG_DEBUG_VM

[Lorenzo Stoakes (ARM)]

On Tue, Nov 10, 2026 at 05:32:17PM +0000, Seunghyun An wrote:
> On Tue, Nov 10, 2026 at 03:10:40PM +0000, Lorenzo Stoakes (ARM) wrote:
> > I introduced commit 17fd82c3abe0 ("mm/vma: add and use
> > vma_assert_stabilised()") with the intent of picking up programmatic bugs
>
> Tested on an AMD EPYC 9634 (84 cores, 12 CCDs), CONFIG_DEBUG_VM=n.
> Applied on d48723711bae; base and patched were measured on the same day.
> Throughput figures are in faults/s; JVM pretouch time is in ms.
>
>                               base      patched
>   shared-VMA faults, 7 CCDs   1.776M    1.917M   +7.9%
>   shared-VMA faults, 12 CCDs  2.303M    2.431M   +5.6%
>   pft, 7 CCDs                 1.910M    2.126M  +11.3%
>   pft, 84 cores               4.201M    5.028M  +19.7%
>   JVM G1 pretouch (32G)       1877 ms   1672 ms -10.9%
>
> Tested-by: Seunghyun An <san55@wisc.edu>

Thanks!

>
> As a follow-up, I also checked whether avoiding the __vm_anon_pgoff_hi
> read in __folio_set_anon() provides any further benefit. For comparison,
> I tested my earlier patch 1 on top, which computes the index from
> vm_pgoff for anonymous VMAs and avoids that read:
>
>                               this patch  + patch 1
>   shared-VMA faults, 12 CCDs  2.431M      2.941M   +21%
>   pft, 12 CCDs                2.434M      3.474M   +43%
>
> With pft on 12 CCDs, this patch alone is essentially unchanged from
> base (2.442M faults/s). The additional improvement with patch 1 suggests
> that avoiding the __vm_anon_pgoff_hi read may be worth investigating on
> CPUs with high coherency costs between L3 cache domains.

OK that suggests there are cases other than the one you mentioned that have
an issue.

I'm disinclined to do something like that patch, as it's rather a hack.

Cna you give me the workload you're actually exercising here? Ideally
something that's easy for me to run + analyse locally.

I have a zen 5 with 8 CCDs so can probably exercise some stuff over here.

>
> Happy to run anything you'd like to try.

Thanks!

>
> Thanks,
> Seunghyun

--
Cheers, Lorenzo

RE: [PATCH] mm/vma: predicate vma_assert_stabilised() on CONFIG_DEBUG_VM

[Seunghyun An]

On Tue, Nov 10, 2026, Lorenzo Stoakes (ARM) wrote:
> I'm disinclined to do something like that patch, as it's rather a hack.

I agree on this point. I used it here as a probe to measure the 
cost of that read.

> Cna you give me the workload you're actually exercising here? Ideally
> something that's easy for me to run + analyse locally.

Sure. The "shared-VMA faults" numbers come from a small test that boils
down to the program below: one anonymous mapping (a single VMA), one
thread per CPU given on the command line, each thread writing one byte
per page of its own 128M slice and then MADV_DONTNEED'ing the slice, in
a loop.

On your Zen 5 I'd compare one thread per CCD (8 threads, one CPU from
each L3 domain) against 8 threads in a single CCD. perf c2c record on 
the cross-CCD run should show the vm_refcnt line and the loads on it.

For pft, e.g. on the 9634 (7 cores per CCD), one CPU per CCD:

  taskset -c 4,11,18,25,32,39,46,53,60,67,74,81 ./pft -a -m 48G -t 12

(shared_vma_faults takes the same CPUs as arguments). This pft revision
does not enumerate sparse CPU masks correctly, so I used this fix
(against b5cfe1b in https://github.com/gormanm/pft):

diff --git a/pft.c b/pft.c
--- a/pft.c
+++ b/pft.c
@@ -317,11 +317,11 @@ cpus_allowed_init(void)
 	 * Populate cpus_allowed[] dense array.
 	 */
 	nr_cpus_allowed = 0;
-	cpus_allowed = calloc(sizeof(int), max_cpus_allowed);
+	cpus_allowed = calloc(sizeof(int), numa_num_possible_cpus());
 	if (!cpus_allowed)
 		perrorx("Can't allocate cpus_allowed array");
 
-	for (i=0; i < max_cpus_allowed; ++i) {
+	for (i=0; i < numa_num_possible_cpus(); ++i) {
 		if (cpu_allowed(i)) {
 			max_cpu_allowed = i;
 			cpus_allowed[nr_cpus_allowed++] = i;

--- shared_vma_faults.c ---
// Threads fault disjoint slices of ONE anonymous VMA concurrently.
// Thread i is pinned to the i-th CPU given and loops: write one byte per
// 4K page of its slice, then MADV_DONTNEED the slice. THP is disabled on
// the mapping, so every write to a fresh page is one 4K fault.
// Prints faults/s.
//
// Build: gcc -O2 -pthread -o shared_vma_faults shared_vma_faults.c
// Run:   ./shared_vma_faults <seconds> <cpu> [<cpu> ...]
#define _GNU_SOURCE
#include <pthread.h>
#include <sched.h>
#include <stdatomic.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>
#include <time.h>
#include <unistd.h>

#define SLICE (128UL << 20)	/* per-thread slice */
#define MAXT 512

static char *base;
static long pgsz;
static atomic_int go, stop;
static pthread_barrier_t ready;
static unsigned long faults[MAXT];

struct arg { int idx, cpu, nthreads; };

static void *worker(void *p)
{
	struct arg *a = p;
	cpu_set_t set;
	char *s = base + (unsigned long)a->idx * SLICE;
	/* stagger the first pass so threads do not fault and zap in lockstep */
	unsigned long off = SLICE / pgsz * a->idx / a->nthreads * pgsz;
	unsigned long n = 0;
	int err;

	CPU_ZERO(&set);
	CPU_SET(a->cpu, &set);
	err = pthread_setaffinity_np(pthread_self(), sizeof(set), &set);
	if (err) {
		fprintf(stderr, "pthread_setaffinity_np(cpu %d): %s\n",
			a->cpu, strerror(err));
		exit(1);
	}
	pthread_barrier_wait(&ready);
	while (!atomic_load_explicit(&go, memory_order_acquire))
		;
	while (!atomic_load_explicit(&stop, memory_order_relaxed)) {
		for (; off < SLICE; off += pgsz, n++)
			s[off] = 1;
		if (madvise(s, SLICE, MADV_DONTNEED)) {
			perror("madvise(MADV_DONTNEED)");
			exit(1);
		}
		off = 0;
	}
	faults[a->idx] = n;	/* all faults, including the last pass */
	return NULL;
}

int main(int argc, char **argv)
{
	int nthreads = argc - 2, secs, err;
	pthread_t th[MAXT];
	struct arg args[MAXT];
	struct timespec t0, t1;
	unsigned long total = 0;

	if (argc < 3 || nthreads > MAXT) {
		fprintf(stderr, "usage: %s <seconds> <cpu> [<cpu> ...]\n",
			argv[0]);
		return 2;
	}
	secs = atoi(argv[1]);
	if (secs <= 0) {
		fprintf(stderr, "seconds must be > 0\n");
		return 2;
	}
	for (int i = 0; i < nthreads; i++) {
		int cpu = atoi(argv[i + 2]);

		if (cpu < 0 || cpu >= CPU_SETSIZE) {
			fprintf(stderr, "bad cpu %s\n", argv[i + 2]);
			return 2;
		}
	}
	pgsz = sysconf(_SC_PAGESIZE);
	base = mmap(NULL, nthreads * SLICE, PROT_READ | PROT_WRITE,
		    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
	if (base == MAP_FAILED) {
		perror("mmap");
		return 1;
	}
	if (madvise(base, nthreads * SLICE, MADV_NOHUGEPAGE)) {
		perror("madvise(MADV_NOHUGEPAGE)");
		return 1;
	}
	pthread_barrier_init(&ready, NULL, nthreads + 1);
	for (int i = 0; i < nthreads; i++) {
		args[i] = (struct arg){ .idx = i, .cpu = atoi(argv[i + 2]),
					.nthreads = nthreads };
		err = pthread_create(&th[i], NULL, worker, &args[i]);
		if (err) {
			fprintf(stderr, "pthread_create: %s\n", strerror(err));
			return 1;
		}
	}
	pthread_barrier_wait(&ready);	/* all workers pinned */
	clock_gettime(CLOCK_MONOTONIC, &t0);
	atomic_store_explicit(&go, 1, memory_order_release);
	sleep(secs);
	atomic_store(&stop, 1);
	for (int i = 0; i < nthreads; i++)
		pthread_join(th[i], NULL);
	clock_gettime(CLOCK_MONOTONIC, &t1);	/* includes the last pass */
	for (int i = 0; i < nthreads; i++)
		total += faults[i];
	printf("%d threads: %.3f M faults/s\n", nthreads,
	       total / ((t1.tv_sec - t0.tv_sec) +
			(t1.tv_nsec - t0.tv_nsec) / 1e9) / 1e6);
	return 0;
}

Thanks,
Seunghyun

Re: [PATCH] mm/vma: predicate vma_assert_stabilised() on CONFIG_DEBUG_VM

[Pedro Falcato]

On Tue, Nov 10, 2026 at 05:37:27PM +0000, Lorenzo Stoakes (ARM) wrote:
> On Tue, Nov 10, 2026 at 05:32:17PM +0000, Seunghyun An wrote:
> > On Tue, Nov 10, 2026 at 03:10:40PM +0000, Lorenzo Stoakes (ARM) wrote:
> > > I introduced commit 17fd82c3abe0 ("mm/vma: add and use
> > > vma_assert_stabilised()") with the intent of picking up programmatic bugs
> >
> > Tested on an AMD EPYC 9634 (84 cores, 12 CCDs), CONFIG_DEBUG_VM=n.
> > Applied on d48723711bae; base and patched were measured on the same day.
> > Throughput figures are in faults/s; JVM pretouch time is in ms.
> >
> >                               base      patched
> >   shared-VMA faults, 7 CCDs   1.776M    1.917M   +7.9%
> >   shared-VMA faults, 12 CCDs  2.303M    2.431M   +5.6%
> >   pft, 7 CCDs                 1.910M    2.126M  +11.3%
> >   pft, 84 cores               4.201M    5.028M  +19.7%
> >   JVM G1 pretouch (32G)       1877 ms   1672 ms -10.9%
> >
> > Tested-by: Seunghyun An <san55@wisc.edu>
> 
> Thanks!
> 
> >
> > As a follow-up, I also checked whether avoiding the __vm_anon_pgoff_hi
> > read in __folio_set_anon() provides any further benefit. For comparison,
> > I tested my earlier patch 1 on top, which computes the index from
> > vm_pgoff for anonymous VMAs and avoids that read:
> >
> >                               this patch  + patch 1
> >   shared-VMA faults, 12 CCDs  2.431M      2.941M   +21%
> >   pft, 12 CCDs                2.434M      3.474M   +43%
> >
> > With pft on 12 CCDs, this patch alone is essentially unchanged from
> > base (2.442M faults/s). The additional improvement with patch 1 suggests
> > that avoiding the __vm_anon_pgoff_hi read may be worth investigating on
> > CPUs with high coherency costs between L3 cache domains.
> 
> OK that suggests there are cases other than the one you mentioned that have
> an issue.
> 
> I'm disinclined to do something like that patch, as it's rather a hack.
> 
> Cna you give me the workload you're actually exercising here? Ideally
> something that's easy for me to run + analyse locally.
> 
> I have a zen 5 with 8 CCDs so can probably exercise some stuff over here.

FWIW, some thoughts:

I don't think this is worth worrying about. For starters, the vm_refcnt
cacheline will _always_ be a point of contention, and in extreme page
faulting cases (like the benchmarks Senghyun is running) it will be
dirtied to hell. Say you want to naively get this refcount its own cacheline.
Ok, fine. You're now wasting 56 bytes of a VMA just for padding. Can we fit
more stuff in there? Ehh, kind of, but not really. In theory, perhaps struct
{} shared or the anon_vma_chain that can sometimes change. But those aren't
written to often anyway (certainly not the AVC).

On the other hand, can we get rid of this contention completely? Probably!
But also maybe not. You could try and replace the vm_refcnt with a percpu
counter. That could work, but the percpu allocator does not scale. That would
need to be addressed first.

Theoretically, were per-vma locking structured differently, you could try and
resolve page faults fully under RCU, without needing to write anywhere. It's
a nice thought, but atm is super unfeasible.

Finally, since this is a microbenchmark, I'm not sure this is worth thinking
about. Do we have (many? any?) cases where real world benchmarks hammer
page faults hard (especially on one single VMA) enough where you can see
cacheline contention? I would wager that's not very common.

-- 
Pedro