various perf fixes

RFC PATCH In review Peter Zijlstra 9 participants 67 messages Jan 11, 2016 – Mar 8, 2016 perf events

lore

Share & tools

lore

Peter Zijlstra

author maintainer

2016-01-11 16:35 UTC #0 lore raw

cover various perf fixes

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.

Peter Zijlstra

author maintainer

2016-01-11 16:35 UTC #1 lore raw

RFC PATCH 1/12 perf: Add lockdep assertions

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

---
 kernel/events/core.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

kernel/events/core.c +8 -2

@@ -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))

Peter Zijlstra

author maintainer

2016-01-11 16:36 UTC #2 lore raw

RFC PATCH 2/12 perf: Fix cgroup event scheduling

In reply to Peter Zijlstra

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 <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

---
 kernel/events/core.c |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

kernel/events/core.c +10 -7

@@ -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);

Stephane Eranian

2016-01-11 19:43 UTC #3 lore raw

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

Peter,

Quoted 5 lines · Peter Zijlstra

There appears to be a problemin __perf_event_task_sched_in() wrt cgroup event scheduling.
The normal event scheduling order is:

Quoted 8 lines

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?

Quoted 44 lines

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

Cc Stephane Eranian <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

---
 kernel/events/core.c |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

kernel/events/core.c +10 -7

@@ -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);

Peter Zijlstra

author maintainer

2016-01-11 22:03 UTC #4 lore raw

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

Quoted 13 lines · Stephane Eranian

Peter,
Quoted 10 lines · Peter Zijlstra
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

Quoted 6 lines

Quoted 3 lines
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.

Quoted 1 line

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)

Peter Zijlstra

author maintainer

2016-01-11 16:34 UTC #5 lore raw

RFC PATCH 3/12 perf: Fix cgroup scheduling in enable_on_exec

In reply to Peter Zijlstra

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 <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

---
 kernel/events/core.c |   31 +++++++------------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

kernel/events/core.c +7 ~~-24~~

@@ -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);

Peter Zijlstra

author maintainer

2016-01-11 16:34 UTC #6 lore raw

RFC PATCH 4/12 perf: Remove stale comment

In reply to Peter Zijlstra

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

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

---
 kernel/events/core.c |    7 -------
 1 file changed, 7 deletions(-)

kernel/events/core.c +0 -7

@@ -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,

Peter Zijlstra

author maintainer

2016-01-11 16:38 UTC #7 lore raw

RFC PATCH 5/12 perf: Fix enable_on_exec event scheduling

In reply to Peter Zijlstra

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) <[email protected]>

---
 kernel/events/core.c |   47 +++++++++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 20 deletions(-)

kernel/events/core.c +27 ~~-20~~

