Commit 3e349507 authored by Peter Zijlstra's avatar Peter Zijlstra Committed by Ingo Molnar

perf: Fix perf_enable_on_exec() event scheduling

There are two problems with the current perf_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: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 5947f657
...@@ -2036,7 +2036,8 @@ static void add_event_to_ctx(struct perf_event *event, ...@@ -2036,7 +2036,8 @@ static void add_event_to_ctx(struct perf_event *event,
event->tstamp_stopped = tstamp; 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 static void
ctx_sched_in(struct perf_event_context *ctx, ctx_sched_in(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx, struct perf_cpu_context *cpuctx,
...@@ -2067,6 +2068,17 @@ static void ___perf_install_in_context(void *info) ...@@ -2067,6 +2068,17 @@ static void ___perf_install_in_context(void *info)
add_event_to_ctx(event, ctx); 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 * Cross CPU call to install and enable a performance event
* *
...@@ -2087,7 +2099,7 @@ static int __perf_install_in_context(void *info) ...@@ -2087,7 +2099,7 @@ static int __perf_install_in_context(void *info)
* If there was an active task_ctx schedule it out. * If there was an active task_ctx schedule it out.
*/ */
if (task_ctx) 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 * If the context we're installing events in is not the
...@@ -2629,10 +2641,9 @@ void __perf_event_task_sched_out(struct task_struct *task, ...@@ -2629,10 +2641,9 @@ void __perf_event_task_sched_out(struct task_struct *task,
perf_cgroup_sched_out(task, next); 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) if (!cpuctx->task_ctx)
return; return;
...@@ -3096,34 +3107,30 @@ static int event_enable_on_exec(struct perf_event *event, ...@@ -3096,34 +3107,30 @@ static int event_enable_on_exec(struct perf_event *event,
static void perf_event_enable_on_exec(int ctxn) static void perf_event_enable_on_exec(int ctxn)
{ {
struct perf_event_context *ctx, *clone_ctx = NULL; struct perf_event_context *ctx, *clone_ctx = NULL;
struct perf_cpu_context *cpuctx;
struct perf_event *event; struct perf_event *event;
unsigned long flags; unsigned long flags;
int enabled = 0; int enabled = 0;
int ret;
local_irq_save(flags); local_irq_save(flags);
ctx = current->perf_event_ctxp[ctxn]; ctx = current->perf_event_ctxp[ctxn];
if (!ctx || !ctx->nr_events) if (!ctx || !ctx->nr_events)
goto out; goto out;
raw_spin_lock(&ctx->lock); cpuctx = __get_cpu_context(ctx);
task_ctx_sched_out(ctx); perf_ctx_lock(cpuctx, ctx);
list_for_each_entry(event, &ctx->event_list, event_entry)
list_for_each_entry(event, &ctx->event_list, event_entry) { enabled |= event_enable_on_exec(event, ctx);
ret = event_enable_on_exec(event, ctx);
if (ret)
enabled = 1;
}
/* /*
* 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); 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: out:
local_irq_restore(flags); local_irq_restore(flags);
...@@ -8737,7 +8744,7 @@ static void perf_event_exit_task_context(struct task_struct *child, int ctxn) ...@@ -8737,7 +8744,7 @@ static void perf_event_exit_task_context(struct task_struct *child, int ctxn)
* incremented the context's refcount before we do put_ctx below. * incremented the context's refcount before we do put_ctx below.
*/ */
raw_spin_lock(&child_ctx->lock); 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; child->perf_event_ctxp[ctxn] = NULL;
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment