[PATCH v3 00/15] perf: Convert mmap() related reference counts to refcount_t

[PATCH v3 00/15] perf: Convert mmap() related reference counts to refcount_t

[Peter Zijlstra]

Took over the series from Thomas; much thanks to him for cleaning this up.

The recently fixed reference count leaks could have been detected by using
refcount_t and refcount_t would have mitigated the potential overflow at
least.

It turned out that converting the code as is does not work as the
allocation code ends up doing a refcount_inc() for the first allocation,
which causes refcount_t sanity checks to emit a UAF warning.

The reason is that the code is sharing functionality at the wrong level and
ends up being overly complicated for no reason. That's what inevitable led
to the refcount leak problems.

Address this by splitting the ringbuffer and the AUX buffer mapping and
allocation parts out into seperate functions, which handle the reference
counts in a sane way.

That not only simplifies the code and makes it halfways comprehensible, but
also allows to convert the mmap() related reference counts to refcount_t.

It survives lightweight testing with perf and passes the perf/mmap
selftest.

Also available at:

  git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core

---
v2: https://lkml.kernel.org/r/20250811123458.050061356@linutronix.de

Changes vs v2:
 - replaced patches 4,5 with fine grained steps

[PATCH v3 01/15] perf: Remove redundant condition for AUX buffer size

[Peter Zijlstra]

From: Thomas Gleixner <tglx@linutronix.de>

It is already checked whether the VMA size is the same as
nr_pages * PAGE_SIZE, so later checking both:

      aux_size == vma_size && aux_size == nr_pages * PAGE_SIZE

is redundant. Remove the vma_size check as nr_pages is what is actually
used in the allocation function. That prepares for splitting out the buffer
allocation into seperate functions, so that only nr_pages needs to be
handed in.

No functional change.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lkml.kernel.org/r/20250811070620.389961756@linutronix.de
---
 kernel/events/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7043,7 +7043,7 @@ static int perf_mmap(struct file *file,
 		if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
 			goto aux_unlock;
 
-		if (aux_size != vma_size || aux_size != nr_pages * PAGE_SIZE)
+		if (aux_size != nr_pages * PAGE_SIZE)
 			goto aux_unlock;
 
 		/* already mapped with a different size */

Re: [PATCH v3 01/15] perf: Remove redundant condition for AUX buffer size

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:38:59PM +0200, Peter Zijlstra wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> It is already checked whether the VMA size is the same as
> nr_pages * PAGE_SIZE, so later checking both:
>
>       aux_size == vma_size && aux_size == nr_pages * PAGE_SIZE
>
> is redundant. Remove the vma_size check as nr_pages is what is actually
> used in the allocation function. That prepares for splitting out the buffer
> allocation into seperate functions, so that only nr_pages needs to be

NIT: still get seperate -> separate typo there. Not a big deal obviously :)

> handed in.
>
> No functional change.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Link: https://lkml.kernel.org/r/20250811070620.389961756@linutronix.de
> ---
>  kernel/events/core.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7043,7 +7043,7 @@ static int perf_mmap(struct file *file,
>  		if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
>  			goto aux_unlock;
>
> -		if (aux_size != vma_size || aux_size != nr_pages * PAGE_SIZE)
> +		if (aux_size != nr_pages * PAGE_SIZE)
>  			goto aux_unlock;
>
>  		/* already mapped with a different size */
>
>

[tip: perf/core] perf: Remove redundant condition for AUX buffer size

[tip-bot2 for Thomas Gleixner]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     e8c4f6ee8eeed8e02800bed6afb9aa22fc3476a1
Gitweb:        https://git.kernel.org/tip/e8c4f6ee8eeed8e02800bed6afb9aa22fc3476a1
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Tue, 12 Aug 2025 12:38:59 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:12:58 +02:00

perf: Remove redundant condition for AUX buffer size

It is already checked whether the VMA size is the same as
nr_pages * PAGE_SIZE, so later checking both:

      aux_size == vma_size && aux_size == nr_pages * PAGE_SIZE

is redundant. Remove the vma_size check as nr_pages is what is actually
used in the allocation function. That prepares for splitting out the buffer
allocation into separate functions, so that only nr_pages needs to be
handed in.

No functional change.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104018.424519320@infradead.org
---
 kernel/events/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 8060c28..eea3a7d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7043,7 +7043,7 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
 			goto aux_unlock;
 
-		if (aux_size != vma_size || aux_size != nr_pages * PAGE_SIZE)
+		if (aux_size != nr_pages * PAGE_SIZE)
 			goto aux_unlock;
 
 		/* already mapped with a different size */

[PATCH v3 02/15] perf: Split out mlock limit handling

[Peter Zijlstra]

From: Thomas Gleixner <tglx@linutronix.de>

To prepare for splitting the buffer allocation out into seperate functions
for the ring buffer and the AUX buffer, split out mlock limit handling into
a helper function, which can be called from both.

No functional change intended.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lkml.kernel.org/r/20250811070620.463634790@linutronix.de
---
 kernel/events/core.c |   77 +++++++++++++++++++++++++--------------------------
 1 file changed, 38 insertions(+), 39 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6927,17 +6927,49 @@ static int map_range(struct perf_buffer
 	return err;
 }
 
+static bool perf_mmap_calc_limits(struct vm_area_struct *vma, long *user_extra, long *extra)
+{
+	unsigned long user_locked, user_lock_limit, locked, lock_limit;
+	struct user_struct *user = current_user();
+
+	user_lock_limit = sysctl_perf_event_mlock >> (PAGE_SHIFT - 10);
+	/* Increase the limit linearly with more CPUs */
+	user_lock_limit *= num_online_cpus();
+
+	user_locked = atomic_long_read(&user->locked_vm);
+
+	/*
+	 * sysctl_perf_event_mlock may have changed, so that
+	 *     user->locked_vm > user_lock_limit
+	 */
+	if (user_locked > user_lock_limit)
+		user_locked = user_lock_limit;
+	user_locked += *user_extra;
+
+	if (user_locked > user_lock_limit) {
+		/*
+		 * charge locked_vm until it hits user_lock_limit;
+		 * charge the rest from pinned_vm
+		 */
+		*extra = user_locked - user_lock_limit;
+		*user_extra -= *extra;
+	}
+
+	lock_limit = rlimit(RLIMIT_MEMLOCK);
+	lock_limit >>= PAGE_SHIFT;
+	locked = atomic64_read(&vma->vm_mm->pinned_vm) + *extra;
+
+	return locked <= lock_limit || !perf_is_paranoid() || capable(CAP_IPC_LOCK);
+}
+
 static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct perf_event *event = file->private_data;
-	unsigned long user_locked, user_lock_limit;
 	struct user_struct *user = current_user();
+	unsigned long vma_size, nr_pages;
+	long user_extra = 0, extra = 0;
 	struct mutex *aux_mutex = NULL;
 	struct perf_buffer *rb = NULL;
-	unsigned long locked, lock_limit;
-	unsigned long vma_size;
-	unsigned long nr_pages;
-	long user_extra = 0, extra = 0;
 	int ret, flags = 0;
 	mapped_f mapped;
 
@@ -7063,38 +7093,7 @@ static int perf_mmap(struct file *file,
 		}
 	}
 
-	user_lock_limit = sysctl_perf_event_mlock >> (PAGE_SHIFT - 10);
-
-	/*
-	 * Increase the limit linearly with more CPUs:
-	 */
-	user_lock_limit *= num_online_cpus();
-
-	user_locked = atomic_long_read(&user->locked_vm);
-
-	/*
-	 * sysctl_perf_event_mlock may have changed, so that
-	 *     user->locked_vm > user_lock_limit
-	 */
-	if (user_locked > user_lock_limit)
-		user_locked = user_lock_limit;
-	user_locked += user_extra;
-
-	if (user_locked > user_lock_limit) {
-		/*
-		 * charge locked_vm until it hits user_lock_limit;
-		 * charge the rest from pinned_vm
-		 */
-		extra = user_locked - user_lock_limit;
-		user_extra -= extra;
-	}
-
-	lock_limit = rlimit(RLIMIT_MEMLOCK);
-	lock_limit >>= PAGE_SHIFT;
-	locked = atomic64_read(&vma->vm_mm->pinned_vm) + extra;
-
-	if ((locked > lock_limit) && perf_is_paranoid() &&
-		!capable(CAP_IPC_LOCK)) {
+	if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
 		ret = -EPERM;
 		goto unlock;
 	}

Re: [PATCH v3 02/15] perf: Split out mlock limit handling

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:00PM +0200, Peter Zijlstra wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> To prepare for splitting the buffer allocation out into seperate functions

NIT: Same comment as 1/2, seperate -> separate, again doesn't hugely matter
but just FYI!

> for the ring buffer and the AUX buffer, split out mlock limit handling into
> a helper function, which can be called from both.
>
> No functional change intended.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Link: https://lkml.kernel.org/r/20250811070620.463634790@linutronix.de
> ---
>  kernel/events/core.c |   77 +++++++++++++++++++++++++--------------------------
>  1 file changed, 38 insertions(+), 39 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6927,17 +6927,49 @@ static int map_range(struct perf_buffer
>  	return err;
>  }
>
> +static bool perf_mmap_calc_limits(struct vm_area_struct *vma, long *user_extra, long *extra)
> +{
> +	unsigned long user_locked, user_lock_limit, locked, lock_limit;
> +	struct user_struct *user = current_user();
> +
> +	user_lock_limit = sysctl_perf_event_mlock >> (PAGE_SHIFT - 10);
> +	/* Increase the limit linearly with more CPUs */
> +	user_lock_limit *= num_online_cpus();
> +
> +	user_locked = atomic_long_read(&user->locked_vm);
> +
> +	/*
> +	 * sysctl_perf_event_mlock may have changed, so that
> +	 *     user->locked_vm > user_lock_limit
> +	 */
> +	if (user_locked > user_lock_limit)
> +		user_locked = user_lock_limit;
> +	user_locked += *user_extra;
> +
> +	if (user_locked > user_lock_limit) {
> +		/*
> +		 * charge locked_vm until it hits user_lock_limit;
> +		 * charge the rest from pinned_vm
> +		 */
> +		*extra = user_locked - user_lock_limit;
> +		*user_extra -= *extra;
> +	}
> +
> +	lock_limit = rlimit(RLIMIT_MEMLOCK);
> +	lock_limit >>= PAGE_SHIFT;
> +	locked = atomic64_read(&vma->vm_mm->pinned_vm) + *extra;
> +
> +	return locked <= lock_limit || !perf_is_paranoid() || capable(CAP_IPC_LOCK);
> +}
> +
>  static int perf_mmap(struct file *file, struct vm_area_struct *vma)
>  {
>  	struct perf_event *event = file->private_data;
> -	unsigned long user_locked, user_lock_limit;
>  	struct user_struct *user = current_user();
> +	unsigned long vma_size, nr_pages;
> +	long user_extra = 0, extra = 0;
>  	struct mutex *aux_mutex = NULL;
>  	struct perf_buffer *rb = NULL;
> -	unsigned long locked, lock_limit;
> -	unsigned long vma_size;
> -	unsigned long nr_pages;
> -	long user_extra = 0, extra = 0;
>  	int ret, flags = 0;
>  	mapped_f mapped;
>
> @@ -7063,38 +7093,7 @@ static int perf_mmap(struct file *file,
>  		}
>  	}
>
> -	user_lock_limit = sysctl_perf_event_mlock >> (PAGE_SHIFT - 10);
> -
> -	/*
> -	 * Increase the limit linearly with more CPUs:
> -	 */
> -	user_lock_limit *= num_online_cpus();
> -
> -	user_locked = atomic_long_read(&user->locked_vm);
> -
> -	/*
> -	 * sysctl_perf_event_mlock may have changed, so that
> -	 *     user->locked_vm > user_lock_limit
> -	 */
> -	if (user_locked > user_lock_limit)
> -		user_locked = user_lock_limit;
> -	user_locked += user_extra;
> -
> -	if (user_locked > user_lock_limit) {
> -		/*
> -		 * charge locked_vm until it hits user_lock_limit;
> -		 * charge the rest from pinned_vm
> -		 */
> -		extra = user_locked - user_lock_limit;
> -		user_extra -= extra;
> -	}
> -
> -	lock_limit = rlimit(RLIMIT_MEMLOCK);
> -	lock_limit >>= PAGE_SHIFT;
> -	locked = atomic64_read(&vma->vm_mm->pinned_vm) + extra;
> -
> -	if ((locked > lock_limit) && perf_is_paranoid() &&
> -		!capable(CAP_IPC_LOCK)) {
> +	if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
>  		ret = -EPERM;
>  		goto unlock;
>  	}
>
>

[tip: perf/core] perf: Split out mlock limit handling

[tip-bot2 for Thomas Gleixner]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     81e026ca47b386e4213c1beff069038a3ba8bb76
Gitweb:        https://git.kernel.org/tip/81e026ca47b386e4213c1beff069038a3ba8bb76
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Tue, 12 Aug 2025 12:39:00 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:12:58 +02:00

perf: Split out mlock limit handling

To prepare for splitting the buffer allocation out into separate functions
for the ring buffer and the AUX buffer, split out mlock limit handling into
a helper function, which can be called from both.

No functional change intended.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104018.541975109@infradead.org
---
 kernel/events/core.c | 75 +++++++++++++++++++++----------------------
 1 file changed, 38 insertions(+), 37 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index eea3a7d..f629901 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6927,17 +6927,49 @@ static int map_range(struct perf_buffer *rb, struct vm_area_struct *vma)
 	return err;
 }
 
+static bool perf_mmap_calc_limits(struct vm_area_struct *vma, long *user_extra, long *extra)
+{
+	unsigned long user_locked, user_lock_limit, locked, lock_limit;
+	struct user_struct *user = current_user();
+
+	user_lock_limit = sysctl_perf_event_mlock >> (PAGE_SHIFT - 10);
+	/* Increase the limit linearly with more CPUs */
+	user_lock_limit *= num_online_cpus();
+
+	user_locked = atomic_long_read(&user->locked_vm);
+
+	/*
+	 * sysctl_perf_event_mlock may have changed, so that
+	 *     user->locked_vm > user_lock_limit
+	 */
+	if (user_locked > user_lock_limit)
+		user_locked = user_lock_limit;
+	user_locked += *user_extra;
+
+	if (user_locked > user_lock_limit) {
+		/*
+		 * charge locked_vm until it hits user_lock_limit;
+		 * charge the rest from pinned_vm
+		 */
+		*extra = user_locked - user_lock_limit;
+		*user_extra -= *extra;
+	}
+
+	lock_limit = rlimit(RLIMIT_MEMLOCK);
+	lock_limit >>= PAGE_SHIFT;
+	locked = atomic64_read(&vma->vm_mm->pinned_vm) + *extra;
+
+	return locked <= lock_limit || !perf_is_paranoid() || capable(CAP_IPC_LOCK);
+}
+
 static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct perf_event *event = file->private_data;
-	unsigned long user_locked, user_lock_limit;
 	struct user_struct *user = current_user();
+	unsigned long vma_size, nr_pages;
+	long user_extra = 0, extra = 0;
 	struct mutex *aux_mutex = NULL;
 	struct perf_buffer *rb = NULL;
-	unsigned long locked, lock_limit;
-	unsigned long vma_size;
-	unsigned long nr_pages;
-	long user_extra = 0, extra = 0;
 	int ret, flags = 0;
 	mapped_f mapped;
 
@@ -7063,38 +7095,7 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		}
 	}
 
-	user_lock_limit = sysctl_perf_event_mlock >> (PAGE_SHIFT - 10);
-
-	/*
-	 * Increase the limit linearly with more CPUs:
-	 */
-	user_lock_limit *= num_online_cpus();
-
-	user_locked = atomic_long_read(&user->locked_vm);
-
-	/*
-	 * sysctl_perf_event_mlock may have changed, so that
-	 *     user->locked_vm > user_lock_limit
-	 */
-	if (user_locked > user_lock_limit)
-		user_locked = user_lock_limit;
-	user_locked += user_extra;
-
-	if (user_locked > user_lock_limit) {
-		/*
-		 * charge locked_vm until it hits user_lock_limit;
-		 * charge the rest from pinned_vm
-		 */
-		extra = user_locked - user_lock_limit;
-		user_extra -= extra;
-	}
-
-	lock_limit = rlimit(RLIMIT_MEMLOCK);
-	lock_limit >>= PAGE_SHIFT;
-	locked = atomic64_read(&vma->vm_mm->pinned_vm) + extra;
-
-	if ((locked > lock_limit) && perf_is_paranoid() &&
-		!capable(CAP_IPC_LOCK)) {
+	if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
 		ret = -EPERM;
 		goto unlock;
 	}

[PATCH v3 03/15] perf: Split out VM accounting

[Peter Zijlstra]

From: Thomas Gleixner <tglx@linutronix.de>

Similary to the mlock limit calculation the VM accounting is required for
both the ringbuffer and the AUX buffer allocations.

To prepare for splitting them out into seperate functions, move the
accounting into a helper function.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lkml.kernel.org/r/20250811070620.527392167@linutronix.de
---
 kernel/events/core.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6962,10 +6962,17 @@ static bool perf_mmap_calc_limits(struct
 	return locked <= lock_limit || !perf_is_paranoid() || capable(CAP_IPC_LOCK);
 }
 
+static void perf_mmap_account(struct vm_area_struct *vma, long user_extra, long extra)
+{
+	struct user_struct *user = current_user();
+
+	atomic_long_add(user_extra, &user->locked_vm);
+	atomic64_add(extra, &vma->vm_mm->pinned_vm);
+}
+
 static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct perf_event *event = file->private_data;
-	struct user_struct *user = current_user();
 	unsigned long vma_size, nr_pages;
 	long user_extra = 0, extra = 0;
 	struct mutex *aux_mutex = NULL;
