[RFC][PATCH 00/12] various perf fixes

[RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

Hi,

I've been hunting perf bugs for the past few weeks. This resulted in this pile
of patches, that mostly seems to work -- I still get an occasional fail so
something is still off.

But I've been sitting on this for far too long, so here goes.

(I've not at all tested perf-cgroup)

The code compiles in between patches, but I've not bothered trying to run/test
any intermediate stage. There's just too many inter-related fail.

Alexander, Stephane, could you guys please have a hard look at this?

Andi, Dmitry, patch 10 should explain the getting stuck in
perf_install_in_context() forever thing you both have observed.

[RFC][PATCH 01/12] perf: Add lockdep assertions

[Peter Zijlstra]

[-- Attachment #1: peterz-perf-fixes-1.patch --]
[-- Type: text/plain, Size: 1137 bytes --]


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

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1246,6 +1246,8 @@ ctx_group_list(struct perf_event *event,
 static void
 list_add_event(struct perf_event *event, struct perf_event_context *ctx)
 {
+	lockdep_assert_held(&ctx->lock);
+
 	WARN_ON_ONCE(event->attach_state & PERF_ATTACH_CONTEXT);
 	event->attach_state |= PERF_ATTACH_CONTEXT;
 
@@ -2342,8 +2344,10 @@ static void ctx_sched_out(struct perf_ev
 			  struct perf_cpu_context *cpuctx,
 			  enum event_type_t event_type)
 {
-	struct perf_event *event;
 	int is_active = ctx->is_active;
+	struct perf_event *event;
+
+	lockdep_assert_held(&ctx->lock);
 
 	ctx->is_active &= ~event_type;
 	if (likely(!ctx->nr_events))
@@ -2725,8 +2729,10 @@ ctx_sched_in(struct perf_event_context *
 	     enum event_type_t event_type,
 	     struct task_struct *task)
 {
-	u64 now;
 	int is_active = ctx->is_active;
+	u64 now;
+
+	lockdep_assert_held(&ctx->lock);
 
 	ctx->is_active |= event_type;
 	if (likely(!ctx->nr_events))

[RFC][PATCH 02/12] perf: Fix cgroup event scheduling

[Peter Zijlstra]

[-- Attachment #1: peterz-perf-fixes-2.patch --]
[-- Type: text/plain, Size: 1670 bytes --]

There appears to be a problemin __perf_event_task_sched_in() wrt
cgroup event scheduling.

The normal event scheduling order is:

	CPU pinned
	Task pinned
	CPU flexible
	Task flexible

And since perf_cgroup_sched*() only schedules the cpu context, we must
call this _before_ adding the task events.

Note: double check what happens on the ctx switch optimization where
the task ctx isn't scheduled.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2804,6 +2804,16 @@ void __perf_event_task_sched_in(struct t
 	struct perf_event_context *ctx;
 	int ctxn;
 
+	/*
+	 * If cgroup events exist on this CPU, then we need to check if we have
+	 * to switch in PMU state; cgroup event are system-wide mode only.
+	 *
+	 * Since cgroup events are CPU events, we must schedule these in before
+	 * we schedule in the task events.
+	 */
+	if (atomic_read(this_cpu_ptr(&perf_cgroup_events)))
+		perf_cgroup_sched_in(prev, task);
+
 	for_each_task_context_nr(ctxn) {
 		ctx = task->perf_event_ctxp[ctxn];
 		if (likely(!ctx))
@@ -2811,13 +2821,6 @@ void __perf_event_task_sched_in(struct t
 
 		perf_event_context_sched_in(ctx, task);
 	}
-	/*
-	 * if cgroup events exist on this CPU, then we need
-	 * to check if we have to switch in PMU state.
-	 * cgroup event are system-wide mode only
-	 */
-	if (atomic_read(this_cpu_ptr(&perf_cgroup_events)))
-		perf_cgroup_sched_in(prev, task);
 
 	if (atomic_read(&nr_switch_events))
 		perf_event_switch(task, prev, true);

[RFC][PATCH 03/12] perf: Fix cgroup scheduling in enable_on_exec

[Peter Zijlstra]

[-- Attachment #1: peterz-perf-fixes-3.patch --]
[-- Type: text/plain, Size: 2428 bytes --]

There is a comment that states that perf_event_context_sched_in() will
also switch in the cgroup events, I cannot find it does so. Therefore
all the resulting logic goes out the window too.

Clean that up.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |   31 +++++++------------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -579,13 +579,7 @@ static inline void perf_cgroup_sched_out
 	 * we are holding the rcu lock
 	 */
 	cgrp1 = perf_cgroup_from_task(task, NULL);
-
-	/*
-	 * next is NULL when called from perf_event_enable_on_exec()
-	 * that will systematically cause a cgroup_switch()
-	 */
-	if (next)
-		cgrp2 = perf_cgroup_from_task(next, NULL);
+	cgrp2 = perf_cgroup_from_task(next, NULL);
 
 	/*
 	 * only schedule out current cgroup events if we know
@@ -611,8 +605,6 @@ static inline void perf_cgroup_sched_in(
 	 * we are holding the rcu lock
 	 */
 	cgrp1 = perf_cgroup_from_task(task, NULL);
-
-	/* prev can never be NULL */
 	cgrp2 = perf_cgroup_from_task(prev, NULL);
 
 	/*
@@ -1450,11 +1442,14 @@ list_del_event(struct perf_event *event,
 
 	if (is_cgroup_event(event)) {
 		ctx->nr_cgroups--;
+		/*
+		 * Because cgroup events are always per-cpu events, this will
+		 * always be called from the right CPU.
+		 */
 		cpuctx = __get_cpu_context(ctx);
 		/*
-		 * if there are no more cgroup events
-		 * then cler cgrp to avoid stale pointer
-		 * in update_cgrp_time_from_cpuctx()
+		 * If there are no more cgroup events then clear cgrp to avoid
+		 * stale pointer in update_cgrp_time_from_cpuctx().
 		 */
 		if (!ctx->nr_cgroups)
 			cpuctx->cgrp = NULL;
@@ -3118,15 +3113,6 @@ static void perf_event_enable_on_exec(in
 	if (!ctx || !ctx->nr_events)
 		goto out;
 
-	/*
-	 * We must ctxsw out cgroup events to avoid conflict
-	 * when invoking perf_task_event_sched_in() later on
-	 * in this function. Otherwise we end up trying to
-	 * ctxswin cgroup events which are already scheduled
-	 * in.
-	 */
-	perf_cgroup_sched_out(current, NULL);
-
 	raw_spin_lock(&ctx->lock);
 	task_ctx_sched_out(ctx);
 
@@ -3144,9 +3130,6 @@ static void perf_event_enable_on_exec(in
 
 	raw_spin_unlock(&ctx->lock);
 
-	/*
-	 * Also calls ctxswin for cgroup events, if any:
-	 */
 	perf_event_context_sched_in(ctx, ctx->task);
 out:
 	local_irq_restore(flags);

[RFC][PATCH 04/12] perf: Remove stale comment

[Peter Zijlstra]

[-- Attachment #1: peterz-perf-fixes-4.patch --]
[-- Type: text/plain, Size: 740 bytes --]

The comment here is horribly out of date, remove it.

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

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2130,13 +2130,6 @@ static int  __perf_install_in_context(vo
 
 /*
  * Attach a performance event to a context
- *
- * First we add the event to the list with the hardware enable bit
- * in event->hw_config cleared.
- *
- * If the event is attached to a task which is on a CPU we use a smp
- * call to enable it in the task context. The task might have been
- * scheduled away, but we check this in the smp call again.
  */
 static void
 perf_install_in_context(struct perf_event_context *ctx,

[RFC][PATCH 05/12] perf: Fix enable_on_exec event scheduling

[Peter Zijlstra]

[-- Attachment #1: peterz-perf-fixes-6.patch --]
[-- Type: text/plain, Size: 3806 bytes --]

There are two problems with the current enable_on_exec event
scheduling:

  - the newly enabled events will be immediately scheduled
    irrespective of their ctx event list order.

  - there's a hole in the ctx->lock between scheduling the events
    out and putting them back on.

Esp. the latter issue is a real problem because a hole in event
scheduling leaves the thing in an observable inconsistent state,
confusing things.

Fix both issues by first doing the enable iteration and at the end,
when there are newly enabled events, reschedule the ctx in one go.

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

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2036,7 +2036,8 @@ static void add_event_to_ctx(struct perf
 	event->tstamp_stopped = tstamp;
 }
 
-static void task_ctx_sched_out(struct perf_event_context *ctx);
+static void task_ctx_sched_out(struct perf_cpu_context *cpuctx,
+			       struct perf_event_context *ctx);
 static void
 ctx_sched_in(struct perf_event_context *ctx,
 	     struct perf_cpu_context *cpuctx,
@@ -2067,6 +2068,17 @@ static void ___perf_install_in_context(v
 	add_event_to_ctx(event, ctx);
 }
 
+static void ctx_resched(struct perf_cpu_context *cpuctx,
+			struct perf_event_context *task_ctx)
+{
+	perf_pmu_disable(cpuctx->ctx.pmu);
+	if (task_ctx)
+		task_ctx_sched_out(cpuctx, task_ctx);
+	cpu_ctx_sched_out(cpuctx, EVENT_ALL);
+	perf_event_sched_in(cpuctx, task_ctx, current);
+	perf_pmu_enable(cpuctx->ctx.pmu);
+}
+
 /*
  * Cross CPU call to install and enable a performance event
  *
@@ -2087,7 +2099,7 @@ static int  __perf_install_in_context(vo
 	 * If there was an active task_ctx schedule it out.
 	 */
 	if (task_ctx)
-		task_ctx_sched_out(task_ctx);
+		task_ctx_sched_out(cpuctx, task_ctx);
 
 	/*
 	 * If the context we're installing events in is not the
@@ -2629,10 +2641,9 @@ void __perf_event_task_sched_out(struct
 		perf_cgroup_sched_out(task, next);
 }
 
-static void task_ctx_sched_out(struct perf_event_context *ctx)
+static void task_ctx_sched_out(struct perf_cpu_context *cpuctx,
+			       struct perf_event_context *ctx)
 {
-	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
-
 	if (!cpuctx->task_ctx)
 		return;
 
@@ -3096,34 +3107,30 @@ static int event_enable_on_exec(struct p
 static void perf_event_enable_on_exec(int ctxn)
 {
 	struct perf_event_context *ctx, *clone_ctx = NULL;
+	struct perf_cpu_context *cpuctx;
 	struct perf_event *event;
 	unsigned long flags;
 	int enabled = 0;
-	int ret;
 
 	local_irq_save(flags);
 	ctx = current->perf_event_ctxp[ctxn];
 	if (!ctx || !ctx->nr_events)
 		goto out;
 
-	raw_spin_lock(&ctx->lock);
-	task_ctx_sched_out(ctx);
-
-	list_for_each_entry(event, &ctx->event_list, event_entry) {
-		ret = event_enable_on_exec(event, ctx);
-		if (ret)
-			enabled = 1;
-	}
+	cpuctx = __get_cpu_context(ctx);
+	perf_ctx_lock(cpuctx, ctx);
+	list_for_each_entry(event, &ctx->event_list, event_entry)
+		enabled |= event_enable_on_exec(event, ctx);
 
 	/*
-	 * Unclone this context if we enabled any event.
+	 * Unclone and reschedule this context if we enabled any event.
 	 */
-	if (enabled)
+	if (enabled) {
 		clone_ctx = unclone_ctx(ctx);
+		ctx_resched(cpuctx, ctx);
+	}
+	perf_ctx_unlock(cpuctx, ctx);
 
-	raw_spin_unlock(&ctx->lock);
-
-	perf_event_context_sched_in(ctx, ctx->task);
 out:
 	local_irq_restore(flags);
 
@@ -8737,7 +8744,7 @@ static void perf_event_exit_task_context
 	 * incremented the context's refcount before we do put_ctx below.
 	 */
 	raw_spin_lock(&child_ctx->lock);
-	task_ctx_sched_out(child_ctx);
+	task_ctx_sched_out(__get_cpu_context(child_ctx), child_ctx);
 	child->perf_event_ctxp[ctxn] = NULL;
 
 	/*

[RFC][PATCH 06/12] perf: Use task_ctx_sched_out()

[Peter Zijlstra]

[-- Attachment #1: peterz-perf-fixes-7.patch --]
[-- Type: text/plain, Size: 578 bytes --]

We have a function that does exactly what we want here, use it. This
reduces the amount of cpuctx->task_ctx muckery.

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

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2545,8 +2545,7 @@ static void perf_event_context_sched_out
 
 	if (do_switch) {
 		raw_spin_lock(&ctx->lock);
-		ctx_sched_out(ctx, cpuctx, EVENT_ALL);
-		cpuctx->task_ctx = NULL;
+		task_ctx_sched_out(cpuctx, ctx);
 		raw_spin_unlock(&ctx->lock);
 	}
 }

[RFC][PATCH 07/12] perf: Simplify/fix perf_event_enable() event scheduling

[Peter Zijlstra]

[-- Attachment #1: peterz-perf-fixes-8.patch --]
[-- Type: text/plain, Size: 2032 bytes --]

Like enable_on_exec, perf_event_enable() event scheduling has problems
respecting the context hierarchy when trying to schedule events (for
example, it will try and add a pinned event without first removing
existing flexible events).

So simplify it by using the new ctx_resched() call which will DTRT.

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

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2188,7 +2188,7 @@ static int __perf_event_enable(void *inf
 	struct perf_event_context *ctx = event->ctx;
 	struct perf_event *leader = event->group_leader;
 	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
-	int err;
+	struct perf_event_context *task_ctx = cpuctx->task_ctx;
 
 	/*
 	 * There's a time window between 'ctx->is_active' check
@@ -2202,7 +2202,8 @@ static int __perf_event_enable(void *inf
 	if (!ctx->is_active)
 		return -EINVAL;
 
-	raw_spin_lock(&ctx->lock);
+	perf_ctx_lock(cpuctx, task_ctx);
+	WARN_ON_ONCE(&cpuctx->ctx != ctx && task_ctx != ctx);
 	update_context_time(ctx);
 
 	if (event->state >= PERF_EVENT_STATE_INACTIVE)
@@ -2228,32 +2229,10 @@ static int __perf_event_enable(void *inf
 	if (leader != event && leader->state != PERF_EVENT_STATE_ACTIVE)
 		goto unlock;
 
-	if (!group_can_go_on(event, cpuctx, 1)) {
-		err = -EEXIST;
-	} else {
-		if (event == leader)
-			err = group_sched_in(event, cpuctx, ctx);
-		else
-			err = event_sched_in(event, cpuctx, ctx);
-	}
-
-	if (err) {
-		/*
-		 * If this event can't go on and it's part of a
-		 * group, then the whole group has to come off.
-		 */
-		if (leader != event) {
-			group_sched_out(leader, cpuctx, ctx);
-			perf_mux_hrtimer_restart(cpuctx);
-		}
-		if (leader->attr.pinned) {
-			update_group_times(leader);
-			leader->state = PERF_EVENT_STATE_ERROR;
-		}
-	}
+	ctx_resched(cpuctx, task_ctx);
 
 unlock:
-	raw_spin_unlock(&ctx->lock);
+	perf_ctx_unlock(cpuctx, task_ctx);
 
 	return 0;
 }

[RFC][PATCH 08/12] perf: Optimize perf_sched_events usage

[Peter Zijlstra]

[-- Attachment #1: peterz-perf-fixes-9.patch --]
[-- Type: text/plain, Size: 2082 bytes --]

It doesn't make sense to take up-to _4_ references on
perf_sched_events per event, avoid doing this.

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

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3493,11 +3493,13 @@ static void unaccount_event_cpu(struct p
 
 static void unaccount_event(struct perf_event *event)
 {
+	bool dec = false;
+
 	if (event->parent)
 		return;
 
 	if (event->attach_state & PERF_ATTACH_TASK)
-		static_key_slow_dec_deferred(&perf_sched_events);
+		dec = true;
 	if (event->attr.mmap || event->attr.mmap_data)
 		atomic_dec(&nr_mmap_events);
 	if (event->attr.comm)
@@ -3507,12 +3509,15 @@ static void unaccount_event(struct perf_
 	if (event->attr.freq)
 		atomic_dec(&nr_freq_events);
 	if (event->attr.context_switch) {
-		static_key_slow_dec_deferred(&perf_sched_events);
+		dec = true;
 		atomic_dec(&nr_switch_events);
 	}
 	if (is_cgroup_event(event))
-		static_key_slow_dec_deferred(&perf_sched_events);
+		dec = true;
 	if (has_branch_stack(event))
+		dec = true;
+
+	if (dec)
 		static_key_slow_dec_deferred(&perf_sched_events);
 
 	unaccount_event_cpu(event, event->cpu);
@@ -7728,11 +7733,13 @@ static void account_event_cpu(struct per
 
 static void account_event(struct perf_event *event)
 {
+	bool inc = false;
+
 	if (event->parent)
 		return;
 
 	if (event->attach_state & PERF_ATTACH_TASK)
-		static_key_slow_inc(&perf_sched_events.key);
+		inc = true;
 	if (event->attr.mmap || event->attr.mmap_data)
 		atomic_inc(&nr_mmap_events);
 	if (event->attr.comm)
@@ -7745,11 +7752,14 @@ static void account_event(struct perf_ev
 	}
 	if (event->attr.context_switch) {
 		atomic_inc(&nr_switch_events);
-		static_key_slow_inc(&perf_sched_events.key);
+		inc = true;
 	}
 	if (has_branch_stack(event))
-		static_key_slow_inc(&perf_sched_events.key);
+		inc = true;
 	if (is_cgroup_event(event))
+		inc = true;
+
+	if (inc)
 		static_key_slow_inc(&perf_sched_events.key);
 
 	account_event_cpu(event, event->cpu);

[RFC][PATCH 09/12] perf: Make ctx->is_active and cpuctx->task_ctx consistent

[Peter Zijlstra]

[-- Attachment #1: peterz-perf-fixes-10.patch --]
[-- Type: text/plain, Size: 1615 bytes --]

For no apparent reason and to great confusion the rules for
ctx->is_active and cpuctx->task_ctx are different. This means that its
not always possible to find all active (task) contexts.

Fix this such that if ctx->is_active gets set, we also set (or verify)
cpuctx->task_ctx.

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

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2330,6 +2330,12 @@ static void ctx_sched_out(struct perf_ev
 	lockdep_assert_held(&ctx->lock);
 
 	ctx->is_active &= ~event_type;
+	if (ctx->task) {
+		WARN_ON_ONCE(cpuctx->task_ctx != ctx);
+		if (!ctx->is_active)
+			cpuctx->task_ctx = NULL;
+	}
+
 	if (likely(!ctx->nr_events))
 		return;
 
@@ -2631,7 +2637,6 @@ static void task_ctx_sched_out(struct pe
 		return;
 
 	ctx_sched_out(ctx, cpuctx, EVENT_ALL);
-	cpuctx->task_ctx = NULL;
 }
 
 /*
@@ -2714,6 +2719,13 @@ ctx_sched_in(struct perf_event_context *
 	lockdep_assert_held(&ctx->lock);
 
 	ctx->is_active |= event_type;
+	if (ctx->task) {
+		if (!is_active)
+			cpuctx->task_ctx = ctx;
+		else
+			WARN_ON_ONCE(cpuctx->task_ctx != ctx);
+	}
+
 	if (likely(!ctx->nr_events))
 		return;
 
@@ -2758,12 +2770,7 @@ static void perf_event_context_sched_in(
 	 * cpu flexible, task flexible.
 	 */
 	cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE);
-
-	if (ctx->nr_events)
-		cpuctx->task_ctx = ctx;
-
-	perf_event_sched_in(cpuctx, cpuctx->task_ctx, task);
-
+	perf_event_sched_in(cpuctx, ctx, task);
 	perf_pmu_enable(ctx->pmu);
 	perf_ctx_unlock(cpuctx, ctx);
 }

[RFC][PATCH 10/12] perf: Fix task context scheduling

[Peter Zijlstra]

[-- Attachment #1: peterz-perf-fixes-12.patch --]
[-- Type: text/plain, Size: 8154 bytes --]

There is a very nasty problem wrt disabling the perf task scheduling
hooks.

Currently we {set,clear} ctx->is_active on every
__perf_event_task_sched_{in,out}, _however_ this means that if we
disable these calls we'll have task contexts with ->is_active set that
are not active and 'active' task contexts without ->is_active set.

This can result in event_function_call() looping on the ctx->is_active
condition basically indefinitely.

Resolve this by changing things such that contexts without events do
not set ->is_active like we used to. From this invariant it trivially
follows that if there are no (task) events, every task ctx is inactive
and disabling the context switch hooks is harmless.

This leaves two places that need attention (and already had
accumulated weird and wonderful hacks to work around, without
recognising this actual problem).

Namely:

 - perf_install_in_context() will need to deal with installing events
   in an inactive context, meaning it cannot rely on ctx-is_active for
   its IPIs.

 - perf_remove_from_context() will have to mark a context as inactive
   when it removes the last event.

For specific detail, see the patch/comments.

Cc: Andi Kleen <andi@firstfloor.org>
Cc: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |  155 +++++++++++++++++++++++++++++----------------------
 1 file changed, 91 insertions(+), 64 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -126,6 +126,38 @@ static int cpu_function_call(int cpu, re
 	return data.ret;
 }
 
+/*
+ * On task ctx scheduling...
+ *
+ * When !ctx->nr_events a task context will not be scheduled. This means
+ * we can disable the scheduler hooks (for performance) without leaving
+ * pending task ctx state.
+ *
+ * This however results in two special cases:
+ *
+ *  - removing the last event from a task ctx; this is relatively straight
+ *    forward and is done in __perf_remove_from_context.
+ *
+ *  - adding the first event to a task ctx; this is tricky because we cannot
+ *    rely on ctx->is_active and therefore cannot use event_function_call().
+ *    See perf_install_in_context().
+ *
+ * This is because we need a ctx->lock serialized variable (ctx->is_active)
+ * to reliably determine if a particular task/context is scheduled in. The
+ * task_curr() use in task_function_call() is racy in that a remote context
+ * switch is not a single atomic operation.
+ *
+ * As is, the situation is 'safe' because we set rq->curr before we do the
+ * actual context switch. This means that task_curr() will fail early, but
+ * we'll continue spinning on ctx->is_active until we've passed
+ * perf_event_task_sched_out().
+ *
+ * Without this ctx->lock serialized variable we could have race where we find
+ * the task (and hence the context) would not be active while in fact they are.
+ *
+ * If ctx->nr_events, then ctx->is_active and cpuctx->task_ctx are set.
+ */
+
 static void event_function_call(struct perf_event *event,
 				int (*active)(void *),
 				void (*inactive)(void *),
@@ -1686,9 +1718,13 @@ static int __perf_remove_from_context(vo
 	if (re->detach_group)
 		perf_group_detach(event);
 	list_del_event(event, ctx);
-	if (!ctx->nr_events && cpuctx->task_ctx == ctx) {
+
+	if (!ctx->nr_events && ctx->is_active) {
 		ctx->is_active = 0;
-		cpuctx->task_ctx = NULL;
+		if (ctx->task) {
+			WARN_ON_ONCE(cpuctx->task_ctx != ctx);
+			cpuctx->task_ctx = NULL;
+		}
 	}
 	raw_spin_unlock(&ctx->lock);
 
@@ -2056,18 +2092,6 @@ static void perf_event_sched_in(struct p
 		ctx_sched_in(ctx, cpuctx, EVENT_FLEXIBLE, task);
 }
 
-static void ___perf_install_in_context(void *info)
-{
-	struct perf_event *event = info;
-	struct perf_event_context *ctx = event->ctx;
-
-	/*
-	 * Since the task isn't running, its safe to add the event, us holding
-	 * the ctx->lock ensures the task won't get scheduled in.
-	 */
-	add_event_to_ctx(event, ctx);
-}
-
 static void ctx_resched(struct perf_cpu_context *cpuctx,
 			struct perf_event_context *task_ctx)
 {
@@ -2086,55 +2110,27 @@ static void ctx_resched(struct perf_cpu_
  */
 static int  __perf_install_in_context(void *info)
 {
-	struct perf_event *event = info;
-	struct perf_event_context *ctx = event->ctx;
+	struct perf_event_context *ctx = info;
 	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
 	struct perf_event_context *task_ctx = cpuctx->task_ctx;
-	struct task_struct *task = current;
 
-	perf_ctx_lock(cpuctx, task_ctx);
-	perf_pmu_disable(cpuctx->ctx.pmu);
-
-	/*
-	 * If there was an active task_ctx schedule it out.
-	 */
-	if (task_ctx)
-		task_ctx_sched_out(cpuctx, task_ctx);
+	if (ctx->task) {
+		/*
+		 * If we hit the 'wrong' task, we've since scheduled and
+		 * everything should be sorted, nothing to do!
+		 */
+		if (ctx->task != current)
+			return 0;
 
-	/*
-	 * If the context we're installing events in is not the
-	 * active task_ctx, flip them.
-	 */
-	if (ctx->task && task_ctx != ctx) {
-		if (task_ctx)
-			raw_spin_unlock(&task_ctx->lock);
-		raw_spin_lock(&ctx->lock);
+		/*
+		 * If task_ctx is set, it had better be to us.
+		 */
+		WARN_ON_ONCE(cpuctx->task_ctx != ctx && cpuctx->task_ctx);
 		task_ctx = ctx;
 	}
 
-	if (task_ctx) {
-		cpuctx->task_ctx = task_ctx;
-		task = task_ctx->task;
-	}
-
-	cpu_ctx_sched_out(cpuctx, EVENT_ALL);
-
-	update_context_time(ctx);
-	/*
-	 * update cgrp time only if current cgrp
-	 * matches event->cgrp. Must be done before
-	 * calling add_event_to_ctx()
-	 */
-	update_cgrp_time_from_event(event);
-
-	add_event_to_ctx(event, ctx);
-
-	/*
-	 * Schedule everything back in
-	 */
-	perf_event_sched_in(cpuctx, task_ctx, task);
-
-	perf_pmu_enable(cpuctx->ctx.pmu);
+	perf_ctx_lock(cpuctx, task_ctx);
+	ctx_resched(cpuctx, task_ctx);
 	perf_ctx_unlock(cpuctx, task_ctx);
 
 	return 0;
@@ -2148,14 +2144,38 @@ perf_install_in_context(struct perf_even
 			struct perf_event *event,
 			int cpu)
 {
+	struct task_struct *task = NULL;
+
 	lockdep_assert_held(&ctx->mutex);
 
 	event->ctx = ctx;
 	if (event->cpu != -1)
 		event->cpu = cpu;
 
-	event_function_call(event, __perf_install_in_context,
-			    ___perf_install_in_context, event);
+	/*
+	 * Installing events is tricky because we cannot rely on ctx->is_active
+	 * to be set in case this is the nr_events 0 -> 1 transition.
+	 *
+	 * So what we do is we add the event to the list here, which will allow
+	 * a future context switch to DTRT and then send a racy IPI. If the IPI
+	 * fails to hit the right task, this means a context switch must have
+	 * happened and that will have taken care of business.
+	 */
+	raw_spin_lock_irq(&ctx->lock);
+	update_context_time(ctx);
+	/*
+	 * Update cgrp time only if current cgrp matches event->cgrp.
+	 * Must be done before calling add_event_to_ctx().
+	 */
+	update_cgrp_time_from_event(event);
+	add_event_to_ctx(event, ctx);
+	task = ctx->task;
+	raw_spin_unlock_irq(&ctx->lock);
+
+	if (task)
+		task_function_call(task, __perf_install_in_context, ctx);
+	else
+		cpu_function_call(cpu, __perf_install_in_context, ctx);
 }
 
 /*
@@ -2328,6 +2348,16 @@ static void ctx_sched_out(struct perf_ev
 
 	lockdep_assert_held(&ctx->lock);
 
+	if (likely(!ctx->nr_events)) {
+		/*
+		 * See __perf_remove_from_context().
+		 */
+		WARN_ON_ONCE(ctx->is_active);
+		if (ctx->task)
+			WARN_ON_ONCE(cpuctx->task_ctx);
+		return;
+	}
+
 	ctx->is_active &= ~event_type;
 	if (ctx->task) {
 		WARN_ON_ONCE(cpuctx->task_ctx != ctx);
@@ -2335,9 +2365,6 @@ static void ctx_sched_out(struct perf_ev
 			cpuctx->task_ctx = NULL;
 	}
 
-	if (likely(!ctx->nr_events))
-		return;
-
 	update_context_time(ctx);
 	update_cgrp_time_from_cpuctx(cpuctx);
 	if (!ctx->nr_active)
@@ -2716,6 +2743,9 @@ ctx_sched_in(struct perf_event_context *
 
 	lockdep_assert_held(&ctx->lock);
 
+	if (likely(!ctx->nr_events))
+		return;
+
 	ctx->is_active |= event_type;
 	if (ctx->task) {
 		if (!is_active)
@@ -2724,9 +2754,6 @@ ctx_sched_in(struct perf_event_context *
 			WARN_ON_ONCE(cpuctx->task_ctx != ctx);
 	}
 
-	if (likely(!ctx->nr_events))
-		return;
-
 	now = perf_clock();
 	ctx->timestamp = now;
 	perf_cgroup_set_timestamp(task, ctx);

[RFC][PATCH 11/12] perf: Specialize perf_event_exit_task()

[Peter Zijlstra]

[-- Attachment #1: peterz-perf-fixes-14.patch --]
[-- Type: text/plain, Size: 1953 bytes --]

The perf_remove_from_context() usage in __perf_event_exit_task() is
different from the other usages in that this site has already
detached and scheduled out the task context.

This will stand in the way of stronger assertions checking the (task)
context scheduling invariants.

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

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8726,7 +8726,13 @@ __perf_event_exit_task(struct perf_event
 	 * Do destroy all inherited groups, we don't care about those
 	 * and being thorough is better.
 	 */
-	perf_remove_from_context(child_event, !!child_event->parent);
+	raw_spin_lock_irq(&child_ctx->lock);
+	WARN_ON_ONCE(child_ctx->is_active);
+
+	if (!!child_event->parent)
+		perf_group_detach(child_event);
+	list_del_event(child_event, child_ctx);
+	raw_spin_unlock_irq(&child_ctx->lock);
 
 	/*
 	 * It can happen that the parent exits first, and has events
@@ -8746,17 +8752,15 @@ static void perf_event_exit_task_context
 {
 	struct perf_event *child_event, *next;
 	struct perf_event_context *child_ctx, *clone_ctx = NULL;
-	unsigned long flags;
 
 	if (likely(!child->perf_event_ctxp[ctxn]))
 		return;
 
-	local_irq_save(flags);
+	local_irq_disable();
+	WARN_ON_ONCE(child != current);
 	/*
 	 * We can't reschedule here because interrupts are disabled,
-	 * and either child is current or it is a task that can't be
-	 * scheduled, so we are now safe from rescheduling changing
-	 * our context.
+	 * and child must be current.
 	 */
 	child_ctx = rcu_dereference_raw(child->perf_event_ctxp[ctxn]);
 
@@ -8776,7 +8780,7 @@ static void perf_event_exit_task_context
 	 */
 	clone_ctx = unclone_ctx(child_ctx);
 	update_context_time(child_ctx);
-	raw_spin_unlock_irqrestore(&child_ctx->lock, flags);
+	raw_spin_unlock_irq(&child_ctx->lock);
 
 	if (clone_ctx)
 		put_ctx(clone_ctx);

[RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Peter Zijlstra]

[-- Attachment #1: peterz-perf-fixes-13.patch --]
[-- Type: text/plain, Size: 15526 bytes --]

There is one common bug left in all the event_function_call() users,
between loading ctx->task and getting to the remote_function(),
ctx->task can already have been changed.

Therefore we need to double check and retry if ctx->task != current.

Insert another trampoline specific to event_function_call() that
checks for this and further validates state. This also allows getting
rid of the active/inactive functions.


Note: Stephane, can you please look at __perf_event_enable()?

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/perf_event.h    |    2 
 kernel/events/core.c          |  362 ++++++++++++++++++------------------------
 kernel/events/hw_breakpoint.c |    2 
 3 files changed, 164 insertions(+), 202 deletions(-)

--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1044,7 +1044,7 @@ extern void perf_swevent_put_recursion_c
 extern u64 perf_swevent_set_period(struct perf_event *event);
 extern void perf_event_enable(struct perf_event *event);
 extern void perf_event_disable(struct perf_event *event);
-extern int __perf_event_disable(void *info);
+extern void perf_event_disable_local(struct perf_event *event);
 extern void perf_event_task_tick(void);
 #else /* !CONFIG_PERF_EVENTS: */
 static inline void *
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -126,6 +126,28 @@ static int cpu_function_call(int cpu, re
 	return data.ret;
 }
 
+static inline struct perf_cpu_context *
+__get_cpu_context(struct perf_event_context *ctx)
+{
+	return this_cpu_ptr(ctx->pmu->pmu_cpu_context);
+}
+
+static void perf_ctx_lock(struct perf_cpu_context *cpuctx,
+			  struct perf_event_context *ctx)
+{
+	raw_spin_lock(&cpuctx->ctx.lock);
+	if (ctx)
+		raw_spin_lock(&ctx->lock);
+}
+
+static void perf_ctx_unlock(struct perf_cpu_context *cpuctx,
+			    struct perf_event_context *ctx)
+{
+	if (ctx)
+		raw_spin_unlock(&ctx->lock);
+	raw_spin_unlock(&cpuctx->ctx.lock);
+}
+
 /*
  * On task ctx scheduling...
  *
@@ -158,21 +180,96 @@ static int cpu_function_call(int cpu, re
  * If ctx->nr_events, then ctx->is_active and cpuctx->task_ctx are set.
  */
 
-static void event_function_call(struct perf_event *event,
-				int (*active)(void *),
-				void (*inactive)(void *),
-				void *data)
+typedef void (*event_f)(struct perf_event *, struct perf_cpu_context *,
+			struct perf_event_context *, void *);
+
+struct event_function_struct {
+	struct perf_event *event;
+	event_f func;
+	void *data;
+};
+
+static int event_function(void *info)
+{
+	struct event_function_struct *efs = info;
+	struct perf_event *event = efs->event;
+	struct perf_event_context *ctx = event->ctx;
+	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
+	struct perf_event_context *task_ctx = cpuctx->task_ctx;
+
+	WARN_ON_ONCE(!irqs_disabled());
+
+	/*
+	 * Since we do the IPI call without holding ctx->lock things can have
+	 * changed, double check we hit the task we set out to hit.
+	 *
+	 * If ctx->task == current, we know things must remain valid because
+	 * we have IRQs disabled so we cannot schedule.
+	 */
+	if (ctx->task) {
+		if (ctx->task != current)
+			return -EAGAIN;
+
+		WARN_ON_ONCE(task_ctx != ctx);
+	} else {
+		WARN_ON_ONCE(&cpuctx->ctx != ctx);
+	}
+
+	perf_ctx_lock(cpuctx, task_ctx);
+	/*
+	 * Now that we hold locks, double check state. Paranoia pays.
+	 */
+	if (task_ctx) {
+		WARN_ON_ONCE(task_ctx->task != current);
+		/*
+		 * We only use event_function_call() on established contexts,
+		 * and event_function() is only ever called when active (or
+		 * rather, we'll have bailed in task_function_call() or the
+		 * above ctx->task != current test), therefore we must have
+		 * ctx->is_active here.
+		 */
+		WARN_ON_ONCE(!ctx->is_active);
+		/*
+		 * And since we have ctx->is_active, cpuctx->task_ctx must
+		 * match.
+		 */
+		WARN_ON_ONCE(cpuctx->task_ctx != task_ctx);
+	}
+	efs->func(event, cpuctx, ctx, efs->data);
+	perf_ctx_unlock(cpuctx, task_ctx);
+
+	return 0;
+}
+
+static void event_function_local(struct perf_event *event, event_f func, void *data)
+{
+	struct event_function_struct efs = {
+		.event = event,
+		.func = func,
+		.data = data,
+	};
+
+	int ret = event_function(&efs);
+	WARN_ON_ONCE(ret);
+}
+
+static void event_function_call(struct perf_event *event, event_f func, void *data)
 {
 	struct perf_event_context *ctx = event->ctx;
 	struct task_struct *task = ctx->task;
+	struct event_function_struct efs = {
+		.event = event,
+		.func = func,
+		.data = data,
+	};
 
 	if (!task) {
-		cpu_function_call(event->cpu, active, data);
+		cpu_function_call(event->cpu, event_function, &efs);
 		return;
 	}
 
 again:
-	if (!task_function_call(task, active, data))
+	if (!task_function_call(task, event_function, &efs))
 		return;
 
 	raw_spin_lock_irq(&ctx->lock);
@@ -185,7 +282,7 @@ static void event_function_call(struct p
 		raw_spin_unlock_irq(&ctx->lock);
 		goto again;
 	}
-	inactive(data);
+	func(event, NULL, ctx, data);
 	raw_spin_unlock_irq(&ctx->lock);
 }
 
@@ -400,28 +497,6 @@ static inline u64 perf_event_clock(struc
 	return event->clock();
 }
 
-static inline struct perf_cpu_context *
-__get_cpu_context(struct perf_event_context *ctx)
-{
-	return this_cpu_ptr(ctx->pmu->pmu_cpu_context);
-}
-
-static void perf_ctx_lock(struct perf_cpu_context *cpuctx,
-			  struct perf_event_context *ctx)
-{
-	raw_spin_lock(&cpuctx->ctx.lock);
-	if (ctx)
-		raw_spin_lock(&ctx->lock);
-}
-
-static void perf_ctx_unlock(struct perf_cpu_context *cpuctx,
-			    struct perf_event_context *ctx)
-{
-	if (ctx)
-		raw_spin_unlock(&ctx->lock);
-	raw_spin_unlock(&cpuctx->ctx.lock);
-}
-
 #ifdef CONFIG_CGROUP_PERF
 
 static inline bool
@@ -1684,38 +1759,22 @@ group_sched_out(struct perf_event *group
 		cpuctx->exclusive = 0;
 }
 
-struct remove_event {
-	struct perf_event *event;
-	bool detach_group;
-};
-
-static void ___perf_remove_from_context(void *info)
-{
-	struct remove_event *re = info;
-	struct perf_event *event = re->event;
-	struct perf_event_context *ctx = event->ctx;
-
-	if (re->detach_group)
-		perf_group_detach(event);
-	list_del_event(event, ctx);
-}
-
 /*
  * Cross CPU call to remove a performance event
  *
  * We disable the event on the hardware level first. After that we
  * remove it from the context list.
  */
-static int __perf_remove_from_context(void *info)
+static void
+__perf_remove_from_context(struct perf_event *event,
+			   struct perf_cpu_context *cpuctx,
+			   struct perf_event_context *ctx,
+			   void *info)
 {
-	struct remove_event *re = info;
-	struct perf_event *event = re->event;
-	struct perf_event_context *ctx = event->ctx;
-	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
+	bool detach_group = (unsigned long)info;
 
-	raw_spin_lock(&ctx->lock);
 	event_sched_out(event, cpuctx, ctx);
-	if (re->detach_group)
+	if (detach_group)
 		perf_group_detach(event);
 	list_del_event(event, ctx);
 
@@ -1726,17 +1785,11 @@ static int __perf_remove_from_context(vo
 			cpuctx->task_ctx = NULL;
 		}
 	}
-	raw_spin_unlock(&ctx->lock);
-
-	return 0;
 }
 
 /*
  * Remove the event from a task's (or a CPU's) list of events.
  *
- * CPU events are removed with a smp call. For task events we only
- * call when the task is on a CPU.
- *
  * If event->ctx is a cloned context, callers must make sure that
  * every task struct that event->ctx->task could possibly point to
  * remains valid.  This is OK when called from perf_release since
@@ -1746,71 +1799,31 @@ static int __perf_remove_from_context(vo
  */
 static void perf_remove_from_context(struct perf_event *event, bool detach_group)
 {
-	struct perf_event_context *ctx = event->ctx;
-	struct remove_event re = {
-		.event = event,
-		.detach_group = detach_group,
-	};
-
-	lockdep_assert_held(&ctx->mutex);
+	lockdep_assert_held(&event->ctx->mutex);
 
 	event_function_call(event, __perf_remove_from_context,
-			    ___perf_remove_from_context, &re);
+			    (void *)(unsigned long)detach_group);
 }
 
 /*
  * Cross CPU call to disable a performance event
  */
-int __perf_event_disable(void *info)
-{
-	struct perf_event *event = info;
-	struct perf_event_context *ctx = event->ctx;
-	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
-
-	/*
-	 * If this is a per-task event, need to check whether this
-	 * event's task is the current task on this cpu.
-	 *
-	 * Can trigger due to concurrent perf_event_context_sched_out()
-	 * flipping contexts around.
-	 */
-	if (ctx->task && cpuctx->task_ctx != ctx)
-		return -EINVAL;
-
-	raw_spin_lock(&ctx->lock);
-
-	/*
-	 * If the event is on, turn it off.
-	 * If it is in error state, leave it in error state.
-	 */
-	if (event->state >= PERF_EVENT_STATE_INACTIVE) {
-		update_context_time(ctx);
-		update_cgrp_time_from_event(event);
-		update_group_times(event);
-		if (event == event->group_leader)
-			group_sched_out(event, cpuctx, ctx);
-		else
-			event_sched_out(event, cpuctx, ctx);
-		event->state = PERF_EVENT_STATE_OFF;
-	}
-
-	raw_spin_unlock(&ctx->lock);
-
-	return 0;
-}
-
-void ___perf_event_disable(void *info)
+static void __perf_event_disable(struct perf_event *event,
+				 struct perf_cpu_context *cpuctx,
+				 struct perf_event_context *ctx,
+				 void *info)
 {
-	struct perf_event *event = info;
+	if (event->state < PERF_EVENT_STATE_INACTIVE)
+		return;
 
-	/*
-	 * Since we have the lock this context can't be scheduled
-	 * in, so we can change the state safely.
-	 */
-	if (event->state == PERF_EVENT_STATE_INACTIVE) {
-		update_group_times(event);
-		event->state = PERF_EVENT_STATE_OFF;
-	}
+	update_context_time(ctx);
+	update_cgrp_time_from_event(event);
+	update_group_times(event);
+	if (event == event->group_leader)
+		group_sched_out(event, cpuctx, ctx);
+	else
+		event_sched_out(event, cpuctx, ctx);
+	event->state = PERF_EVENT_STATE_OFF;
 }
 
 /*
@@ -1837,8 +1850,12 @@ static void _perf_event_disable(struct p
 	}
 	raw_spin_unlock_irq(&ctx->lock);
 
-	event_function_call(event, __perf_event_disable,
-			    ___perf_event_disable, event);
+	event_function_call(event, __perf_event_disable, NULL);
+}
+
+void perf_event_disable_local(struct perf_event *event)
+{
+	event_function_local(event, __perf_event_disable, NULL);
 }
 
 /*
@@ -2202,44 +2219,28 @@ static void __perf_event_mark_enabled(st
 /*
  * Cross CPU call to enable a performance event
  */
-static int __perf_event_enable(void *info)
+static void __perf_event_enable(struct perf_event *event,
+				struct perf_cpu_context *cpuctx,
+				struct perf_event_context *ctx,
+				void *info)
 {
-	struct perf_event *event = info;
-	struct perf_event_context *ctx = event->ctx;
 	struct perf_event *leader = event->group_leader;
-	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
-	struct perf_event_context *task_ctx = cpuctx->task_ctx;
-
-	/*
-	 * There's a time window between 'ctx->is_active' check
-	 * in perf_event_enable function and this place having:
-	 *   - IRQs on
-	 *   - ctx->lock unlocked
-	 *
-	 * where the task could be killed and 'ctx' deactivated
-	 * by perf_event_exit_task.
-	 */
-	if (!ctx->is_active)
-		return -EINVAL;
-
-	perf_ctx_lock(cpuctx, task_ctx);
-	WARN_ON_ONCE(&cpuctx->ctx != ctx && task_ctx != ctx);
-	update_context_time(ctx);
 
 	if (event->state >= PERF_EVENT_STATE_INACTIVE)
-		goto unlock;
-
-	/*
-	 * set current task's cgroup time reference point
-	 */
-	perf_cgroup_set_timestamp(current, ctx);
+		return;
 
+	update_context_time(ctx);
 	__perf_event_mark_enabled(event);
 
+	if (!ctx->is_active)
+		return;
+
 	if (!event_filter_match(event)) {
-		if (is_cgroup_event(event))
+		if (is_cgroup_event(event)) {
+			perf_cgroup_set_timestamp(current, ctx); // XXX ?
 			perf_cgroup_defer_enabled(event);
-		goto unlock;
+		}
+		return;
 	}
 
 	/*
@@ -2247,19 +2248,9 @@ static int __perf_event_enable(void *inf
 	 * then don't put it on unless the group is on.
 	 */
 	if (leader != event && leader->state != PERF_EVENT_STATE_ACTIVE)
-		goto unlock;
-
-	ctx_resched(cpuctx, task_ctx);
-
-unlock:
-	perf_ctx_unlock(cpuctx, task_ctx);
-
-	return 0;
-}
+		return;
 
-void ___perf_event_enable(void *info)
-{
-	__perf_event_mark_enabled((struct perf_event *)info);
+	ctx_resched(cpuctx, ctx);
 }
 
 /*
@@ -2292,8 +2283,7 @@ static void _perf_event_enable(struct pe
 		event->state = PERF_EVENT_STATE_OFF;
 	raw_spin_unlock_irq(&ctx->lock);
 
-	event_function_call(event, __perf_event_enable,
-			    ___perf_event_enable, event);
+	event_function_call(event, __perf_event_enable, NULL);
 }
 
 /*
@@ -4095,36 +4085,14 @@ static void perf_event_for_each(struct p
 		perf_event_for_each_child(sibling, func);
 }
 
-struct period_event {
-	struct perf_event *event;
-	u64 value;
-};
-
-static void ___perf_event_period(void *info)
-{
-	struct period_event *pe = info;
-	struct perf_event *event = pe->event;
-	u64 value = pe->value;
-
-	if (event->attr.freq) {
-		event->attr.sample_freq = value;
-	} else {
-		event->attr.sample_period = value;
-		event->hw.sample_period = value;
-	}
-
-	local64_set(&event->hw.period_left, 0);
-}
-
-static int __perf_event_period(void *info)
+static void __perf_event_period(struct perf_event *event,
+				struct perf_cpu_context *cpuctx,
+				struct perf_event_context *ctx,
+				void *info)
 {
-	struct period_event *pe = info;
-	struct perf_event *event = pe->event;
-	struct perf_event_context *ctx = event->ctx;
-	u64 value = pe->value;
+	u64 value = *((u64 *)info);
 	bool active;
 
-	raw_spin_lock(&ctx->lock);
 	if (event->attr.freq) {
 		event->attr.sample_freq = value;
 	} else {
@@ -4144,14 +4112,10 @@ static int __perf_event_period(void *inf
 		event->pmu->start(event, PERF_EF_RELOAD);
 		perf_pmu_enable(ctx->pmu);
 	}
-	raw_spin_unlock(&ctx->lock);
-
-	return 0;
 }
 
 static int perf_event_period(struct perf_event *event, u64 __user *arg)
 {
-	struct period_event pe = { .event = event, };
 	u64 value;
 
 	if (!is_sampling_event(event))
@@ -4166,10 +4130,7 @@ static int perf_event_period(struct perf
 	if (event->attr.freq && value > sysctl_perf_event_sample_rate)
 		return -EINVAL;
 
-	pe.value = value;
-
-	event_function_call(event, __perf_event_period,
-			    ___perf_event_period, &pe);
+	event_function_call(event, __perf_event_period, &value);
 
 	return 0;
 }
@@ -4941,7 +4902,7 @@ static void perf_pending_event(struct ir
 
 	if (event->pending_disable) {
 		event->pending_disable = 0;
-		__perf_event_disable(event);
+		perf_event_disable_local(event);
 	}
 
 	if (event->pending_wakeup) {
@@ -9239,13 +9200,14 @@ static void perf_event_init_cpu(int cpu)
 #if defined CONFIG_HOTPLUG_CPU || defined CONFIG_KEXEC_CORE
 static void __perf_event_exit_context(void *__info)
 {
-	struct remove_event re = { .detach_group = true };
 	struct perf_event_context *ctx = __info;
+	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
+	struct perf_event *event;
 
-	rcu_read_lock();
-	list_for_each_entry_rcu(re.event, &ctx->event_list, event_entry)
-		__perf_remove_from_context(&re);
-	rcu_read_unlock();
+	raw_spin_lock(&ctx->lock);
+	list_for_each_entry(event, &ctx->event_list, event_entry)
+		__perf_remove_from_context(event, cpuctx, ctx, (void *)(unsigned long)true);
+	raw_spin_unlock(&ctx->lock);
 }
 
 static void perf_event_exit_cpu_context(int cpu)
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -444,7 +444,7 @@ int modify_user_hw_breakpoint(struct per
 	 * current task.
 	 */
 	if (irqs_disabled() && bp->ctx && bp->ctx->task == current)
-		__perf_event_disable(bp);
+		perf_event_disable_local(bp);
 	else
 		perf_event_disable(bp);
 

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Mon, Jan 11, 2016 at 5:24 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> Hi,
>
> I've been hunting perf bugs for the past few weeks. This resulted in this pile
> of patches, that mostly seems to work -- I still get an occasional fail so
> something is still off.
>
> But I've been sitting on this for far too long, so here goes.
>
> (I've not at all tested perf-cgroup)
>
> The code compiles in between patches, but I've not bothered trying to run/test
> any intermediate stage. There's just too many inter-related fail.
>
> Alexander, Stephane, could you guys please have a hard look at this?
>
> Andi, Dmitry, patch 10 should explain the getting stuck in
> perf_install_in_context() forever thing you both have observed.


I've tried to apply this series, but it failed exactly on patch 10.
Among other things I don't have event_function_call() function at all.
I am on the latest Linus tree afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc
(Jan 10, 4.4). What other patches do I need to apply this?

Re: [RFC][PATCH 02/12] perf: Fix cgroup event scheduling

[Stephane Eranian]

Peter,

On Mon, Jan 11, 2016 at 8:25 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> There appears to be a problemin __perf_event_task_sched_in() wrt
> cgroup event scheduling.
>
> The normal event scheduling order is:
>
>         CPU pinned
>         Task pinned
>         CPU flexible
>         Task flexible
>
> And since perf_cgroup_sched*() only schedules the cpu context, we must
> call this _before_ adding the task events.
>
I understand this but I am trying to understand why cgroup system-wide event
would be treated differently from regular system-wide events w.r.t. task events
here. If I do a cgroup flexible and a task pinned, what happens?

> Note: double check what happens on the ctx switch optimization where
> the task ctx isn't scheduled.
>
> Cc: Stephane Eranian <eranian@google.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  kernel/events/core.c |   17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -2804,6 +2804,16 @@ void __perf_event_task_sched_in(struct t
>         struct perf_event_context *ctx;
>         int ctxn;
>
> +       /*
> +        * If cgroup events exist on this CPU, then we need to check if we have
> +        * to switch in PMU state; cgroup event are system-wide mode only.
> +        *
> +        * Since cgroup events are CPU events, we must schedule these in before
> +        * we schedule in the task events.
> +        */
> +       if (atomic_read(this_cpu_ptr(&perf_cgroup_events)))
> +               perf_cgroup_sched_in(prev, task);
> +
>         for_each_task_context_nr(ctxn) {
>                 ctx = task->perf_event_ctxp[ctxn];
>                 if (likely(!ctx))
> @@ -2811,13 +2821,6 @@ void __perf_event_task_sched_in(struct t
>
>                 perf_event_context_sched_in(ctx, task);
>         }
> -       /*
> -        * if cgroup events exist on this CPU, then we need
> -        * to check if we have to switch in PMU state.
> -        * cgroup event are system-wide mode only
> -        */
> -       if (atomic_read(this_cpu_ptr(&perf_cgroup_events)))
> -               perf_cgroup_sched_in(prev, task);
>
>         if (atomic_read(&nr_switch_events))
>                 perf_event_switch(task, prev, true);
>
>

Re: [RFC][PATCH 00/12] various perf fixes

[Andi Kleen]

> 
> I've tried to apply this series, but it failed exactly on patch 10.
> Among other things I don't have event_function_call() function at all.
> I am on the latest Linus tree afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc
> (Jan 10, 4.4). What other patches do I need to apply this?

Likely perf/core from
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

On Mon, Jan 11, 2016 at 07:44:55PM +0100, Dmitry Vyukov wrote:
> 
> I've tried to apply this series, but it failed exactly on patch 10.
> Among other things I don't have event_function_call() function at all.
> I am on the latest Linus tree afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc
> (Jan 10, 4.4). What other patches do I need to apply this?

As Andi said, its based on tip/perf/core

I've put up a git branch with these patches in on top of the latest tip,
I've not yet compiled or tested this exact tree, but they did apply
cleanly.

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

Re: [RFC][PATCH 02/12] perf: Fix cgroup event scheduling

[Peter Zijlstra]

On Mon, Jan 11, 2016 at 11:43:40AM -0800, Stephane Eranian wrote:
> Peter,
> 
> On Mon, Jan 11, 2016 at 8:25 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > There appears to be a problemin __perf_event_task_sched_in() wrt
> > cgroup event scheduling.
> >
> > The normal event scheduling order is:
> >
> >         CPU pinned
> >         Task pinned
> >         CPU flexible
> >         Task flexible
> >
> > And since perf_cgroup_sched*() only schedules the cpu context, we must
> > call this _before_ adding the task events.
> >
> I understand this but I am trying to understand why cgroup system-wide event
> would be treated differently from regular system-wide events w.r.t. task events
> here. 

This patch is about making cgroup events behave similarly to regular
system-wide events.

> If I do a cgroup flexible and a task pinned, what happens?

Look at perf_event_context_sched_in(), it will first remove the
cpu-flexible and then install everything:

  cpu-pinned (skipped, because that would still be present)
  task-pinned (new)
  cpu-flexible (re-issue, possibly truncated due to the new pinned events)
  task-flexible (new)

Re: [RFC][PATCH 00/12] various perf fixes

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Mon, Jan 11, 2016 at 07:44:55PM +0100, Dmitry Vyukov wrote:
> > 
> > I've tried to apply this series, but it failed exactly on patch 10.
> > Among other things I don't have event_function_call() function at all.
> > I am on the latest Linus tree afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc
> > (Jan 10, 4.4). What other patches do I need to apply this?
> 
> As Andi said, its based on tip/perf/core
> 
> I've put up a git branch with these patches in on top of the latest tip,
> I've not yet compiled or tested this exact tree, but they did apply
> cleanly.
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/core

The patches will also apply cleanly to Linus's very latest upstream tree (commit 
5cb52b5e1654 and later) - which has perf/core merged.

Thanks,

	Ingo

Re: [RFC][PATCH 00/12] various perf fixes

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> Hi,
> 
> I've been hunting perf bugs for the past few weeks. This resulted in this pile
> of patches, that mostly seems to work -- I still get an occasional fail so
> something is still off.
> 
> But I've been sitting on this for far too long, so here goes.
> 
> (I've not at all tested perf-cgroup)
> 
> The code compiles in between patches, but I've not bothered trying to run/test
> any intermediate stage. There's just too many inter-related fail.
> 
> Alexander, Stephane, could you guys please have a hard look at this?
> 
> Andi, Dmitry, patch 10 should explain the getting stuck in
> perf_install_in_context() forever thing you both have observed.

Btw., if there's no test failures I plan to apply and push this to Linus fairly 
soon, so guys please give it all the review and testing you can.

Thanks,

	Ingo

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Tue, Jan 12, 2016 at 11:11 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Peter Zijlstra <peterz@infradead.org> wrote:
>
>> Hi,
>>
>> I've been hunting perf bugs for the past few weeks. This resulted in this pile
>> of patches, that mostly seems to work -- I still get an occasional fail so
>> something is still off.
>>
>> But I've been sitting on this for far too long, so here goes.
>>
>> (I've not at all tested perf-cgroup)
>>
>> The code compiles in between patches, but I've not bothered trying to run/test
>> any intermediate stage. There's just too many inter-related fail.
>>
>> Alexander, Stephane, could you guys please have a hard look at this?
>>
>> Andi, Dmitry, patch 10 should explain the getting stuck in
>> perf_install_in_context() forever thing you both have observed.
>
> Btw., if there's no test failures I plan to apply and push this to Linus fairly
> soon, so guys please give it all the review and testing you can.


Yes, I was able to apply it on top of the latest Linus tree. Thanks.
Now testing perf with these patches at full capacity.

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Tue, Jan 12, 2016 at 11:57 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Tue, Jan 12, 2016 at 11:11 AM, Ingo Molnar <mingo@kernel.org> wrote:
>>
>> * Peter Zijlstra <peterz@infradead.org> wrote:
>>
>>> Hi,
>>>
>>> I've been hunting perf bugs for the past few weeks. This resulted in this pile
>>> of patches, that mostly seems to work -- I still get an occasional fail so
>>> something is still off.
>>>
>>> But I've been sitting on this for far too long, so here goes.
>>>
>>> (I've not at all tested perf-cgroup)
>>>
>>> The code compiles in between patches, but I've not bothered trying to run/test
>>> any intermediate stage. There's just too many inter-related fail.
>>>
>>> Alexander, Stephane, could you guys please have a hard look at this?
>>>
>>> Andi, Dmitry, patch 10 should explain the getting stuck in
>>> perf_install_in_context() forever thing you both have observed.
>>
>> Btw., if there's no test failures I plan to apply and push this to Linus fairly
>> soon, so guys please give it all the review and testing you can.
>
>
> Yes, I was able to apply it on top of the latest Linus tree. Thanks.
> Now testing perf with these patches at full capacity.

Hit this in a minute:

------------[ cut here ]------------
WARNING: CPU: 0 PID: 12905 at kernel/events/core.c:2651
task_ctx_sched_out+0x8d/0xa0()
Modules linked in:
CPU: 0 PID: 12905 Comm: syz-executor Not tainted 4.4.0+ #224
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 00000000ffffffff ffff88003290f4e8 ffffffff8290d92d 0000000000000000
 ffff8800348c5f00 ffffffff85fc05c0 ffff88003290f528 ffffffff813429e9
 ffffffff81617ffd ffffffff85fc05c0 0000000000000a5b ffff880035c09188
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff8290d92d>] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
 [<ffffffff813429e9>] warn_slowpath_common+0xd9/0x140 kernel/panic.c:483
 [<ffffffff81342c19>] warn_slowpath_null+0x29/0x30 kernel/panic.c:516
 [<ffffffff81617ffd>] task_ctx_sched_out+0x8d/0xa0 kernel/events/core.c:2651
 [<     inline     >] perf_event_context_sched_out kernel/events/core.c:2550
 [<ffffffff81622263>] __perf_event_task_sched_out+0x5d3/0x1230
kernel/events/core.c:2634
 [<     inline     >] perf_event_task_sched_out include/linux/perf_event.h:948
 [<     inline     >] prepare_task_switch kernel/sched/core.c:2613
 [<     inline     >] context_switch kernel/sched/core.c:2771
 [<ffffffff85e7d2d7>] __schedule+0xb27/0x1c50 kernel/sched/core.c:3282
 [<ffffffff85e7eed2>] preempt_schedule_common+0x42/0x70 kernel/sched/core.c:3352
 [<ffffffff85e7ef17>] _cond_resched+0x17/0x20 kernel/sched/core.c:4720
 [<     inline     >] __wait_for_common kernel/sched/completion.c:90
 [<     inline     >] wait_for_common kernel/sched/completion.c:101
 [<ffffffff85e8158b>] wait_for_completion+0x8b/0x300
kernel/sched/completion.c:122
 [<ffffffff8149255e>] __wait_rcu_gp+0x12e/0x1a0 kernel/rcu/update.c:365
 [<ffffffff81498218>] synchronize_sched.part.58+0x88/0xb0 kernel/rcu/tree.c:3220
 [<ffffffff814a19a3>] synchronize_sched+0xa3/0x120 kernel/rcu/tree.c:3217
 [<     inline     >] synchronize_rcu include/linux/rcupdate.h:320
 [<ffffffff817dd6be>] namespace_unlock+0xee/0x100 fs/namespace.c:1349
 [<ffffffff817e671e>] drop_collected_mounts+0x8e/0xa0 fs/namespace.c:1762
 [<ffffffff817eaf1c>] put_mnt_ns+0x4c/0x70 fs/namespace.c:3149
 [<ffffffff813a7594>] free_nsproxy+0x44/0x1d0 kernel/nsproxy.c:161
 [<ffffffff813a7992>] switch_task_namespaces+0xa2/0xc0 kernel/nsproxy.c:213
 [<ffffffff813a79c7>] exit_task_namespaces+0x17/0x20 kernel/nsproxy.c:218
 [<ffffffff8134bb69>] do_exit+0x8b9/0x2b80 kernel/exit.c:749
 [<ffffffff8134dfa8>] do_group_exit+0x108/0x330 kernel/exit.c:880
 [<ffffffff81371214>] get_signal+0x5e4/0x1500 kernel/signal.c:2307
 [<ffffffff81193db3>] do_signal+0x83/0x1c90 arch/x86/kernel/signal.c:712
 [<ffffffff81006685>] exit_to_usermode_loop+0x1a5/0x210
arch/x86/entry/common.c:247
 [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:282
 [<ffffffff8100851a>] syscall_return_slowpath+0x2ba/0x340
arch/x86/entry/common.c:344
 [<ffffffff85e8dba2>] int_ret_from_sys_call+0x25/0x9f
arch/x86/entry/entry_64.S:281
---[ end trace 93985d046a082b72 ]---

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Tue, Jan 12, 2016 at 12:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Tue, Jan 12, 2016 at 11:57 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Tue, Jan 12, 2016 at 11:11 AM, Ingo Molnar <mingo@kernel.org> wrote:
>>>
>>> * Peter Zijlstra <peterz@infradead.org> wrote:
>>>
>>>> Hi,
>>>>
>>>> I've been hunting perf bugs for the past few weeks. This resulted in this pile
>>>> of patches, that mostly seems to work -- I still get an occasional fail so
>>>> something is still off.
>>>>
>>>> But I've been sitting on this for far too long, so here goes.
>>>>
>>>> (I've not at all tested perf-cgroup)
>>>>
>>>> The code compiles in between patches, but I've not bothered trying to run/test
>>>> any intermediate stage. There's just too many inter-related fail.
>>>>
>>>> Alexander, Stephane, could you guys please have a hard look at this?
>>>>
>>>> Andi, Dmitry, patch 10 should explain the getting stuck in
>>>> perf_install_in_context() forever thing you both have observed.
>>>
>>> Btw., if there's no test failures I plan to apply and push this to Linus fairly
>>> soon, so guys please give it all the review and testing you can.
>>
>>
>> Yes, I was able to apply it on top of the latest Linus tree. Thanks.
>> Now testing perf with these patches at full capacity.
>
> Hit this in a minute:
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 12905 at kernel/events/core.c:2651
> task_ctx_sched_out+0x8d/0xa0()
> Modules linked in:
> CPU: 0 PID: 12905 Comm: syz-executor Not tainted 4.4.0+ #224
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>  00000000ffffffff ffff88003290f4e8 ffffffff8290d92d 0000000000000000
>  ffff8800348c5f00 ffffffff85fc05c0 ffff88003290f528 ffffffff813429e9
>  ffffffff81617ffd ffffffff85fc05c0 0000000000000a5b ffff880035c09188
> Call Trace:
>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>  [<ffffffff8290d92d>] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
>  [<ffffffff813429e9>] warn_slowpath_common+0xd9/0x140 kernel/panic.c:483
>  [<ffffffff81342c19>] warn_slowpath_null+0x29/0x30 kernel/panic.c:516
>  [<ffffffff81617ffd>] task_ctx_sched_out+0x8d/0xa0 kernel/events/core.c:2651
>  [<     inline     >] perf_event_context_sched_out kernel/events/core.c:2550
>  [<ffffffff81622263>] __perf_event_task_sched_out+0x5d3/0x1230
> kernel/events/core.c:2634
>  [<     inline     >] perf_event_task_sched_out include/linux/perf_event.h:948
>  [<     inline     >] prepare_task_switch kernel/sched/core.c:2613
>  [<     inline     >] context_switch kernel/sched/core.c:2771
>  [<ffffffff85e7d2d7>] __schedule+0xb27/0x1c50 kernel/sched/core.c:3282
>  [<ffffffff85e7eed2>] preempt_schedule_common+0x42/0x70 kernel/sched/core.c:3352
>  [<ffffffff85e7ef17>] _cond_resched+0x17/0x20 kernel/sched/core.c:4720
>  [<     inline     >] __wait_for_common kernel/sched/completion.c:90
>  [<     inline     >] wait_for_common kernel/sched/completion.c:101
>  [<ffffffff85e8158b>] wait_for_completion+0x8b/0x300
> kernel/sched/completion.c:122
>  [<ffffffff8149255e>] __wait_rcu_gp+0x12e/0x1a0 kernel/rcu/update.c:365
>  [<ffffffff81498218>] synchronize_sched.part.58+0x88/0xb0 kernel/rcu/tree.c:3220
>  [<ffffffff814a19a3>] synchronize_sched+0xa3/0x120 kernel/rcu/tree.c:3217
>  [<     inline     >] synchronize_rcu include/linux/rcupdate.h:320
>  [<ffffffff817dd6be>] namespace_unlock+0xee/0x100 fs/namespace.c:1349
>  [<ffffffff817e671e>] drop_collected_mounts+0x8e/0xa0 fs/namespace.c:1762
>  [<ffffffff817eaf1c>] put_mnt_ns+0x4c/0x70 fs/namespace.c:3149
>  [<ffffffff813a7594>] free_nsproxy+0x44/0x1d0 kernel/nsproxy.c:161
>  [<ffffffff813a7992>] switch_task_namespaces+0xa2/0xc0 kernel/nsproxy.c:213
>  [<ffffffff813a79c7>] exit_task_namespaces+0x17/0x20 kernel/nsproxy.c:218
>  [<ffffffff8134bb69>] do_exit+0x8b9/0x2b80 kernel/exit.c:749
>  [<ffffffff8134dfa8>] do_group_exit+0x108/0x330 kernel/exit.c:880
>  [<ffffffff81371214>] get_signal+0x5e4/0x1500 kernel/signal.c:2307
>  [<ffffffff81193db3>] do_signal+0x83/0x1c90 arch/x86/kernel/signal.c:712
>  [<ffffffff81006685>] exit_to_usermode_loop+0x1a5/0x210
> arch/x86/entry/common.c:247
>  [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:282
>  [<ffffffff8100851a>] syscall_return_slowpath+0x2ba/0x340
> arch/x86/entry/common.c:344
>  [<ffffffff85e8dba2>] int_ret_from_sys_call+0x25/0x9f
> arch/x86/entry/entry_64.S:281
> ---[ end trace 93985d046a082b72 ]---


And this:

------------[ cut here ]------------
WARNING: CPU: 3 PID: 24027 at kernel/events/core.c:2744
ctx_sched_in+0x253/0x1390()
Modules linked in:
CPU: 3 PID: 24027 Comm: syz-executor Not tainted 4.4.0+ #224
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 00000000ffffffff ffff8800620079d0 ffffffff8290d92d 0000000000000000
 ffff880065652f80 ffffffff85fc05c0 ffff880062007a10 ffffffff813429e9
 ffffffff8161bd13 ffffffff85fc05c0 0000000000000ab8 0000000000000003
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff8290d92d>] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
 [<ffffffff813429e9>] warn_slowpath_common+0xd9/0x140 kernel/panic.c:483
 [<ffffffff81342c19>] warn_slowpath_null+0x29/0x30 kernel/panic.c:516
 [<ffffffff8161bd13>] ctx_sched_in+0x253/0x1390 kernel/events/core.c:2744
 [<ffffffff8161ce97>] perf_event_sched_in+0x47/0xa0 kernel/events/core.c:2106
 [<ffffffff8161cfcc>] ctx_resched+0xdc/0x1d0 kernel/events/core.c:2119
 [<ffffffff8161db6b>] __perf_install_in_context+0x12b/0x1b0
kernel/events/core.c:2150
 [<ffffffff8160c36f>] remote_function+0x12f/0x1b0 kernel/events/core.c:74
 [<ffffffff814eab83>] generic_exec_single+0x253/0x450 kernel/smp.c:156
 [<ffffffff814eaf96>] smp_call_function_single+0x216/0x340 kernel/smp.c:300
 [<ffffffff81606b7c>] task_function_call+0x11c/0x130 kernel/events/core.c:101
 [<ffffffff81611bc8>] perf_install_in_context+0x318/0x640
kernel/events/core.c:2193
 [<ffffffff81628a20>] SYSC_perf_event_open+0x1720/0x1fa0
kernel/events/core.c:8457
 [<ffffffff8162fad9>] SyS_perf_event_open+0x39/0x50 kernel/events/core.c:8153
 [<ffffffff85e8da36>] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185
---[ end trace 3930c39e9993a16d ]---

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Tue, Jan 12, 2016 at 12:01 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Tue, Jan 12, 2016 at 12:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Tue, Jan 12, 2016 at 11:57 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>> On Tue, Jan 12, 2016 at 11:11 AM, Ingo Molnar <mingo@kernel.org> wrote:
>>>>
>>>> * Peter Zijlstra <peterz@infradead.org> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I've been hunting perf bugs for the past few weeks. This resulted in this pile
>>>>> of patches, that mostly seems to work -- I still get an occasional fail so
>>>>> something is still off.
>>>>>
>>>>> But I've been sitting on this for far too long, so here goes.
>>>>>
>>>>> (I've not at all tested perf-cgroup)
>>>>>
>>>>> The code compiles in between patches, but I've not bothered trying to run/test
>>>>> any intermediate stage. There's just too many inter-related fail.
>>>>>
>>>>> Alexander, Stephane, could you guys please have a hard look at this?
>>>>>
>>>>> Andi, Dmitry, patch 10 should explain the getting stuck in
>>>>> perf_install_in_context() forever thing you both have observed.
>>>>
>>>> Btw., if there's no test failures I plan to apply and push this to Linus fairly
>>>> soon, so guys please give it all the review and testing you can.
>>>
>>>
>>> Yes, I was able to apply it on top of the latest Linus tree. Thanks.
>>> Now testing perf with these patches at full capacity.
>>
>> Hit this in a minute:
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 12905 at kernel/events/core.c:2651
>> task_ctx_sched_out+0x8d/0xa0()
>> Modules linked in:
>> CPU: 0 PID: 12905 Comm: syz-executor Not tainted 4.4.0+ #224
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>  00000000ffffffff ffff88003290f4e8 ffffffff8290d92d 0000000000000000
>>  ffff8800348c5f00 ffffffff85fc05c0 ffff88003290f528 ffffffff813429e9
>>  ffffffff81617ffd ffffffff85fc05c0 0000000000000a5b ffff880035c09188
>> Call Trace:
>>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>>  [<ffffffff8290d92d>] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
>>  [<ffffffff813429e9>] warn_slowpath_common+0xd9/0x140 kernel/panic.c:483
>>  [<ffffffff81342c19>] warn_slowpath_null+0x29/0x30 kernel/panic.c:516
>>  [<ffffffff81617ffd>] task_ctx_sched_out+0x8d/0xa0 kernel/events/core.c:2651
>>  [<     inline     >] perf_event_context_sched_out kernel/events/core.c:2550
>>  [<ffffffff81622263>] __perf_event_task_sched_out+0x5d3/0x1230
>> kernel/events/core.c:2634
>>  [<     inline     >] perf_event_task_sched_out include/linux/perf_event.h:948
>>  [<     inline     >] prepare_task_switch kernel/sched/core.c:2613
>>  [<     inline     >] context_switch kernel/sched/core.c:2771
>>  [<ffffffff85e7d2d7>] __schedule+0xb27/0x1c50 kernel/sched/core.c:3282
>>  [<ffffffff85e7eed2>] preempt_schedule_common+0x42/0x70 kernel/sched/core.c:3352
>>  [<ffffffff85e7ef17>] _cond_resched+0x17/0x20 kernel/sched/core.c:4720
>>  [<     inline     >] __wait_for_common kernel/sched/completion.c:90
>>  [<     inline     >] wait_for_common kernel/sched/completion.c:101
>>  [<ffffffff85e8158b>] wait_for_completion+0x8b/0x300
>> kernel/sched/completion.c:122
>>  [<ffffffff8149255e>] __wait_rcu_gp+0x12e/0x1a0 kernel/rcu/update.c:365
>>  [<ffffffff81498218>] synchronize_sched.part.58+0x88/0xb0 kernel/rcu/tree.c:3220
>>  [<ffffffff814a19a3>] synchronize_sched+0xa3/0x120 kernel/rcu/tree.c:3217
>>  [<     inline     >] synchronize_rcu include/linux/rcupdate.h:320
>>  [<ffffffff817dd6be>] namespace_unlock+0xee/0x100 fs/namespace.c:1349
>>  [<ffffffff817e671e>] drop_collected_mounts+0x8e/0xa0 fs/namespace.c:1762
>>  [<ffffffff817eaf1c>] put_mnt_ns+0x4c/0x70 fs/namespace.c:3149
>>  [<ffffffff813a7594>] free_nsproxy+0x44/0x1d0 kernel/nsproxy.c:161
>>  [<ffffffff813a7992>] switch_task_namespaces+0xa2/0xc0 kernel/nsproxy.c:213
>>  [<ffffffff813a79c7>] exit_task_namespaces+0x17/0x20 kernel/nsproxy.c:218
>>  [<ffffffff8134bb69>] do_exit+0x8b9/0x2b80 kernel/exit.c:749
>>  [<ffffffff8134dfa8>] do_group_exit+0x108/0x330 kernel/exit.c:880
>>  [<ffffffff81371214>] get_signal+0x5e4/0x1500 kernel/signal.c:2307
>>  [<ffffffff81193db3>] do_signal+0x83/0x1c90 arch/x86/kernel/signal.c:712
>>  [<ffffffff81006685>] exit_to_usermode_loop+0x1a5/0x210
>> arch/x86/entry/common.c:247
>>  [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:282
>>  [<ffffffff8100851a>] syscall_return_slowpath+0x2ba/0x340
>> arch/x86/entry/common.c:344
>>  [<ffffffff85e8dba2>] int_ret_from_sys_call+0x25/0x9f
>> arch/x86/entry/entry_64.S:281
>> ---[ end trace 93985d046a082b72 ]---
>
>
> And this:
>
> ------------[ cut here ]------------
> WARNING: CPU: 3 PID: 24027 at kernel/events/core.c:2744
> ctx_sched_in+0x253/0x1390()
> Modules linked in:
> CPU: 3 PID: 24027 Comm: syz-executor Not tainted 4.4.0+ #224
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>  00000000ffffffff ffff8800620079d0 ffffffff8290d92d 0000000000000000
>  ffff880065652f80 ffffffff85fc05c0 ffff880062007a10 ffffffff813429e9
>  ffffffff8161bd13 ffffffff85fc05c0 0000000000000ab8 0000000000000003
> Call Trace:
>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>  [<ffffffff8290d92d>] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
>  [<ffffffff813429e9>] warn_slowpath_common+0xd9/0x140 kernel/panic.c:483
>  [<ffffffff81342c19>] warn_slowpath_null+0x29/0x30 kernel/panic.c:516
>  [<ffffffff8161bd13>] ctx_sched_in+0x253/0x1390 kernel/events/core.c:2744
>  [<ffffffff8161ce97>] perf_event_sched_in+0x47/0xa0 kernel/events/core.c:2106
>  [<ffffffff8161cfcc>] ctx_resched+0xdc/0x1d0 kernel/events/core.c:2119
>  [<ffffffff8161db6b>] __perf_install_in_context+0x12b/0x1b0
> kernel/events/core.c:2150
>  [<ffffffff8160c36f>] remote_function+0x12f/0x1b0 kernel/events/core.c:74
>  [<ffffffff814eab83>] generic_exec_single+0x253/0x450 kernel/smp.c:156
>  [<ffffffff814eaf96>] smp_call_function_single+0x216/0x340 kernel/smp.c:300
>  [<ffffffff81606b7c>] task_function_call+0x11c/0x130 kernel/events/core.c:101
>  [<ffffffff81611bc8>] perf_install_in_context+0x318/0x640
> kernel/events/core.c:2193
>  [<ffffffff81628a20>] SYSC_perf_event_open+0x1720/0x1fa0
> kernel/events/core.c:8457
>  [<ffffffff8162fad9>] SyS_perf_event_open+0x39/0x50 kernel/events/core.c:8153
>  [<ffffffff85e8da36>] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
> ---[ end trace 3930c39e9993a16d ]---


Whole assortment:

2016/01/12 11:57:23 qemu-11: saving crash 'WARNING: CPU: 0 PID: 12905
at kernel/events/core.c:2651 task_ctx_sched_out+0x8d/0xa0()' to
crash-qemu-11-1452596243752451147
2016/01/12 11:59:50 qemu-0: saving crash 'WARNING: CPU: 3 PID: 24027
at kernel/events/core.c:2744 ctx_sched_in+0x253/0x1390()' to
crash-qemu-0-1452596390210686680
2016/01/12 12:05:30 qemu-18: saving crash 'WARNING: CPU: 2 PID: 10497
at kernel/events/core.c:2145 __perf_install_in_context+0x177/0x1b0()'
to crash-qemu-18-1452596730362194373
2016/01/12 12:08:26 qemu-12: saving crash 'WARNING: CPU: 2 PID: 26946
at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
crash-qemu-12-1452596906889159952
2016/01/12 12:11:09 qemu-3: saving crash 'WARNING: CPU: 1 PID: 27863
at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
crash-qemu-3-1452597069411271690
2016/01/12 12:13:38 qemu-5: saving crash 'WARNING: CPU: 1 PID: 18036
at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
crash-qemu-5-1452597218740474180
2016/01/12 12:14:00 qemu-2: saving crash 'WARNING: CPU: 1 PID: 27287
at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
crash-qemu-2-1452597240810824249
2016/01/12 12:14:21 qemu-13: saving crash 'WARNING: CPU: 0 PID: 24151
at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
crash-qemu-13-1452597261384610361
2016/01/12 12:15:40 qemu-12: saving crash 'WARNING: CPU: 2 PID: 1802
at kernel/events/core.c:1698 event_sched_out.isra.86+0x79c/0xf40()' to
crash-qemu-12-1452597340496838168
2016/01/12 12:16:17 qemu-1: saving crash 'WARNING: CPU: 2 PID: 5975 at
kernel/events/core.c:213 event_function+0x4ba/0x590()' to
crash-qemu-1-1452597377189059606
2016/01/12 12:16:51 qemu-2: saving crash 'WARNING: CPU: 2 PID: 8948 at
kernel/events/core.c:1698 event_sched_out.isra.86+0x79c/0xf40()' to
crash-qemu-2-1452597411120105533
2016/01/12 12:17:56 qemu-1: saving crash 'WARNING: CPU: 2 PID: 30319
at kernel/events/core.c:1698 event_sched_out.isra.86+0x79c/0xf40()' to
crash-qemu-1-1452597476734445707
2016/01/12 12:18:52 qemu-7: saving crash 'WARNING: CPU: 1 PID: 31877
at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
crash-qemu-7-1452597532508063377
2016/01/12 12:19:07 qemu-13: saving crash 'WARNING: CPU: 1 PID: 964 at
kernel/events/core.c:2145 __perf_install_in_context+0x177/0x1b0()' to
crash-qemu-13-1452597547020900569


Peter,

What do you think if we work on making syzkaller work for you locally?
I think it will be more efficient than testing on my side. I am ready
to provide any necessary help. If you are interested please drop a
email to syzkaller@googlegroups.com.

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Tue, Jan 12, 2016 at 12:26 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> Whole assortment:
>
> 2016/01/12 11:57:23 qemu-11: saving crash 'WARNING: CPU: 0 PID: 12905
> at kernel/events/core.c:2651 task_ctx_sched_out+0x8d/0xa0()' to
> crash-qemu-11-1452596243752451147
> 2016/01/12 11:59:50 qemu-0: saving crash 'WARNING: CPU: 3 PID: 24027
> at kernel/events/core.c:2744 ctx_sched_in+0x253/0x1390()' to
> crash-qemu-0-1452596390210686680
> 2016/01/12 12:05:30 qemu-18: saving crash 'WARNING: CPU: 2 PID: 10497
> at kernel/events/core.c:2145 __perf_install_in_context+0x177/0x1b0()'
> to crash-qemu-18-1452596730362194373
> 2016/01/12 12:08:26 qemu-12: saving crash 'WARNING: CPU: 2 PID: 26946
> at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
> crash-qemu-12-1452596906889159952
> 2016/01/12 12:11:09 qemu-3: saving crash 'WARNING: CPU: 1 PID: 27863
> at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
> crash-qemu-3-1452597069411271690
> 2016/01/12 12:13:38 qemu-5: saving crash 'WARNING: CPU: 1 PID: 18036
> at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
> crash-qemu-5-1452597218740474180
> 2016/01/12 12:14:00 qemu-2: saving crash 'WARNING: CPU: 1 PID: 27287
> at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
> crash-qemu-2-1452597240810824249
> 2016/01/12 12:14:21 qemu-13: saving crash 'WARNING: CPU: 0 PID: 24151
> at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
> crash-qemu-13-1452597261384610361
> 2016/01/12 12:15:40 qemu-12: saving crash 'WARNING: CPU: 2 PID: 1802
> at kernel/events/core.c:1698 event_sched_out.isra.86+0x79c/0xf40()' to
> crash-qemu-12-1452597340496838168
> 2016/01/12 12:16:17 qemu-1: saving crash 'WARNING: CPU: 2 PID: 5975 at
> kernel/events/core.c:213 event_function+0x4ba/0x590()' to
> crash-qemu-1-1452597377189059606
> 2016/01/12 12:16:51 qemu-2: saving crash 'WARNING: CPU: 2 PID: 8948 at
> kernel/events/core.c:1698 event_sched_out.isra.86+0x79c/0xf40()' to
> crash-qemu-2-1452597411120105533
> 2016/01/12 12:17:56 qemu-1: saving crash 'WARNING: CPU: 2 PID: 30319
> at kernel/events/core.c:1698 event_sched_out.isra.86+0x79c/0xf40()' to
> crash-qemu-1-1452597476734445707
> 2016/01/12 12:18:52 qemu-7: saving crash 'WARNING: CPU: 1 PID: 31877
> at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
> crash-qemu-7-1452597532508063377
> 2016/01/12 12:19:07 qemu-13: saving crash 'WARNING: CPU: 1 PID: 964 at
> kernel/events/core.c:2145 __perf_install_in_context+0x177/0x1b0()' to
> crash-qemu-13-1452597547020900569
>
>
> Peter,
>
> What do you think if we work on making syzkaller work for you locally?
> I think it will be more efficient than testing on my side. I am ready
> to provide any necessary help. If you are interested please drop a
> email to syzkaller@googlegroups.com.


Also got 2 different stalls, so it's not just WARNINGS.
Difficult to say whether the situation become better or worse, because
it was actively failing in different ways before as well...

Re: [RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

On Tue, Jan 12, 2016 at 12:26:27PM +0100, Dmitry Vyukov wrote:
> Peter,
> 
> What do you think if we work on making syzkaller work for you locally?
> I think it will be more efficient than testing on my side. I am ready
> to provide any necessary help. If you are interested please drop a
> email to syzkaller@googlegroups.com.

I did manage to hit some fail locally (as stated in the cover letter),
let me try and hunt that down before I try that.

I think you hit some of the same, but way faster than I did :-)

Thanks for testing though!

Re: [RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

On Tue, Jan 12, 2016 at 11:11:09AM +0100, Ingo Molnar wrote:
> 
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > Hi,
> > 
> > I've been hunting perf bugs for the past few weeks. This resulted in this pile
> > of patches, that mostly seems to work -- I still get an occasional fail so
> > something is still off.

 ^^^^

> Btw., if there's no test failures I plan to apply and push this to Linus fairly 
> soon, so guys please give it all the review and testing you can.

Dmitry also hit plenty fail, so please wait a little while. I'll take
another look tomorrow, but I just have to stare at something else for a
little while to reset my brain.

Re: [RFC][PATCH 00/12] various perf fixes

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Jan 12, 2016 at 11:11:09AM +0100, Ingo Molnar wrote:
> > 
> > * Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > > Hi,
> > > 
> > > I've been hunting perf bugs for the past few weeks. This resulted in this pile
> > > of patches, that mostly seems to work -- I still get an occasional fail so
> > > something is still off.
> 
>  ^^^^
> 
> > Btw., if there's no test failures I plan to apply and push this to Linus fairly 
> > soon, so guys please give it all the review and testing you can.
> 
> Dmitry also hit plenty fail, so please wait a little while. I'll take
> another look tomorrow, but I just have to stare at something else for a
> little while to reset my brain.

Ok, fair enough!

Thanks,

	Ingo

Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Peter Zijlstra]

On Mon, Jan 11, 2016 at 05:25:10PM +0100, Peter Zijlstra wrote:
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -2202,44 +2219,28 @@ static void __perf_event_mark_enabled(st
>  /*
>   * Cross CPU call to enable a performance event
>   */
> +static void __perf_event_enable(struct perf_event *event,
> +				struct perf_cpu_context *cpuctx,
> +				struct perf_event_context *ctx,
> +				void *info)
>  {
>  	struct perf_event *leader = event->group_leader;
> -	struct perf_event_context *task_ctx = cpuctx->task_ctx;

deleted too much ^^

>  
>  	if (event->state >= PERF_EVENT_STATE_INACTIVE)
> +		return;
>  
> +	update_context_time(ctx);
>  	__perf_event_mark_enabled(event);
>  
> +	if (!ctx->is_active)
> +		return;
> +
>  	if (!event_filter_match(event)) {
> +		if (is_cgroup_event(event)) {
> +			perf_cgroup_set_timestamp(current, ctx); // XXX ?
>  			perf_cgroup_defer_enabled(event);
> +		}
> +		return;
>  	}
>  
>  	/*
> @@ -2247,19 +2248,9 @@ static int __perf_event_enable(void *inf
>  	 * then don't put it on unless the group is on.
>  	 */
>  	if (leader != event && leader->state != PERF_EVENT_STATE_ACTIVE)
> +		return;
>  
> +	ctx_resched(cpuctx, ctx);
>  }


--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2225,6 +2243,7 @@ static void __perf_event_enable(struct p
 				void *info)
 {
 	struct perf_event *leader = event->group_leader;
+	struct perf_event_context *task_ctx;
 
 	if (event->state >= PERF_EVENT_STATE_INACTIVE)
 		return;
@@ -2250,7 +2269,11 @@ static void __perf_event_enable(struct p
 	if (leader != event && leader->state != PERF_EVENT_STATE_ACTIVE)
 		return;
 
-	ctx_resched(cpuctx, ctx);
+	task_ctx = cpuctx->task_ctx;
+	if (ctx->task)
+		WARN_ON_ONCE(task_ctx != ctx);
+
+	ctx_resched(cpuctx, task_ctx);
 }
 
 /*

Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Alexander Shishkin]

Peter Zijlstra <peterz@infradead.org> writes:

> @@ -2250,7 +2269,11 @@ static void __perf_event_enable(struct p
>  	if (leader != event && leader->state != PERF_EVENT_STATE_ACTIVE)
>  		return;
>  
> -	ctx_resched(cpuctx, ctx);
> +	task_ctx = cpuctx->task_ctx;
> +	if (ctx->task)
> +		WARN_ON_ONCE(task_ctx != ctx);
> +
> +	ctx_resched(cpuctx, task_ctx);

Afaict, ctx_resched() path already does this in task_ctx_sched_out().

Regards,
--
Alex

Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Alexander Shishkin]

I think I caught one, below.

Peter Zijlstra <peterz@infradead.org> writes:

> +static int event_function(void *info)
> +{
> +	struct event_function_struct *efs = info;
> +	struct perf_event *event = efs->event;
> +	struct perf_event_context *ctx = event->ctx;
> +	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
> +	struct perf_event_context *task_ctx = cpuctx->task_ctx;
> +
> +	WARN_ON_ONCE(!irqs_disabled());
> +
> +	/*
> +	 * Since we do the IPI call without holding ctx->lock things can have
> +	 * changed, double check we hit the task we set out to hit.
> +	 *
> +	 * If ctx->task == current, we know things must remain valid because
> +	 * we have IRQs disabled so we cannot schedule.
> +	 */
> +	if (ctx->task) {
> +		if (ctx->task != current)
> +			return -EAGAIN;
> +
> +		WARN_ON_ONCE(task_ctx != ctx);

Looks like between dropping ctx::lock in event_function_call() and here,
cpuctx::task_ctx may still become NULL.

> +	} else {
> +		WARN_ON_ONCE(&cpuctx->ctx != ctx);
> +	}
> +
> +	perf_ctx_lock(cpuctx, task_ctx);
> +	/*
> +	 * Now that we hold locks, double check state. Paranoia pays.
> +	 */
> +	if (task_ctx) {
> +		WARN_ON_ONCE(task_ctx->task != current);
> +		/*
> +		 * We only use event_function_call() on established contexts,
> +		 * and event_function() is only ever called when active (or
> +		 * rather, we'll have bailed in task_function_call() or the
> +		 * above ctx->task != current test), therefore we must have
> +		 * ctx->is_active here.
> +		 */
> +		WARN_ON_ONCE(!ctx->is_active);
> +		/*
> +		 * And since we have ctx->is_active, cpuctx->task_ctx must
> +		 * match.
> +		 */
> +		WARN_ON_ONCE(cpuctx->task_ctx != task_ctx);
> +	}
> +	efs->func(event, cpuctx, ctx, efs->data);

In which case we probably don't want to call the callback.

Not sure if this is what Dmitry ran into, his logs contain warnings from
this function, but hard to tell exactly which ones.

Regards,
--
Alex

Re: [RFC][PATCH 00/12] various perf fixes

[Alexander Shishkin]

Dmitry Vyukov <dvyukov@google.com> writes:

> 2016/01/12 12:08:26 qemu-12: saving crash 'WARNING: CPU: 2 PID: 26946
> at kernel/events/core.c:213 event_function+0x4ba/0x590()' to

If you still have the code, can you check which of the WARN_ON_ONCE()s
is this?

Thanks,
--
Alex

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Wed, Jan 13, 2016 at 4:18 PM, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> Dmitry Vyukov <dvyukov@google.com> writes:
>
>> 2016/01/12 12:08:26 qemu-12: saving crash 'WARNING: CPU: 2 PID: 26946
>> at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
>
> If you still have the code, can you check which of the WARN_ON_ONCE()s
> is this?

Here is my core.c
https://gist.github.com/dvyukov/3b3a4993ade37e344636


>
> Thanks,
> --
> Alex

Re: [RFC][PATCH 00/12] various perf fixes

[Alexander Shishkin]

Dmitry Vyukov <dvyukov@google.com> writes:

> On Wed, Jan 13, 2016 at 4:18 PM, Alexander Shishkin
> <alexander.shishkin@linux.intel.com> wrote:
>> Dmitry Vyukov <dvyukov@google.com> writes:
>>
>>> 2016/01/12 12:08:26 qemu-12: saving crash 'WARNING: CPU: 2 PID: 26946
>>> at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
>>
>> If you still have the code, can you check which of the WARN_ON_ONCE()s
>> is this?
>
> Here is my core.c
> https://gist.github.com/dvyukov/3b3a4993ade37e344636

Bingo! Thanks.

Regards,
--
Alex

Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Alexander Shishkin]

Alexander Shishkin <alexander.shishkin@linux.intel.com> writes:

> Not sure if this is what Dmitry ran into, his logs contain warnings from
> this function, but hard to tell exactly which ones.

Looks like it is:

>  2016/01/12 12:08:26 qemu-12: saving crash 'WARNING: CPU: 2 PID: 26946
>  at kernel/events/core.c:213 event_function+0x4ba/0x590()' to
>  crash-qemu-12-1452596906889159952

https://gist.github.com/dvyukov/3b3a4993ade37e344636#file-gistfile1-txt-L213

Regards,
--
Alex

Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Peter Zijlstra]

On Wed, Jan 13, 2016 at 03:46:58PM +0200, Alexander Shishkin wrote:
> Peter Zijlstra <peterz@infradead.org> writes:
> 
> > @@ -2250,7 +2269,11 @@ static void __perf_event_enable(struct p
> >  	if (leader != event && leader->state != PERF_EVENT_STATE_ACTIVE)
> >  		return;
> >  
> > -	ctx_resched(cpuctx, ctx);
> > +	task_ctx = cpuctx->task_ctx;
> > +	if (ctx->task)
> > +		WARN_ON_ONCE(task_ctx != ctx);
> > +
> > +	ctx_resched(cpuctx, task_ctx);
> 
> Afaict, ctx_resched() path already does this in task_ctx_sched_out().

It does not; that got changed somewhere along the way :-)

Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Peter Zijlstra]

On Wed, Jan 13, 2016 at 05:00:50PM +0200, Alexander Shishkin wrote:
> I think I caught one, below.
> 
> Peter Zijlstra <peterz@infradead.org> writes:
> 
> > +static int event_function(void *info)
> > +{
> > +	struct event_function_struct *efs = info;
> > +	struct perf_event *event = efs->event;
> > +	struct perf_event_context *ctx = event->ctx;
> > +	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
> > +	struct perf_event_context *task_ctx = cpuctx->task_ctx;
> > +
> > +	WARN_ON_ONCE(!irqs_disabled());
> > +
> > +	/*
> > +	 * Since we do the IPI call without holding ctx->lock things can have
> > +	 * changed, double check we hit the task we set out to hit.
> > +	 *
> > +	 * If ctx->task == current, we know things must remain valid because
> > +	 * we have IRQs disabled so we cannot schedule.
> > +	 */
> > +	if (ctx->task) {
> > +		if (ctx->task != current)
> > +			return -EAGAIN;
> > +
> > +		WARN_ON_ONCE(task_ctx != ctx);
> 
> Looks like between dropping ctx::lock in event_function_call() and here,
> cpuctx::task_ctx may still become NULL.

Hmm yes I think you're right. If the event is being migrated to another
context concurrently the remove_from_context() might have gone through,
we'll still be waiting for sync_rcu and then this (say enabled) happens
and we're looking at a 'dead' context.

Thanks!

Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Peter Zijlstra]

On Wed, Jan 13, 2016 at 07:10:16PM +0100, Peter Zijlstra wrote:
> On Wed, Jan 13, 2016 at 05:00:50PM +0200, Alexander Shishkin wrote:
> > I think I caught one, below.
> > 
> > Peter Zijlstra <peterz@infradead.org> writes:
> > 
> > > +static int event_function(void *info)
> > > +{
> > > +	struct event_function_struct *efs = info;
> > > +	struct perf_event *event = efs->event;
> > > +	struct perf_event_context *ctx = event->ctx;
> > > +	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
> > > +	struct perf_event_context *task_ctx = cpuctx->task_ctx;
> > > +
> > > +	WARN_ON_ONCE(!irqs_disabled());
> > > +
> > > +	/*
> > > +	 * Since we do the IPI call without holding ctx->lock things can have
> > > +	 * changed, double check we hit the task we set out to hit.
> > > +	 *
> > > +	 * If ctx->task == current, we know things must remain valid because
> > > +	 * we have IRQs disabled so we cannot schedule.
> > > +	 */
> > > +	if (ctx->task) {
> > > +		if (ctx->task != current)
> > > +			return -EAGAIN;
> > > +
> > > +		WARN_ON_ONCE(task_ctx != ctx);
> > 
> > Looks like between dropping ctx::lock in event_function_call() and here,
> > cpuctx::task_ctx may still become NULL.
> 
> Hmm yes I think you're right. If the event is being migrated to another
> context concurrently the remove_from_context() might have gone through,
> we'll still be waiting for sync_rcu and then this (say enabled) happens
> and we're looking at a 'dead' context.

Hmm, no. This cannot be, all event_function_call() users should hold
ctx->mutex.

Re: [RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

On Tue, Jan 12, 2016 at 12:26:27PM +0100, Dmitry Vyukov wrote:
> What do you think if we work on making syzkaller work for you locally?

There's no easy way to do this right? I had a look at that project on
github and it looks like the most complex test setup possible :-(

I don't generally do VMs so I'm not much good at setting those up nor do
I speak Go (although I did play the game a lot of years ago).

Isn't there an easy way to just run syz-fuzzer on a machine without all
the bells and whistles on?

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Thu, Jan 14, 2016 at 10:35 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Jan 12, 2016 at 12:26:27PM +0100, Dmitry Vyukov wrote:
>> What do you think if we work on making syzkaller work for you locally?
>
> There's no easy way to do this right? I had a look at that project on
> github and it looks like the most complex test setup possible :-(
>
> I don't generally do VMs so I'm not much good at setting those up nor do
> I speak Go (although I did play the game a lot of years ago).
>
> Isn't there an easy way to just run syz-fuzzer on a machine without all
> the bells and whistles on?


There is a way to run it without coverage on a local machine.

First, you need to setup Go toolchain: download latest Go distribution
from https://golang.org/dl:
https://storage.googleapis.com/golang/go1.5.3.linux-amd64.tar.gz
Unpack it to $HOME/go1.5.

$ export GOROOT=$HOME/go1.5
$ export GOPATH=$HOME/gopath

Download syzkaller sources:

$ go get github.com/google/syzkaller

Build necessary syzkaller binaries:

$ cd $GOPATH/src/github.com/google/syzkaller
$ make

Then save the following content into
$GOPATH/src/github.com/google/syzkaller/perf.cfg

{
        "http": "localhost:50000",
        "workdir": "home/gopath/src/github.com/google/syzkaller/workdir",
        "syzkaller": "/home/gopath/src/github.com/google/syzkaller",
        "vmlinux": "-",
        "type": "local",
        "count": 1,
        "procs": 16,
        "nocover": true,
        "nodropprivs": true,
        "enable_syscalls": [
                "getpid",
                "perf_event_open",
                "ioctl$PERF*",
                "prctl$void",
                "bpf$*",
                "sched_yield"
        ]
}

Alter paths as necessary. Also you can change procs parameter (number
of parallel test processes), something like NCPU*4 would be a good
number. Also you can add additional syscalls to the mix.

Then run:

$ bin/syz-manager -config perf.cfg

If you run it on a separate test machine, then scp syzkaller/bin dir
and perf.cfg to the machine (the syzkaller param in config is where it
will search for the bin dir).

If syz-manager does not appear to be doing anything useful, then pleas
run it in the following mode and post output:

$ bin/syz-manager -config perf.cfg -v 1 -debug

Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Peter Zijlstra]

On Wed, Jan 13, 2016 at 05:00:50PM +0200, Alexander Shishkin wrote:
> I think I caught one, below.
> 
> Peter Zijlstra <peterz@infradead.org> writes:
> 
> > +static int event_function(void *info)
> > +{
> > +	struct event_function_struct *efs = info;
> > +	struct perf_event *event = efs->event;
> > +	struct perf_event_context *ctx = event->ctx;
> > +	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
> > +	struct perf_event_context *task_ctx = cpuctx->task_ctx;
> > +
> > +	WARN_ON_ONCE(!irqs_disabled());
> > +
> > +	/*
> > +	 * Since we do the IPI call without holding ctx->lock things can have
> > +	 * changed, double check we hit the task we set out to hit.
> > +	 *
> > +	 * If ctx->task == current, we know things must remain valid because
> > +	 * we have IRQs disabled so we cannot schedule.
> > +	 */
> > +	if (ctx->task) {
> > +		if (ctx->task != current)
> > +			return -EAGAIN;
> > +
> > +		WARN_ON_ONCE(task_ctx != ctx);
> 
> Looks like between dropping ctx::lock in event_function_call() and here,
> cpuctx::task_ctx may still become NULL.

You were indeed correct, and I've found out how and why.

Its a race with perf_event_exit_task(), which will schedule the context
out. I think there's a bunch of races around this area, but in general I
was planning on making event_function_call() a no-op if !(event->attach
& PERF_ATTACH_CONTEXT).

You cannot rely on ->is_active here, because while the context will be
inactive, you still do not want it to call ->func().

Let me prod at this a bit more.

Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Peter Zijlstra]

On Thu, Jan 14, 2016 at 11:44:49AM +0100, Peter Zijlstra wrote:
> On Wed, Jan 13, 2016 at 05:00:50PM +0200, Alexander Shishkin wrote:
> > I think I caught one, below.

> > Looks like between dropping ctx::lock in event_function_call() and here,
> > cpuctx::task_ctx may still become NULL.
> 
> You were indeed correct, and I've found out how and why.
> 
> Its a race with perf_event_exit_task(), which will schedule the context
> out. I think there's a bunch of races around this area, but in general I
> was planning on making event_function_call() a no-op if !(event->attach
> & PERF_ATTACH_CONTEXT).
> 
> You cannot rely on ->is_active here, because while the context will be
> inactive, you still do not want it to call ->func().
> 
> Let me prod at this a bit more.

Does this make sense?

---
Subject: perf: Fix perf_event_exit_task() race
From: Peter Zijlstra <peterz@infradead.org>
Date: Thu Jan 14 16:05:37 CET 2016

There is a race against perf_event_exit_task() vs
event_function_call(),find_get_context(),perf_install_in_context()
(iow, everyone).

Since there is no permanent marker on a context that its dead, it is
quite possible that we access (and even modify) a context after its
passed through perf_event_exit_task().

For instance, find_get_context() might find the context still
installed, but by the time we get to perf_install_in_context() it
might already have passed through perf_event_exit_task() and be
considered dead, we will however still add the event to it.

Solve this by marking a ctx dead by setting its ctx->task value to -1,
it must be !0 so we still know its a (former) task context.

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

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -148,6 +148,13 @@ static void perf_ctx_unlock(struct perf_
 	raw_spin_unlock(&cpuctx->ctx.lock);
 }
 
+#define TASK_TOMBSTONE ((void *)-1L)
+
+static bool is_kernel_event(struct perf_event *event)
+{
+	return event->owner == TASK_TOMBSTONE;
+}
+
 /*
  * On task ctx scheduling...
  *
@@ -196,31 +203,21 @@ static int event_function(void *info)
 	struct perf_event_context *ctx = event->ctx;
 	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
 	struct perf_event_context *task_ctx = cpuctx->task_ctx;
+	int ret = 0;
 
 	WARN_ON_ONCE(!irqs_disabled());
 
+	perf_ctx_lock(cpuctx, task_ctx);
 	/*
 	 * Since we do the IPI call without holding ctx->lock things can have
 	 * changed, double check we hit the task we set out to hit.
-	 *
-	 * If ctx->task == current, we know things must remain valid because
-	 * we have IRQs disabled so we cannot schedule.
 	 */
 	if (ctx->task) {
-		if (ctx->task != current)
-			return -EAGAIN;
-
-		WARN_ON_ONCE(task_ctx != ctx);
-	} else {
-		WARN_ON_ONCE(&cpuctx->ctx != ctx);
-	}
+		if (ctx->task != current) {
+			ret = -EAGAIN;
+			goto unlock;
+		}
 
-	perf_ctx_lock(cpuctx, task_ctx);
-	/*
-	 * Now that we hold locks, double check state. Paranoia pays.
-	 */
-	if (task_ctx) {
-		WARN_ON_ONCE(task_ctx->task != current);
 		/*
 		 * We only use event_function_call() on established contexts,
 		 * and event_function() is only ever called when active (or
@@ -233,12 +230,16 @@ static int event_function(void *info)
 		 * And since we have ctx->is_active, cpuctx->task_ctx must
 		 * match.
 		 */
-		WARN_ON_ONCE(cpuctx->task_ctx != task_ctx);
+		WARN_ON_ONCE(task_ctx != ctx);
+	} else {
+		WARN_ON_ONCE(&cpuctx->ctx != ctx);
 	}
+
 	efs->func(event, cpuctx, ctx, efs->data);
+unlock:
 	perf_ctx_unlock(cpuctx, task_ctx);
 
-	return 0;
+	return ret;
 }
 
 static void event_function_local(struct perf_event *event, event_f func, void *data)
@@ -256,7 +257,7 @@ static void event_function_local(struct
 static void event_function_call(struct perf_event *event, event_f func, void *data)
 {
 	struct perf_event_context *ctx = event->ctx;
-	struct task_struct *task = ctx->task;
+	struct task_struct *task = READ_ONCE(ctx->task); /* verified in event_function */
 	struct event_function_struct efs = {
 		.event = event,
 		.func = func,
@@ -278,30 +279,28 @@ static void event_function_call(struct p
 	}
 
 again:
+	if (task == TASK_TOMBSTONE)
+		return;
+
 	if (!task_function_call(task, event_function, &efs))
 		return;
 
 	raw_spin_lock_irq(&ctx->lock);
-	if (ctx->is_active) {
-		/*
-		 * Reload the task pointer, it might have been changed by
-		 * a concurrent perf_event_context_sched_out().
-		 */
-		task = ctx->task;
-		raw_spin_unlock_irq(&ctx->lock);
-		goto again;
+	/*
+	 * Reload the task pointer, it might have been changed by
+	 * a concurrent perf_event_context_sched_out().
+	 */
+	task = ctx->task;
+	if (task != TASK_TOMBSTONE) {
+		if (ctx->is_active) {
+			raw_spin_unlock_irq(&ctx->lock);
+			goto again;
+		}
+		func(event, NULL, ctx, data);
 	}
-	func(event, NULL, ctx, data);
 	raw_spin_unlock_irq(&ctx->lock);
 }
 
-#define EVENT_OWNER_KERNEL ((void *) -1)
-
-static bool is_kernel_event(struct perf_event *event)
-{
-	return event->owner == EVENT_OWNER_KERNEL;
-}
-
 #define PERF_FLAG_ALL (PERF_FLAG_FD_NO_GROUP |\
 		       PERF_FLAG_FD_OUTPUT  |\
 		       PERF_FLAG_PID_CGROUP |\
@@ -1025,7 +1024,7 @@ static void put_ctx(struct perf_event_co
 	if (atomic_dec_and_test(&ctx->refcount)) {
 		if (ctx->parent_ctx)
 			put_ctx(ctx->parent_ctx);
-		if (ctx->task)
+		if (ctx->task && ctx->task != TASK_TOMBSTONE)
 			put_task_struct(ctx->task);
 		call_rcu(&ctx->rcu_head, free_ctx);
 	}
@@ -1186,6 +1185,7 @@ static u64 primary_event_id(struct perf_
 
 /*
  * Get the perf_event_context for a task and lock it.
+ *
  * This has to cope with with the fact that until it is locked,
  * the context could get moved to another task.
  */
@@ -1226,10 +1226,13 @@ perf_lock_task_context(struct task_struc
 			goto retry;
 		}
 
-		if (!atomic_inc_not_zero(&ctx->refcount)) {
+		if (ctx->task == TASK_TOMBSTONE ||
+		    !atomic_inc_not_zero(&ctx->refcount)) {
 			raw_spin_unlock(&ctx->lock);
 			ctx = NULL;
 		}
+
+		WARN_ON_ONCE(ctx->task != task);
 	}
 	rcu_read_unlock();
 	if (!ctx)
@@ -2140,23 +2143,27 @@ static int  __perf_install_in_context(vo
 	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
 	struct perf_event_context *task_ctx = cpuctx->task_ctx;
 
+	raw_spin_lock(&cpuctx->ctx.lock);
 	if (ctx->task) {
+		raw_spin_lock(&ctx->lock);
 		/*
 		 * If we hit the 'wrong' task, we've since scheduled and
 		 * everything should be sorted, nothing to do!
 		 */
+		task_ctx = ctx;
 		if (ctx->task != current)
-			return 0;
+			goto unlock;
 
 		/*
 		 * If task_ctx is set, it had better be to us.
 		 */
 		WARN_ON_ONCE(cpuctx->task_ctx != ctx && cpuctx->task_ctx);
-		task_ctx = ctx;
+	} else if (task_ctx) {
+		raw_spin_lock(&task_ctx->lock);
 	}
 
-	perf_ctx_lock(cpuctx, task_ctx);
 	ctx_resched(cpuctx, task_ctx);
+unlock:
 	perf_ctx_unlock(cpuctx, task_ctx);
 
 	return 0;
@@ -2188,6 +2195,17 @@ perf_install_in_context(struct perf_even
 	 * happened and that will have taken care of business.
 	 */
 	raw_spin_lock_irq(&ctx->lock);
+	task = ctx->task;
+	/*
+	 * Worse, we cannot even rely on the ctx actually existing anymore. If
+	 * between find_get_context() and perf_install_in_context() the task
+	 * went through perf_event_exit_task() its dead and we should not be
+	 * adding new events.
+	 */
+	if (task == TASK_TOMBSTONE) {
+		raw_spin_unlock_irq(&ctx->lock);
+		return;
+	}
 	update_context_time(ctx);
 	/*
 	 * Update cgrp time only if current cgrp matches event->cgrp.
@@ -2195,7 +2213,6 @@ perf_install_in_context(struct perf_even
 	 */
 	update_cgrp_time_from_event(event);
 	add_event_to_ctx(event, ctx);
-	task = ctx->task;
 	raw_spin_unlock_irq(&ctx->lock);
 
 	if (task)
@@ -2538,17 +2555,21 @@ static void perf_event_context_sched_out
 		raw_spin_lock(&ctx->lock);
 		raw_spin_lock_nested(&next_ctx->lock, SINGLE_DEPTH_NESTING);
 		if (context_equiv(ctx, next_ctx)) {
-			/*
-			 * XXX do we need a memory barrier of sorts
-			 * wrt to rcu_dereference() of perf_event_ctxp
-			 */
-			task->perf_event_ctxp[ctxn] = next_ctx;
-			next->perf_event_ctxp[ctxn] = ctx;
-			ctx->task = next;
-			next_ctx->task = task;
+			WRITE_ONCE(ctx->task, next);
+			WRITE_ONCE(next_ctx->task, task);
 
 			swap(ctx->task_ctx_data, next_ctx->task_ctx_data);
 
+			/*
+			 * RCU_INIT_POINTER here is safe because we've not
+			 * modified the ctx and the above modification of
+			 * ctx->task and ctx->task_ctx_data are immaterial
+			 * since those values are always verified under
+			 * ctx->lock which we're now holding.
+			 */
+			RCU_INIT_POINTER(task->perf_event_ctxp[ctxn], next_ctx);
+			RCU_INIT_POINTER(next->perf_event_ctxp[ctxn], ctx);
+
 			do_switch = 0;
 
 			perf_event_sync_stat(ctx, next_ctx);
@@ -8545,7 +8566,7 @@ perf_event_create_kernel_counter(struct
 	}
 
 	/* Mark owner so we could distinguish it from user events. */
-	event->owner = EVENT_OWNER_KERNEL;
+	event->owner = TASK_TOMBSTONE;
 
 	account_event(event);
 
@@ -8725,28 +8746,26 @@ __perf_event_exit_task(struct perf_event
 
 static void perf_event_exit_task_context(struct task_struct *child, int ctxn)
 {
-	struct perf_event *child_event, *next;
 	struct perf_event_context *child_ctx, *clone_ctx = NULL;
+	struct perf_event *child_event, *next;
+	unsigned long flags;
+
+	WARN_ON_ONCE(child != current);
 
-	if (likely(!child->perf_event_ctxp[ctxn]))
+	child_ctx = perf_lock_task_context(child, ctxn, &flags);
+	if (!child_ctx)
 		return;
 
-	local_irq_disable();
-	WARN_ON_ONCE(child != current);
-	/*
-	 * We can't reschedule here because interrupts are disabled,
-	 * and child must be current.
-	 */
-	child_ctx = rcu_dereference_raw(child->perf_event_ctxp[ctxn]);
+	task_ctx_sched_out(__get_cpu_context(child_ctx), child_ctx);
 
 	/*
-	 * Take the context lock here so that if find_get_context is
-	 * reading child->perf_event_ctxp, we wait until it has
-	 * incremented the context's refcount before we do put_ctx below.
+	 * Now that the context is inactive, destroy the task <-> ctx relation
+	 * and mark the context dead.
 	 */
-	raw_spin_lock(&child_ctx->lock);
-	task_ctx_sched_out(__get_cpu_context(child_ctx), child_ctx);
-	child->perf_event_ctxp[ctxn] = NULL;
+	RCU_INIT_POINTER(child->perf_event_ctxp[ctxn], NULL);
+	put_ctx(child_ctx); /* cannot be last */
+	WRITE_ONCE(child_ctx->task, TASK_TOMBSTONE);
+	put_task_struct(current); /* cannot be last */
 
 	/*
 	 * If this context is a clone; unclone it so it can't get
@@ -8755,7 +8774,7 @@ static void perf_event_exit_task_context
 	 */
 	clone_ctx = unclone_ctx(child_ctx);
 	update_context_time(child_ctx);
-	raw_spin_unlock_irq(&child_ctx->lock);
+	raw_spin_unlock_irqrestore(&child_ctx->lock, flags);
 
 	if (clone_ctx)
 		put_ctx(clone_ctx);

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Thu, Jan 14, 2016 at 11:05 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Thu, Jan 14, 2016 at 10:35 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Tue, Jan 12, 2016 at 12:26:27PM +0100, Dmitry Vyukov wrote:
>>> What do you think if we work on making syzkaller work for you locally?
>>
>> There's no easy way to do this right? I had a look at that project on
>> github and it looks like the most complex test setup possible :-(
>>
>> I don't generally do VMs so I'm not much good at setting those up nor do
>> I speak Go (although I did play the game a lot of years ago).
>>
>> Isn't there an easy way to just run syz-fuzzer on a machine without all
>> the bells and whistles on?


Peter, I want to check status of these perf crashes. I see that you
landed a bunch of patches. But I still see perf-related crashes while
running syzkaller. Are there any existing bugs left that you know
about? Or should I report new crashes?
Did you have any luck setting up syzkaller locally? That could
significantly reduce turnaround time.



> There is a way to run it without coverage on a local machine.
>
> First, you need to setup Go toolchain: download latest Go distribution
> from https://golang.org/dl:
> https://storage.googleapis.com/golang/go1.5.3.linux-amd64.tar.gz
> Unpack it to $HOME/go1.5.
>
> $ export GOROOT=$HOME/go1.5
> $ export GOPATH=$HOME/gopath
>
> Download syzkaller sources:
>
> $ go get github.com/google/syzkaller
>
> Build necessary syzkaller binaries:
>
> $ cd $GOPATH/src/github.com/google/syzkaller
> $ make
>
> Then save the following content into
> $GOPATH/src/github.com/google/syzkaller/perf.cfg
>
> {
>         "http": "localhost:50000",
>         "workdir": "home/gopath/src/github.com/google/syzkaller/workdir",
>         "syzkaller": "/home/gopath/src/github.com/google/syzkaller",
>         "vmlinux": "-",
>         "type": "local",
>         "count": 1,
>         "procs": 16,
>         "nocover": true,
>         "nodropprivs": true,
>         "enable_syscalls": [
>                 "getpid",
>                 "perf_event_open",
>                 "ioctl$PERF*",
>                 "prctl$void",
>                 "bpf$*",
>                 "sched_yield"
>         ]
> }
>
> Alter paths as necessary. Also you can change procs parameter (number
> of parallel test processes), something like NCPU*4 would be a good
> number. Also you can add additional syscalls to the mix.
>
> Then run:
>
> $ bin/syz-manager -config perf.cfg
>
> If you run it on a separate test machine, then scp syzkaller/bin dir
> and perf.cfg to the machine (the syzkaller param in config is where it
> will search for the bin dir).
>
> If syz-manager does not appear to be doing anything useful, then pleas
> run it in the following mode and post output:
>
> $ bin/syz-manager -config perf.cfg -v 1 -debug

Re: [RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

On Mon, Feb 15, 2016 at 04:04:36PM +0100, Dmitry Vyukov wrote:
> Peter, I want to check status of these perf crashes. I see that you
> landed a bunch of patches. But I still see perf-related crashes while
> running syzkaller.

Bugger :-)

> Are there any existing bugs left that you know
> about? 

I might have one report, but I've been ill the past two weeks, so I'm
not too sure on that.

> Or should I report new crashes?

Please.

> Did you have any luck setting up syzkaller locally? That could
> significantly reduce turnaround time.

Not yet :/ I managed to reproduce a lot of fail with Vince's perf-fuzzer
and some stress test scripts Ingo had.

I suppose I'll have to go look at making this happen; right bugger for
requiring a weird language though :/

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Mon, Feb 15, 2016 at 4:38 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Feb 15, 2016 at 04:04:36PM +0100, Dmitry Vyukov wrote:
>> Peter, I want to check status of these perf crashes. I see that you
>> landed a bunch of patches. But I still see perf-related crashes while
>> running syzkaller.
>
> Bugger :-)
>
>> Are there any existing bugs left that you know
>> about?
>
> I might have one report, but I've been ill the past two weeks, so I'm
> not too sure on that.
>
>> Or should I report new crashes?
>
> Please.
>
>> Did you have any luck setting up syzkaller locally? That could
>> significantly reduce turnaround time.
>
> Not yet :/ I managed to reproduce a lot of fail with Vince's perf-fuzzer
> and some stress test scripts Ingo had.
>
> I suppose I'll have to go look at making this happen; right bugger for
> requiring a weird language though :/

It should not be _too_ hard :)
Feel free to contact me or syzkaller@googlegroups.com if you have any
issues with setup.

Re: [RFC][PATCH 00/12] various perf fixes

[Vince Weaver]

On Mon, 15 Feb 2016, Peter Zijlstra wrote:

> Not yet :/ I managed to reproduce a lot of fail with Vince's perf-fuzzer
> and some stress test scripts Ingo had.

I was glad to see the fixes go by, since a lot of the
syzkaller reports are very similar to open bugs I have with perf_fuzzer.

I have to admit I've been busy/lazy recently and so was waiting for the 
fixes to hit an actual Linus -rc kernel before ramping up my fuzz testing 
again.

Vince

Re: [RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

On Thu, Jan 14, 2016 at 11:05:50AM +0100, Dmitry Vyukov wrote:
> On Thu, Jan 14, 2016 at 10:35 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Tue, Jan 12, 2016 at 12:26:27PM +0100, Dmitry Vyukov wrote:
> >> What do you think if we work on making syzkaller work for you locally?
> >
> > There's no easy way to do this right? I had a look at that project on
> > github and it looks like the most complex test setup possible :-(
> >
> > I don't generally do VMs so I'm not much good at setting those up nor do
> > I speak Go (although I did play the game a lot of years ago).
> >
> > Isn't there an easy way to just run syz-fuzzer on a machine without all
> > the bells and whistles on?
> 
> 
> There is a way to run it without coverage on a local machine.
> 
> First, you need to setup Go toolchain: download latest Go distribution
> from https://golang.org/dl:
> https://storage.googleapis.com/golang/go1.5.3.linux-amd64.tar.gz
> Unpack it to $HOME/go1.5.

I used: apt-get -t testing install golang, which gets me that same 1.5.3
version of Go (and I still think its a board game!).

> $ export GOROOT=$HOME/go1.5
> $ export GOPATH=$HOME/gopath

Fully expecting not to need nonsense like that.

> Download syzkaller sources:
> 
> $ go get github.com/google/syzkaller

But that yields:

~# go get github.com/google/syzkaller
package github.com/google/syzkaller: cannot download, $GOPATH not set.  For more details see: go help gopath

Clearly this stuff isn't meant to be used :/ Why doesn't it set a system
GOPATH in profile.d or whatnot. Why do I, as a user, need to be bothered
with crap like this.

Re: [RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

On Thu, Jan 14, 2016 at 11:05:50AM +0100, Dmitry Vyukov wrote:
> There is a way to run it without coverage on a local machine.
> 
> First, you need to setup Go toolchain: download latest Go distribution
> from https://golang.org/dl:
> https://storage.googleapis.com/golang/go1.5.3.linux-amd64.tar.gz
> Unpack it to $HOME/go1.5.
> 
> $ export GOROOT=$HOME/go1.5

> $ export GOPATH=$HOME/gopath

So after I put that in a profile.d file:

> Download syzkaller sources:
> 
> $ go get github.com/google/syzkaller

package github.com/google/syzkaller: no buildable Go source files in /root/gopath/src/github.com/google/syzkaller

> Build necessary syzkaller binaries:
> 
> $ cd $GOPATH/src/github.com/google/syzkaller
> $ make

That seems to have done its thing

> Then save the following content into
> $GOPATH/src/github.com/google/syzkaller/perf.cfg
> 

{
	"http": "localhost:50000",
	"workdir": "root/gopath/src/github.com/google/syzkaller/workdir",
	"syzkaller": "/root/gopath/src/github.com/google/syzkaller",
	"vmlinux": "-",
	"type": "local",
	"count": 1,
	"procs": 160,
	"nocover": true,
	"nodropprivs": true,
	"enable_syscalls": [
		"perf_event_open",
		"ioctl$PERF*",
		"prctl$void",
		"bpf$*",
		"sched_yield"
	]
}

> Alter paths as necessary. Also you can change procs parameter (number
> of parallel test processes), something like NCPU*4 would be a good
> number. Also you can add additional syscalls to the mix.

per the above

> Then run:
> 
> $ bin/syz-manager -config perf.cfg
> 
> If you run it on a separate test machine, then scp syzkaller/bin dir
> and perf.cfg to the machine (the syzkaller param in config is where it
> will search for the bin dir).
> 
> If syz-manager does not appear to be doing anything useful, then pleas
> run it in the following mode and post output:
> 
> $ bin/syz-manager -config perf.cfg -v 1 -debug

root@ivb-ep:~/gopath/src/github.com/google/syzkaller# bin/syz-manager -config perf.cfg -v 1 -debug
2016/02/15 17:12:50 bad config syzkaller param: can't find bin/syz-execprog

And its right, no such thing as bin/syz-execprog

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Mon, Feb 15, 2016 at 5:17 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Jan 14, 2016 at 11:05:50AM +0100, Dmitry Vyukov wrote:
>> On Thu, Jan 14, 2016 at 10:35 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> > On Tue, Jan 12, 2016 at 12:26:27PM +0100, Dmitry Vyukov wrote:
>> >> What do you think if we work on making syzkaller work for you locally?
>> >
>> > There's no easy way to do this right? I had a look at that project on
>> > github and it looks like the most complex test setup possible :-(
>> >
>> > I don't generally do VMs so I'm not much good at setting those up nor do
>> > I speak Go (although I did play the game a lot of years ago).
>> >
>> > Isn't there an easy way to just run syz-fuzzer on a machine without all
>> > the bells and whistles on?
>>
>>
>> There is a way to run it without coverage on a local machine.
>>
>> First, you need to setup Go toolchain: download latest Go distribution
>> from https://golang.org/dl:
>> https://storage.googleapis.com/golang/go1.5.3.linux-amd64.tar.gz
>> Unpack it to $HOME/go1.5.
>
> I used: apt-get -t testing install golang, which gets me that same 1.5.3
> version of Go (and I still think its a board game!).
>
>> $ export GOROOT=$HOME/go1.5
>> $ export GOPATH=$HOME/gopath
>
> Fully expecting not to need nonsense like that.
>
>> Download syzkaller sources:
>>
>> $ go get github.com/google/syzkaller
>
> But that yields:
>
> ~# go get github.com/google/syzkaller
> package github.com/google/syzkaller: cannot download, $GOPATH not set.  For more details see: go help gopath
>
> Clearly this stuff isn't meant to be used :/ Why doesn't it set a system
> GOPATH in profile.d or whatnot. Why do I, as a user, need to be bothered
> with crap like this.

If you get Go via apt-get, then you don't need to set GOROOT, but you
still need to set GOPATH to some path. That's the path where 'go get'
will download syzkaller and dependent packages.

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Mon, Feb 15, 2016 at 5:29 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Jan 14, 2016 at 11:05:50AM +0100, Dmitry Vyukov wrote:
>> There is a way to run it without coverage on a local machine.
>>
>> First, you need to setup Go toolchain: download latest Go distribution
>> from https://golang.org/dl:
>> https://storage.googleapis.com/golang/go1.5.3.linux-amd64.tar.gz
>> Unpack it to $HOME/go1.5.
>>
>> $ export GOROOT=$HOME/go1.5
>
>> $ export GOPATH=$HOME/gopath
>
> So after I put that in a profile.d file:
>
>> Download syzkaller sources:
>>
>> $ go get github.com/google/syzkaller
>
> package github.com/google/syzkaller: no buildable Go source files in /root/gopath/src/github.com/google/syzkaller
>
>> Build necessary syzkaller binaries:
>>
>> $ cd $GOPATH/src/github.com/google/syzkaller
>> $ make
>
> That seems to have done its thing
>
>> Then save the following content into
>> $GOPATH/src/github.com/google/syzkaller/perf.cfg
>>
>
> {
>         "http": "localhost:50000",
>         "workdir": "root/gopath/src/github.com/google/syzkaller/workdir",
>         "syzkaller": "/root/gopath/src/github.com/google/syzkaller",
>         "vmlinux": "-",
>         "type": "local",
>         "count": 1,
>         "procs": 160,
>         "nocover": true,
>         "nodropprivs": true,
>         "enable_syscalls": [
>                 "perf_event_open",
>                 "ioctl$PERF*",
>                 "prctl$void",
>                 "bpf$*",
>                 "sched_yield"
>         ]
> }
>
>> Alter paths as necessary. Also you can change procs parameter (number
>> of parallel test processes), something like NCPU*4 would be a good
>> number. Also you can add additional syscalls to the mix.
>
> per the above
>
>> Then run:
>>
>> $ bin/syz-manager -config perf.cfg
>>
>> If you run it on a separate test machine, then scp syzkaller/bin dir
>> and perf.cfg to the machine (the syzkaller param in config is where it
>> will search for the bin dir).
>>
>> If syz-manager does not appear to be doing anything useful, then pleas
>> run it in the following mode and post output:
>>
>> $ bin/syz-manager -config perf.cfg -v 1 -debug
>
> root@ivb-ep:~/gopath/src/github.com/google/syzkaller# bin/syz-manager -config perf.cfg -v 1 -debug
> 2016/02/15 17:12:50 bad config syzkaller param: can't find bin/syz-execprog
>
> And its right, no such thing as bin/syz-execprog


Please also do:

$ make execprog

And replace:

        "nocover": true,
        "nodropprivs": true,

in config file with:

        "cover": false,
        "dropprivs": false,

(that's changed since I wrote the instructions).

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Mon, Feb 15, 2016 at 5:35 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Mon, Feb 15, 2016 at 5:29 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Thu, Jan 14, 2016 at 11:05:50AM +0100, Dmitry Vyukov wrote:
>>> There is a way to run it without coverage on a local machine.
>>>
>>> First, you need to setup Go toolchain: download latest Go distribution
>>> from https://golang.org/dl:
>>> https://storage.googleapis.com/golang/go1.5.3.linux-amd64.tar.gz
>>> Unpack it to $HOME/go1.5.
>>>
>>> $ export GOROOT=$HOME/go1.5
>>
>>> $ export GOPATH=$HOME/gopath
>>
>> So after I put that in a profile.d file:
>>
>>> Download syzkaller sources:
>>>
>>> $ go get github.com/google/syzkaller
>>
>> package github.com/google/syzkaller: no buildable Go source files in /root/gopath/src/github.com/google/syzkaller
>>
>>> Build necessary syzkaller binaries:
>>>
>>> $ cd $GOPATH/src/github.com/google/syzkaller
>>> $ make
>>
>> That seems to have done its thing
>>
>>> Then save the following content into
>>> $GOPATH/src/github.com/google/syzkaller/perf.cfg
>>>
>>
>> {
>>         "http": "localhost:50000",
>>         "workdir": "root/gopath/src/github.com/google/syzkaller/workdir",
>>         "syzkaller": "/root/gopath/src/github.com/google/syzkaller",
>>         "vmlinux": "-",
>>         "type": "local",
>>         "count": 1,
>>         "procs": 160,
>>         "nocover": true,
>>         "nodropprivs": true,
>>         "enable_syscalls": [
>>                 "perf_event_open",
>>                 "ioctl$PERF*",
>>                 "prctl$void",
>>                 "bpf$*",
>>                 "sched_yield"
>>         ]
>> }
>>
>>> Alter paths as necessary. Also you can change procs parameter (number
>>> of parallel test processes), something like NCPU*4 would be a good
>>> number. Also you can add additional syscalls to the mix.
>>
>> per the above
>>
>>> Then run:
>>>
>>> $ bin/syz-manager -config perf.cfg
>>>
>>> If you run it on a separate test machine, then scp syzkaller/bin dir
>>> and perf.cfg to the machine (the syzkaller param in config is where it
>>> will search for the bin dir).
>>>
>>> If syz-manager does not appear to be doing anything useful, then pleas
>>> run it in the following mode and post output:
>>>
>>> $ bin/syz-manager -config perf.cfg -v 1 -debug
>>
>> root@ivb-ep:~/gopath/src/github.com/google/syzkaller# bin/syz-manager -config perf.cfg -v 1 -debug
>> 2016/02/15 17:12:50 bad config syzkaller param: can't find bin/syz-execprog
>>
>> And its right, no such thing as bin/syz-execprog
>
>
> Please also do:
>
> $ make execprog
>
> And replace:
>
>         "nocover": true,
>         "nodropprivs": true,
>
> in config file with:
>
>         "cover": false,
>         "dropprivs": false,
>
> (that's changed since I wrote the instructions).


Just to compensate the pain, here is what I've just got when
re-enabled perf in my config :)

2016/02/15 16:52:18 qemu-15: saving crash 'WARNING: CPU: 0 PID: 18465
at kernel/events/core.c:2743 ctx_sched_in+0x255/0x17f0()' to
crash-qemu-15-1455551538366997265
2016/02/15 16:54:02 qemu-17: saving crash 'WARNING: CPU: 0 PID: 1072
at kernel/events/core.c:2743 ctx_sched_in+0x255/0x17f0()' to
crash-qemu-17-1455551642205132730
2016/02/15 16:54:44 qemu-30: saving crash 'WARNING: CPU: 3 PID: 19858
at kernel/events/core.c:2122 __perf_install_in_context+0x1c4/0x220()'
to crash-qemu-30-1455551684741709699
2016/02/15 16:55:02 qemu-20: saving crash 'WARNING: CPU: 2 PID: 9873
at kernel/events/core.c:226 event_function+0x359/0x3e0()' to
crash-qemu-20-1455551702744185903
2016/02/15 16:56:22 qemu-27: saving crash 'WARNING: CPU: 3 PID: 10602
at kernel/events/core.c:2122 __perf_install_in_context+0x1c4/0x220()'
to crash-qemu-27-1455551782768040747
2016/02/15 16:56:45 qemu-27: saving crash 'WARNING: CPU: 0 PID: 12779
at kernel/events/core.c:8693 perf_event_exit_task+0x708/0x900()' to
crash-qemu-27-1455551805030612662
2016/02/15 16:56:49 qemu-2: saving crash 'WARNING: CPU: 3 PID: 17062
at kernel/events/core.c:226 event_function+0x359/0x3e0()' to
crash-qemu-2-1455551809378028704
2016/02/15 16:56:58 qemu-20: saving crash 'WARNING: CPU: 3 PID: 22426
at kernel/events/core.c:2743 ctx_sched_in+0x255/0x17f0()' to
crash-qemu-20-1455551818913067860
2016/02/15 16:57:08 qemu-33: saving crash 'WARNING: CPU: 3 PID: 23837
at kernel/events/core.c:2122 __perf_install_in_context+0x1c4/0x220()'
to crash-qemu-33-1455551828622810673
2016/02/15 16:57:11 qemu-27: saving crash 'INFO: rcu_sched detected
stalls on CPUs/tasks:' to crash-qemu-27-1455551831109315050
2016/02/15 16:57:34 qemu-33: saving crash 'INFO: rcu_sched
self-detected stall on CPU' to crash-qemu-33-1455551854588053356
2016/02/15 16:57:44 qemu-33: saving crash 'BUG: workqueue lockup -
pool cpus=0 node=0 flags=0x0 nice=-20 stuck for 34s!' to
crash-qemu-33-1455551864688663395

Re: [RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

On Mon, Feb 15, 2016 at 05:35:51PM +0100, Dmitry Vyukov wrote:
> > root@ivb-ep:~/gopath/src/github.com/google/syzkaller# bin/syz-manager -config perf.cfg -v 1 -debug
> > 2016/02/15 17:12:50 bad config syzkaller param: can't find bin/syz-execprog
> >
> > And its right, no such thing as bin/syz-execprog
> 
> 
> Please also do:
> 
> $ make execprog
> 
> And replace:
> 
>         "nocover": true,
>         "nodropprivs": true,
> 
> in config file with:
> 
>         "cover": false,
>         "dropprivs": false,
> 
> (that's changed since I wrote the instructions).

OK, now I have syz-fuzzer at ~950% CPU time and a gazillion syz-executor
tasks running.

No splats yet, I'll leave it running for a while.


In order to get coverage support, I have to apply that one patch to my
local kernel, right?

Re: [RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

On Mon, Feb 15, 2016 at 05:38:36PM +0100, Dmitry Vyukov wrote:
> Just to compensate the pain, here is what I've just got when
> re-enabled perf in my config :)
> 
> 2016/02/15 16:52:18 qemu-15: saving crash 'WARNING: CPU: 0 PID: 18465
> at kernel/events/core.c:2743 ctx_sched_in+0x255/0x17f0()' to
> crash-qemu-15-1455551538366997265
> 2016/02/15 16:54:02 qemu-17: saving crash 'WARNING: CPU: 0 PID: 1072
> at kernel/events/core.c:2743 ctx_sched_in+0x255/0x17f0()' to
> crash-qemu-17-1455551642205132730
> 2016/02/15 16:54:44 qemu-30: saving crash 'WARNING: CPU: 3 PID: 19858
> at kernel/events/core.c:2122 __perf_install_in_context+0x1c4/0x220()'
> to crash-qemu-30-1455551684741709699
> 2016/02/15 16:55:02 qemu-20: saving crash 'WARNING: CPU: 2 PID: 9873
> at kernel/events/core.c:226 event_function+0x359/0x3e0()' to
> crash-qemu-20-1455551702744185903

That looks like a splat every minute or so. I've now had it run for
almost 10 minutes and I've not seen anything except:

2016/02/15 17:26:25 local-0: saving crash 'not executing programs' to crash-local-0-1455553585720015733
2016/02/15 17:29:25 local-0: saving crash 'not executing programs' to crash-local-0-1455553765740119317
2016/02/15 17:32:25 local-0: saving crash 'not executing programs' to crash-local-0-1455553945767974819
2016/02/15 17:35:25 local-0: saving crash 'not executing programs' to crash-local-0-1455554125793957186

And those files are completely empty of useful.

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Mon, Feb 15, 2016 at 5:41 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Feb 15, 2016 at 05:35:51PM +0100, Dmitry Vyukov wrote:
>> > root@ivb-ep:~/gopath/src/github.com/google/syzkaller# bin/syz-manager -config perf.cfg -v 1 -debug
>> > 2016/02/15 17:12:50 bad config syzkaller param: can't find bin/syz-execprog
>> >
>> > And its right, no such thing as bin/syz-execprog
>>
>>
>> Please also do:
>>
>> $ make execprog
>>
>> And replace:
>>
>>         "nocover": true,
>>         "nodropprivs": true,
>>
>> in config file with:
>>
>>         "cover": false,
>>         "dropprivs": false,
>>
>> (that's changed since I wrote the instructions).
>
> OK, now I have syz-fuzzer at ~950% CPU time and a gazillion syz-executor
> tasks running.

If you want more or less, it is controlled by the "procs": 160 config parameter.

> No splats yet, I'll leave it running for a while.
>
>
> In order to get coverage support, I have to apply that one patch to my
> local kernel, right?

Unfortunately you also need a very fresh gcc. Kcov support was
committed to gcc in revision 231296 (Dec 4). So you either need to
build gcc manually or obtain some kind of nightly build.

Yes, you need to apply the kcov patch to kernel. It is now in
linux-next tree, so you can pull from there.
Enable CONFIG_KCOV and CONFIG_DEBUGFS and build kernel with 'make
CC=your/fresh/gcc'.

And then change "cover": false in config to "cover": true.

Re: [RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

On Mon, Feb 15, 2016 at 05:54:51PM +0100, Dmitry Vyukov wrote:
> > OK, now I have syz-fuzzer at ~950% CPU time and a gazillion syz-executor
> > tasks running.
> 
> If you want more or less, it is controlled by the "procs": 160 config parameter.

I've got 2*10 cores, each with SMT enabled for 40 'CPU's, 160 is the
4*NR_CPUS thing you suggested.

Should be good enough to keep the machine busy I suppose.

> > No splats yet, I'll leave it running for a while.
> >
> >
> > In order to get coverage support, I have to apply that one patch to my
> > local kernel, right?
> 
> Unfortunately you also need a very fresh gcc. Kcov support was
> committed to gcc in revision 231296 (Dec 4). So you either need to
> build gcc manually or obtain some kind of nightly build.

My last GCC build is 5.3-branch or so, is that fresh enough?

> Yes, you need to apply the kcov patch to kernel. It is now in
> linux-next tree, so you can pull from there.
> Enable CONFIG_KCOV and CONFIG_DEBUGFS and build kernel with 'make
> CC=your/fresh/gcc'.
> 
> And then change "cover": false in config to "cover": true.

Right, I'll see if I can pick the patch from there.

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Mon, Feb 15, 2016 at 5:52 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Feb 15, 2016 at 05:38:36PM +0100, Dmitry Vyukov wrote:
>> Just to compensate the pain, here is what I've just got when
>> re-enabled perf in my config :)
>>
>> 2016/02/15 16:52:18 qemu-15: saving crash 'WARNING: CPU: 0 PID: 18465
>> at kernel/events/core.c:2743 ctx_sched_in+0x255/0x17f0()' to
>> crash-qemu-15-1455551538366997265
>> 2016/02/15 16:54:02 qemu-17: saving crash 'WARNING: CPU: 0 PID: 1072
>> at kernel/events/core.c:2743 ctx_sched_in+0x255/0x17f0()' to
>> crash-qemu-17-1455551642205132730
>> 2016/02/15 16:54:44 qemu-30: saving crash 'WARNING: CPU: 3 PID: 19858
>> at kernel/events/core.c:2122 __perf_install_in_context+0x1c4/0x220()'
>> to crash-qemu-30-1455551684741709699
>> 2016/02/15 16:55:02 qemu-20: saving crash 'WARNING: CPU: 2 PID: 9873
>> at kernel/events/core.c:226 event_function+0x359/0x3e0()' to
>> crash-qemu-20-1455551702744185903
>
> That looks like a splat every minute or so. I've now had it run for
> almost 10 minutes and I've not seen anything except:
>
> 2016/02/15 17:26:25 local-0: saving crash 'not executing programs' to crash-local-0-1455553585720015733
> 2016/02/15 17:29:25 local-0: saving crash 'not executing programs' to crash-local-0-1455553765740119317
> 2016/02/15 17:32:25 local-0: saving crash 'not executing programs' to crash-local-0-1455553945767974819
> 2016/02/15 17:35:25 local-0: saving crash 'not executing programs' to crash-local-0-1455554125793957186
>
> And those files are completely empty of useful.

There is something wrong and I guess it is not actually working.
Please post your config file so that I can double check. In
particular, did you set "cover": false?

Re: [RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

On Mon, Feb 15, 2016 at 06:04:15PM +0100, Dmitry Vyukov wrote:
> There is something wrong and I guess it is not actually working.
> Please post your config file so that I can double check. In
> particular, did you set "cover": false?


{
	"http": "localhost:50000",
	"workdir": "root/gopath/src/github.com/google/syzkaller/workdir",
	"syzkaller": "/root/gopath/src/github.com/google/syzkaller",
	"vmlinux": "-",
	"type": "local",
	"count": 1,
	"procs": 160,
	"cover": false,
	"dropprivs": false,
	"enable_syscalls": [
		"perf_event_open",
		"ioctl$PERF*",
		"prctl$void",
		"bpf$*",
		"sched_yield"
	]
}

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]

On Mon, Feb 15, 2016 at 6:07 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Feb 15, 2016 at 06:04:15PM +0100, Dmitry Vyukov wrote:
>> There is something wrong and I guess it is not actually working.
>> Please post your config file so that I can double check. In
>> particular, did you set "cover": false?
>
>
> {
>         "http": "localhost:50000",
>         "workdir": "root/gopath/src/github.com/google/syzkaller/workdir",
>         "syzkaller": "/root/gopath/src/github.com/google/syzkaller",
>         "vmlinux": "-",
>         "type": "local",
>         "count": 1,
>         "procs": 160,
>         "cover": false,
>         "dropprivs": false,
>         "enable_syscalls": [
>                 "perf_event_open",
>                 "ioctl$PERF*",
>                 "prctl$void",
>                 "bpf$*",
>                 "sched_yield"
>         ]
> }


Ah, OK, it's actually working, everything is fine. You can double
check it by executing:

$ curl http://localhost:50000 | grep "exec total"

On my machine it produces (this is number of executed test programs per second):
exec total: 1196/sec<br>

However, you need to add getpid and gettid syscalls to the list of
enabled syscalls in config (they are required for perf_event_open):

        "enable_syscalls": [
                "getpid",
                "gettid",
                "perf_event_open",
                "ioctl$PERF*",
                "prctl$void",
                "bpf$*",
                "sched_yield"
        ]

(you can also add few other syscalls if you think they can be
relevant, but I am able to trigger perf crashes with just these)

And note that syz-manager binary won't notify you about any crashes,
you need to look in dmesg (and maybe at hanged processes).

I've collected some corpus of perf-related programs with coverage in
my setup. If you extract the attached archive into
root/gopath/src/github.com/google/syzkaller/workdir/corpus/ dir, it
should significantly speed up triggering of bugs (note that you did
not add / in front of the path, so it should be in your cwd).
While collecting the corpus I've triggered whole lot of crashes in few minutes:
https://gist.githubusercontent.com/dvyukov/9433b40adb094cc22cf2/raw/8642ad53ea44f604168189c568b419ba498dd5f3/gistfile1.txt

[-- Attachment #2: corpus.zip --]
[-- Type: application/zip, Size: 140780 bytes --]

Re: [RFC][PATCH 00/12] various perf fixes

[Peter Zijlstra]

On Mon, Feb 15, 2016 at 06:45:59PM +0100, Dmitry Vyukov wrote:
> However, you need to add getpid and gettid syscalls to the list of
> enabled syscalls in config (they are required for perf_event_open):
> 
>         "enable_syscalls": [
>                 "getpid",
>                 "gettid",
>                 "perf_event_open",
>                 "ioctl$PERF*",
>                 "prctl$void",
>                 "bpf$*",
>                 "sched_yield"
>         ]
> 

Yep, that made it work. I'll go have a look.

Thanks!

Re: [RFC][PATCH 00/12] various perf fixes

[Dmitry Vyukov]

On Mon, Feb 15, 2016 at 7:01 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Feb 15, 2016 at 06:45:59PM +0100, Dmitry Vyukov wrote:
>> However, you need to add getpid and gettid syscalls to the list of
>> enabled syscalls in config (they are required for perf_event_open):
>>
>>         "enable_syscalls": [
>>                 "getpid",
>>                 "gettid",
>>                 "perf_event_open",
>>                 "ioctl$PERF*",
>>                 "prctl$void",
>>                 "bpf$*",
>>                 "sched_yield"
>>         ]
>>
>
> Yep, that made it work. I'll go have a look.


Glad it works for you. Thanks for bearing with me.

I will fix the confusing "not executing programs" messages.

Re gcc 5.3: it was released on the day I submitted the patch and 5.3
claims to contain only bug fixes, so I guess it is not there.


Btw, do you have CONFIG_KASAN enabled? I also see some some
use-after-free bugs. gcc 5.3 should fully support KASAN including
CONFIG_KASAN_INLINE=y (which is faster).


BUG: KASAN: use-after-free in perf_event_task_tick+0xb47/0xbd0 at addr
ffff880036cff2b8
Read of size 8 by task ksoftirqd/1/12

INFO: Allocated in alloc_perf_context+0x4c/0xf0 age=272 cpu=1 pid=1904
[<      none      >] ___slab_alloc+0x564/0x5b0 mm/slub.c:2470
[<      none      >] __slab_alloc+0x66/0xc0 mm/slub.c:2499
[<     inline     >] slab_alloc_node mm/slub.c:2562
[<     inline     >] slab_alloc mm/slub.c:2604
[<      none      >] kmem_cache_alloc_trace+0x25c/0x300 mm/slub.c:2621
[<     inline     >] kmalloc include/linux/slab.h:463
[<     inline     >] kzalloc include/linux/slab.h:607
[<      none      >] alloc_perf_context+0x4c/0xf0 kernel/events/core.c:3337
[<      none      >] find_get_context.isra.76+0x12a/0x5b0
kernel/events/core.c:3444
[<      none      >] SYSC_perf_event_open+0xed1/0x1ef0 kernel/events/core.c:8312
[<      none      >] SyS_perf_event_open+0x39/0x50 kernel/events/core.c:8173
[<      none      >] tracesys_phase2+0x88/0x8d arch/x86/entry/entry_64.S:269

INFO: Freed in free_ctx+0x47/0x60 age=0 cpu=3 pid=0
[<      none      >] __slab_free+0x1fc/0x320 mm/slub.c:2680
[<     inline     >] slab_free mm/slub.c:2835
[<      none      >] kfree+0x2ac/0x2c0 mm/slub.c:3664
[<      none      >] free_ctx+0x47/0x60 kernel/events/core.c:1017
[<     inline     >] __rcu_reclaim kernel/rcu/rcu.h:118
[<     inline     >] rcu_do_batch kernel/rcu/tree.c:2704
[<     inline     >] invoke_rcu_callbacks kernel/rcu/tree.c:2970
[<     inline     >] __rcu_process_callbacks kernel/rcu/tree.c:2937
[<      none      >] rcu_process_callbacks+0xd5b/0x1440 kernel/rcu/tree.c:2954
[<      none      >] __do_softirq+0x26a/0x920 kernel/softirq.c:273
[<     inline     >] invoke_softirq kernel/softirq.c:350
[<      none      >] irq_exit+0x18f/0x1d0 kernel/softirq.c:391
[<     inline     >] exiting_irq ./arch/x86/include/asm/apic.h:659
[<      none      >] smp_apic_timer_interrupt+0x7e/0xa0
arch/x86/kernel/apic/apic.c:932
[<      none      >] apic_timer_interrupt+0x8c/0xa0
arch/x86/entry/entry_64.S:520
[<     inline     >] arch_safe_halt ./arch/x86/include/asm/paravirt.h:117
[<      none      >] default_idle+0x52/0x2e0 arch/x86/kernel/process.c:304
[<      none      >] arch_cpu_idle+0xa/0x10 arch/x86/kernel/process.c:295
[<      none      >] default_idle_call+0x48/0xa0 kernel/sched/idle.c:92
[<     inline     >] cpuidle_idle_call kernel/sched/idle.c:150
[<     inline     >] cpu_idle_loop kernel/sched/idle.c:246
[<      none      >] cpu_startup_entry+0x57e/0x720 kernel/sched/idle.c:294
[<      none      >] start_secondary+0x30a/0x450 arch/x86/kernel/smpboot.c:251

INFO: Slab 0xffffea0000db3f00 objects=16 used=1 fp=0xffff880036cff1f8
flags=0x1fffc0000004080
INFO: Object 0xffff880036cff1f8 @offset=12792 fp=0xffff880036cfee20

CPU: 1 PID: 12 Comm: ksoftirqd/1 Tainted: G    B   W       4.5.0-rc4+ #327
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffffffff87b05080 ffff88003ed07bc8 ffffffff82be46cf ffffffff00db3f00
 fffffbfff0f60a10 ffff88003e804f00 ffff880036cff1f8 ffff880036cfc000
 ffffea0000db3f00 ffff88003e25c740 ffff88003ed07bf8 ffffffff8175dea4

Call Trace:
 [<ffffffff817678ce>] __asan_report_load8_noabort+0x3e/0x40
mm/kasan/report.c:295
 [<ffffffff81629e17>] perf_event_task_tick+0xb47/0xbd0 kernel/events/core.c:3085
 [<ffffffff813e7ac2>] scheduler_tick+0x102/0x290 kernel/sched/core.c:2976
 [<ffffffff814b5f0c>] update_process_times+0x5c/0x70 kernel/time/timer.c:1425
 [<ffffffff814e1b4f>] tick_sched_handle.isra.21+0xaf/0xe0
kernel/time/tick-sched.c:152
 [<ffffffff814e2272>] tick_sched_timer+0x72/0x100 kernel/time/tick-sched.c:1088
 [<     inline     >] __run_hrtimer kernel/time/hrtimer.c:1248
 [<ffffffff814b7df3>] __hrtimer_run_queues+0x363/0xc20
kernel/time/hrtimer.c:1312
 [<ffffffff814b9e12>] hrtimer_interrupt+0x182/0x430 kernel/time/hrtimer.c:1346
 [<ffffffff8124e1d2>] local_apic_timer_interrupt+0x72/0xe0
arch/x86/kernel/apic/apic.c:907
 [<ffffffff812516f9>] smp_apic_timer_interrupt+0x79/0xa0
arch/x86/kernel/apic/apic.c:931
 [<ffffffff8666342c>] apic_timer_interrupt+0x8c/0xa0
arch/x86/entry/entry_64.S:520
 <EOI>  [<ffffffff8666213e>] _raw_spin_unlock_irqrestore+0x5e/0xc0
kernel/locking/spinlock.c:191
 [<     inline     >] spin_unlock_irqrestore include/linux/spinlock.h:362
 [<ffffffff817632ee>] __slab_free+0x1ae/0x320 mm/slub.c:2687

Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Sasha Levin]

On 01/11/2016 11:25 AM, Peter Zijlstra wrote:
> There is one common bug left in all the event_function_call() users,
> between loading ctx->task and getting to the remote_function(),
> ctx->task can already have been changed.
> 
> Therefore we need to double check and retry if ctx->task != current.
> 
> Insert another trampoline specific to event_function_call() that
> checks for this and further validates state. This also allows getting
> rid of the active/inactive functions.
> 
> 
> Note: Stephane, can you please look at __perf_event_enable()?
> 
> Cc: Stephane Eranian <eranian@google.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Hey Peter,

I seem to be hitting a warning added by this patch:

[  258.890172] WARNING: CPU: 5 PID: 14801 at kernel/events/core.c:226 event_function+0x3a7/0x550()
[  258.891356] Modules linked in:[  258.892440] CPU: 5 PID: 14801 Comm: syz-executor Not tainted 4.5.0-rc4-next-20160217-sasha-00024-g888c008-dirty #2976
[  258.893818]  1ffff1006c57bf07 ffff880362bdf8c0 ffffffffa43d511d ffffffff00000005
[  258.895071]  fffffbfff60265b8 0000000041b58ab3 ffffffffafb1d490 ffffffffa43d4f85
[  258.896127]  ffffffffa25984c0 0000000000000038 0000000000000282 ffff880362bdf898
[  258.897182] Call Trace:
[  258.897559]  [<ffffffffa43d511d>] dump_stack+0x198/0x21b
[  258.898258]  [<ffffffffa43d4f85>] ? arch_local_irq_restore+0x5f/0x5f
[  258.899099]  [<ffffffffa25984c0>] ? is_module_text_address+0x20/0x20
[  258.899934]  [<ffffffffa23a6851>] warn_slowpath_common+0xe1/0x160
[  258.900723]  [<ffffffffa268cf07>] ? event_function+0x3a7/0x550
[  258.901473]  [<ffffffffa23a6a99>] warn_slowpath_null+0x29/0x30
[  258.902186]  [<ffffffffa268cf07>] event_function+0x3a7/0x550
[  258.902913]  [<ffffffffa268cb60>] ? perf_event_read_event+0x3c0/0x3c0
[  258.906328]  [<ffffffffa267d7a0>] ? free_ctx+0x70/0x70
[  258.907633]  [<ffffffffa267d8ea>] remote_function+0x14a/0x200
[  258.908776]  [<ffffffffa2582cb8>] generic_exec_single+0x2e8/0x5a0
[  258.913186]  [<ffffffffa258314d>] smp_call_function_single+0x1dd/0x350
[  258.917536]  [<ffffffffa267bc83>] task_function_call+0x123/0x160
[  258.938733]  [<ffffffffa267dbdb>] event_function_call+0x23b/0x440
[  258.949427]  [<ffffffffa267df7f>] _perf_event_enable+0xbf/0x100
[  258.952368]  [<ffffffffa267e04c>] _perf_event_refresh+0x8c/0xd0
[  258.953776]  [<ffffffffa26aa49f>] perf_ioctl+0x2af/0x870
[  258.963056]  [<ffffffffa28fcc60>] do_vfs_ioctl+0x1b0/0x1250
[  258.975817]  [<ffffffffa28fdd8f>] SyS_ioctl+0x8f/0xc0
[  258.976277]  [<ffffffffad5c96c0>] entry_SYSCALL_64_fastpath+0x23/0xc1


Thanks,
Sasha

Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Peter Zijlstra]

On Wed, Feb 17, 2016 at 05:38:43PM -0500, Sasha Levin wrote:
> Hey Peter,
> 
> I seem to be hitting a warning added by this patch:
> 
> [  258.890172] WARNING: CPU: 5 PID: 14801 at kernel/events/core.c:226 event_function+0x3a7/0x550()

Yes, I've been chasing this one for the past few days.

I have a bunch of traces that make absolutely no sense what so ever, and
a patch that appears to make it go away, but I'm not confident.

I'll cleanup and share what I have.

Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users

[Peter Zijlstra]

On Thu, Feb 18, 2016 at 08:46:20AM +0100, Peter Zijlstra wrote:
> On Wed, Feb 17, 2016 at 05:38:43PM -0500, Sasha Levin wrote:
> > Hey Peter,
> > 
> > I seem to be hitting a warning added by this patch:
> > 
> > [  258.890172] WARNING: CPU: 5 PID: 14801 at kernel/events/core.c:226 event_function+0x3a7/0x550()
> 
> Yes, I've been chasing this one for the past few days.
> 
> I have a bunch of traces that make absolutely no sense what so ever, and
> a patch that appears to make it go away, but I'm not confident.
> 
> I'll cleanup and share what I have.

*sigh*, all cleaned up and debug cruft removed it hits again :/


Back to poking at it I suppose...

Re: [RFC][PATCH 07/12] perf: Simplify/fix perf_event_enable() event scheduling

[James Morse]

Hi Peter!

On 11/01/16 16:25, Peter Zijlstra wrote:
> Like enable_on_exec, perf_event_enable() event scheduling has problems
> respecting the context hierarchy when trying to schedule events (for
> example, it will try and add a pinned event without first removing
> existing flexible events).
> 
> So simplify it by using the new ctx_resched() call which will DTRT.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

I'm seeing 'failures' for the ltp test perf_event_open02[0] with v4.5-rcs.
git bisect points to this patch[1].

I suspect the test is over-reliant on the behaviour of the removed code.

This has been seen on arm64 and x86_64[2].


Thanks,

James


[0]
https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/perf_event_open/perf_event_open02.c

[1] Bisect log
----------------------------------------
git bisect start
# good: [afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc] Linux 4.4
git bisect good afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc
# bad: [18558cae0272f8fd9647e69d3fec1565a7949865] Linux 4.5-rc4
git bisect bad 18558cae0272f8fd9647e69d3fec1565a7949865
# good: [875fc4f5ddf35605581f9a5900c14afef48611f2] Merge branch 'akpm' (patches
from Andrew)
git bisect good 875fc4f5ddf35605581f9a5900c14afef48611f2
# good: [9f273c24ec5f4a6f785bb83e931b3808a07b459e] MAINTAINERS: add/fix git URLs
for various subsystems
git bisect good 9f273c24ec5f4a6f785bb83e931b3808a07b459e
# good: [2c9b3ebd5913c2d1371b749a8057ac32972b410d] Merge tag 'mmc-v4.5-rc1' of
git://git.linaro.org/people/ulf.hansson/mmc
git bisect good 2c9b3ebd5913c2d1371b749a8057ac32972b410d
# bad: [54e3f3e30245abb0d47e3bc53a1b3c75434616f1] Merge tag 'tty-4.5-rc2' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty
git bisect bad 54e3f3e30245abb0d47e3bc53a1b3c75434616f1
# good: [e1c10879ed59436cde537b723545430b04d4dec0] Merge tag
'platform-drivers-x86-v4.5-2' of
git://git.infradead.org/users/dvhart/linux-platform-drivers-x86
git bisect good e1c10879ed59436cde537b723545430b04d4dec0
# good: [f51d4d7826d64dee2c421267c7bd44c4d52805c5] Merge tag
'hwmon-for-linus-v4.5-rc2' of
git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging
git bisect good f51d4d7826d64dee2c421267c7bd44c4d52805c5
# good: [bbfb239a106d41d793f58befdaf5c806e34ea97e] Merge branch
'locking-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good bbfb239a106d41d793f58befdaf5c806e34ea97e
# bad: [7ab85d4a85160ea2ffc96b1255443cbc83be180f] Merge branch
'sched-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 7ab85d4a85160ea2ffc96b1255443cbc83be180f
# bad: [45c815f06b80031659c63d7b93e580015d6024dd] perf: Synchronously free aux
pages in case of allocation failure
git bisect bad 45c815f06b80031659c63d7b93e580015d6024dd
# good: [70a0165752944e0be0b1de4a9020473079962c18] perf: Fix cgroup scheduling
in perf_enable_on_exec()
git bisect good 70a0165752944e0be0b1de4a9020473079962c18
# bad: [63e30d3e52d4d85854ce6c761ffc6ab55209a630] perf: Make ctx->is_active and
cpuctx->task_ctx consistent
git bisect bad 63e30d3e52d4d85854ce6c761ffc6ab55209a630
# good: [8833d0e286c12fd4456089a7a553faf4921e4b08] perf: Use task_ctx_sched_out()
git bisect good 8833d0e286c12fd4456089a7a553faf4921e4b08
# bad: [25432ae96a9889774a05bf5f0f6fd8dbcdec5e72] perf: Optimize
perf_sched_events() usage
git bisect bad 25432ae96a9889774a05bf5f0f6fd8dbcdec5e72
# bad: [aee7dbc45f8aa976913de9b352fa6da816f1f3cd] perf: Simplify/fix
perf_event_enable() event scheduling
git bisect bad aee7dbc45f8aa976913de9b352fa6da816f1f3cd
# first bad commit: [aee7dbc45f8aa976913de9b352fa6da816f1f3cd] perf:
Simplify/fix perf_event_enable() event scheduling
----------------------------------------


[2]
----------------------------------------
x86_64, v4.2
nanook@trouble-every-day:~$ ./perf_event_open02  -v
at iteration:0 value:3001278671 time_enabled:378865967 time_running:378865967
at iteration:1 value:3001294516 time_enabled:379206798 time_running:379206798
at iteration:2 value:3001304264 time_enabled:379741658 time_running:379741658
at iteration:3 value:2247456463 time_enabled:381782350 time_running:285882994
perf_event_open02    0  TINFO  :  overall task clock: 382444297
perf_event_open02    0  TINFO  :  hw sum: 9004576323, task clock sum: 1146042829
hw counters: 1276985606 1284698955 1291658694 1294253771 1293189575 1285360592
1278429130
task clock counters: 162510606 163544930 164439727 164730880 164600624 163538072
162677990
perf_event_open02    0  TINFO  :  ratio: 3.00
perf_event_open02    1  TPASS  :  test passed
----------------------------------------
x86_64, v4.5-rc5:
nanook@trouble-every-day:~$ ./perf_event_open02  -v
at iteration:0 value:3001332678 time_enabled:379409484 time_running:379408991
perf_event_open02    0  TINFO  :  overall task clock: 381025905
perf_event_open02    0  TINFO  :  hw sum: 9004621962, task clock sum: 1142038427
hw counters: 2249100352 2254366204 2252915996 2248239410
task clock counters: 285302769 285853299 285787328 285095031
perf_event_open02    0  TINFO  :  ratio: 3.00
perf_event_open02    1  TFAIL  :  perf_event_open02.c:333: test failed (ratio
was greater than )
----------------------------------------

Re: [RFC][PATCH 07/12] perf: Simplify/fix perf_event_enable() event scheduling

[Peter Zijlstra]

On Tue, Mar 08, 2016 at 10:04:17AM +0000, James Morse wrote:

> [0]
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/perf_event_open/perf_event_open02.c

I so hate LTP, you can't compile the test cases in isolate and running
'make' doesn't work.

> [2]
> ----------------------------------------
> x86_64, v4.2
> nanook@trouble-every-day:~$ ./perf_event_open02  -v
> at iteration:0 value:3001278671 time_enabled:378865967 time_running:378865967
> at iteration:1 value:3001294516 time_enabled:379206798 time_running:379206798
> at iteration:2 value:3001304264 time_enabled:379741658 time_running:379741658
> at iteration:3 value:2247456463 time_enabled:381782350 time_running:285882994
> perf_event_open02    0  TINFO  :  overall task clock: 382444297
> perf_event_open02    0  TINFO  :  hw sum: 9004576323, task clock sum: 1146042829
> hw counters: 1276985606 1284698955 1291658694 1294253771 1293189575 1285360592
> 1278429130
> task clock counters: 162510606 163544930 164439727 164730880 164600624 163538072
> 162677990
> perf_event_open02    0  TINFO  :  ratio: 3.00
> perf_event_open02    1  TPASS  :  test passed
> ----------------------------------------
> x86_64, v4.5-rc5:
> nanook@trouble-every-day:~$ ./perf_event_open02  -v
> at iteration:0 value:3001332678 time_enabled:379409484 time_running:379408991
> perf_event_open02    0  TINFO  :  overall task clock: 381025905
> perf_event_open02    0  TINFO  :  hw sum: 9004621962, task clock sum: 1142038427
> hw counters: 2249100352 2254366204 2252915996 2248239410
> task clock counters: 285302769 285853299 285787328 285095031
> perf_event_open02    0  TINFO  :  ratio: 3.00
> perf_event_open02    1  TFAIL  :  perf_event_open02.c:333: test failed (ratio
> was greater than )
> ----------------------------------------

Its unfortunate the test doesn't actually tells what it was expecting.
It fails to print 'nhw'.

In any case, please try -rc6, which includes:

  a096309bc467 perf: Fix scaling vs. perf_install_in_context()
  bd2afa49d194 perf: Fix scaling vs. perf_event_enable()
  7fce250915ef perf: Fix scaling vs. perf_event_enable_on_exec()
  3cbaa5906967 perf: Fix ctx time tracking by introducing EVENT_TIME

Which might fix this. I gave up after a few minutes trying to make LTP
work.

Re: [RFC][PATCH 07/12] perf: Simplify/fix perf_event_enable() event scheduling

[James Morse]

On 08/03/16 10:26, Peter Zijlstra wrote:
> In any case, please try -rc6, which includes:

Bother, I should have thought to test with the later rcs, I foolishly stopped
with rc5.

>   a096309bc467 perf: Fix scaling vs. perf_install_in_context()
>   bd2afa49d194 perf: Fix scaling vs. perf_event_enable()
>   7fce250915ef perf: Fix scaling vs. perf_event_enable_on_exec()
>   3cbaa5906967 perf: Fix ctx time tracking by introducing EVENT_TIME
> 
> Which might fix this.

Yes, one of these has fixed the issue. For arm64 it no longer shows up with
v4.5-rc6.

Sorry for the noise!


Thanks,

James

Re: [RFC][PATCH 07/12] perf: Simplify/fix perf_event_enable() event scheduling

[Peter Zijlstra]

On Tue, Mar 08, 2016 at 10:42:12AM +0000, James Morse wrote:
> On 08/03/16 10:26, Peter Zijlstra wrote:
> > In any case, please try -rc6, which includes:
> 
> Bother, I should have thought to test with the later rcs, I foolishly stopped
> with rc5.
> 
> >   a096309bc467 perf: Fix scaling vs. perf_install_in_context()
> >   bd2afa49d194 perf: Fix scaling vs. perf_event_enable()
> >   7fce250915ef perf: Fix scaling vs. perf_event_enable_on_exec()
> >   3cbaa5906967 perf: Fix ctx time tracking by introducing EVENT_TIME
> > 
> > Which might fix this.
> 
> Yes, one of these has fixed the issue. For arm64 it no longer shows up with
> v4.5-rc6.

Awesome, one less worry ;-)

> Sorry for the noise!

No problem.