@@ -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;
 
 	/*

Peter Zijlstra

author maintainer

2016-01-11 16:34 UTC #8 lore raw

RFC PATCH 6/12 perf: Use task_ctx_sched_out()

In reply to Peter Zijlstra

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) <[email protected]>

---
 kernel/events/core.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

kernel/events/core.c +1 -2

@@ -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);
 	}
 }

Peter Zijlstra

author maintainer

2016-01-11 16:34 UTC #9 lore raw

RFC PATCH 7/12 perf: Simplify/fix perf_event_enable() event scheduling

In reply to Peter Zijlstra

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) <[email protected]>

---
 kernel/events/core.c |   31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

kernel/events/core.c +5 ~~-26~~

@@ -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;
 }

James Morse

2016-03-08 10:05 UTC #10 lore raw

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

Hi Peter!

Quoted 8 lines · Peter Zijlstra

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) <[email protected]>

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 )
----------------------------------------

Peter Zijlstra

author maintainer

2016-03-08 10:26 UTC #11 lore raw

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

Quoted 2 lines · James Morse

[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.

Quoted 20 lines

[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

Quoted 8 lines

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.

James Morse

2016-03-08 10:43 UTC #12 lore raw

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

Quoted 1 line · Peter Zijlstra

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

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

Quoted 6 lines

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

Peter Zijlstra

author maintainer

2016-03-08 11:29 UTC #13 lore raw

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

Quoted 11 lines · James Morse

Quoted 1 line · Peter Zijlstra
In any case, please try -rc6, which includes:
Bother, I should have thought to test with the later rcs, I foolishly stopped with rc5.
Quoted 5 lines
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

Quoted 4 lines

Quoted 1 line
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 ;-)

Quoted 1 line

Sorry for the noise!

No problem.

Peter Zijlstra

author maintainer

2016-01-11 16:35 UTC #14 lore raw

RFC PATCH 8/12 perf: Optimize perf_sched_events usage

In reply to Peter Zijlstra

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) <[email protected]>

---
 kernel/events/core.c |   22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

kernel/events/core.c +16 -6

@@ -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);

Peter Zijlstra

author maintainer

2016-01-11 16:38 UTC #15 lore raw

RFC PATCH 9/12 perf: Make ctx->is_active and cpuctx->task_ctx consistent

In reply to Peter Zijlstra

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) <[email protected]>

---
 kernel/events/core.c |   21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

kernel/events/core.c +14 -7

@@ -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);
 }

Peter Zijlstra

author maintainer

2016-01-11 16:36 UTC #16 lore raw

RFC PATCH 10/12 perf: Fix task context scheduling

In reply to Peter Zijlstra

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 <[email protected]>

Cc Dmitry Vyukov <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

---
 kernel/events/core.c |  155 +++++++++++++++++++++++++++++----------------------
 1 file changed, 91 insertions(+), 64 deletions(-)

kernel/events/core.c +91 ~~-64~~

@@ -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);

Peter Zijlstra

author maintainer

2016-01-11 16:34 UTC #17 lore raw

RFC PATCH 11/12 perf: Specialize perf_event_exit_task()

In reply to Peter Zijlstra

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) <[email protected]>

---
 kernel/events/core.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

kernel/events/core.c +11 -7

@@ -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);

Peter Zijlstra

author maintainer

2016-01-11 16:37 UTC #18 lore raw

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

In reply to Peter Zijlstra

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 <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

---
 include/linux/perf_event.h    |    2 
 kernel/events/core.c          |  362 ++++++++++++++++++------------------------
 kernel/events/hw_breakpoint.c |    2 
 3 files changed, 164 insertions(+), 202 deletions(-)

include/linux/perf_event.h +1 -1

@@ -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 *

kernel/events/core.c +162 ~~-200~~

509 lines not on this page · raw message

kernel/events/hw_breakpoint.c +1 -1

@@ -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);

Peter Zijlstra

author maintainer

2016-01-13 10:51 UTC #19 lore raw

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

kernel/events/core.c static void __perf_event_mark_enabled(st · line 2207

Quoted 13 lines · Peter Zijlstra

kernel/events/core.c +4 -1

@@ -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 ^^

Quoted 20 lines

kernel/events/core.c +9 -0

 	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;
 	}
 
 	/*

kernel/events/core.c static int __perf_event_enable(void *inf · line 2254

Quoted 7 lines

kernel/events/core.c +2 -0

@@ -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);
 }

kernel/events/core.c +6 -1

@@ -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);
 }
 
 /*

Alexander Shishkin

reviewer

2016-01-13 13:49 UTC #20 lore raw

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

kernel/events/core.c static void __perf_event_enable(struct p · line 2276

Quoted 10 lines · Peter Zijlstra

kernel/events/core.c +5 -1

@@ -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

Peter Zijlstra

author maintainer

2016-01-13 17:30 UTC #21 lore raw

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

Quoted 6 lines · Alexander Shishkin

Quoted 4 lines · Peter Zijlstra

kernel/events/core.c +0 -0

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

Quoted 8 lines

kernel/events/core.c static void __perf_event_enable(struct p · line 2276
Quoted 6 lines
kernel/events/core.c +5 -1
-	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 :-)

Alexander Shishkin

reviewer

2016-01-13 15:00 UTC #22 lore raw

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

In reply to Peter Zijlstra

I think I caught one, below.

Quoted 12 lines · Peter Zijlstra

kernel/events/core.c +12 -0

+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

kernel/events/core.c static int cpu_function_call(int cpu, re · line 213

Quoted 10 lines

kernel/events/core.c +10 -0

+	 * 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.

Quoted 15 lines

kernel/events/core.c +15 -0

+	} 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

kernel/events/core.c static int cpu_function_call(int cpu, re · line 238

Quoted 10 lines

kernel/events/core.c +10 -0

+		 * 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

Alexander Shishkin

reviewer

2016-01-13 15:40 UTC #23 lore raw

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

Quoted 2 lines · Alexander Shishkin

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:

Quoted 3 lines

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

Peter Zijlstra

author maintainer

2016-01-13 18:10 UTC #24 lore raw

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

In reply to Alexander Shishkin

Quoted 21 lines · Alexander Shishkin

I think I caught one, below.

Quoted 17 lines · Peter Zijlstra

kernel/events/core.c +17 -0

+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.
+	 */

