Re: [BUG] lock issue in perf cgroup with jump label enabled

[Peter Zijlstra <peterz@infradead.org>]

On Thu, 2011-03-03 at 16:31 +0800, Li Zefan wrote:
> With CONFIG_JUMP_LABEL enabled:
> 
> # mount -t cgroup -o perf_event xxx /mnt
> # mkdir /mnt/test
> # ./perf stat -B -a -e cycles:u -G test -- sleep 1
> 
> lockdep warning will be triggered immediately, and the machine
> will become unresponsive.
> 
> I guess this is the culprit?
> 
> In the "perf cgroup support" patch:
> 
> @@ -419,6 +781,17 @@ list_add_event(struct perf_event *event, struct perf_event_con
>                 list_add_tail(&event->group_entry, list);
>         }
>  
> +       if (is_cgroup_event(event)) {
> +               ctx->nr_cgroups++;
> +               /*
> +                * one more event:
> +                * - that has cgroup constraint on event->cpu
> +                * - that may need work on context switch
> +                */
> +               atomic_inc(&per_cpu(perf_cgroup_events, event->cpu));
> +               jump_label_inc(&perf_sched_events);
> +       }
> 
> list_add_event() is called with spin lock ctl->lock held,  while
> jump_label_inc() will acquire jump_label_mutex..

I think something like the below ought to cure that.

---
 kernel/perf_event.c |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index 64a018e..6bc69a4 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -817,16 +817,8 @@ list_add_event(struct perf_event *event, struct perf_event_context *ctx)
 		list_add_tail(&event->group_entry, list);
 	}
 
-	if (is_cgroup_event(event)) {
+	if (is_cgroup_event(event))
 		ctx->nr_cgroups++;
-		/*
-		 * one more event:
-		 * - that has cgroup constraint on event->cpu
-		 * - that may need work on context switch
-		 */
-		atomic_inc(&per_cpu(perf_cgroup_events, event->cpu));
-		jump_label_inc(&perf_sched_events);
-	}
 
 	list_add_rcu(&event->event_entry, &ctx->event_list);
 	if (!ctx->nr_events)
@@ -6553,6 +6545,16 @@ SYSCALL_DEFINE5(perf_event_open,
 		goto err_context;
 	}
 
+	if (is_cgroup_event(event)) {
+		/*
+		 * one more event:
+		 * - that has cgroup constraint on event->cpu
+		 * - that may need work on context switch
+		 */
+		atomic_inc(&per_cpu(perf_cgroup_events, event->cpu));
+		jump_label_inc(&perf_sched_events);
+	}
+
 	if (move_group) {
 		struct perf_event_context *gctx = group_leader->ctx;