@@ -7136,9 +7143,7 @@ static int perf_mmap(struct file *file,
 
 unlock:
 	if (!ret) {
-		atomic_long_add(user_extra, &user->locked_vm);
-		atomic64_add(extra, &vma->vm_mm->pinned_vm);
-
+		perf_mmap_account(vma, user_extra, extra);
 		atomic_inc(&event->mmap_count);
 	} else if (rb) {
 		/* AUX allocation failed */

Re: [PATCH v3 03/15] perf: Split out VM accounting

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:01PM +0200, Peter Zijlstra wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> Similary to the mlock limit calculation the VM accounting is required for

NIT: Similary -> Similarly

> both the ringbuffer and the AUX buffer allocations.
>
> To prepare for splitting them out into seperate functions, move the

NIT: Seperate -> Separate

And that's the end of the typo nits ;)

> accounting into a helper function.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Link: https://lkml.kernel.org/r/20250811070620.527392167@linutronix.de
> ---
>  kernel/events/core.c |   13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6962,10 +6962,17 @@ static bool perf_mmap_calc_limits(struct
>  	return locked <= lock_limit || !perf_is_paranoid() || capable(CAP_IPC_LOCK);
>  }
>
> +static void perf_mmap_account(struct vm_area_struct *vma, long user_extra, long extra)
> +{
> +	struct user_struct *user = current_user();
> +
> +	atomic_long_add(user_extra, &user->locked_vm);
> +	atomic64_add(extra, &vma->vm_mm->pinned_vm);
> +}
> +
>  static int perf_mmap(struct file *file, struct vm_area_struct *vma)
>  {
>  	struct perf_event *event = file->private_data;
> -	struct user_struct *user = current_user();
>  	unsigned long vma_size, nr_pages;
>  	long user_extra = 0, extra = 0;
>  	struct mutex *aux_mutex = NULL;
> @@ -7136,9 +7143,7 @@ static int perf_mmap(struct file *file,
>
>  unlock:
>  	if (!ret) {
> -		atomic_long_add(user_extra, &user->locked_vm);
> -		atomic64_add(extra, &vma->vm_mm->pinned_vm);
> -
> +		perf_mmap_account(vma, user_extra, extra);
>  		atomic_inc(&event->mmap_count);
>  	} else if (rb) {
>  		/* AUX allocation failed */
>
>

[tip: perf/core] perf: Split out VM accounting

[tip-bot2 for Thomas Gleixner]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     1ea3e3b0dadc06c5e6c1bdf5312e70ee861b1ba0
Gitweb:        https://git.kernel.org/tip/1ea3e3b0dadc06c5e6c1bdf5312e70ee861b1ba0
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Tue, 12 Aug 2025 12:39:01 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:12:59 +02:00

perf: Split out VM accounting

Similarly to the mlock limit calculation the VM accounting is required for
both the ringbuffer and the AUX buffer allocations.

To prepare for splitting them out into separate functions, move the
accounting into a helper function.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104018.660347811@infradead.org
---
 kernel/events/core.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index f629901..f908471 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6962,10 +6962,17 @@ static bool perf_mmap_calc_limits(struct vm_area_struct *vma, long *user_extra, 
 	return locked <= lock_limit || !perf_is_paranoid() || capable(CAP_IPC_LOCK);
 }
 
+static void perf_mmap_account(struct vm_area_struct *vma, long user_extra, long extra)
+{
+	struct user_struct *user = current_user();
+
+	atomic_long_add(user_extra, &user->locked_vm);
+	atomic64_add(extra, &vma->vm_mm->pinned_vm);
+}
+
 static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct perf_event *event = file->private_data;
-	struct user_struct *user = current_user();
 	unsigned long vma_size, nr_pages;
 	long user_extra = 0, extra = 0;
 	struct mutex *aux_mutex = NULL;
@@ -7136,9 +7143,7 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 
 unlock:
 	if (!ret) {
-		atomic_long_add(user_extra, &user->locked_vm);
-		atomic64_add(extra, &vma->vm_mm->pinned_vm);
-
+		perf_mmap_account(vma, user_extra, extra);
 		atomic_inc(&event->mmap_count);
 	} else if (rb) {
 		/* AUX allocation failed */

[PATCH v3 04/15] perf: Move perf_mmap_calc_limits() into both rb and aux branches

[Peter Zijlstra]

  if (cond) {
    A;
  } else {
    B;
  }
  C;

into

  if (cond) {
    A;
    C;
  } else {
    B;
    C;
  }

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |   26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7054,6 +7054,16 @@ static int perf_mmap(struct file *file,
 			ring_buffer_attach(event, NULL);
 		}
 
+		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
+			ret = -EPERM;
+			goto unlock;
+		}
+
+		WARN_ON(!rb && event->rb);
+
+		if (vma->vm_flags & VM_WRITE)
+			flags |= RING_BUFFER_WRITABLE;
+
 	} else {
 		/*
 		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
@@ -7100,17 +7110,17 @@ static int perf_mmap(struct file *file,
 			ret = 0;
 			goto unlock;
 		}
-	}
 
-	if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
-		ret = -EPERM;
-		goto unlock;
-	}
+		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
+			ret = -EPERM;
+			goto unlock;
+		}
 
-	WARN_ON(!rb && event->rb);
+		WARN_ON(!rb && event->rb);
 
-	if (vma->vm_flags & VM_WRITE)
-		flags |= RING_BUFFER_WRITABLE;
+		if (vma->vm_flags & VM_WRITE)
+			flags |= RING_BUFFER_WRITABLE;
+	}
 
 	if (!rb) {
 		rb = rb_alloc(nr_pages,

Re: [PATCH v3 04/15] perf: Move perf_mmap_calc_limits() into both rb and aux branches

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:02PM +0200, Peter Zijlstra wrote:
>   if (cond) {
>     A;
>   } else {
>     B;
>   }
>   C;
>
> into
>
>   if (cond) {
>     A;
>     C;
>   } else {
>     B;
>     C;
>   }
>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Seems sensible enough, guessing trade off on duplication vs. complexity of
having to keep track of 'C' being in place for both and I assume makes
later refactorings easier :)

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  kernel/events/core.c |   26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7054,6 +7054,16 @@ static int perf_mmap(struct file *file,
>  			ring_buffer_attach(event, NULL);
>  		}
>
> +		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
> +			ret = -EPERM;
> +			goto unlock;
> +		}
> +
> +		WARN_ON(!rb && event->rb);
> +
> +		if (vma->vm_flags & VM_WRITE)
> +			flags |= RING_BUFFER_WRITABLE;
> +
>  	} else {
>  		/*
>  		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
> @@ -7100,17 +7110,17 @@ static int perf_mmap(struct file *file,
>  			ret = 0;
>  			goto unlock;
>  		}
> -	}
>
> -	if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
> -		ret = -EPERM;
> -		goto unlock;
> -	}
> +		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
> +			ret = -EPERM;
> +			goto unlock;
> +		}
>
> -	WARN_ON(!rb && event->rb);
> +		WARN_ON(!rb && event->rb);
>
> -	if (vma->vm_flags & VM_WRITE)
> -		flags |= RING_BUFFER_WRITABLE;
> +		if (vma->vm_flags & VM_WRITE)
> +			flags |= RING_BUFFER_WRITABLE;
> +	}
>
>  	if (!rb) {
>  		rb = rb_alloc(nr_pages,
>
>

[tip: perf/core] perf: Move perf_mmap_calc_limits() into both rb and aux branches

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     86a0a7c59845e7093c9c73a7115c9d86349499d1
Gitweb:        https://git.kernel.org/tip/86a0a7c59845e7093c9c73a7115c9d86349499d1
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 12 Aug 2025 12:39:02 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:12:59 +02:00

perf: Move perf_mmap_calc_limits() into both rb and aux branches

  if (cond) {
    A;
  } else {
    B;
  }
  C;

into

  if (cond) {
    A;
    C;
  } else {
    B;
    C;
  }

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104018.781244099@infradead.org
---
 kernel/events/core.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index f908471..9f19c61 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7054,6 +7054,16 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 			ring_buffer_attach(event, NULL);
 		}
 
+		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
+			ret = -EPERM;
+			goto unlock;
+		}
+
+		WARN_ON(!rb && event->rb);
+
+		if (vma->vm_flags & VM_WRITE)
+			flags |= RING_BUFFER_WRITABLE;
+
 	} else {
 		/*
 		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
@@ -7100,17 +7110,17 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 			ret = 0;
 			goto unlock;
 		}
-	}
 
-	if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
-		ret = -EPERM;
-		goto unlock;
-	}
+		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
+			ret = -EPERM;
+			goto unlock;
+		}
 
-	WARN_ON(!rb && event->rb);
+		WARN_ON(!rb && event->rb);
 
-	if (vma->vm_flags & VM_WRITE)
-		flags |= RING_BUFFER_WRITABLE;
+		if (vma->vm_flags & VM_WRITE)
+			flags |= RING_BUFFER_WRITABLE;
+	}
 
 	if (!rb) {
 		rb = rb_alloc(nr_pages,

[PATCH v3 05/15] perf: Merge consecutive conditionals in perf_mmap()

[Peter Zijlstra]

  if (cond) {
    A;
  } else {
    B;
  }

  if (cond) {
    C;
  } else {
    D;
  }

into:

  if (cond) {
    A;
    C;
  } else {
    B;
    D;
  }

Notably the conditions are not identical in form, but are equivalent.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |   41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7064,6 +7064,25 @@ static int perf_mmap(struct file *file,
 		if (vma->vm_flags & VM_WRITE)
 			flags |= RING_BUFFER_WRITABLE;
 
+		rb = rb_alloc(nr_pages,
+			      event->attr.watermark ? event->attr.wakeup_watermark : 0,
+			      event->cpu, flags);
+
+		if (!rb) {
+			ret = -ENOMEM;
+			goto unlock;
+		}
+
+		atomic_set(&rb->mmap_count, 1);
+		rb->mmap_user = get_current_user();
+		rb->mmap_locked = extra;
+
+		ring_buffer_attach(event, rb);
+
+		perf_event_update_time(event);
+		perf_event_init_userpage(event);
+		perf_event_update_userpage(event);
+		ret = 0;
 	} else {
 		/*
 		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
@@ -7120,29 +7139,7 @@ static int perf_mmap(struct file *file,
 
 		if (vma->vm_flags & VM_WRITE)
 			flags |= RING_BUFFER_WRITABLE;
-	}
 
-	if (!rb) {
-		rb = rb_alloc(nr_pages,
-			      event->attr.watermark ? event->attr.wakeup_watermark : 0,
-			      event->cpu, flags);
-
-		if (!rb) {
-			ret = -ENOMEM;
-			goto unlock;
-		}
-
-		atomic_set(&rb->mmap_count, 1);
-		rb->mmap_user = get_current_user();
-		rb->mmap_locked = extra;
-
-		ring_buffer_attach(event, rb);
-
-		perf_event_update_time(event);
-		perf_event_init_userpage(event);
-		perf_event_update_userpage(event);
-		ret = 0;
-	} else {
 		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
 				   event->attr.aux_watermark, flags);
 		if (!ret) {

Re: [PATCH v3 05/15] perf: Merge consecutive conditionals in perf_mmap()

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:03PM +0200, Peter Zijlstra wrote:
>   if (cond) {
>     A;
>   } else {
>     B;
>   }
>
>   if (cond) {
>     C;
>   } else {
>     D;
>   }
>
> into:
>
>   if (cond) {
>     A;
>     C;
>   } else {
>     B;
>     D;
>   }
>
> Notably the conditions are not identical in form, but are equivalent.

Good to point out.

>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Seems logical and reduces complexity, so LGTM and:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  kernel/events/core.c |   41 +++++++++++++++++++----------------------
>  1 file changed, 19 insertions(+), 22 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7064,6 +7064,25 @@ static int perf_mmap(struct file *file,
>  		if (vma->vm_flags & VM_WRITE)
>  			flags |= RING_BUFFER_WRITABLE;
>
> +		rb = rb_alloc(nr_pages,
> +			      event->attr.watermark ? event->attr.wakeup_watermark : 0,
> +			      event->cpu, flags);
> +
> +		if (!rb) {
> +			ret = -ENOMEM;
> +			goto unlock;
> +		}
> +
> +		atomic_set(&rb->mmap_count, 1);
> +		rb->mmap_user = get_current_user();
> +		rb->mmap_locked = extra;
> +
> +		ring_buffer_attach(event, rb);
> +
> +		perf_event_update_time(event);
> +		perf_event_init_userpage(event);
> +		perf_event_update_userpage(event);
> +		ret = 0;
>  	} else {
>  		/*
>  		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
> @@ -7120,29 +7139,7 @@ static int perf_mmap(struct file *file,
>
>  		if (vma->vm_flags & VM_WRITE)
>  			flags |= RING_BUFFER_WRITABLE;
> -	}
>
> -	if (!rb) {
> -		rb = rb_alloc(nr_pages,
> -			      event->attr.watermark ? event->attr.wakeup_watermark : 0,
> -			      event->cpu, flags);
> -
> -		if (!rb) {
> -			ret = -ENOMEM;
> -			goto unlock;
> -		}
> -
> -		atomic_set(&rb->mmap_count, 1);
> -		rb->mmap_user = get_current_user();
> -		rb->mmap_locked = extra;
> -
> -		ring_buffer_attach(event, rb);
> -
> -		perf_event_update_time(event);
> -		perf_event_init_userpage(event);
> -		perf_event_update_userpage(event);
> -		ret = 0;
> -	} else {
>  		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
>  				   event->attr.aux_watermark, flags);
>  		if (!ret) {
>
>

[tip: perf/core] perf: Merge consecutive conditionals in perf_mmap()

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     3821f258686691cf12bbfc636ab22fa2b049dc86
Gitweb:        https://git.kernel.org/tip/3821f258686691cf12bbfc636ab22fa2b049dc86
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 12 Aug 2025 12:39:03 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:12:59 +02:00

perf: Merge consecutive conditionals in perf_mmap()

  if (cond) {
    A;
  } else {
    B;
  }

  if (cond) {
    C;
  } else {
    D;
  }

into:

  if (cond) {
    A;
    C;
  } else {
    B;
    D;
  }

Notably the conditions are not identical in form, but are equivalent.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104018.900078502@infradead.org
---
 kernel/events/core.c | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 9f19c61..085f36f 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7064,6 +7064,25 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		if (vma->vm_flags & VM_WRITE)
 			flags |= RING_BUFFER_WRITABLE;
 
+		rb = rb_alloc(nr_pages,
+			      event->attr.watermark ? event->attr.wakeup_watermark : 0,
+			      event->cpu, flags);
+
+		if (!rb) {
+			ret = -ENOMEM;
+			goto unlock;
+		}
+
+		atomic_set(&rb->mmap_count, 1);
+		rb->mmap_user = get_current_user();
+		rb->mmap_locked = extra;
+
+		ring_buffer_attach(event, rb);
+
+		perf_event_update_time(event);
+		perf_event_init_userpage(event);
+		perf_event_update_userpage(event);
+		ret = 0;
 	} else {
 		/*
 		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
@@ -7120,29 +7139,7 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 
 		if (vma->vm_flags & VM_WRITE)
 			flags |= RING_BUFFER_WRITABLE;
-	}
 
-	if (!rb) {
-		rb = rb_alloc(nr_pages,
-			      event->attr.watermark ? event->attr.wakeup_watermark : 0,
-			      event->cpu, flags);
-
-		if (!rb) {
-			ret = -ENOMEM;
-			goto unlock;
-		}
-
-		atomic_set(&rb->mmap_count, 1);
-		rb->mmap_user = get_current_user();
-		rb->mmap_locked = extra;
-
-		ring_buffer_attach(event, rb);
-
-		perf_event_update_time(event);
-		perf_event_init_userpage(event);
-		perf_event_update_userpage(event);
-		ret = 0;
-	} else {
 		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
 				   event->attr.aux_watermark, flags);
 		if (!ret) {

[PATCH v3 06/15] perf: Move common code into both rb and aux branches

[Peter Zijlstra]

  if (cond) {
    A;
  } else {
    B;
  }
  C;

into

  if (cond) {
    A;
    C;
  } else {
    B;
    C;
  }

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |   25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7043,6 +7043,8 @@ static int perf_mmap(struct file *file,
 				ret = 0;
 				/* We need the rb to map pages. */
 				rb = event->rb;
+				perf_mmap_account(vma, user_extra, extra);
+				atomic_inc(&event->mmap_count);
 				goto unlock;
 			}
 
@@ -7083,6 +7085,9 @@ static int perf_mmap(struct file *file,
 		perf_event_init_userpage(event);
 		perf_event_update_userpage(event);
 		ret = 0;
+
+		perf_mmap_account(vma, user_extra, extra);
+		atomic_inc(&event->mmap_count);
 	} else {
 		/*
 		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
@@ -7127,11 +7132,12 @@ static int perf_mmap(struct file *file,
 		if (rb_has_aux(rb)) {
 			atomic_inc(&rb->aux_mmap_count);
 			ret = 0;
-			goto unlock;
+			goto aux_success;
 		}
 
 		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
 			ret = -EPERM;
+			atomic_dec(&rb->mmap_count);
 			goto unlock;
 		}
 
@@ -7142,20 +7148,19 @@ static int perf_mmap(struct file *file,
 
 		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
 				   event->attr.aux_watermark, flags);
-		if (!ret) {
-			atomic_set(&rb->aux_mmap_count, 1);
-			rb->aux_mmap_locked = extra;
+		if (ret) {
+			atomic_dec(&rb->mmap_count);
+			goto unlock;
 		}
-	}
 
-unlock:
-	if (!ret) {
+		atomic_set(&rb->aux_mmap_count, 1);
+		rb->aux_mmap_locked = extra;
+aux_success:
 		perf_mmap_account(vma, user_extra, extra);
 		atomic_inc(&event->mmap_count);
-	} else if (rb) {
-		/* AUX allocation failed */
-		atomic_dec(&rb->mmap_count);
 	}
+
+unlock:
 aux_unlock:
 	if (aux_mutex)
 		mutex_unlock(aux_mutex);

Re: [PATCH v3 06/15] perf: Move common code into both rb and aux branches

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:04PM +0200, Peter Zijlstra wrote:
>   if (cond) {
>     A;
>   } else {
>     B;
>   }
>   C;
>
> into
>
>   if (cond) {
>     A;
>     C;
>   } else {
>     B;
>     C;
>   }
>

Hm we're doing more than that here though, we're refactoring other stuff at
the same time.

I guess you're speaking broad strokes here, but maybe worth mentioniing the
tricksy hobbitses around the rb_has_aux() bit.

Anyway LGTM so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  kernel/events/core.c |   25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7043,6 +7043,8 @@ static int perf_mmap(struct file *file,
>  				ret = 0;
>  				/* We need the rb to map pages. */
>  				rb = event->rb;
> +				perf_mmap_account(vma, user_extra, extra);
> +				atomic_inc(&event->mmap_count);
>  				goto unlock;
>  			}
>
> @@ -7083,6 +7085,9 @@ static int perf_mmap(struct file *file,
>  		perf_event_init_userpage(event);
>  		perf_event_update_userpage(event);
>  		ret = 0;
> +
> +		perf_mmap_account(vma, user_extra, extra);
> +		atomic_inc(&event->mmap_count);
>  	} else {
>  		/*
>  		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
> @@ -7127,11 +7132,12 @@ static int perf_mmap(struct file *file,
>  		if (rb_has_aux(rb)) {
>  			atomic_inc(&rb->aux_mmap_count);
>  			ret = 0;
> -			goto unlock;
> +			goto aux_success;
>  		}
>
>  		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
>  			ret = -EPERM;
> +			atomic_dec(&rb->mmap_count);
>  			goto unlock;
>  		}
>
> @@ -7142,20 +7148,19 @@ static int perf_mmap(struct file *file,
>
>  		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
>  				   event->attr.aux_watermark, flags);
> -		if (!ret) {
> -			atomic_set(&rb->aux_mmap_count, 1);
> -			rb->aux_mmap_locked = extra;
> +		if (ret) {

You dropped the 'AUX allocation failed' comment here, but honestly I think
that's fine, it's pretty obvious that's the case given the literal previous
line is you trying the AUX allocation... :)

> +			atomic_dec(&rb->mmap_count);
> +			goto unlock;
>  		}
> -	}
>
> -unlock:
> -	if (!ret) {
> +		atomic_set(&rb->aux_mmap_count, 1);
> +		rb->aux_mmap_locked = extra;
> +aux_success:
>  		perf_mmap_account(vma, user_extra, extra);
>  		atomic_inc(&event->mmap_count);
> -	} else if (rb) {
> -		/* AUX allocation failed */
> -		atomic_dec(&rb->mmap_count);
>  	}
> +
> +unlock:
>  aux_unlock:

Hm, this sucks, I assume this is a temporary thing to reduce churn above
and effective prove that the code paths are equivalently going to the same
place?

Which, given the complexity of the code, and the enormous ease with which
you can miss stuff (from personal experience...!), this is probably a
sensible way of doing it.

>  	if (aux_mutex)
>  		mutex_unlock(aux_mutex);
>
>

Re: [PATCH v3 06/15] perf: Move common code into both rb and aux branches

[Peter Zijlstra]

On Wed, Aug 13, 2025 at 06:55:51AM +0100, Lorenzo Stoakes wrote:
> On Tue, Aug 12, 2025 at 12:39:04PM +0200, Peter Zijlstra wrote:
> >   if (cond) {
> >     A;
> >   } else {
> >     B;
> >   }
> >   C;
> >
> > into
> >
> >   if (cond) {
> >     A;
> >     C;
> >   } else {
> >     B;
> >     C;
> >   }
> >
> 
> Hm we're doing more than that here though, we're refactoring other stuff at
> the same time.
> 
> I guess you're speaking broad strokes here, but maybe worth mentioniing the
> tricksy hobbitses around the rb_has_aux() bit.

Does something like so clarify:

Notably C has a success branch and both A and B have two places for
success. For A (rb case), duplicate the success case because later
patches will result in them no longer being identical. For B (aux
case), share using goto (cleaned up later).

> Anyway LGTM so:
> 
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> 
> > Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> >  kernel/events/core.c |   25 +++++++++++++++----------
> >  1 file changed, 15 insertions(+), 10 deletions(-)
> >
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -7043,6 +7043,8 @@ static int perf_mmap(struct file *file,
> >  				ret = 0;
> >  				/* We need the rb to map pages. */
> >  				rb = event->rb;
> > +				perf_mmap_account(vma, user_extra, extra);
> > +				atomic_inc(&event->mmap_count);
> >  				goto unlock;
> >  			}
> >
> > @@ -7083,6 +7085,9 @@ static int perf_mmap(struct file *file,
> >  		perf_event_init_userpage(event);
> >  		perf_event_update_userpage(event);
> >  		ret = 0;
> > +
> > +		perf_mmap_account(vma, user_extra, extra);
> > +		atomic_inc(&event->mmap_count);
> >  	} else {
> >  		/*
> >  		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
> > @@ -7127,11 +7132,12 @@ static int perf_mmap(struct file *file,
> >  		if (rb_has_aux(rb)) {
> >  			atomic_inc(&rb->aux_mmap_count);
> >  			ret = 0;
> > -			goto unlock;
> > +			goto aux_success;
> >  		}
> >
> >  		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
> >  			ret = -EPERM;
> > +			atomic_dec(&rb->mmap_count);
> >  			goto unlock;
> >  		}
> >
> > @@ -7142,20 +7148,19 @@ static int perf_mmap(struct file *file,
> >
> >  		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
> >  				   event->attr.aux_watermark, flags);
> > -		if (!ret) {
> > -			atomic_set(&rb->aux_mmap_count, 1);
> > -			rb->aux_mmap_locked = extra;
> > +		if (ret) {
> 
> You dropped the 'AUX allocation failed' comment here, but honestly I think
> that's fine, it's pretty obvious that's the case given the literal previous
> line is you trying the AUX allocation... :)

Yes that :-)

Re: [PATCH v3 06/15] perf: Move common code into both rb and aux branches

[Lorenzo Stoakes]

On Wed, Aug 13, 2025 at 10:25:24AM +0200, Peter Zijlstra wrote:
> On Wed, Aug 13, 2025 at 06:55:51AM +0100, Lorenzo Stoakes wrote:
> > On Tue, Aug 12, 2025 at 12:39:04PM +0200, Peter Zijlstra wrote:
> > >   if (cond) {
> > >     A;
> > >   } else {
> > >     B;
> > >   }
> > >   C;
> > >
> > > into
> > >
> > >   if (cond) {
> > >     A;
> > >     C;
> > >   } else {
> > >     B;
> > >     C;
> > >   }
> > >
> >
> > Hm we're doing more than that here though, we're refactoring other stuff at
> > the same time.
> >
> > I guess you're speaking broad strokes here, but maybe worth mentioniing the
> > tricksy hobbitses around the rb_has_aux() bit.
>
> Does something like so clarify:
>
> Notably C has a success branch and both A and B have two places for
> success. For A (rb case), duplicate the success case because later
> patches will result in them no longer being identical. For B (aux
> case), share using goto (cleaned up later).

Sounds great, thanks!

>
> > Anyway LGTM so:
> >
> > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> >
> > > Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > > ---
> > >  kernel/events/core.c |   25 +++++++++++++++----------
> > >  1 file changed, 15 insertions(+), 10 deletions(-)

[snip]

> > > @@ -7142,20 +7148,19 @@ static int perf_mmap(struct file *file,
> > >
> > >  		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
> > >  				   event->attr.aux_watermark, flags);
> > > -		if (!ret) {
> > > -			atomic_set(&rb->aux_mmap_count, 1);
> > > -			rb->aux_mmap_locked = extra;
> > > +		if (ret) {
> >
> > You dropped the 'AUX allocation failed' comment here, but honestly I think
> > that's fine, it's pretty obvious that's the case given the literal previous
> > line is you trying the AUX allocation... :)
>
> Yes that :-)

:)

[tip: perf/core] perf: Move common code into both rb and aux branches

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     4118994b33bb628dd9aeb941c5af6f950f1dea90
Gitweb:        https://git.kernel.org/tip/4118994b33bb628dd9aeb941c5af6f950f1dea90
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 12 Aug 2025 12:39:04 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:12:59 +02:00

perf: Move common code into both rb and aux branches

  if (cond) {
    A;
  } else {
    B;
  }
  C;

into

  if (cond) {
    A;
    C;
  } else {
    B;
    C;
  }

Notably C has a success branch and both A and B have two places for
success. For A (rb case), duplicate the success case because later
patches will result in them no longer being identical. For B (aux
case), share using goto (cleaned up later).

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104019.016252852@infradead.org
---
 kernel/events/core.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 085f36f..dfe09b0 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7043,6 +7043,8 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 				ret = 0;
 				/* We need the rb to map pages. */
 				rb = event->rb;
+				perf_mmap_account(vma, user_extra, extra);
+				atomic_inc(&event->mmap_count);
 				goto unlock;
 			}
 
@@ -7083,6 +7085,9 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		perf_event_init_userpage(event);
 		perf_event_update_userpage(event);
 		ret = 0;
+
+		perf_mmap_account(vma, user_extra, extra);
+		atomic_inc(&event->mmap_count);
 	} else {
 		/*
 		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
@@ -7127,11 +7132,12 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		if (rb_has_aux(rb)) {
 			atomic_inc(&rb->aux_mmap_count);
 			ret = 0;
-			goto unlock;
+			goto aux_success;
 		}
 
 		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
 			ret = -EPERM;
+			atomic_dec(&rb->mmap_count);
 			goto unlock;
 		}
 
@@ -7142,20 +7148,19 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 
 		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
 				   event->attr.aux_watermark, flags);
-		if (!ret) {
-			atomic_set(&rb->aux_mmap_count, 1);
-			rb->aux_mmap_locked = extra;
+		if (ret) {
+			atomic_dec(&rb->mmap_count);
+			goto unlock;
 		}
-	}
 
-unlock:
-	if (!ret) {
+		atomic_set(&rb->aux_mmap_count, 1);
+		rb->aux_mmap_locked = extra;
+aux_success:
 		perf_mmap_account(vma, user_extra, extra);
 		atomic_inc(&event->mmap_count);
-	} else if (rb) {
-		/* AUX allocation failed */
-		atomic_dec(&rb->mmap_count);
 	}
+
+unlock:
 aux_unlock:
 	if (aux_mutex)
 		mutex_unlock(aux_mutex);

[PATCH v3 07/15] perf: Remove redundant aux_unlock label

[Peter Zijlstra]

unlock and aux_unlock are now identical, remove the aux_unlock one.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7095,7 +7095,7 @@ static int perf_mmap(struct file *file,
 
 		rb = event->rb;
 		if (!rb)
-			goto aux_unlock;
+			goto unlock;
 
 		aux_mutex = &rb->aux_mutex;
 		mutex_lock(aux_mutex);
@@ -7104,27 +7104,27 @@ static int perf_mmap(struct file *file,
 		aux_size = READ_ONCE(rb->user_page->aux_size);
 
 		if (aux_offset < perf_data_size(rb) + PAGE_SIZE)
-			goto aux_unlock;
+			goto unlock;
 
 		if (aux_offset != vma->vm_pgoff << PAGE_SHIFT)
-			goto aux_unlock;
+			goto unlock;
 
 		/* already mapped with a different offset */
 		if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
-			goto aux_unlock;
+			goto unlock;
 
 		if (aux_size != nr_pages * PAGE_SIZE)
-			goto aux_unlock;
+			goto unlock;
 
 		/* already mapped with a different size */
 		if (rb_has_aux(rb) && rb->aux_nr_pages != nr_pages)
-			goto aux_unlock;
+			goto unlock;
 
 		if (!is_power_of_2(nr_pages))
-			goto aux_unlock;
+			goto unlock;
 
 		if (!atomic_inc_not_zero(&rb->mmap_count))
-			goto aux_unlock;
+			goto unlock;
 
 		if (rb_has_aux(rb)) {
 			atomic_inc(&rb->aux_mmap_count);
@@ -7158,7 +7158,6 @@ static int perf_mmap(struct file *file,
 	}
 
 unlock:
-aux_unlock:
 	if (aux_mutex)
 		mutex_unlock(aux_mutex);
 	mutex_unlock(&event->mmap_mutex);

Re: [PATCH v3 07/15] perf: Remove redundant aux_unlock label

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:05PM +0200, Peter Zijlstra wrote:
> unlock and aux_unlock are now identical, remove the aux_unlock one.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

And in classic fashion, of course you addressed my comment from the last
patch in the very next one :P

LGTM, so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  kernel/events/core.c |   17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7095,7 +7095,7 @@ static int perf_mmap(struct file *file,
>
>  		rb = event->rb;
>  		if (!rb)
> -			goto aux_unlock;
> +			goto unlock;
>
>  		aux_mutex = &rb->aux_mutex;
>  		mutex_lock(aux_mutex);
> @@ -7104,27 +7104,27 @@ static int perf_mmap(struct file *file,
>  		aux_size = READ_ONCE(rb->user_page->aux_size);
>
>  		if (aux_offset < perf_data_size(rb) + PAGE_SIZE)
> -			goto aux_unlock;
> +			goto unlock;
>
>  		if (aux_offset != vma->vm_pgoff << PAGE_SHIFT)
> -			goto aux_unlock;
> +			goto unlock;
>
>  		/* already mapped with a different offset */
>  		if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
> -			goto aux_unlock;
> +			goto unlock;
>
>  		if (aux_size != nr_pages * PAGE_SIZE)
> -			goto aux_unlock;
> +			goto unlock;
>
>  		/* already mapped with a different size */
>  		if (rb_has_aux(rb) && rb->aux_nr_pages != nr_pages)
> -			goto aux_unlock;
> +			goto unlock;
>
>  		if (!is_power_of_2(nr_pages))
> -			goto aux_unlock;
> +			goto unlock;
>
>  		if (!atomic_inc_not_zero(&rb->mmap_count))
> -			goto aux_unlock;
> +			goto unlock;
>
>  		if (rb_has_aux(rb)) {
>  			atomic_inc(&rb->aux_mmap_count);
> @@ -7158,7 +7158,6 @@ static int perf_mmap(struct file *file,
>  	}
>
>  unlock:
> -aux_unlock:
>  	if (aux_mutex)
>  		mutex_unlock(aux_mutex);
>  	mutex_unlock(&event->mmap_mutex);
>
>

[tip: perf/core] perf: Remove redundant aux_unlock label

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     41b80e1d74bdef5e48ea63d186244b9f6f82a4da
Gitweb:        https://git.kernel.org/tip/41b80e1d74bdef5e48ea63d186244b9f6f82a4da
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 12 Aug 2025 12:39:05 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:13:00 +02:00

perf: Remove redundant aux_unlock label

unlock and aux_unlock are now identical, remove the aux_unlock one.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104019.131293512@infradead.org
---
 kernel/events/core.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index dfe09b0..89fb069 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7098,7 +7098,7 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 
 		rb = event->rb;
 		if (!rb)
-			goto aux_unlock;
+			goto unlock;
 
 		aux_mutex = &rb->aux_mutex;
 		mutex_lock(aux_mutex);
@@ -7107,27 +7107,27 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		aux_size = READ_ONCE(rb->user_page->aux_size);
 
 		if (aux_offset < perf_data_size(rb) + PAGE_SIZE)
-			goto aux_unlock;
+			goto unlock;
 
 		if (aux_offset != vma->vm_pgoff << PAGE_SHIFT)
-			goto aux_unlock;
+			goto unlock;
 
 		/* already mapped with a different offset */
 		if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
-			goto aux_unlock;
+			goto unlock;
 
 		if (aux_size != nr_pages * PAGE_SIZE)
-			goto aux_unlock;
+			goto unlock;
 
 		/* already mapped with a different size */
 		if (rb_has_aux(rb) && rb->aux_nr_pages != nr_pages)
-			goto aux_unlock;
+			goto unlock;
 
 		if (!is_power_of_2(nr_pages))
-			goto aux_unlock;
+			goto unlock;
 
 		if (!atomic_inc_not_zero(&rb->mmap_count))
-			goto aux_unlock;
+			goto unlock;
 
 		if (rb_has_aux(rb)) {
 			atomic_inc(&rb->aux_mmap_count);
@@ -7161,7 +7161,6 @@ aux_success:
 	}
 
 unlock:
-aux_unlock:
 	if (aux_mutex)
 		mutex_unlock(aux_mutex);
 	mutex_unlock(&event->mmap_mutex);

[PATCH v3 08/15] perf: Use guard() for aux_mutex in perf_mmap()

[Peter Zijlstra]

After duplicating the common code into the rb/aux branches is it
possible to use a simple guard() for the aux_mutex. Making the aux
branch self-contained.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6975,7 +6975,6 @@ static int perf_mmap(struct file *file,
 	struct perf_event *event = file->private_data;
 	unsigned long vma_size, nr_pages;
 	long user_extra = 0, extra = 0;
-	struct mutex *aux_mutex = NULL;
 	struct perf_buffer *rb = NULL;
 	int ret, flags = 0;
 	mapped_f mapped;
@@ -7098,8 +7097,7 @@ static int perf_mmap(struct file *file,
 		if (!rb)
 			goto unlock;
 
-		aux_mutex = &rb->aux_mutex;
-		mutex_lock(aux_mutex);
+		guard(mutex)(&rb->aux_mutex);
 
 		aux_offset = READ_ONCE(rb->user_page->aux_offset);
 		aux_size = READ_ONCE(rb->user_page->aux_size);
@@ -7159,8 +7157,6 @@ static int perf_mmap(struct file *file,
 	}
 
 unlock:
-	if (aux_mutex)
-		mutex_unlock(aux_mutex);
 	mutex_unlock(&event->mmap_mutex);
 
 	if (ret)

Re: [PATCH v3 08/15] perf: Use guard() for aux_mutex in perf_mmap()

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:06PM +0200, Peter Zijlstra wrote:
> After duplicating the common code into the rb/aux branches is it
> possible to use a simple guard() for the aux_mutex. Making the aux
> branch self-contained.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

LGTM, I really am going to look into how we can use guards in mm to
simplify things... :)

So:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  kernel/events/core.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6975,7 +6975,6 @@ static int perf_mmap(struct file *file,
>  	struct perf_event *event = file->private_data;
>  	unsigned long vma_size, nr_pages;
>  	long user_extra = 0, extra = 0;
> -	struct mutex *aux_mutex = NULL;
>  	struct perf_buffer *rb = NULL;
>  	int ret, flags = 0;
>  	mapped_f mapped;
> @@ -7098,8 +7097,7 @@ static int perf_mmap(struct file *file,
>  		if (!rb)
>  			goto unlock;
>
> -		aux_mutex = &rb->aux_mutex;
> -		mutex_lock(aux_mutex);
> +		guard(mutex)(&rb->aux_mutex);
>
>  		aux_offset = READ_ONCE(rb->user_page->aux_offset);
>  		aux_size = READ_ONCE(rb->user_page->aux_size);
> @@ -7159,8 +7157,6 @@ static int perf_mmap(struct file *file,
>  	}
>
>  unlock:
> -	if (aux_mutex)
> -		mutex_unlock(aux_mutex);
>  	mutex_unlock(&event->mmap_mutex);
>
>  	if (ret)
>
>

[tip: perf/core] perf: Use guard() for aux_mutex in perf_mmap()

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     b33a51564e3eb6c468979f9f08d9b4ad8451bed7
Gitweb:        https://git.kernel.org/tip/b33a51564e3eb6c468979f9f08d9b4ad8451bed7
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 12 Aug 2025 12:39:06 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:13:00 +02:00

perf: Use guard() for aux_mutex in perf_mmap()

After duplicating the common code into the rb/aux branches is it
possible to use a simple guard() for the aux_mutex. Making the aux
branch self-contained.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104019.246250452@infradead.org
---
 kernel/events/core.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 89fb069..236c60a 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6975,7 +6975,6 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 	struct perf_event *event = file->private_data;
 	unsigned long vma_size, nr_pages;
 	long user_extra = 0, extra = 0;
-	struct mutex *aux_mutex = NULL;
 	struct perf_buffer *rb = NULL;
 	int ret, flags = 0;
 	mapped_f mapped;
@@ -7100,8 +7099,7 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		if (!rb)
 			goto unlock;
 
-		aux_mutex = &rb->aux_mutex;
-		mutex_lock(aux_mutex);
+		guard(mutex)(&rb->aux_mutex);
 
 		aux_offset = READ_ONCE(rb->user_page->aux_offset);
 		aux_size = READ_ONCE(rb->user_page->aux_size);
@@ -7161,8 +7159,6 @@ aux_success:
 	}
 
 unlock:
-	if (aux_mutex)
-		mutex_unlock(aux_mutex);
 	mutex_unlock(&event->mmap_mutex);
 
 	if (ret)

[PATCH v3 09/15] perf: Reflow to get rid of aux_success label

[Peter Zijlstra]

Mostly re-indent noise needed to get rid of that label.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |   37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7127,30 +7127,29 @@ static int perf_mmap(struct file *file,
 		if (rb_has_aux(rb)) {
 			atomic_inc(&rb->aux_mmap_count);
 			ret = 0;
-			goto aux_success;
-		}
 
-		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
-			ret = -EPERM;
-			atomic_dec(&rb->mmap_count);
-			goto unlock;
-		}
+		} else {
+			if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
+				ret = -EPERM;
+				atomic_dec(&rb->mmap_count);
+				goto unlock;
+			}
 
-		WARN_ON(!rb && event->rb);
+			WARN_ON(!rb && event->rb);
 
-		if (vma->vm_flags & VM_WRITE)
-			flags |= RING_BUFFER_WRITABLE;
+			if (vma->vm_flags & VM_WRITE)
+				flags |= RING_BUFFER_WRITABLE;
 
-		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
-				   event->attr.aux_watermark, flags);
-		if (ret) {
-			atomic_dec(&rb->mmap_count);
-			goto unlock;
-		}
+			ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
+					   event->attr.aux_watermark, flags);
+			if (ret) {
+				atomic_dec(&rb->mmap_count);
+				goto unlock;
+			}
 
-		atomic_set(&rb->aux_mmap_count, 1);
-		rb->aux_mmap_locked = extra;
-aux_success:
+			atomic_set(&rb->aux_mmap_count, 1);
+			rb->aux_mmap_locked = extra;
+		}
 		perf_mmap_account(vma, user_extra, extra);
 		atomic_inc(&event->mmap_count);
 	}

Re: [PATCH v3 09/15] perf: Reflow to get rid of aux_success label

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:07PM +0200, Peter Zijlstra wrote:
> Mostly re-indent noise needed to get rid of that label.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

This is where a side-by-side git diff pager comes in handy :)

LGTM apart from nit/comment below so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  kernel/events/core.c |   37 ++++++++++++++++++-------------------
>  1 file changed, 18 insertions(+), 19 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7127,30 +7127,29 @@ static int perf_mmap(struct file *file,
>  		if (rb_has_aux(rb)) {
>  			atomic_inc(&rb->aux_mmap_count);
>  			ret = 0;
> -			goto aux_success;
> -		}
>
> -		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
> -			ret = -EPERM;
> -			atomic_dec(&rb->mmap_count);
> -			goto unlock;
> -		}

NIT: These leaves  a space above:

		if (rb_has_aux(rb)) {
			atomic_inc(&rb->aux_mmap_count);
			ret = 0;

		} else {


> +		} else {
> +			if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
> +				ret = -EPERM;
> +				atomic_dec(&rb->mmap_count);
> +				goto unlock;
> +			}
>
> -		WARN_ON(!rb && event->rb);
> +			WARN_ON(!rb && event->rb);
>
> -		if (vma->vm_flags & VM_WRITE)
> -			flags |= RING_BUFFER_WRITABLE;
> +			if (vma->vm_flags & VM_WRITE)
> +				flags |= RING_BUFFER_WRITABLE;
>
> -		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
> -				   event->attr.aux_watermark, flags);
> -		if (ret) {
> -			atomic_dec(&rb->mmap_count);
> -			goto unlock;
> -		}
> +			ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
> +					   event->attr.aux_watermark, flags);
> +			if (ret) {
> +				atomic_dec(&rb->mmap_count);
> +				goto unlock;
> +			}
>
> -		atomic_set(&rb->aux_mmap_count, 1);
> -		rb->aux_mmap_locked = extra;
> -aux_success:
> +			atomic_set(&rb->aux_mmap_count, 1);
> +			rb->aux_mmap_locked = extra;
> +		}

This gets rid of the label but leave spretty horrid nesting, but I'm
guessing further refactorings will tame it.

>  		perf_mmap_account(vma, user_extra, extra);
>  		atomic_inc(&event->mmap_count);
>  	}
>
>

Re: [PATCH v3 09/15] perf: Reflow to get rid of aux_success label

[Peter Zijlstra]

On Wed, Aug 13, 2025 at 07:03:01AM +0100, Lorenzo Stoakes wrote:
> On Tue, Aug 12, 2025 at 12:39:07PM +0200, Peter Zijlstra wrote:
> > Mostly re-indent noise needed to get rid of that label.
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> 
> This is where a side-by-side git diff pager comes in handy :)
> 
> LGTM apart from nit/comment below so:
> 
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> 
> > ---
> >  kernel/events/core.c |   37 ++++++++++++++++++-------------------
> >  1 file changed, 18 insertions(+), 19 deletions(-)
> >
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -7127,30 +7127,29 @@ static int perf_mmap(struct file *file,
> >  		if (rb_has_aux(rb)) {
> >  			atomic_inc(&rb->aux_mmap_count);
> >  			ret = 0;
> > -			goto aux_success;
> > -		}
> >
> > -		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
> > -			ret = -EPERM;
> > -			atomic_dec(&rb->mmap_count);
> > -			goto unlock;
> > -		}
> 
> NIT: These leaves  a space above:
> 
> 		if (rb_has_aux(rb)) {
> 			atomic_inc(&rb->aux_mmap_count);
> 			ret = 0;
> 
> 		} else {
> 

Yeah, you mention that elsewhere as well. I tend to do that to visually
separate the else branch from the previous block. Although I'm not very
consistent with it. I can remove these I suppose.

> > +		} else {
> > +			if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
> > +				ret = -EPERM;
> > +				atomic_dec(&rb->mmap_count);
> > +				goto unlock;
> > +			}
> >
> > -		WARN_ON(!rb && event->rb);
> > +			WARN_ON(!rb && event->rb);
> >
> > -		if (vma->vm_flags & VM_WRITE)
> > -			flags |= RING_BUFFER_WRITABLE;
> > +			if (vma->vm_flags & VM_WRITE)
> > +				flags |= RING_BUFFER_WRITABLE;
> >
> > -		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
> > -				   event->attr.aux_watermark, flags);
> > -		if (ret) {
> > -			atomic_dec(&rb->mmap_count);
> > -			goto unlock;
> > -		}
> > +			ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
> > +					   event->attr.aux_watermark, flags);
> > +			if (ret) {
> > +				atomic_dec(&rb->mmap_count);
> > +				goto unlock;
> > +			}
> >
> > -		atomic_set(&rb->aux_mmap_count, 1);
> > -		rb->aux_mmap_locked = extra;
> > -aux_success:
> > +			atomic_set(&rb->aux_mmap_count, 1);
> > +			rb->aux_mmap_locked = extra;
> > +		}
> 
> This gets rid of the label but leave spretty horrid nesting, but I'm
> guessing further refactorings will tame it.