Quoted 8 lines

kernel/events/core.c static int cpu_function_call(int cpu, re · line 213
Quoted 5 lines
kernel/events/core.c +5 -0
+	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!

Peter Zijlstra

author maintainer

2016-01-13 20:44 UTC #25 lore raw

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

Quoted 31 lines · Peter Zijlstra

Quoted 21 lines · Alexander Shishkin

I think I caught one, below.

Quoted 17 lines · Peter Zijlstra

kernel/events/core.c +17 -0

+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.
+	 */

Quoted 8 lines

kernel/events/core.c static int cpu_function_call(int cpu, re · line 213
Quoted 5 lines
kernel/events/core.c +5 -0
+	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.

Quoted 4 lines

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.

Peter Zijlstra

author maintainer

2016-01-14 10:44 UTC #26 lore raw

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

In reply to Alexander Shishkin

Quoted 21 lines · Alexander Shishkin

I think I caught one, below.

Quoted 17 lines · Peter Zijlstra

kernel/events/core.c +17 -0

+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.
+	 */

Quoted 8 lines

kernel/events/core.c static int cpu_function_call(int cpu, re · line 213
Quoted 5 lines
kernel/events/core.c +5 -0
+	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.

Peter Zijlstra

author maintainer

2016-01-14 16:31 UTC #27 lore raw

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

Quoted 13 lines · Peter Zijlstra

Quoted 4 lines · Alexander Shishkin
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).

Quoted 4 lines

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 <[email protected]>
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) <[email protected]>

---
 kernel/events/core.c |  151 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 85 insertions(+), 66 deletions(-)

kernel/events/core.c +85 ~~-66~~

@@ -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);

Sasha Levin

2016-02-17 22:40 UTC #28 lore raw

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

In reply to Peter Zijlstra

Quoted 11 lines · Peter Zijlstra

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.

Quoted 4 lines

Note: Stephane, can you please look at __perf_event_enable()?
Cc Stephane Eranian <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>

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

Peter Zijlstra

author maintainer

2016-02-18 07:46 UTC #29 lore raw

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

Quoted 5 lines · Sasha Levin

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.

Peter Zijlstra

author maintainer

2016-02-18 12:37 UTC #30 lore raw

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

Quoted 5 lines · Peter Zijlstra

Quoted 4 lines · Sasha Levin
Hey Peter,
I seem to be hitting a warning added by this patch:

Quoted 8 lines

Quoted 1 line
[  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...

Dmitry Vyukov

2016-01-11 18:45 UTC #31 lore raw

In reply to Peter Zijlstra

Quoted 11 lines · 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)

Quoted 7 lines

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?

Andi Kleen

2016-01-11 19:56 UTC #32 lore raw

Quoted 5 lines

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

Signature · 1 line

[email protected] -- Speaking for myself only.

Peter Zijlstra

author maintainer

2016-01-11 22:00 UTC #33 lore raw

In reply to Dmitry Vyukov

Quoted 5 lines · Dmitry Vyukov

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

Ingo Molnar

2016-01-12 09:59 UTC #34 lore raw

Quoted 7 lines · Peter Zijlstra

Quoted 5 lines · Dmitry Vyukov
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?

Quoted 7 lines

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

Ingo Molnar

2016-01-12 10:11 UTC #35 lore raw

In reply to Peter Zijlstra

Quoted 10 lines · 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)

Quoted 7 lines

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

Dmitry Vyukov

2016-01-12 10:57 UTC #36 lore raw

Quoted 16 lines · Ingo Molnar

Quoted 13 lines · 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.

Quoted 7 lines

Quoted 4 lines
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.

Dmitry Vyukov

2016-01-12 11:00 UTC #37 lore raw

Quoted 22 lines · Dmitry Vyukov

Quoted 21 lines · Ingo Molnar
Quoted 17 lines · 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.

Quoted 6 lines

Quoted 2 lines
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 ]---

Dmitry Vyukov

2016-01-12 11:01 UTC #38 lore raw

