[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170715110049.36jvxnidy2flh6ll@gmail.com>
Date: Sat, 15 Jul 2017 13:00:49 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Vince Weaver <vincent.weaver@...ne.edu>
Cc: linux-kernel@...r.kernel.org,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Peter Zijlstra <peterz@...radead.org>,
Stephane Eranian <eranian@...il.com>,
Jiri Olsa <jolsa@...hat.com>,
Arnaldo Carvalho de Melo <acme@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: perf: bisected sampling bug in Linux 4.11-rc1
* Vince Weaver <vincent.weaver@...ne.edu> wrote:
> 487f05e18aa4efacee6357480f293a5afe6593b5 is the first bad commit
>
> commit 487f05e18aa4efacee6357480f293a5afe6593b5
> Author: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> Date: Thu Jan 19 18:43:30 2017 +0200
>
> perf/core: Optimize event rescheduling on active contexts
BTW., just to prepare for the eventuality: below is a (completely untested...)
revert of this commit, against recent kernels, with conflicts fixed up.
Does this fix your testcase?
Thanks,
Ingo
>From fe0deecf2a8e9f5097013bcf89a9ef4b80715be1 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@...nel.org>
Date: Sat, 15 Jul 2017 12:57:51 +0200
Subject: [PATCH] Revert "perf/core: Optimize event rescheduling on active contexts"
This reverts commit 487f05e18aa4efacee6357480f293a5afe6593b5.
---
kernel/events/core.c | 80 ++++++++--------------------------------------------
1 file changed, 11 insertions(+), 69 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 9747e422ab20..778aa2548142 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -359,8 +359,6 @@ enum event_type_t {
EVENT_FLEXIBLE = 0x1,
EVENT_PINNED = 0x2,
EVENT_TIME = 0x4,
- /* see ctx_resched() for details */
- EVENT_CPU = 0x8,
EVENT_ALL = EVENT_FLEXIBLE | EVENT_PINNED,
};
@@ -1445,20 +1443,6 @@ static void update_group_times(struct perf_event *leader)
update_event_times(event);
}
-static enum event_type_t get_event_type(struct perf_event *event)
-{
- struct perf_event_context *ctx = event->ctx;
- enum event_type_t event_type;
-
- lockdep_assert_held(&ctx->lock);
-
- event_type = event->attr.pinned ? EVENT_PINNED : EVENT_FLEXIBLE;
- if (!ctx->task)
- event_type |= EVENT_CPU;
-
- return event_type;
-}
-
static struct list_head *
ctx_group_list(struct perf_event *event, struct perf_event_context *ctx)
{
@@ -2232,8 +2216,7 @@ ctx_sched_in(struct perf_event_context *ctx,
struct task_struct *task);
static void task_ctx_sched_out(struct perf_cpu_context *cpuctx,
- struct perf_event_context *ctx,
- enum event_type_t event_type)
+ struct perf_event_context *ctx)
{
if (!cpuctx->task_ctx)
return;
@@ -2241,7 +2224,7 @@ static void task_ctx_sched_out(struct perf_cpu_context *cpuctx,
if (WARN_ON_ONCE(ctx != cpuctx->task_ctx))
return;
- ctx_sched_out(ctx, cpuctx, event_type);
+ ctx_sched_out(ctx, cpuctx, EVENT_ALL);
}
static void perf_event_sched_in(struct perf_cpu_context *cpuctx,
@@ -2256,51 +2239,13 @@ static void perf_event_sched_in(struct perf_cpu_context *cpuctx,
ctx_sched_in(ctx, cpuctx, EVENT_FLEXIBLE, task);
}
-/*
- * We want to maintain the following priority of scheduling:
- * - CPU pinned (EVENT_CPU | EVENT_PINNED)
- * - task pinned (EVENT_PINNED)
- * - CPU flexible (EVENT_CPU | EVENT_FLEXIBLE)
- * - task flexible (EVENT_FLEXIBLE).
- *
- * In order to avoid unscheduling and scheduling back in everything every
- * time an event is added, only do it for the groups of equal priority and
- * below.
- *
- * This can be called after a batch operation on task events, in which case
- * event_type is a bit mask of the types of events involved. For CPU events,
- * event_type is only either EVENT_PINNED or EVENT_FLEXIBLE.
- */
static void ctx_resched(struct perf_cpu_context *cpuctx,
- struct perf_event_context *task_ctx,
- enum event_type_t event_type)
+ struct perf_event_context *task_ctx)
{
- enum event_type_t ctx_event_type = event_type & EVENT_ALL;
- bool cpu_event = !!(event_type & EVENT_CPU);
-
- /*
- * If pinned groups are involved, flexible groups also need to be
- * scheduled out.
- */
- if (event_type & EVENT_PINNED)
- event_type |= EVENT_FLEXIBLE;
-
perf_pmu_disable(cpuctx->ctx.pmu);
if (task_ctx)
- task_ctx_sched_out(cpuctx, task_ctx, event_type);
-
- /*
- * Decide which cpu ctx groups to schedule out based on the types
- * of events that caused rescheduling:
- * - EVENT_CPU: schedule out corresponding groups;
- * - EVENT_PINNED task events: schedule out EVENT_FLEXIBLE groups;
- * - otherwise, do nothing more.
- */
- if (cpu_event)
- cpu_ctx_sched_out(cpuctx, ctx_event_type);
- else if (ctx_event_type & EVENT_PINNED)
- cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE);
-
+ 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);
}
@@ -2347,7 +2292,7 @@ static int __perf_install_in_context(void *info)
if (reprogram) {
ctx_sched_out(ctx, cpuctx, EVENT_TIME);
add_event_to_ctx(event, ctx);
- ctx_resched(cpuctx, task_ctx, get_event_type(event));
+ ctx_resched(cpuctx, task_ctx);
} else {
add_event_to_ctx(event, ctx);
}
@@ -2514,7 +2459,7 @@ static void __perf_event_enable(struct perf_event *event,
if (ctx->task)
WARN_ON_ONCE(task_ctx != ctx);
- ctx_resched(cpuctx, task_ctx, get_event_type(event));
+ ctx_resched(cpuctx, task_ctx);
}
/*
@@ -2941,7 +2886,7 @@ static void perf_event_context_sched_out(struct task_struct *task, int ctxn,
if (do_switch) {
raw_spin_lock(&ctx->lock);
- task_ctx_sched_out(cpuctx, ctx, EVENT_ALL);
+ task_ctx_sched_out(cpuctx, ctx);
raw_spin_unlock(&ctx->lock);
}
}
@@ -3498,7 +3443,6 @@ static int event_enable_on_exec(struct perf_event *event,
static void perf_event_enable_on_exec(int ctxn)
{
struct perf_event_context *ctx, *clone_ctx = NULL;
- enum event_type_t event_type = 0;
struct perf_cpu_context *cpuctx;
struct perf_event *event;
unsigned long flags;
@@ -3512,17 +3456,15 @@ static void perf_event_enable_on_exec(int ctxn)
cpuctx = __get_cpu_context(ctx);
perf_ctx_lock(cpuctx, ctx);
ctx_sched_out(ctx, cpuctx, EVENT_TIME);
- 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);
- event_type |= get_event_type(event);
- }
/*
* Unclone and reschedule this context if we enabled any event.
*/
if (enabled) {
clone_ctx = unclone_ctx(ctx);
- ctx_resched(cpuctx, ctx, event_type);
+ ctx_resched(cpuctx, ctx);
} else {
ctx_sched_in(ctx, cpuctx, EVENT_TIME, current);
}
@@ -10466,7 +10408,7 @@ static void perf_event_exit_task_context(struct task_struct *child, int ctxn)
* in.
*/
raw_spin_lock_irq(&child_ctx->lock);
- task_ctx_sched_out(__get_cpu_context(child_ctx), child_ctx, EVENT_ALL);
+ task_ctx_sched_out(__get_cpu_context(child_ctx), child_ctx);
/*
* Now that the context is inactive, destroy the task <-> ctx relation
Powered by blists - more mailing lists