The split out in the next patch removes one indent level.

[tip: perf/core] perf: Reflow to get rid of aux_success label

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     8558dca9fbdf825edf30b5fb74fbbbf3e6ba5dce
Gitweb:        https://git.kernel.org/tip/8558dca9fbdf825edf30b5fb74fbbbf3e6ba5dce
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 12 Aug 2025 12:39:07 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:13:00 +02:00

perf: Reflow to get rid of aux_success label

Mostly re-indent noise needed to get rid of that label.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104019.362581570@infradead.org
---
 kernel/events/core.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 236c60a..5bbea81 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7130,30 +7130,29 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		if (rb_has_aux(rb)) {
 			atomic_inc(&rb->aux_mmap_count);
 			ret = 0;
-			goto aux_success;
-		}
 
-		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
-			ret = -EPERM;
-			atomic_dec(&rb->mmap_count);
-			goto unlock;
-		}
+		} else {
+			if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
+				ret = -EPERM;
+				atomic_dec(&rb->mmap_count);
+				goto unlock;
+			}
 
-		WARN_ON(!rb && event->rb);
+			WARN_ON(!rb && event->rb);
 
-		if (vma->vm_flags & VM_WRITE)
-			flags |= RING_BUFFER_WRITABLE;
+			if (vma->vm_flags & VM_WRITE)
+				flags |= RING_BUFFER_WRITABLE;
 
