[RFC][PATCH] perf: Fix perf_addr_filters_afjust()

[RFC][PATCH] perf: Fix perf_addr_filters_afjust()

[Peter Zijlstra]

 - adjust lock order; since perf_event_addr_filters_apply() takes mmap_lock
   inside child_mutex.

 - note that perf_event_mmap() is called by the thread doing the
   mmap()/mprotect()/.. operation, but not the other threads in the address
   space that might have inherited the same event.

 - have perf_addr_filters_adjust() iterate the child_list to find all other
   events for tasks that have the same mm.

 - since this can now adjust addr_filter_ranges[] for remote events, make sure
   to adjust things while holding ctx->lock; local_irq_disable() as implied by
   the ifh->lock is no longer suffient to serialize against event scheduling.

 - per the previous locking order; perf_event_mmap(), which is called holding
   mmap_lock, cannot take child_mutex, as such, make child_list RCU protected
   and iterate under RCU.

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

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1454,10 +1454,11 @@ static void put_ctx(struct perf_event_co
  *	  perf_event_context::mutex
  *	    perf_event::child_mutex;
  *	      perf_event_context::lock
- *	    mmap_lock
- *	      perf_event::mmap_mutex
- *	        perf_buffer::aux_mutex
- *	      perf_addr_filters_head::lock
+ *	        perf_addr_filters_head::lock
+ *	      mmap_lock
+ *	        perf_event::mmap_mutex
+ *	          perf_buffer::aux_mutex
+ *	        perf_addr_filters_head::lock
  *
  *    cpu_hotplug_lock
  *      pmus_lock
@@ -2429,7 +2430,7 @@ static void perf_child_detach(struct per
 	lockdep_assert_held(&parent_event->child_mutex);
 	 */
 
-	list_del_init(&event->child_list);
+	list_del_rcu(&event->child_list);
 }
 
 static bool is_orphaned_event(struct perf_event *event)
@@ -9902,35 +9903,64 @@ static bool perf_addr_filter_vma_adjust(
 	return true;
 }
 
-static void __perf_addr_filters_adjust(struct perf_event *event, void *data)
+static int perf_child_addr_filters_adjust(struct perf_event *event,
+					  struct vm_area_struct *vma)
 {
 	struct perf_addr_filters_head *ifh = perf_event_addr_filters(event);
-	struct vm_area_struct *vma = data;
-	struct perf_addr_filter *filter;
 	unsigned int restart = 0, count = 0;
-	unsigned long flags;
-
-	if (!has_addr_filter(event))
-		return;
+	struct perf_addr_filter *filter;
 
-	if (!vma->vm_file)
-		return;
+	guard(raw_spinlock)(&ifh->lock);
 
-	raw_spin_lock_irqsave(&ifh->lock, flags);
 	list_for_each_entry(filter, &ifh->list, entry) {
 		if (perf_addr_filter_vma_adjust(filter, vma,
 						&event->addr_filter_ranges[count]))
 			restart++;
 
 		count++;
+
 	}
 
 	if (restart)
 		event->addr_filters_gen++;
-	raw_spin_unlock_irqrestore(&ifh->lock, flags);
 
-	if (restart)
-		perf_event_stop(event, 1);
+	return restart;
+}
+
+static void __perf_addr_filters_adjust(struct perf_event *event, void *data)
+{
+	struct perf_event *child, *parent = event->parent ?: event;
+	struct vm_area_struct *vma = data;
+
+	if (!has_addr_filter(event))
+		return;
+
+	if (!vma->vm_file)
+		return;
+
+	list_for_each_entry_rcu(child, &parent->child_list, child_list) {
+		struct perf_event_context *ctx;
+		struct task_struct *task;
+		int restart = 0;
+
+again:
+		ctx = READ_ONCE(child->ctx);
+		scoped_guard (raw_spinlock_irqsave, &ctx->lock) {
+			if (child->ctx != ctx)
+				goto again;
+
+			task = ctx->task;
+			if (!task || task == TASK_TOMBSTONE)
+				continue;
+
+			if (vma->vm_mm != task->mm)
+				continue;
+
+			restart = perf_child_addr_filters_adjust(child, vma);
+		}
+		if (restart)
+			perf_event_stop(child, 1);
+	}
 }
 
 /*
@@ -9947,11 +9977,10 @@ static void perf_addr_filters_adjust(str
 	if (!(vma->vm_flags & VM_EXEC))
 		return;
 
-	rcu_read_lock();
+	guard(rcu)();
 	ctx = rcu_dereference(current->perf_event_ctxp);
 	if (ctx)
 		perf_iterate_ctx(ctx, __perf_addr_filters_adjust, vma, true);
-	rcu_read_unlock();
 }
 
 void perf_event_mmap(struct vm_area_struct *vma)

[RFC][PATCH v2] perf: Fix perf_addr_filters_afjust()

[Peter Zijlstra]

 - adjust lock order; since perf_event_addr_filters_apply() takes mmap_lock
   inside child_mutex.

 - note that perf_event_mmap() is called by the thread doing the
   mmap()/mprotect()/.. operation, but not the other threads in the address
   space that might have inherited the same event.

 - have perf_addr_filters_adjust() iterate the child_list to find all other
   events for tasks that have the same mm.

 - since this can now adjust addr_filter_ranges[] for remote events, make sure
   to adjust things while holding ctx->lock; local_irq_disable() as implied by
   the ifh->lock is no longer suffient to serialize against event scheduling.

 - per the previous locking order; perf_event_mmap(), which is called holding
   mmap_lock, cannot take child_mutex, as such, make child_list RCU protected
   and iterate under RCU.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
addressed sashiko feedback

 kernel/events/core.c |   72 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 53 insertions(+), 19 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1454,10 +1454,11 @@ static void put_ctx(struct perf_event_co
  *	  perf_event_context::mutex
  *	    perf_event::child_mutex;
  *	      perf_event_context::lock
- *	    mmap_lock
- *	      perf_event::mmap_mutex
- *	        perf_buffer::aux_mutex
- *	      perf_addr_filters_head::lock
+ *	        perf_addr_filters_head::lock
+ *	      mmap_lock
+ *	        perf_event::mmap_mutex
+ *	          perf_buffer::aux_mutex
+ *	        perf_addr_filters_head::lock
  *
  *    cpu_hotplug_lock
  *      pmus_lock
@@ -2429,7 +2430,7 @@ static void perf_child_detach(struct per
 	lockdep_assert_held(&parent_event->child_mutex);
 	 */
 
-	list_del_init(&event->child_list);
+	list_del_rcu(&event->child_list);
 }
 
 static bool is_orphaned_event(struct perf_event *event)