Quoted 74 lines · Dmitry Vyukov

Quoted 22 lines · Dmitry Vyukov

Quoted 21 lines · Ingo Molnar
Quoted 17 lines · 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.

Quoted 6 lines

Quoted 2 lines
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

Quoted 8 lines

 [<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 ]---

Dmitry Vyukov

2016-01-12 11:26 UTC #39 lore raw

Quoted 110 lines · Dmitry Vyukov

Quoted 74 lines · Dmitry Vyukov

Quoted 22 lines · Dmitry Vyukov

Quoted 21 lines · Ingo Molnar
Quoted 17 lines · 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.

Quoted 6 lines

Quoted 2 lines
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

Quoted 8 lines

 [<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

Quoted 8 lines

 [<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 [email protected].

Dmitry Vyukov

2016-01-12 11:36 UTC #40 lore raw

Quoted 46 lines · Dmitry Vyukov

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

Quoted 6 lines

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 [email protected].

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...

Peter Zijlstra

author maintainer

2016-01-12 12:01 UTC #41 lore raw

In reply to Dmitry Vyukov

Quoted 6 lines · Dmitry Vyukov

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 [email protected].

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!

Alexander Shishkin

reviewer

2016-01-13 15:18 UTC #42 lore raw

In reply to Dmitry Vyukov

Quoted 2 lines · Dmitry Vyukov

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

Dmitry Vyukov

2016-01-13 15:23 UTC #43 lore raw

Quoted 7 lines · Alexander Shishkin

Quoted 2 lines · Dmitry Vyukov
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

Quoted 4 lines

Thanks,
--
Alex

Alexander Shishkin

reviewer

2016-01-13 15:35 UTC #44 lore raw

Quoted 4 lines · Dmitry Vyukov

Quoted 2 lines · Alexander Shishkin
Dmitry Vyukov <[email protected]> writes:

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
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

Peter Zijlstra

author maintainer

2016-01-14 09:35 UTC #45 lore raw

In reply to Dmitry Vyukov

Quoted 1 line · Dmitry Vyukov

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?

Dmitry Vyukov

2016-01-14 10:06 UTC #46 lore raw

Quoted 11 lines · Peter Zijlstra

Quoted 1 line · Dmitry Vyukov
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

Dmitry Vyukov

2016-02-15 15:04 UTC #47 lore raw

Quoted 4 lines · Dmitry Vyukov

Quoted 3 lines · Peter Zijlstra
Quoted 1 line · Dmitry Vyukov
What do you think if we work on making syzkaller work for you locally?

Quoted 8 lines

Quoted 8 lines
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.

Quoted 58 lines

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

Peter Zijlstra

author maintainer

2016-02-15 15:41 UTC #48 lore raw

Quoted 3 lines · Dmitry Vyukov

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 :-)

Quoted 2 lines

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.

Quoted 1 line

Or should I report new crashes?

Please.

Quoted 2 lines

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 :/

Dmitry Vyukov

2016-02-15 15:48 UTC #49 lore raw

Quoted 17 lines · Peter Zijlstra

Quoted 3 lines · Dmitry Vyukov
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 :-)
Quoted 2 lines
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.
Quoted 1 line
Or should I report new crashes?
Please.

Quoted 8 lines

Quoted 2 lines
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 [email protected] if you have any issues with setup.

Vince Weaver

2016-02-15 16:09 UTC #50 lore raw

In reply to Peter Zijlstra

Quoted 2 lines · Peter Zijlstra

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

Peter Zijlstra

author maintainer

2016-02-15 16:19 UTC #51 lore raw

In reply to Dmitry Vyukov

Quoted 14 lines · Dmitry Vyukov

Quoted 11 lines · Peter Zijlstra
Quoted 1 line · Dmitry Vyukov
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?

Quoted 6 lines

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!).

Quoted 2 lines

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

Fully expecting not to need nonsense like that.

Quoted 3 lines

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.

Dmitry Vyukov

2016-02-15 16:29 UTC #52 lore raw

Quoted 34 lines · Peter Zijlstra

Quoted 14 lines · Dmitry Vyukov
Quoted 11 lines · Peter Zijlstra
Quoted 1 line · Dmitry Vyukov
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?
Quoted 6 lines
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!).
Quoted 2 lines
$ export GOROOT=$HOME/go1.5
$ export GOPATH=$HOME/gopath
Fully expecting not to need nonsense like that.
Quoted 3 lines
Download syzkaller sources:
$ go get github.com/google/syzkaller