-		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
-				   event->attr.aux_watermark, flags);
-		if (ret) {
-			atomic_dec(&rb->mmap_count);
-			goto unlock;
-		}
+			ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
+					   event->attr.aux_watermark, flags);
+			if (ret) {
+				atomic_dec(&rb->mmap_count);
+				goto unlock;
+			}
 
-		atomic_set(&rb->aux_mmap_count, 1);
-		rb->aux_mmap_locked = extra;
-aux_success:
+			atomic_set(&rb->aux_mmap_count, 1);
+			rb->aux_mmap_locked = extra;
+		}
 		perf_mmap_account(vma, user_extra, extra);
 		atomic_inc(&event->mmap_count);
 	}

[PATCH v3 10/15] perf: Split out the AUX buffer allocation

[Peter Zijlstra]

Move the AUX buffer allocation branch into its own function.

Originally-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |  144 +++++++++++++++++++++++++++------------------------
 1 file changed, 77 insertions(+), 67 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6970,6 +6970,82 @@ static void perf_mmap_account(struct vm_
 	atomic64_add(extra, &vma->vm_mm->pinned_vm);
 }
 
+static int perf_mmap_aux(struct vm_area_struct *vma, struct perf_event *event,
+			 unsigned long nr_pages)
+{
+	long extra = 0, user_extra = nr_pages;
+	u64 aux_offset, aux_size;
+	struct perf_buffer *rb;
+	int ret, rb_flags = 0;
+
+	rb = event->rb;
+	if (!rb)
+		return -EINVAL;
+
+	guard(mutex)(&rb->aux_mutex);
+
+	/*
+	 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
+	 * mapped, all subsequent mappings should have the same size
+	 * and offset. Must be above the normal perf buffer.
+	 */
+	aux_offset = READ_ONCE(rb->user_page->aux_offset);
+	aux_size = READ_ONCE(rb->user_page->aux_size);
+
+	if (aux_offset < perf_data_size(rb) + PAGE_SIZE)
+		return -EINVAL;
+
+	if (aux_offset != vma->vm_pgoff << PAGE_SHIFT)
+		return -EINVAL;
+
+	/* already mapped with a different offset */
+	if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
+		return -EINVAL;
+
+	if (aux_size != nr_pages * PAGE_SIZE)
+		return -EINVAL;
+
+	/* already mapped with a different size */
+	if (rb_has_aux(rb) && rb->aux_nr_pages != nr_pages)
+		return -EINVAL;
+
+	if (!is_power_of_2(nr_pages))
+		return -EINVAL;
+
+	if (!atomic_inc_not_zero(&rb->mmap_count))
+		return -EINVAL;
+
+	if (rb_has_aux(rb)) {
+		atomic_inc(&rb->aux_mmap_count);
+
+	} else {
+		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
+			atomic_dec(&rb->mmap_count);
+			return -EPERM;
+		}
+
+		WARN_ON(!rb && event->rb);
+
+		if (vma->vm_flags & VM_WRITE)
+			rb_flags |= RING_BUFFER_WRITABLE;
+
+		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
+				   event->attr.aux_watermark, rb_flags);
+		if (ret) {
+			atomic_dec(&rb->mmap_count);
+			return ret;
+		}
+
+		atomic_set(&rb->aux_mmap_count, 1);
+		rb->aux_mmap_locked = extra;
+	}
+
+	perf_mmap_account(vma, user_extra, extra);
+	atomic_inc(&event->mmap_count);
+
+	return 0;
+}
+
 static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct perf_event *event = file->private_data;
@@ -7087,73 +7163,7 @@ static int perf_mmap(struct file *file,
 		perf_mmap_account(vma, user_extra, extra);
 		atomic_inc(&event->mmap_count);
 	} else {
-		/*
-		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
-		 * mapped, all subsequent mappings should have the same size
-		 * and offset. Must be above the normal perf buffer.
-		 */
-		u64 aux_offset, aux_size;
-
-		rb = event->rb;
-		if (!rb)
-			goto unlock;
-
-		guard(mutex)(&rb->aux_mutex);
-
-		aux_offset = READ_ONCE(rb->user_page->aux_offset);
-		aux_size = READ_ONCE(rb->user_page->aux_size);
-
-		if (aux_offset < perf_data_size(rb) + PAGE_SIZE)
-			goto unlock;
-
-		if (aux_offset != vma->vm_pgoff << PAGE_SHIFT)
-			goto unlock;
-
-		/* already mapped with a different offset */
-		if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
-			goto unlock;
-
-		if (aux_size != nr_pages * PAGE_SIZE)
-			goto unlock;
-
-		/* already mapped with a different size */
-		if (rb_has_aux(rb) && rb->aux_nr_pages != nr_pages)
-			goto unlock;
-
-		if (!is_power_of_2(nr_pages))
-			goto unlock;
-
-		if (!atomic_inc_not_zero(&rb->mmap_count))
-			goto unlock;
-
-		if (rb_has_aux(rb)) {
-			atomic_inc(&rb->aux_mmap_count);
-			ret = 0;
-
-		} else {
-			if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
-				ret = -EPERM;
-				atomic_dec(&rb->mmap_count);
-				goto unlock;
-			}
-
-			WARN_ON(!rb && event->rb);
-
-			if (vma->vm_flags & VM_WRITE)
-				flags |= RING_BUFFER_WRITABLE;
-
-			ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
-					   event->attr.aux_watermark, flags);
-			if (ret) {
-				atomic_dec(&rb->mmap_count);
-				goto unlock;
-			}
-
-			atomic_set(&rb->aux_mmap_count, 1);
-			rb->aux_mmap_locked = extra;
-		}
-		perf_mmap_account(vma, user_extra, extra);
-		atomic_inc(&event->mmap_count);
+		ret = perf_mmap_aux(vma, event, nr_pages);
 	}
 
 unlock:

Re: [PATCH v3 10/15] perf: Split out the AUX buffer allocation

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:08PM +0200, Peter Zijlstra wrote:
> Move the AUX buffer allocation branch into its own function.
>
> Originally-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