@@ -9902,21 +9903,15 @@ static bool perf_addr_filter_vma_adjust(
 	return true;
 }
 
-static void __perf_addr_filters_adjust(struct perf_event *event, void *data)
+static int __perf_event_addr_filters_adjust(struct perf_event *event,
+					    struct vm_area_struct *vma)
 {
 	struct perf_addr_filters_head *ifh = perf_event_addr_filters(event);
-	struct vm_area_struct *vma = data;
-	struct perf_addr_filter *filter;
 	unsigned int restart = 0, count = 0;
-	unsigned long flags;
-
-	if (!has_addr_filter(event))
-		return;
+	struct perf_addr_filter *filter;
 
-	if (!vma->vm_file)
-		return;
+	guard(raw_spinlock)(&ifh->lock);
 
-	raw_spin_lock_irqsave(&ifh->lock, flags);
 	list_for_each_entry(filter, &ifh->list, entry) {
 		if (perf_addr_filter_vma_adjust(filter, vma,
 						&event->addr_filter_ranges[count]))
@@ -9927,12 +9922,52 @@ static void __perf_addr_filters_adjust(s
 
 	if (restart)
 		event->addr_filters_gen++;
-	raw_spin_unlock_irqrestore(&ifh->lock, flags);
 
+	return restart;
+}
+
+static void perf_event_addr_filters_adjust(struct perf_event *event,
+					   struct vm_area_struct *vma)
+{
+	struct perf_event_context *ctx;
+	struct task_struct *task;
+	int restart = 0;
+
+again:
+	ctx = READ_ONCE(event->ctx);
+	scoped_guard (raw_spinlock_irqsave, &ctx->lock) {
+		if (event->ctx != ctx)
+			goto again;
+
+		task = ctx->task;
+		if (!task || task == TASK_TOMBSTONE)
+			continue;
+
+		if (vma->vm_mm != task->mm)
+			continue;
+
+		restart = __perf_event_addr_filters_adjust(event, vma);
+	}
 	if (restart)
 		perf_event_stop(event, 1);
 }
 
+static void __perf_addr_filters_adjust(struct perf_event *event, void *data)
+{
+	struct perf_event *child, *parent = event->parent ?: event;
+	struct vm_area_struct *vma = data;
+
+	if (!has_addr_filter(event))
+		return;
+
+	if (!vma->vm_file)
+		return;
+
+	perf_event_addr_filters_adjust(parent, vma);
+	list_for_each_entry_rcu(child, &parent->child_list, child_list)
+		perf_event_addr_filters_adjust(child, vma);
+}
+
 /*
  * Adjust all task's events' filters to the new vma
  */
@@ -9947,11 +9982,10 @@ static void perf_addr_filters_adjust(str
 	if (!(vma->vm_flags & VM_EXEC))
 		return;
 
-	rcu_read_lock();
+	guard(rcu)();
 	ctx = rcu_dereference(current->perf_event_ctxp);
 	if (ctx)
 		perf_iterate_ctx(ctx, __perf_addr_filters_adjust, vma, true);
-	rcu_read_unlock();
 }
 
 void perf_event_mmap(struct vm_area_struct *vma)
@@ -14880,7 +14914,7 @@ inherit_event(struct perf_event *parent_
 	/*
 	 * Link this into the parent event's child list
 	 */
-	list_add_tail(&child_event->child_list, &parent_event->child_list);
+	list_add_tail_rcu(&child_event->child_list, &parent_event->child_list);
 	mutex_unlock(&parent_event->child_mutex);
 
 	return child_event;

Re: [RFC][PATCH v2] perf: Fix perf_addr_filters_afjust()

[Leo Yan]

On Tue, Jun 23, 2026 at 11:14:26AM +0200, Peter Zijlstra wrote:
> 
>  - adjust lock order; since perf_event_addr_filters_apply() takes mmap_lock
>    inside child_mutex.
> 
>  - note that perf_event_mmap() is called by the thread doing the
>    mmap()/mprotect()/.. operation, but not the other threads in the address
>    space that might have inherited the same event.
> 
>  - have perf_addr_filters_adjust() iterate the child_list to find all other
>    events for tasks that have the same mm.
> 
>  - since this can now adjust addr_filter_ranges[] for remote events, make sure
>    to adjust things while holding ctx->lock; local_irq_disable() as implied by
>    the ifh->lock is no longer suffient to serialize against event scheduling.
> 
>  - per the previous locking order; perf_event_mmap(), which is called holding
>    mmap_lock, cannot take child_mutex, as such, make child_list RCU protected
>    and iterate under RCU.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Just verified this patch with Arm CoreSight:

  perf record -e cs_etm//u --filter 'filter traced_region @/root/test' -- /root/test

The workload forks a child process to inherited the event, also used
trace_printk() to confirm perf_event_addr_filters_adjust() has been hit.
No warning reported by LOCKDEP.

Tested-by: Leo Yan <leo.yan@arm.com>

If you want me to try any specific test, pleaset let me know and I am
glad to give a run.

Maybe it is good to fix typo in the subject: s/afjust()/adjust()/.