Quoted 8 lines

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.

Peter Zijlstra

author maintainer

2016-02-15 16:30 UTC #53 lore raw

In reply to Dmitry Vyukov

Quoted 10 lines · Dmitry Vyukov

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:

Quoted 3 lines

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

Quoted 4 lines

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

That seems to have done its thing

Quoted 3 lines

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"
	]
}

Quoted 3 lines

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

Quoted 4 lines

Then run:

$ bin/syz-manager -config perf.cfg

Quoted 8 lines

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

Dmitry Vyukov

2016-02-15 16:36 UTC #54 lore raw

Quoted 67 lines · Peter Zijlstra

Quoted 10 lines · Dmitry Vyukov
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:
Quoted 3 lines
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
Quoted 4 lines
Build necessary syzkaller binaries:
$ cd $GOPATH/src/github.com/google/syzkaller
$ make
That seems to have done its thing
Quoted 3 lines
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"
        ]
}
Quoted 3 lines
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
Quoted 11 lines
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:

Quoted 6 lines

Quoted 1 line

$ 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).

Dmitry Vyukov

2016-02-15 16:39 UTC #55 lore raw

Quoted 85 lines · Dmitry Vyukov

Quoted 67 lines · Peter Zijlstra
Quoted 10 lines · Dmitry Vyukov
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:
Quoted 3 lines
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
Quoted 4 lines
Build necessary syzkaller binaries:
$ cd $GOPATH/src/github.com/google/syzkaller
$ make
That seems to have done its thing
Quoted 3 lines
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"
        ]
}
Quoted 3 lines
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
Quoted 11 lines
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:
Quoted 6 lines
Quoted 1 line
$ 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,

Quoted 6 lines

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

Peter Zijlstra

author maintainer

2016-02-15 16:53 UTC #56 lore raw

Quoted 15 lines · Dmitry Vyukov

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.

Dmitry Vyukov

2016-02-15 17:04 UTC #57 lore raw

Quoted 20 lines · Peter Zijlstra

Quoted 15 lines · Dmitry Vyukov
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:

Quoted 6 lines

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?

Peter Zijlstra

author maintainer

2016-02-15 17:07 UTC #58 lore raw

Quoted 3 lines · Dmitry Vyukov

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"
	]
}

Dmitry Vyukov

2016-02-15 17:46 UTC #59 lore raw

Quoted 16 lines · Peter Zijlstra

Quoted 3 lines · Dmitry Vyukov
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,

Quoted 8 lines

        "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

Peter Zijlstra

author maintainer

2016-02-15 18:03 UTC #60 lore raw

Quoted 13 lines · Dmitry Vyukov

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!

Dmitry Vyukov

2016-02-15 18:34 UTC #61 lore raw

Quoted 15 lines · Peter Zijlstra

Quoted 13 lines · Dmitry Vyukov
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"
        ]

Quoted 1 line

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

Peter Zijlstra

author maintainer

2016-02-15 16:42 UTC #62 lore raw

In reply to Dmitry Vyukov

Quoted 15 lines · Dmitry Vyukov

Quoted 4 lines
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,

Quoted 6 lines

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?

Dmitry Vyukov

2016-02-15 16:55 UTC #63 lore raw

Quoted 18 lines · Peter Zijlstra

Quoted 17 lines · Dmitry Vyukov
Quoted 4 lines
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:

Quoted 7 lines

Quoted 4 lines
        "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.

Quoted 5 lines

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.

Peter Zijlstra

author maintainer

2016-02-15 17:00 UTC #64 lore raw

Quoted 4 lines · Dmitry Vyukov

Quoted 2 lines
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.

Quoted 9 lines

Quoted 5 lines
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?

Quoted 6 lines

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.

Peter Zijlstra

author maintainer

2016-01-12 13:14 UTC #65 lore raw

In reply to Ingo Molnar

Quoted 8 lines · Ingo Molnar

Quoted 5 lines · 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.

^^^^

Quoted 2 lines

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.

Ingo Molnar

2016-01-12 13:31 UTC #66 lore raw

Quoted 10 lines · Peter Zijlstra

Quoted 8 lines · Ingo Molnar
Quoted 5 lines · 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.

Quoted 8 lines

^^^^
Quoted 2 lines
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