LGTM (one nitty note below :), so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  kernel/events/core.c |  144 +++++++++++++++++++++++++++------------------------
>  1 file changed, 77 insertions(+), 67 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6970,6 +6970,82 @@ static void perf_mmap_account(struct vm_
>  	atomic64_add(extra, &vma->vm_mm->pinned_vm);
>  }
>
> +static int perf_mmap_aux(struct vm_area_struct *vma, struct perf_event *event,
> +			 unsigned long nr_pages)
> +{
> +	long extra = 0, user_extra = nr_pages;
> +	u64 aux_offset, aux_size;
> +	struct perf_buffer *rb;
> +	int ret, rb_flags = 0;
> +
> +	rb = event->rb;
> +	if (!rb)
> +		return -EINVAL;
> +
> +	guard(mutex)(&rb->aux_mutex);
> +
> +	/*
> +	 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
> +	 * mapped, all subsequent mappings should have the same size
> +	 * and offset. Must be above the normal perf buffer.
> +	 */
> +	aux_offset = READ_ONCE(rb->user_page->aux_offset);
> +	aux_size = READ_ONCE(rb->user_page->aux_size);
> +
> +	if (aux_offset < perf_data_size(rb) + PAGE_SIZE)
> +		return -EINVAL;
> +
> +	if (aux_offset != vma->vm_pgoff << PAGE_SHIFT)
> +		return -EINVAL;
> +
> +	/* already mapped with a different offset */
> +	if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
> +		return -EINVAL;
> +
> +	if (aux_size != nr_pages * PAGE_SIZE)
> +		return -EINVAL;
> +
> +	/* already mapped with a different size */
> +	if (rb_has_aux(rb) && rb->aux_nr_pages != nr_pages)
> +		return -EINVAL;
> +
> +	if (!is_power_of_2(nr_pages))
> +		return -EINVAL;
> +
> +	if (!atomic_inc_not_zero(&rb->mmap_count))
> +		return -EINVAL;
> +
> +	if (rb_has_aux(rb)) {
> +		atomic_inc(&rb->aux_mmap_count);
> +

Still that extra line :>)

> +	} else {
> +		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
> +			atomic_dec(&rb->mmap_count);
> +			return -EPERM;
> +		}
> +
> +		WARN_ON(!rb && event->rb);
> +
> +		if (vma->vm_flags & VM_WRITE)
> +			rb_flags |= RING_BUFFER_WRITABLE;
> +
> +		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
> +				   event->attr.aux_watermark, rb_flags);
> +		if (ret) {
> +			atomic_dec(&rb->mmap_count);
> +			return ret;
> +		}
> +
> +		atomic_set(&rb->aux_mmap_count, 1);
> +		rb->aux_mmap_locked = extra;
> +	}
> +
> +	perf_mmap_account(vma, user_extra, extra);
> +	atomic_inc(&event->mmap_count);
> +
> +	return 0;
> +}
> +
>  static int perf_mmap(struct file *file, struct vm_area_struct *vma)
>  {
>  	struct perf_event *event = file->private_data;
> @@ -7087,73 +7163,7 @@ static int perf_mmap(struct file *file,
>  		perf_mmap_account(vma, user_extra, extra);
>  		atomic_inc(&event->mmap_count);
>  	} else {
> -		/*
> -		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
> -		 * mapped, all subsequent mappings should have the same size
> -		 * and offset. Must be above the normal perf buffer.
> -		 */
> -		u64 aux_offset, aux_size;
> -
> -		rb = event->rb;
> -		if (!rb)
> -			goto unlock;
> -
> -		guard(mutex)(&rb->aux_mutex);
> -
> -		aux_offset = READ_ONCE(rb->user_page->aux_offset);
> -		aux_size = READ_ONCE(rb->user_page->aux_size);
> -
> -		if (aux_offset < perf_data_size(rb) + PAGE_SIZE)
> -			goto unlock;
> -
> -		if (aux_offset != vma->vm_pgoff << PAGE_SHIFT)
> -			goto unlock;
> -
> -		/* already mapped with a different offset */
> -		if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
> -			goto unlock;
> -
> -		if (aux_size != nr_pages * PAGE_SIZE)
> -			goto unlock;
> -
> -		/* already mapped with a different size */
> -		if (rb_has_aux(rb) && rb->aux_nr_pages != nr_pages)
> -			goto unlock;
> -
> -		if (!is_power_of_2(nr_pages))
> -			goto unlock;
> -
> -		if (!atomic_inc_not_zero(&rb->mmap_count))
> -			goto unlock;
> -
> -		if (rb_has_aux(rb)) {
> -			atomic_inc(&rb->aux_mmap_count);
> -			ret = 0;
> -
> -		} else {
> -			if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
> -				ret = -EPERM;
> -				atomic_dec(&rb->mmap_count);
> -				goto unlock;
> -			}
> -
> -			WARN_ON(!rb && event->rb);
> -
> -			if (vma->vm_flags & VM_WRITE)
> -				flags |= RING_BUFFER_WRITABLE;
> -
> -			ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
> -					   event->attr.aux_watermark, flags);
> -			if (ret) {
> -				atomic_dec(&rb->mmap_count);
> -				goto unlock;
> -			}
> -
> -			atomic_set(&rb->aux_mmap_count, 1);
> -			rb->aux_mmap_locked = extra;
> -		}
> -		perf_mmap_account(vma, user_extra, extra);
> -		atomic_inc(&event->mmap_count);
> +		ret = perf_mmap_aux(vma, event, nr_pages);
>  	}
>
>  unlock:
>
>

[tip: perf/core] perf: Split out the AUX buffer allocation

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     2aee37682391332d26c01e703170e0d9358c7252
Gitweb:        https://git.kernel.org/tip/2aee37682391332d26c01e703170e0d9358c7252
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 12 Aug 2025 12:39:08 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:13:00 +02:00

perf: Split out the AUX buffer allocation

Move the AUX buffer allocation branch into its own function.

Originally-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104019.494205648@infradead.org
---
 kernel/events/core.c | 144 ++++++++++++++++++++++--------------------
 1 file changed, 77 insertions(+), 67 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5bbea81..e76afd9 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6970,6 +6970,82 @@ static void perf_mmap_account(struct vm_area_struct *vma, long user_extra, long 
 	atomic64_add(extra, &vma->vm_mm->pinned_vm);
 }
 
+static int perf_mmap_aux(struct vm_area_struct *vma, struct perf_event *event,
+			 unsigned long nr_pages)
+{
+	long extra = 0, user_extra = nr_pages;
+	u64 aux_offset, aux_size;
+	struct perf_buffer *rb;
+	int ret, rb_flags = 0;
+
+	rb = event->rb;
+	if (!rb)
+		return -EINVAL;
+
+	guard(mutex)(&rb->aux_mutex);
+
+	/*
+	 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
+	 * mapped, all subsequent mappings should have the same size
+	 * and offset. Must be above the normal perf buffer.
+	 */
+	aux_offset = READ_ONCE(rb->user_page->aux_offset);
+	aux_size = READ_ONCE(rb->user_page->aux_size);
+
+	if (aux_offset < perf_data_size(rb) + PAGE_SIZE)
+		return -EINVAL;
+
+	if (aux_offset != vma->vm_pgoff << PAGE_SHIFT)
+		return -EINVAL;
+
+	/* already mapped with a different offset */
+	if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
+		return -EINVAL;
+
+	if (aux_size != nr_pages * PAGE_SIZE)
+		return -EINVAL;
+
+	/* already mapped with a different size */
+	if (rb_has_aux(rb) && rb->aux_nr_pages != nr_pages)
+		return -EINVAL;
+
+	if (!is_power_of_2(nr_pages))
+		return -EINVAL;
+
+	if (!atomic_inc_not_zero(&rb->mmap_count))
+		return -EINVAL;
+
+	if (rb_has_aux(rb)) {
+		atomic_inc(&rb->aux_mmap_count);
+
+	} else {
+		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
+			atomic_dec(&rb->mmap_count);
+			return -EPERM;
+		}
+
+		WARN_ON(!rb && event->rb);
+
+		if (vma->vm_flags & VM_WRITE)
+			rb_flags |= RING_BUFFER_WRITABLE;
+
+		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
+				   event->attr.aux_watermark, rb_flags);
+		if (ret) {
+			atomic_dec(&rb->mmap_count);
+			return ret;
+		}
+
+		atomic_set(&rb->aux_mmap_count, 1);
+		rb->aux_mmap_locked = extra;
+	}
+
+	perf_mmap_account(vma, user_extra, extra);
+	atomic_inc(&event->mmap_count);
+
+	return 0;
+}
+
 static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct perf_event *event = file->private_data;
@@ -7088,73 +7164,7 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		perf_mmap_account(vma, user_extra, extra);
 		atomic_inc(&event->mmap_count);
 	} else {
-		/*
-		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
-		 * mapped, all subsequent mappings should have the same size
-		 * and offset. Must be above the normal perf buffer.
-		 */
-		u64 aux_offset, aux_size;
-
-		rb = event->rb;
-		if (!rb)
-			goto unlock;
-
-		guard(mutex)(&rb->aux_mutex);
-
-		aux_offset = READ_ONCE(rb->user_page->aux_offset);
-		aux_size = READ_ONCE(rb->user_page->aux_size);
-
-		if (aux_offset < perf_data_size(rb) + PAGE_SIZE)
-			goto unlock;
-
-		if (aux_offset != vma->vm_pgoff << PAGE_SHIFT)
-			goto unlock;
-
-		/* already mapped with a different offset */
-		if (rb_has_aux(rb) && rb->aux_pgoff != vma->vm_pgoff)
-			goto unlock;
-
-		if (aux_size != nr_pages * PAGE_SIZE)
-			goto unlock;
-
-		/* already mapped with a different size */
-		if (rb_has_aux(rb) && rb->aux_nr_pages != nr_pages)
-			goto unlock;
-
-		if (!is_power_of_2(nr_pages))
-			goto unlock;
-
-		if (!atomic_inc_not_zero(&rb->mmap_count))
-			goto unlock;
-
-		if (rb_has_aux(rb)) {
-			atomic_inc(&rb->aux_mmap_count);
-			ret = 0;
-
-		} else {
-			if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
-				ret = -EPERM;
-				atomic_dec(&rb->mmap_count);
-				goto unlock;
-			}
-
-			WARN_ON(!rb && event->rb);
-
-			if (vma->vm_flags & VM_WRITE)
-				flags |= RING_BUFFER_WRITABLE;
-
-			ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
-					   event->attr.aux_watermark, flags);
-			if (ret) {
-				atomic_dec(&rb->mmap_count);
-				goto unlock;
-			}
-
-			atomic_set(&rb->aux_mmap_count, 1);
-			rb->aux_mmap_locked = extra;
-		}
-		perf_mmap_account(vma, user_extra, extra);
-		atomic_inc(&event->mmap_count);
+		ret = perf_mmap_aux(vma, event, nr_pages);
 	}
 
 unlock:

[PATCH v3 11/15] perf: Make RB allocation branch self sufficient

[Peter Zijlstra]

Ensure @rb usage doesn't extend out of the branch block.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7116,8 +7116,6 @@ static int perf_mmap(struct file *file,
 				 * multiple times.
 				 */
 				ret = 0;
-				/* We need the rb to map pages. */
-				rb = event->rb;
 				perf_mmap_account(vma, user_extra, extra);
 				atomic_inc(&event->mmap_count);
 				goto unlock;
@@ -7136,8 +7134,6 @@ static int perf_mmap(struct file *file,
 			goto unlock;
 		}
 
-		WARN_ON(!rb && event->rb);
-
 		if (vma->vm_flags & VM_WRITE)
 			flags |= RING_BUFFER_WRITABLE;
 
@@ -7190,7 +7186,7 @@ static int perf_mmap(struct file *file,
 	 * full cleanup in this case and therefore does not invoke
 	 * vmops::close().
 	 */
-	ret = map_range(rb, vma);
+	ret = map_range(event->rb, vma);
 	if (ret)
 		perf_mmap_close(vma);
 

Re: [PATCH v3 11/15] perf: Make RB allocation branch self sufficient

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:09PM +0200, Peter Zijlstra wrote:
> Ensure @rb usage doesn't extend out of the branch block.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Nice to separate this out and make this bit clear.

I'm guessing you're holding off on putting the rb decl into the branch as
you intend in a future patch to separate out the rb branch too into a
helper like tglx's original did?

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  kernel/events/core.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7116,8 +7116,6 @@ static int perf_mmap(struct file *file,
>  				 * multiple times.
>  				 */
>  				ret = 0;
> -				/* We need the rb to map pages. */
> -				rb = event->rb;
>  				perf_mmap_account(vma, user_extra, extra);
>  				atomic_inc(&event->mmap_count);
>  				goto unlock;
> @@ -7136,8 +7134,6 @@ static int perf_mmap(struct file *file,
>  			goto unlock;
>  		}
>
> -		WARN_ON(!rb && event->rb);
> -
>  		if (vma->vm_flags & VM_WRITE)
>  			flags |= RING_BUFFER_WRITABLE;
>
> @@ -7190,7 +7186,7 @@ static int perf_mmap(struct file *file,
>  	 * full cleanup in this case and therefore does not invoke
>  	 * vmops::close().
>  	 */
> -	ret = map_range(rb, vma);
> +	ret = map_range(event->rb, vma);
>  	if (ret)
>  		perf_mmap_close(vma);
>
>
>

[tip: perf/core] perf: Make RB allocation branch self sufficient

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     191759e5ea9f6995171ed2ffcc41a2377f946a3a
Gitweb:        https://git.kernel.org/tip/191759e5ea9f6995171ed2ffcc41a2377f946a3a
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 12 Aug 2025 12:39:09 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:13:01 +02:00

perf: Make RB allocation branch self sufficient

Ensure @rb usage doesn't extend out of the branch block.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104019.605285302@infradead.org
---
 kernel/events/core.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index e76afd9..875c27b 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7116,8 +7116,6 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 				 * multiple times.
 				 */
 				ret = 0;
-				/* We need the rb to map pages. */
-				rb = event->rb;
 				perf_mmap_account(vma, user_extra, extra);
 				atomic_inc(&event->mmap_count);
 				goto unlock;
@@ -7136,8 +7134,6 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 			goto unlock;
 		}
 
-		WARN_ON(!rb && event->rb);
-
 		if (vma->vm_flags & VM_WRITE)
 			flags |= RING_BUFFER_WRITABLE;
 
@@ -7190,7 +7186,7 @@ unlock:
 	 * full cleanup in this case and therefore does not invoke
 	 * vmops::close().
 	 */
-	ret = map_range(rb, vma);
+	ret = map_range(event->rb, vma);
 	if (ret)
 		perf_mmap_close(vma);
 

[PATCH v3 12/15] perf: Split out the RB allocation

[Peter Zijlstra]

Move the RB buffer allocation branch into its own function.

Originally-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |  145 +++++++++++++++++++++++++--------------------------
 1 file changed, 73 insertions(+), 72 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6970,6 +6970,75 @@ static void perf_mmap_account(struct vm_
 	atomic64_add(extra, &vma->vm_mm->pinned_vm);
 }
 
+static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
+			unsigned long nr_pages)
+{
+	long extra = 0, user_extra = nr_pages;
+	struct perf_buffer *rb;
+	int rb_flags = 0;
+
+	nr_pages -= 1;
+
+	/*
+	 * If we have rb pages ensure they're a power-of-two number, so we
+	 * can do bitmasks instead of modulo.
+	 */
+	if (nr_pages != 0 && !is_power_of_2(nr_pages))
+		return -EINVAL;
+
+	WARN_ON_ONCE(event->ctx->parent_ctx);
+
+	if (event->rb) {
+		if (data_page_nr(event->rb) != nr_pages)
+			return -EINVAL;
+
+		if (atomic_inc_not_zero(&event->rb->mmap_count)) {
+			/*
+			 * Success -- managed to mmap() the same buffer
+			 * multiple times.
+			 */
+			perf_mmap_account(vma, user_extra, extra);
+			atomic_inc(&event->mmap_count);
+			return 0;
+		}
+
+		/*
+		 * Raced against perf_mmap_close()'s
+		 * atomic_dec_and_mutex_lock() remove the
+		 * event and continue as if !event->rb
+		 */
+		ring_buffer_attach(event, NULL);
+	}
+
+	if (!perf_mmap_calc_limits(vma, &user_extra, &extra))
+		return -EPERM;
+
+	if (vma->vm_flags & VM_WRITE)
+		rb_flags |= RING_BUFFER_WRITABLE;
+
+	rb = rb_alloc(nr_pages,
+		      event->attr.watermark ? event->attr.wakeup_watermark : 0,
+		      event->cpu, rb_flags);
+
+	if (!rb)
+		return -ENOMEM;
+
+	atomic_set(&rb->mmap_count, 1);
+	rb->mmap_user = get_current_user();
+	rb->mmap_locked = extra;
+
+	ring_buffer_attach(event, rb);
+
+	perf_event_update_time(event);
+	perf_event_init_userpage(event);
+	perf_event_update_userpage(event);
+
+	perf_mmap_account(vma, user_extra, extra);
+	atomic_inc(&event->mmap_count);
+
+	return 0;
+}
+
 static int perf_mmap_aux(struct vm_area_struct *vma, struct perf_event *event,
 			 unsigned long nr_pages)
 {
@@ -7050,10 +7119,8 @@ static int perf_mmap(struct file *file,
 {
 	struct perf_event *event = file->private_data;
 	unsigned long vma_size, nr_pages;
-	long user_extra = 0, extra = 0;
-	struct perf_buffer *rb = NULL;
-	int ret, flags = 0;
 	mapped_f mapped;
+	int ret;
 
 	/*
 	 * Don't allow mmap() of inherited per-task counters. This would
@@ -7079,8 +7146,6 @@ static int perf_mmap(struct file *file,
 	if (vma_size != PAGE_SIZE * nr_pages)
 		return -EINVAL;
 
-	user_extra = nr_pages;
-
 	mutex_lock(&event->mmap_mutex);
 	ret = -EINVAL;
 
@@ -7094,74 +7159,10 @@ static int perf_mmap(struct file *file,
 		goto unlock;
 	}
 
-	if (vma->vm_pgoff == 0) {
-		nr_pages -= 1;
-
-		/*
-		 * If we have rb pages ensure they're a power-of-two number, so we
-		 * can do bitmasks instead of modulo.
-		 */
-		if (nr_pages != 0 && !is_power_of_2(nr_pages))
-			goto unlock;
-
-		WARN_ON_ONCE(event->ctx->parent_ctx);
-
-		if (event->rb) {
-			if (data_page_nr(event->rb) != nr_pages)
-				goto unlock;
-
-			if (atomic_inc_not_zero(&event->rb->mmap_count)) {
-				/*
-				 * Success -- managed to mmap() the same buffer
-				 * multiple times.
-				 */
-				ret = 0;
-				perf_mmap_account(vma, user_extra, extra);
-				atomic_inc(&event->mmap_count);
-				goto unlock;
-			}
-
-			/*
-			 * Raced against perf_mmap_close()'s
-			 * atomic_dec_and_mutex_lock() remove the
-			 * event and continue as if !event->rb
-			 */
-			ring_buffer_attach(event, NULL);
-		}
-
-		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
-			ret = -EPERM;
-			goto unlock;
-		}
-
-		if (vma->vm_flags & VM_WRITE)
-			flags |= RING_BUFFER_WRITABLE;
-
-		rb = rb_alloc(nr_pages,
-			      event->attr.watermark ? event->attr.wakeup_watermark : 0,
-			      event->cpu, flags);
-
-		if (!rb) {
-			ret = -ENOMEM;
-			goto unlock;
-		}
-
-		atomic_set(&rb->mmap_count, 1);
-		rb->mmap_user = get_current_user();
-		rb->mmap_locked = extra;
-
-		ring_buffer_attach(event, rb);
-
-		perf_event_update_time(event);
-		perf_event_init_userpage(event);
-		perf_event_update_userpage(event);
-		ret = 0;
-
-		perf_mmap_account(vma, user_extra, extra);
-		atomic_inc(&event->mmap_count);
-	} else {
+	if (vma->vm_pgoff == 0)
+		ret = perf_mmap_rb(vma, event, nr_pages);
+	else
 		ret = perf_mmap_aux(vma, event, nr_pages);
-	}
 
 unlock:
 	mutex_unlock(&event->mmap_mutex);

Re: [PATCH v3 12/15] perf: Split out the RB allocation

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:10PM +0200, Peter Zijlstra wrote:
> Move the RB buffer allocation branch into its own function.
>
> Originally-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Ah yes of course, the very next patch :)

One nit below about ret assignment, though I wonder if you address later?

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  kernel/events/core.c |  145 +++++++++++++++++++++++++--------------------------
>  1 file changed, 73 insertions(+), 72 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6970,6 +6970,75 @@ static void perf_mmap_account(struct vm_
>  	atomic64_add(extra, &vma->vm_mm->pinned_vm);
>  }
>
> +static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
> +			unsigned long nr_pages)
> +{
> +	long extra = 0, user_extra = nr_pages;
> +	struct perf_buffer *rb;
> +	int rb_flags = 0;
> +
> +	nr_pages -= 1;
> +
> +	/*
> +	 * If we have rb pages ensure they're a power-of-two number, so we
> +	 * can do bitmasks instead of modulo.
> +	 */
> +	if (nr_pages != 0 && !is_power_of_2(nr_pages))
> +		return -EINVAL;
> +
> +	WARN_ON_ONCE(event->ctx->parent_ctx);
> +
> +	if (event->rb) {
> +		if (data_page_nr(event->rb) != nr_pages)
> +			return -EINVAL;
> +
> +		if (atomic_inc_not_zero(&event->rb->mmap_count)) {
> +			/*
> +			 * Success -- managed to mmap() the same buffer
> +			 * multiple times.
> +			 */
> +			perf_mmap_account(vma, user_extra, extra);
> +			atomic_inc(&event->mmap_count);
> +			return 0;
> +		}
> +
> +		/*
> +		 * Raced against perf_mmap_close()'s
> +		 * atomic_dec_and_mutex_lock() remove the
> +		 * event and continue as if !event->rb
> +		 */
> +		ring_buffer_attach(event, NULL);
> +	}
> +
> +	if (!perf_mmap_calc_limits(vma, &user_extra, &extra))
> +		return -EPERM;
> +
> +	if (vma->vm_flags & VM_WRITE)
> +		rb_flags |= RING_BUFFER_WRITABLE;
> +

I hadn't noticed before actually, but it's really nice that we only assign
rb from below for the newly allocated rb, and refer to the prior one via
event->rb above.

Which neatly addressed my prior review feedback on this, very nice, cheers!

> +	rb = rb_alloc(nr_pages,
> +		      event->attr.watermark ? event->attr.wakeup_watermark : 0,
> +		      event->cpu, rb_flags);
> +
> +	if (!rb)
> +		return -ENOMEM;
> +
> +	atomic_set(&rb->mmap_count, 1);
> +	rb->mmap_user = get_current_user();
> +	rb->mmap_locked = extra;
> +
> +	ring_buffer_attach(event, rb);
> +
> +	perf_event_update_time(event);
> +	perf_event_init_userpage(event);
> +	perf_event_update_userpage(event);
> +
> +	perf_mmap_account(vma, user_extra, extra);
> +	atomic_inc(&event->mmap_count);
> +
> +	return 0;
> +}
> +
>  static int perf_mmap_aux(struct vm_area_struct *vma, struct perf_event *event,
>  			 unsigned long nr_pages)
>  {
> @@ -7050,10 +7119,8 @@ static int perf_mmap(struct file *file,
>  {
>  	struct perf_event *event = file->private_data;
>  	unsigned long vma_size, nr_pages;
> -	long user_extra = 0, extra = 0;
> -	struct perf_buffer *rb = NULL;
> -	int ret, flags = 0;
>  	mapped_f mapped;
> +	int ret;
>
>  	/*
>  	 * Don't allow mmap() of inherited per-task counters. This would
> @@ -7079,8 +7146,6 @@ static int perf_mmap(struct file *file,
>  	if (vma_size != PAGE_SIZE * nr_pages)
>  		return -EINVAL;
>
> -	user_extra = nr_pages;
> -
>  	mutex_lock(&event->mmap_mutex);
>  	ret = -EINVAL;

This ret assignment is redundant now, since in all cases you assign ret
below.

I wonder if you get rid of this in a later patch though?

>
> @@ -7094,74 +7159,10 @@ static int perf_mmap(struct file *file,
>  		goto unlock;
>  	}
>
> -	if (vma->vm_pgoff == 0) {
> -		nr_pages -= 1;
> -
> -		/*
> -		 * If we have rb pages ensure they're a power-of-two number, so we
> -		 * can do bitmasks instead of modulo.
> -		 */
> -		if (nr_pages != 0 && !is_power_of_2(nr_pages))
> -			goto unlock;
> -
> -		WARN_ON_ONCE(event->ctx->parent_ctx);
> -
> -		if (event->rb) {
> -			if (data_page_nr(event->rb) != nr_pages)
> -				goto unlock;
> -
> -			if (atomic_inc_not_zero(&event->rb->mmap_count)) {
> -				/*
> -				 * Success -- managed to mmap() the same buffer
> -				 * multiple times.
> -				 */
> -				ret = 0;
> -				perf_mmap_account(vma, user_extra, extra);
> -				atomic_inc(&event->mmap_count);
> -				goto unlock;
> -			}
> -
> -			/*
> -			 * Raced against perf_mmap_close()'s
> -			 * atomic_dec_and_mutex_lock() remove the
> -			 * event and continue as if !event->rb
> -			 */
> -			ring_buffer_attach(event, NULL);
> -		}
> -
> -		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
> -			ret = -EPERM;
> -			goto unlock;
> -		}
> -
> -		if (vma->vm_flags & VM_WRITE)
> -			flags |= RING_BUFFER_WRITABLE;
> -
> -		rb = rb_alloc(nr_pages,
> -			      event->attr.watermark ? event->attr.wakeup_watermark : 0,
> -			      event->cpu, flags);
> -
> -		if (!rb) {
> -			ret = -ENOMEM;
> -			goto unlock;
> -		}
> -
> -		atomic_set(&rb->mmap_count, 1);
> -		rb->mmap_user = get_current_user();
> -		rb->mmap_locked = extra;
> -
> -		ring_buffer_attach(event, rb);
> -
> -		perf_event_update_time(event);
> -		perf_event_init_userpage(event);
> -		perf_event_update_userpage(event);
> -		ret = 0;
> -
> -		perf_mmap_account(vma, user_extra, extra);
> -		atomic_inc(&event->mmap_count);
> -	} else {
> +	if (vma->vm_pgoff == 0)
> +		ret = perf_mmap_rb(vma, event, nr_pages);
> +	else
>  		ret = perf_mmap_aux(vma, event, nr_pages);
> -	}
>
>  unlock:
>  	mutex_unlock(&event->mmap_mutex);
>
>

[tip: perf/core] perf: Split out the RB allocation

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     5d299897f1e36025400ca84fd36c15925a383b03
Gitweb:        https://git.kernel.org/tip/5d299897f1e36025400ca84fd36c15925a383b03
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 12 Aug 2025 12:39:10 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:13:01 +02:00

perf: Split out the RB allocation

Move the RB buffer allocation branch into its own function.

Originally-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104019.722214699@infradead.org
---
 kernel/events/core.c | 145 +++++++++++++++++++++---------------------
 1 file changed, 73 insertions(+), 72 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 875c27b..3a5fd2b 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6970,6 +6970,75 @@ static void perf_mmap_account(struct vm_area_struct *vma, long user_extra, long 
 	atomic64_add(extra, &vma->vm_mm->pinned_vm);
 }
 
+static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
+			unsigned long nr_pages)
+{
+	long extra = 0, user_extra = nr_pages;
+	struct perf_buffer *rb;
+	int rb_flags = 0;
+
+	nr_pages -= 1;
+
+	/*
+	 * If we have rb pages ensure they're a power-of-two number, so we
+	 * can do bitmasks instead of modulo.
+	 */
+	if (nr_pages != 0 && !is_power_of_2(nr_pages))
+		return -EINVAL;
+
+	WARN_ON_ONCE(event->ctx->parent_ctx);
+
+	if (event->rb) {
+		if (data_page_nr(event->rb) != nr_pages)
+			return -EINVAL;
+
+		if (atomic_inc_not_zero(&event->rb->mmap_count)) {
+			/*
+			 * Success -- managed to mmap() the same buffer
+			 * multiple times.
+			 */
+			perf_mmap_account(vma, user_extra, extra);
+			atomic_inc(&event->mmap_count);
+			return 0;
+		}
+
+		/*
+		 * Raced against perf_mmap_close()'s
+		 * atomic_dec_and_mutex_lock() remove the
+		 * event and continue as if !event->rb
+		 */
+		ring_buffer_attach(event, NULL);
+	}
+
+	if (!perf_mmap_calc_limits(vma, &user_extra, &extra))
+		return -EPERM;
+
+	if (vma->vm_flags & VM_WRITE)
+		rb_flags |= RING_BUFFER_WRITABLE;
+
+	rb = rb_alloc(nr_pages,
+		      event->attr.watermark ? event->attr.wakeup_watermark : 0,
+		      event->cpu, rb_flags);
+
+	if (!rb)
+		return -ENOMEM;
+
+	atomic_set(&rb->mmap_count, 1);
+	rb->mmap_user = get_current_user();
+	rb->mmap_locked = extra;
+
+	ring_buffer_attach(event, rb);
+
+	perf_event_update_time(event);
+	perf_event_init_userpage(event);
+	perf_event_update_userpage(event);
+
+	perf_mmap_account(vma, user_extra, extra);
+	atomic_inc(&event->mmap_count);
+
+	return 0;
+}
+
 static int perf_mmap_aux(struct vm_area_struct *vma, struct perf_event *event,
 			 unsigned long nr_pages)
 {
@@ -7050,10 +7119,8 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct perf_event *event = file->private_data;
 	unsigned long vma_size, nr_pages;
-	long user_extra = 0, extra = 0;
-	struct perf_buffer *rb = NULL;
-	int ret, flags = 0;
 	mapped_f mapped;
+	int ret;
 
 	/*
 	 * Don't allow mmap() of inherited per-task counters. This would
@@ -7079,8 +7146,6 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 	if (vma_size != PAGE_SIZE * nr_pages)
 		return -EINVAL;
 
-	user_extra = nr_pages;
-
 	mutex_lock(&event->mmap_mutex);
 	ret = -EINVAL;
 
@@ -7094,74 +7159,10 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		goto unlock;
 	}
 
-	if (vma->vm_pgoff == 0) {
-		nr_pages -= 1;
-
-		/*
-		 * If we have rb pages ensure they're a power-of-two number, so we
-		 * can do bitmasks instead of modulo.
-		 */
-		if (nr_pages != 0 && !is_power_of_2(nr_pages))
-			goto unlock;
-
-		WARN_ON_ONCE(event->ctx->parent_ctx);
-
-		if (event->rb) {
-			if (data_page_nr(event->rb) != nr_pages)
-				goto unlock;
-
-			if (atomic_inc_not_zero(&event->rb->mmap_count)) {
-				/*
-				 * Success -- managed to mmap() the same buffer
-				 * multiple times.
-				 */
-				ret = 0;
-				perf_mmap_account(vma, user_extra, extra);
-				atomic_inc(&event->mmap_count);
-				goto unlock;
-			}
-
-			/*
-			 * Raced against perf_mmap_close()'s
-			 * atomic_dec_and_mutex_lock() remove the
-			 * event and continue as if !event->rb
-			 */
-			ring_buffer_attach(event, NULL);
-		}
-
-		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
-			ret = -EPERM;
-			goto unlock;
-		}
-
-		if (vma->vm_flags & VM_WRITE)
-			flags |= RING_BUFFER_WRITABLE;
-
-		rb = rb_alloc(nr_pages,
-			      event->attr.watermark ? event->attr.wakeup_watermark : 0,
-			      event->cpu, flags);
-
-		if (!rb) {
-			ret = -ENOMEM;
-			goto unlock;
-		}
-
-		atomic_set(&rb->mmap_count, 1);
-		rb->mmap_user = get_current_user();
-		rb->mmap_locked = extra;
-
-		ring_buffer_attach(event, rb);
-
-		perf_event_update_time(event);
-		perf_event_init_userpage(event);
-		perf_event_update_userpage(event);
-		ret = 0;
-
-		perf_mmap_account(vma, user_extra, extra);
-		atomic_inc(&event->mmap_count);
-	} else {
+	if (vma->vm_pgoff == 0)
+		ret = perf_mmap_rb(vma, event, nr_pages);
+	else
 		ret = perf_mmap_aux(vma, event, nr_pages);
-	}
 
 unlock:
 	mutex_unlock(&event->mmap_mutex);

[PATCH v3 13/15] perf: Use scoped_guard() for mmap_mutex in perf_mmap()

[Peter Zijlstra]

Mostly just re-indent noise.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |   32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7143,27 +7143,23 @@ static int perf_mmap(struct file *file,
 	if (vma_size != PAGE_SIZE * nr_pages)
 		return -EINVAL;
 
-	mutex_lock(&event->mmap_mutex);
-	ret = -EINVAL;
+	scoped_guard (mutex, &event->mmap_mutex) {
+		/*
+		 * This relies on __pmu_detach_event() taking mmap_mutex after marking
+		 * the event REVOKED. Either we observe the state, or __pmu_detach_event()
+		 * will detach the rb created here.
+		 */
+		if (event->state <= PERF_EVENT_STATE_REVOKED) {
+			ret = -ENODEV;
+			break;
+		}
 
-	/*
-	 * This relies on __pmu_detach_event() taking mmap_mutex after marking
-	 * the event REVOKED. Either we observe the state, or __pmu_detach_event()
-	 * will detach the rb created here.
-	 */
-	if (event->state <= PERF_EVENT_STATE_REVOKED) {
-		ret = -ENODEV;
-		goto unlock;
+		if (vma->vm_pgoff == 0)
+			ret = perf_mmap_rb(vma, event, nr_pages);
+		else
+			ret = perf_mmap_aux(vma, event, nr_pages);
 	}
 
-	if (vma->vm_pgoff == 0)
-		ret = perf_mmap_rb(vma, event, nr_pages);
-	else
-		ret = perf_mmap_aux(vma, event, nr_pages);
-
-unlock:
-	mutex_unlock(&event->mmap_mutex);
-
 	if (ret)
 		return ret;
 

Re: [PATCH v3 13/15] perf: Use scoped_guard() for mmap_mutex in perf_mmap()

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:11PM +0200, Peter Zijlstra wrote:
> Mostly just re-indent noise.
>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

LGTM, so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  kernel/events/core.c |   32 ++++++++++++++------------------
>  1 file changed, 14 insertions(+), 18 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7143,27 +7143,23 @@ static int perf_mmap(struct file *file,
>  	if (vma_size != PAGE_SIZE * nr_pages)
>  		return -EINVAL;
>
> -	mutex_lock(&event->mmap_mutex);

Yeah getting rid of this lock goto stuff is great... mm is at risk of some
more Lorenzo churn to use this stuff over there... :)

> -	ret = -EINVAL;

You see, I'm getting in a pattern here with comment on prior patch and you
reading my mind and fixing it on next ;)

> +	scoped_guard (mutex, &event->mmap_mutex) {
> +		/*
> +		 * This relies on __pmu_detach_event() taking mmap_mutex after marking
> +		 * the event REVOKED. Either we observe the state, or __pmu_detach_event()
> +		 * will detach the rb created here.
> +		 */
> +		if (event->state <= PERF_EVENT_STATE_REVOKED) {
> +			ret = -ENODEV;
> +			break;

I don't absolutely love this break-for-what-is-not-obviously-a-for-loop
formulation (I know scoped_guard in practice _is_ a for loop, but obviously
that's hidden by macro), but I guess hey it's C, and we have to do what we
have to do :)

> +		}
>
> -	/*
> -	 * This relies on __pmu_detach_event() taking mmap_mutex after marking
> -	 * the event REVOKED. Either we observe the state, or __pmu_detach_event()
> -	 * will detach the rb created here.
> -	 */
> -	if (event->state <= PERF_EVENT_STATE_REVOKED) {
> -		ret = -ENODEV;
> -		goto unlock;
> +		if (vma->vm_pgoff == 0)
> +			ret = perf_mmap_rb(vma, event, nr_pages);
> +		else
> +			ret = perf_mmap_aux(vma, event, nr_pages);
>  	}
>
> -	if (vma->vm_pgoff == 0)
> -		ret = perf_mmap_rb(vma, event, nr_pages);
> -	else
> -		ret = perf_mmap_aux(vma, event, nr_pages);
> -
> -unlock:
> -	mutex_unlock(&event->mmap_mutex);
> -
>  	if (ret)
>  		return ret;
>
>
>

Re: [PATCH v3 13/15] perf: Use scoped_guard() for mmap_mutex in perf_mmap()

[Peter Zijlstra]

On Wed, Aug 13, 2025 at 07:42:41AM +0100, Lorenzo Stoakes wrote:

> > +	scoped_guard (mutex, &event->mmap_mutex) {
> > +		/*
> > +		 * This relies on __pmu_detach_event() taking mmap_mutex after marking
> > +		 * the event REVOKED. Either we observe the state, or __pmu_detach_event()
> > +		 * will detach the rb created here.
> > +		 */
> > +		if (event->state <= PERF_EVENT_STATE_REVOKED) {
> > +			ret = -ENODEV;
> > +			break;
> 
> I don't absolutely love this break-for-what-is-not-obviously-a-for-loop
> formulation (I know scoped_guard in practice _is_ a for loop, but obviously
> that's hidden by macro), but I guess hey it's C, and we have to do what we
> have to do :)

Right, don't love it either, but the alternative was a goto and that's
arguably worse, so meh.

> > +		}
> >
> > +		if (vma->vm_pgoff == 0)
> > +			ret = perf_mmap_rb(vma, event, nr_pages);
> > +		else
> > +			ret = perf_mmap_aux(vma, event, nr_pages);
> >  	}
> >
> >  	if (ret)
> >  		return ret;
> >
> >
> >

Re: [PATCH v3 13/15] perf: Use scoped_guard() for mmap_mutex in perf_mmap()

[Peter Zijlstra]

On Wed, Aug 13, 2025 at 10:32:44AM +0200, Peter Zijlstra wrote:
> On Wed, Aug 13, 2025 at 07:42:41AM +0100, Lorenzo Stoakes wrote:
> 
> > > +	scoped_guard (mutex, &event->mmap_mutex) {
> > > +		/*
> > > +		 * This relies on __pmu_detach_event() taking mmap_mutex after marking
> > > +		 * the event REVOKED. Either we observe the state, or __pmu_detach_event()
> > > +		 * will detach the rb created here.
> > > +		 */
> > > +		if (event->state <= PERF_EVENT_STATE_REVOKED) {
> > > +			ret = -ENODEV;
> > > +			break;
> > 
> > I don't absolutely love this break-for-what-is-not-obviously-a-for-loop
> > formulation (I know scoped_guard in practice _is_ a for loop, but obviously
> > that's hidden by macro), but I guess hey it's C, and we have to do what we
> > have to do :)
> 
> Right, don't love it either, but the alternative was a goto and that's
> arguably worse, so meh.
> 
> > > +		}
> > >
> > > +		if (vma->vm_pgoff == 0)
> > > +			ret = perf_mmap_rb(vma, event, nr_pages);
> > > +		else
> > > +			ret = perf_mmap_aux(vma, event, nr_pages);
> > >  	}
> > >
> > >  	if (ret)
> > >  		return ret;
> > >

Nah, I'm an idiot.. How's this?

---

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 20189a3354f2..4b82f8ed6b4e 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7151,20 +7151,17 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		 * the event REVOKED. Either we observe the state, or __pmu_detach_event()
 		 * will detach the rb created here.
 		 */
-		if (event->state <= PERF_EVENT_STATE_REVOKED) {
-			ret = -ENODEV;
-			break;
-		}
+		if (event->state <= PERF_EVENT_STATE_REVOKED)
+			return -ENODEV;
 
 		if (vma->vm_pgoff == 0)
 			ret = perf_mmap_rb(vma, event, nr_pages);
 		else
 			ret = perf_mmap_aux(vma, event, nr_pages);
+		if (ret)
+			return ret;
 	}
 
-	if (ret)
-		return ret;
-
 	/*
 	 * Since pinned accounting is per vm we cannot allow fork() to copy our
 	 * vma.

Re: [PATCH v3 13/15] perf: Use scoped_guard() for mmap_mutex in perf_mmap()

[Lorenzo Stoakes]

On Wed, Aug 13, 2025 at 10:44:48AM +0200, Peter Zijlstra wrote:
> On Wed, Aug 13, 2025 at 10:32:44AM +0200, Peter Zijlstra wrote:
> > On Wed, Aug 13, 2025 at 07:42:41AM +0100, Lorenzo Stoakes wrote:
> >
> > > > +	scoped_guard (mutex, &event->mmap_mutex) {
> > > > +		/*
> > > > +		 * This relies on __pmu_detach_event() taking mmap_mutex after marking
> > > > +		 * the event REVOKED. Either we observe the state, or __pmu_detach_event()
> > > > +		 * will detach the rb created here.
> > > > +		 */
> > > > +		if (event->state <= PERF_EVENT_STATE_REVOKED) {
> > > > +			ret = -ENODEV;
> > > > +			break;
> > >
> > > I don't absolutely love this break-for-what-is-not-obviously-a-for-loop
> > > formulation (I know scoped_guard in practice _is_ a for loop, but obviously
> > > that's hidden by macro), but I guess hey it's C, and we have to do what we
> > > have to do :)
> >
> > Right, don't love it either, but the alternative was a goto and that's
> > arguably worse, so meh.
> >
> > > > +		}
> > > >
> > > > +		if (vma->vm_pgoff == 0)
> > > > +			ret = perf_mmap_rb(vma, event, nr_pages);
> > > > +		else
> > > > +			ret = perf_mmap_aux(vma, event, nr_pages);
> > > >  	}
> > > >
> > > >  	if (ret)
> > > >  		return ret;
> > > >
>
> Nah, I'm an idiot.. How's this?

Right yeah, you could just return instead of break :))

[tip: perf/core] perf: Use scoped_guard() for mmap_mutex in perf_mmap()

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     d23a6dbc0a71741eb7b141fdc04e31360fba46ef
Gitweb:        https://git.kernel.org/tip/d23a6dbc0a71741eb7b141fdc04e31360fba46ef
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 12 Aug 2025 12:39:11 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:13:01 +02:00

perf: Use scoped_guard() for mmap_mutex in perf_mmap()

Mostly just re-indent noise.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104019.838047976@infradead.org
---
 kernel/events/core.c | 35 ++++++++++++++---------------------
 1 file changed, 14 insertions(+), 21 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3a5fd2b..41941df 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7146,30 +7146,23 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 	if (vma_size != PAGE_SIZE * nr_pages)
 		return -EINVAL;
 
-	mutex_lock(&event->mmap_mutex);
-	ret = -EINVAL;
+	scoped_guard (mutex, &event->mmap_mutex) {
+		/*
+		 * This relies on __pmu_detach_event() taking mmap_mutex after marking
+		 * the event REVOKED. Either we observe the state, or __pmu_detach_event()
+		 * will detach the rb created here.
+		 */
+		if (event->state <= PERF_EVENT_STATE_REVOKED)
+			return -ENODEV;
 
-	/*
-	 * This relies on __pmu_detach_event() taking mmap_mutex after marking
-	 * the event REVOKED. Either we observe the state, or __pmu_detach_event()
-	 * will detach the rb created here.
-	 */
-	if (event->state <= PERF_EVENT_STATE_REVOKED) {
-		ret = -ENODEV;
-		goto unlock;
+		if (vma->vm_pgoff == 0)
+			ret = perf_mmap_rb(vma, event, nr_pages);
+		else
+			ret = perf_mmap_aux(vma, event, nr_pages);
+		if (ret)
+			return ret;
 	}
 
-	if (vma->vm_pgoff == 0)
-		ret = perf_mmap_rb(vma, event, nr_pages);
-	else
-		ret = perf_mmap_aux(vma, event, nr_pages);
-
-unlock:
-	mutex_unlock(&event->mmap_mutex);
-
-	if (ret)
-		return ret;
-
 	/*
 	 * Since pinned accounting is per vm we cannot allow fork() to copy our
 	 * vma.

[PATCH v3 14/15] perf: Identify the 0->1 transition for event::mmap_count

[Peter Zijlstra]

Needed because refcount_inc() doesn't allow the 0->1 transition.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7033,7 +7033,7 @@ static int perf_mmap_rb(struct vm_area_s
 	perf_event_update_userpage(event);
 
 	perf_mmap_account(vma, user_extra, extra);
-	atomic_inc(&event->mmap_count);
+	atomic_set(&event->mmap_count, 1);
 
 	return 0;
 }

Re: [PATCH v3 14/15] perf: Identify the 0->1 transition for event::mmap_count

[Peter Zijlstra]

ljs noted that this Changelog was a tad terse, and he had a pending
question here.

On Tue, Aug 12, 2025 at 12:39:12PM +0200, Peter Zijlstra wrote:
> Needed because refcount_inc() doesn't allow the 0->1 transition.

Specifically, this is the case where we've created the RB, this means
there was no RB, and as such there could not have been an mmap.
Additionally we hold mmap_mutex to serialize everything.

This must be the first.

> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  kernel/events/core.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7033,7 +7033,7 @@ static int perf_mmap_rb(struct vm_area_s
>  	perf_event_update_userpage(event);
>  
>  	perf_mmap_account(vma, user_extra, extra);
> -	atomic_inc(&event->mmap_count);
> +	atomic_set(&event->mmap_count, 1);
>  
>  	return 0;
>  }
> 
> 

Re: [PATCH v3 14/15] perf: Identify the 0->1 transition for event::mmap_count

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 01:15:22PM +0200, Peter Zijlstra wrote:
>
> ljs noted that this Changelog was a tad terse, and he had a pending
> question here.
>
> On Tue, Aug 12, 2025 at 12:39:12PM +0200, Peter Zijlstra wrote:
> > Needed because refcount_inc() doesn't allow the 0->1 transition.
>
> Specifically, this is the case where we've created the RB, this means
> there was no RB, and as such there could not have been an mmap.
> Additionally we hold mmap_mutex to serialize everything.
>
> This must be the first.

Thanks, this is great!

And really good to separate this out as a really critical step prepping for
refcount_t change.

If we're being nitty it might be worth mentioning that we plan to move to
refcount_t and that considers inc from 0 to be evidence of a UAF, hence the
absolute requirement for this.

However, since we're clearly stating that this is the first instance of
this mmap_count being incremented, it's moot, as it's simply clearer to do
this anyway.

>
> > Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

LGTM so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> > ---
> >  kernel/events/core.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -7033,7 +7033,7 @@ static int perf_mmap_rb(struct vm_area_s
> >  	perf_event_update_userpage(event);
> >
> >  	perf_mmap_account(vma, user_extra, extra);
> > -	atomic_inc(&event->mmap_count);
> > +	atomic_set(&event->mmap_count, 1);
> >
> >  	return 0;
> >  }
> >
> >

[tip: perf/core] perf: Identify the 0->1 transition for event::mmap_count

[tip-bot2 for Peter Zijlstra]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     59741451b49ce9964a9758c19d6f7df2a1255c75
Gitweb:        https://git.kernel.org/tip/59741451b49ce9964a9758c19d6f7df2a1255c75
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 12 Aug 2025 12:39:12 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:13:02 +02:00

perf: Identify the 0->1 transition for event::mmap_count

Needed because refcount_inc() doesn't allow the 0->1 transition.

Specifically, this is the case where we've created the RB, this means
there was no RB, and as such there could not have been an mmap.
Additionally we hold mmap_mutex to serialize everything.

This must be the first.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20250812104019.956479989@infradead.org
---
 kernel/events/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 41941df..f6211ab 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7034,7 +7034,7 @@ static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
 	perf_event_update_userpage(event);
 
 	perf_mmap_account(vma, user_extra, extra);
-	atomic_inc(&event->mmap_count);
+	atomic_set(&event->mmap_count, 1);
 
 	return 0;
 }

[PATCH v3 15/15] perf: Convert mmap() refcounts to refcount_t

[Peter Zijlstra]

From: Thomas Gleixner <tglx@linutronix.de>

The recently fixed reference count leaks could have been detected by using
refcount_t and refcount_t would have mitigated the potential overflow at
least.

Now that the code is properly structured, convert the mmap() related
mmap_count variants over to refcount_t.

No functional change intended.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20250811070620.716309215@linutronix.de
---
 include/linux/perf_event.h  |    2 +-
 kernel/events/core.c        |   40 ++++++++++++++++++++--------------------
 kernel/events/internal.h    |    4 ++--
 kernel/events/ring_buffer.c |    2 +-
 4 files changed, 24 insertions(+), 24 deletions(-)

--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -859,7 +859,7 @@ struct perf_event {
 
 	/* mmap bits */
 	struct mutex			mmap_mutex;
-	atomic_t			mmap_count;
+	refcount_t			mmap_count;
 
 	struct perf_buffer		*rb;
 	struct list_head		rb_entry;
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3968,7 +3968,7 @@ static noinline int visit_groups_merge(s
  */
 static inline bool event_update_userpage(struct perf_event *event)
 {
-	if (likely(!atomic_read(&event->mmap_count)))
+	if (likely(!refcount_read(&event->mmap_count)))
 		return false;
 
 	perf_event_update_time(event);
@@ -6704,11 +6704,11 @@ static void perf_mmap_open(struct vm_are
 	struct perf_event *event = vma->vm_file->private_data;
 	mapped_f mapped = get_mapped(event, event_mapped);
 
-	atomic_inc(&event->mmap_count);
-	atomic_inc(&event->rb->mmap_count);
+	refcount_inc(&event->mmap_count);
+	refcount_inc(&event->rb->mmap_count);
 
 	if (vma->vm_pgoff)
-		atomic_inc(&event->rb->aux_mmap_count);
+		refcount_inc(&event->rb->aux_mmap_count);
 
 	if (mapped)
 		mapped(event, vma->vm_mm);
@@ -6743,7 +6743,7 @@ static void perf_mmap_close(struct vm_ar
 	 * to avoid complications.
 	 */
 	if (rb_has_aux(rb) && vma->vm_pgoff == rb->aux_pgoff &&
-	    atomic_dec_and_mutex_lock(&rb->aux_mmap_count, &rb->aux_mutex)) {
+	    refcount_dec_and_mutex_lock(&rb->aux_mmap_count, &rb->aux_mutex)) {
 		/*
 		 * Stop all AUX events that are writing to this buffer,
 		 * so that we can free its AUX pages and corresponding PMU
@@ -6763,10 +6763,10 @@ static void perf_mmap_close(struct vm_ar
 		mutex_unlock(&rb->aux_mutex);
 	}
 
-	if (atomic_dec_and_test(&rb->mmap_count))
+	if (refcount_dec_and_test(&rb->mmap_count))
 		detach_rest = true;
 
-	if (!atomic_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
+	if (!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
 		goto out_put;
 
 	ring_buffer_attach(event, NULL);
@@ -6992,19 +6992,19 @@ static int perf_mmap_rb(struct vm_area_s
 		if (data_page_nr(event->rb) != nr_pages)
 			return -EINVAL;
 
-		if (atomic_inc_not_zero(&event->rb->mmap_count)) {
+		if (refcount_inc_not_zero(&event->rb->mmap_count)) {
 			/*
 			 * Success -- managed to mmap() the same buffer
 			 * multiple times.
 			 */
 			perf_mmap_account(vma, user_extra, extra);
-			atomic_inc(&event->mmap_count);
+			refcount_inc(&event->mmap_count);
 			return 0;
 		}
 
 		/*
 		 * Raced against perf_mmap_close()'s
-		 * atomic_dec_and_mutex_lock() remove the
+		 * refcount_dec_and_mutex_lock() remove the
 		 * event and continue as if !event->rb
 		 */
 		ring_buffer_attach(event, NULL);
@@ -7023,7 +7023,7 @@ static int perf_mmap_rb(struct vm_area_s
 	if (!rb)
 		return -ENOMEM;
 
-	atomic_set(&rb->mmap_count, 1);
+	refcount_set(&rb->mmap_count, 1);
 	rb->mmap_user = get_current_user();
 	rb->mmap_locked = extra;
 
@@ -7034,7 +7034,7 @@ static int perf_mmap_rb(struct vm_area_s
 	perf_event_update_userpage(event);
 
 	perf_mmap_account(vma, user_extra, extra);
-	atomic_set(&event->mmap_count, 1);
+	refcount_set(&event->mmap_count, 1);
 
 	return 0;
 }
@@ -7081,15 +7081,15 @@ static int perf_mmap_aux(struct vm_area_
 	if (!is_power_of_2(nr_pages))
 		return -EINVAL;
 
-	if (!atomic_inc_not_zero(&rb->mmap_count))
+	if (!refcount_inc_not_zero(&rb->mmap_count))
 		return -EINVAL;
 
 	if (rb_has_aux(rb)) {
-		atomic_inc(&rb->aux_mmap_count);
+		refcount_inc(&rb->aux_mmap_count);
 
 	} else {
 		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
-			atomic_dec(&rb->mmap_count);
+			refcount_dec(&rb->mmap_count);
 			return -EPERM;
 		}
 
@@ -7101,16 +7101,16 @@ static int perf_mmap_aux(struct vm_area_
 		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
 				   event->attr.aux_watermark, rb_flags);
 		if (ret) {
-			atomic_dec(&rb->mmap_count);
+			refcount_dec(&rb->mmap_count);
 			return ret;
 		}
 
-		atomic_set(&rb->aux_mmap_count, 1);
+		refcount_set(&rb->aux_mmap_count, 1);
 		rb->aux_mmap_locked = extra;
 	}
 
 	perf_mmap_account(vma, user_extra, extra);
-	atomic_inc(&event->mmap_count);
+	refcount_inc(&event->mmap_count);
 
 	return 0;
 }
@@ -13257,7 +13257,7 @@ perf_event_set_output(struct perf_event
 	mutex_lock_double(&event->mmap_mutex, &output_event->mmap_mutex);
 set:
 	/* Can't redirect output if we've got an active mmap() */
-	if (atomic_read(&event->mmap_count))
+	if (refcount_read(&event->mmap_count))
 		goto unlock;
 
 	if (output_event) {
@@ -13270,7 +13270,7 @@ perf_event_set_output(struct perf_event
 			goto unlock;
 
 		/* did we race against perf_mmap_close() */
-		if (!atomic_read(&rb->mmap_count)) {
+		if (!refcount_read(&rb->mmap_count)) {
 			ring_buffer_put(rb);
 			goto unlock;
 		}
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -35,7 +35,7 @@ struct perf_buffer {
 	spinlock_t			event_lock;
 	struct list_head		event_list;
 
-	atomic_t			mmap_count;
+	refcount_t			mmap_count;
 	unsigned long			mmap_locked;
 	struct user_struct		*mmap_user;
 
@@ -47,7 +47,7 @@ struct perf_buffer {
 	unsigned long			aux_pgoff;
 	int				aux_nr_pages;
 	int				aux_overwrite;
-	atomic_t			aux_mmap_count;
+	refcount_t			aux_mmap_count;
 	unsigned long			aux_mmap_locked;
 	void				(*free_aux)(void *);
 	refcount_t			aux_refcount;
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -400,7 +400,7 @@ void *perf_aux_output_begin(struct perf_
 	 * the same order, see perf_mmap_close. Otherwise we end up freeing
 	 * aux pages in this path, which is a bug, because in_atomic().
 	 */
-	if (!atomic_read(&rb->aux_mmap_count))
+	if (!refcount_read(&rb->aux_mmap_count))
 		goto err;
 
 	if (!refcount_inc_not_zero(&rb->aux_refcount))

Re: [PATCH v3 15/15] perf: Convert mmap() refcounts to refcount_t

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:39:13PM +0200, Peter Zijlstra wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> The recently fixed reference count leaks could have been detected by using
> refcount_t and refcount_t would have mitigated the potential overflow at
> least.
>
> Now that the code is properly structured, convert the mmap() related
> mmap_count variants over to refcount_t.
>
> No functional change intended.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

All LGTM, checked the mmap test passes and no splat, the atomic_inc() ->
atomic_set() on &rb->mmap_count (which here becomes refcount_set()) sorted
that out.

This is a nice fix up as is the rest of the series, kudos to tglx and
additionally you Peter for adding further nice changes + fixing up the bit
we missed.

This is a _massive_ improvement :)

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> Link: https://lkml.kernel.org/r/20250811070620.716309215@linutronix.de
> ---
>  include/linux/perf_event.h  |    2 +-
>  kernel/events/core.c        |   40 ++++++++++++++++++++--------------------
>  kernel/events/internal.h    |    4 ++--
>  kernel/events/ring_buffer.c |    2 +-
>  4 files changed, 24 insertions(+), 24 deletions(-)
>
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -859,7 +859,7 @@ struct perf_event {
>
>  	/* mmap bits */
>  	struct mutex			mmap_mutex;
> -	atomic_t			mmap_count;
> +	refcount_t			mmap_count;
>
>  	struct perf_buffer		*rb;
>  	struct list_head		rb_entry;
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -3968,7 +3968,7 @@ static noinline int visit_groups_merge(s
>   */
>  static inline bool event_update_userpage(struct perf_event *event)
>  {
> -	if (likely(!atomic_read(&event->mmap_count)))
> +	if (likely(!refcount_read(&event->mmap_count)))
>  		return false;
>
>  	perf_event_update_time(event);
> @@ -6704,11 +6704,11 @@ static void perf_mmap_open(struct vm_are
>  	struct perf_event *event = vma->vm_file->private_data;
>  	mapped_f mapped = get_mapped(event, event_mapped);
>
> -	atomic_inc(&event->mmap_count);
> -	atomic_inc(&event->rb->mmap_count);
> +	refcount_inc(&event->mmap_count);
> +	refcount_inc(&event->rb->mmap_count);
>
>  	if (vma->vm_pgoff)
> -		atomic_inc(&event->rb->aux_mmap_count);
> +		refcount_inc(&event->rb->aux_mmap_count);
>
>  	if (mapped)
>  		mapped(event, vma->vm_mm);
> @@ -6743,7 +6743,7 @@ static void perf_mmap_close(struct vm_ar
>  	 * to avoid complications.
>  	 */
>  	if (rb_has_aux(rb) && vma->vm_pgoff == rb->aux_pgoff &&
> -	    atomic_dec_and_mutex_lock(&rb->aux_mmap_count, &rb->aux_mutex)) {
> +	    refcount_dec_and_mutex_lock(&rb->aux_mmap_count, &rb->aux_mutex)) {
>  		/*
>  		 * Stop all AUX events that are writing to this buffer,
>  		 * so that we can free its AUX pages and corresponding PMU
> @@ -6763,10 +6763,10 @@ static void perf_mmap_close(struct vm_ar
>  		mutex_unlock(&rb->aux_mutex);
>  	}
>
> -	if (atomic_dec_and_test(&rb->mmap_count))
> +	if (refcount_dec_and_test(&rb->mmap_count))
>  		detach_rest = true;
>
> -	if (!atomic_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
> +	if (!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
>  		goto out_put;
>
>  	ring_buffer_attach(event, NULL);
> @@ -6992,19 +6992,19 @@ static int perf_mmap_rb(struct vm_area_s
>  		if (data_page_nr(event->rb) != nr_pages)
>  			return -EINVAL;
>
> -		if (atomic_inc_not_zero(&event->rb->mmap_count)) {
> +		if (refcount_inc_not_zero(&event->rb->mmap_count)) {
>  			/*
>  			 * Success -- managed to mmap() the same buffer
>  			 * multiple times.
>  			 */
>  			perf_mmap_account(vma, user_extra, extra);
> -			atomic_inc(&event->mmap_count);
> +			refcount_inc(&event->mmap_count);
>  			return 0;
>  		}
>
>  		/*
>  		 * Raced against perf_mmap_close()'s
> -		 * atomic_dec_and_mutex_lock() remove the
> +		 * refcount_dec_and_mutex_lock() remove the
>  		 * event and continue as if !event->rb
>  		 */
>  		ring_buffer_attach(event, NULL);
> @@ -7023,7 +7023,7 @@ static int perf_mmap_rb(struct vm_area_s
>  	if (!rb)
>  		return -ENOMEM;
>
> -	atomic_set(&rb->mmap_count, 1);
> +	refcount_set(&rb->mmap_count, 1);
>  	rb->mmap_user = get_current_user();
>  	rb->mmap_locked = extra;
>
> @@ -7034,7 +7034,7 @@ static int perf_mmap_rb(struct vm_area_s
>  	perf_event_update_userpage(event);
>
>  	perf_mmap_account(vma, user_extra, extra);
> -	atomic_set(&event->mmap_count, 1);
> +	refcount_set(&event->mmap_count, 1);
>
>  	return 0;
>  }
> @@ -7081,15 +7081,15 @@ static int perf_mmap_aux(struct vm_area_
>  	if (!is_power_of_2(nr_pages))
>  		return -EINVAL;
>
> -	if (!atomic_inc_not_zero(&rb->mmap_count))
> +	if (!refcount_inc_not_zero(&rb->mmap_count))
>  		return -EINVAL;
>
>  	if (rb_has_aux(rb)) {
> -		atomic_inc(&rb->aux_mmap_count);
> +		refcount_inc(&rb->aux_mmap_count);
>
>  	} else {
>  		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
> -			atomic_dec(&rb->mmap_count);
> +			refcount_dec(&rb->mmap_count);
>  			return -EPERM;
>  		}
>
> @@ -7101,16 +7101,16 @@ static int perf_mmap_aux(struct vm_area_
>  		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
>  				   event->attr.aux_watermark, rb_flags);
>  		if (ret) {
> -			atomic_dec(&rb->mmap_count);
> +			refcount_dec(&rb->mmap_count);
>  			return ret;
>  		}
>
> -		atomic_set(&rb->aux_mmap_count, 1);
> +		refcount_set(&rb->aux_mmap_count, 1);
>  		rb->aux_mmap_locked = extra;
>  	}
>
>  	perf_mmap_account(vma, user_extra, extra);
> -	atomic_inc(&event->mmap_count);
> +	refcount_inc(&event->mmap_count);
>
>  	return 0;
>  }
> @@ -13257,7 +13257,7 @@ perf_event_set_output(struct perf_event
>  	mutex_lock_double(&event->mmap_mutex, &output_event->mmap_mutex);
>  set:
>  	/* Can't redirect output if we've got an active mmap() */
> -	if (atomic_read(&event->mmap_count))
> +	if (refcount_read(&event->mmap_count))
>  		goto unlock;
>
>  	if (output_event) {
> @@ -13270,7 +13270,7 @@ perf_event_set_output(struct perf_event
>  			goto unlock;
>
>  		/* did we race against perf_mmap_close() */
> -		if (!atomic_read(&rb->mmap_count)) {
> +		if (!refcount_read(&rb->mmap_count)) {
>  			ring_buffer_put(rb);
>  			goto unlock;
>  		}
> --- a/kernel/events/internal.h
> +++ b/kernel/events/internal.h
> @@ -35,7 +35,7 @@ struct perf_buffer {
>  	spinlock_t			event_lock;
>  	struct list_head		event_list;
>
> -	atomic_t			mmap_count;
> +	refcount_t			mmap_count;
>  	unsigned long			mmap_locked;
>  	struct user_struct		*mmap_user;
>
> @@ -47,7 +47,7 @@ struct perf_buffer {
>  	unsigned long			aux_pgoff;
>  	int				aux_nr_pages;
>  	int				aux_overwrite;
> -	atomic_t			aux_mmap_count;
> +	refcount_t			aux_mmap_count;
>  	unsigned long			aux_mmap_locked;
>  	void				(*free_aux)(void *);
>  	refcount_t			aux_refcount;
> --- a/kernel/events/ring_buffer.c
> +++ b/kernel/events/ring_buffer.c
> @@ -400,7 +400,7 @@ void *perf_aux_output_begin(struct perf_
>  	 * the same order, see perf_mmap_close. Otherwise we end up freeing
>  	 * aux pages in this path, which is a bug, because in_atomic().
>  	 */
> -	if (!atomic_read(&rb->aux_mmap_count))
> +	if (!refcount_read(&rb->aux_mmap_count))
>  		goto err;
>
>  	if (!refcount_inc_not_zero(&rb->aux_refcount))
>
>

[tip: perf/core] perf: Convert mmap() refcounts to refcount_t

[tip-bot2 for Thomas Gleixner]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     448f97fba9013ffa13f5dd82febd18836b189499
Gitweb:        https://git.kernel.org/tip/448f97fba9013ffa13f5dd82febd18836b189499
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Tue, 12 Aug 2025 12:39:13 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 15 Aug 2025 13:13:02 +02:00

perf: Convert mmap() refcounts to refcount_t

The recently fixed reference count leaks could have been detected by using
refcount_t and refcount_t would have mitigated the potential overflow at
least.

Now that the code is properly structured, convert the mmap() related
mmap_count variants over to refcount_t.

No functional change intended.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/r/20250812104020.071507932@infradead.org
---
 include/linux/perf_event.h  |  2 +-
 kernel/events/core.c        | 40 ++++++++++++++++++------------------
 kernel/events/internal.h    |  4 ++--
 kernel/events/ring_buffer.c |  2 +-
 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index ec9d960..bfbf9ea 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -859,7 +859,7 @@ struct perf_event {
 
 	/* mmap bits */
 	struct mutex			mmap_mutex;
-	atomic_t			mmap_count;
+	refcount_t			mmap_count;
 
 	struct perf_buffer		*rb;
 	struct list_head		rb_entry;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f6211ab..ea35704 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3968,7 +3968,7 @@ static noinline int visit_groups_merge(struct perf_event_context *ctx,
  */
 static inline bool event_update_userpage(struct perf_event *event)
 {
-	if (likely(!atomic_read(&event->mmap_count)))
+	if (likely(!refcount_read(&event->mmap_count)))
 		return false;
 
 	perf_event_update_time(event);
@@ -6704,11 +6704,11 @@ static void perf_mmap_open(struct vm_area_struct *vma)
 	struct perf_event *event = vma->vm_file->private_data;
 	mapped_f mapped = get_mapped(event, event_mapped);
 
-	atomic_inc(&event->mmap_count);
-	atomic_inc(&event->rb->mmap_count);
+	refcount_inc(&event->mmap_count);
+	refcount_inc(&event->rb->mmap_count);
 
 	if (vma->vm_pgoff)
-		atomic_inc(&event->rb->aux_mmap_count);
+		refcount_inc(&event->rb->aux_mmap_count);
 
 	if (mapped)
 		mapped(event, vma->vm_mm);
@@ -6743,7 +6743,7 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 	 * to avoid complications.
 	 */
 	if (rb_has_aux(rb) && vma->vm_pgoff == rb->aux_pgoff &&
-	    atomic_dec_and_mutex_lock(&rb->aux_mmap_count, &rb->aux_mutex)) {
+	    refcount_dec_and_mutex_lock(&rb->aux_mmap_count, &rb->aux_mutex)) {
 		/*
 		 * Stop all AUX events that are writing to this buffer,
 		 * so that we can free its AUX pages and corresponding PMU
@@ -6763,10 +6763,10 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 		mutex_unlock(&rb->aux_mutex);
 	}
 
-	if (atomic_dec_and_test(&rb->mmap_count))
+	if (refcount_dec_and_test(&rb->mmap_count))
 		detach_rest = true;
 
-	if (!atomic_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
+	if (!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
 		goto out_put;
 
 	ring_buffer_attach(event, NULL);
@@ -6992,19 +6992,19 @@ static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
 		if (data_page_nr(event->rb) != nr_pages)
 			return -EINVAL;
 
-		if (atomic_inc_not_zero(&event->rb->mmap_count)) {
+		if (refcount_inc_not_zero(&event->rb->mmap_count)) {
 			/*
 			 * Success -- managed to mmap() the same buffer
 			 * multiple times.
 			 */
 			perf_mmap_account(vma, user_extra, extra);
-			atomic_inc(&event->mmap_count);
+			refcount_inc(&event->mmap_count);
 			return 0;
 		}
 
 		/*
 		 * Raced against perf_mmap_close()'s
-		 * atomic_dec_and_mutex_lock() remove the
+		 * refcount_dec_and_mutex_lock() remove the
 		 * event and continue as if !event->rb
 		 */
 		ring_buffer_attach(event, NULL);
@@ -7023,7 +7023,7 @@ static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
 	if (!rb)
 		return -ENOMEM;
 
-	atomic_set(&rb->mmap_count, 1);
+	refcount_set(&rb->mmap_count, 1);
 	rb->mmap_user = get_current_user();
 	rb->mmap_locked = extra;
 
@@ -7034,7 +7034,7 @@ static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
 	perf_event_update_userpage(event);
 
 	perf_mmap_account(vma, user_extra, extra);
-	atomic_set(&event->mmap_count, 1);
+	refcount_set(&event->mmap_count, 1);
 
 	return 0;
 }
@@ -7081,15 +7081,15 @@ static int perf_mmap_aux(struct vm_area_struct *vma, struct perf_event *event,
 	if (!is_power_of_2(nr_pages))
 		return -EINVAL;
 
-	if (!atomic_inc_not_zero(&rb->mmap_count))
+	if (!refcount_inc_not_zero(&rb->mmap_count))
 		return -EINVAL;
 
 	if (rb_has_aux(rb)) {
-		atomic_inc(&rb->aux_mmap_count);
+		refcount_inc(&rb->aux_mmap_count);
 
 	} else {
 		if (!perf_mmap_calc_limits(vma, &user_extra, &extra)) {
-			atomic_dec(&rb->mmap_count);
+			refcount_dec(&rb->mmap_count);
 			return -EPERM;
 		}
 
@@ -7101,16 +7101,16 @@ static int perf_mmap_aux(struct vm_area_struct *vma, struct perf_event *event,
 		ret = rb_alloc_aux(rb, event, vma->vm_pgoff, nr_pages,
 				   event->attr.aux_watermark, rb_flags);
 		if (ret) {
-			atomic_dec(&rb->mmap_count);
+			refcount_dec(&rb->mmap_count);
 			return ret;
 		}
 
-		atomic_set(&rb->aux_mmap_count, 1);
+		refcount_set(&rb->aux_mmap_count, 1);
 		rb->aux_mmap_locked = extra;
 	}
 
 	perf_mmap_account(vma, user_extra, extra);
-	atomic_inc(&event->mmap_count);
+	refcount_inc(&event->mmap_count);
 
 	return 0;
 }
@@ -13254,7 +13254,7 @@ perf_event_set_output(struct perf_event *event, struct perf_event *output_event)
 	mutex_lock_double(&event->mmap_mutex, &output_event->mmap_mutex);
 set:
 	/* Can't redirect output if we've got an active mmap() */
-	if (atomic_read(&event->mmap_count))
+	if (refcount_read(&event->mmap_count))
 		goto unlock;
 
 	if (output_event) {
@@ -13267,7 +13267,7 @@ set:
 			goto unlock;
 
 		/* did we race against perf_mmap_close() */
-		if (!atomic_read(&rb->mmap_count)) {
+		if (!refcount_read(&rb->mmap_count)) {
 			ring_buffer_put(rb);
 			goto unlock;
 		}
diff --git a/kernel/events/internal.h b/kernel/events/internal.h
index 249288d..d9cc570 100644
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -35,7 +35,7 @@ struct perf_buffer {
 	spinlock_t			event_lock;
 	struct list_head		event_list;
 
-	atomic_t			mmap_count;
+	refcount_t			mmap_count;
 	unsigned long			mmap_locked;
 	struct user_struct		*mmap_user;
 
@@ -47,7 +47,7 @@ struct perf_buffer {
 	unsigned long			aux_pgoff;
 	int				aux_nr_pages;
 	int				aux_overwrite;
-	atomic_t			aux_mmap_count;
+	refcount_t			aux_mmap_count;
 	unsigned long			aux_mmap_locked;
 	void				(*free_aux)(void *);
 	refcount_t			aux_refcount;
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index aa9a759..20a9050 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -400,7 +400,7 @@ void *perf_aux_output_begin(struct perf_output_handle *handle,
 	 * the same order, see perf_mmap_close. Otherwise we end up freeing
 	 * aux pages in this path, which is a bug, because in_atomic().
 	 */
-	if (!atomic_read(&rb->aux_mmap_count))
+	if (!refcount_read(&rb->aux_mmap_count))
 		goto err;
 
 	if (!refcount_inc_not_zero(&rb->aux_refcount))

Re: [PATCH v3 00/15] perf: Convert mmap() related reference counts to refcount_t

[Lorenzo Stoakes]

On Tue, Aug 12, 2025 at 12:38:58PM +0200, Peter Zijlstra wrote:
> Took over the series from Thomas; much thanks to him for cleaning this up.
>
> The recently fixed reference count leaks could have been detected by using
> refcount_t and refcount_t would have mitigated the potential overflow at
> least.
>
> It turned out that converting the code as is does not work as the
> allocation code ends up doing a refcount_inc() for the first allocation,
> which causes refcount_t sanity checks to emit a UAF warning.
>
> The reason is that the code is sharing functionality at the wrong level and
> ends up being overly complicated for no reason. That's what inevitable led
> to the refcount leak problems.
>
> Address this by splitting the ringbuffer and the AUX buffer mapping and
> allocation parts out into seperate functions, which handle the reference
> counts in a sane way.
>
> That not only simplifies the code and makes it halfways comprehensible, but
> also allows to convert the mmap() related reference counts to refcount_t.
>
> It survives lightweight testing with perf and passes the perf/mmap
> selftest.
>
> Also available at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core
>
> ---
> v2: https://lkml.kernel.org/r/20250811123458.050061356@linutronix.de
>
> Changes vs v2:
>  - replaced patches 4,5 with fine grained steps
>

Whole series LGTM, various nits/comments but nothing of importance.

I also build-checked every commit and checked the new mmap self test all
looks good.

Thanks for this guys, huge improvement! :)