Messages in this thread Patch in this message |  | | | Subject | Re: [PATCH,RFC] perf: panic due to inclied cpu context task_ctx value | | From | Peter Zijlstra <> | | Date | Mon, 28 Mar 2011 16:57:55 +0200 |
| |
On Mon, 2011-03-28 at 15:30 +0200, Oleg Nesterov wrote: > On 03/26, Peter Zijlstra wrote: > > > > On Sat, 2011-03-26 at 18:35 +0100, Oleg Nesterov wrote: > > > On 03/26, Oleg Nesterov wrote: > > > > > > > > On 03/26, Peter Zijlstra wrote: > > > > > > > > > > diff --git a/kernel/perf_event.c b/kernel/perf_event.c > > > > > index c75925c..e9e4e35 100644 > > > > > --- a/kernel/perf_event.c > > > > > +++ b/kernel/perf_event.c > > > > > @@ -1073,6 +1073,8 @@ event_sched_out(struct perf_event *event, > > > > > if (!is_software_event(event)) > > > > > cpuctx->active_oncpu--; > > > > > ctx->nr_active--; > > > > > + if (!ctx->nr_active && cpuctx->task_ctx == ctx) > > > > > + cpuctx->task_ctx = NULL; > > > > > > > > If we clear cpuctx->task_ctx, we should also clear ctx->is_active. > > > > Right. > > Wait... Yes, we have to clear ctx->is_active, otherwise we break, say, > perf_install_in_context(). > > But if we clear ->is_active we break perf_event_enable(). Suppose we > are doing ioctl(PERF_EVENT_IOC_DISABLE) + ioctl(PERF_EVENT_IOC_ENABLE). > PERF_EVENT_IOC_DISABLE can sched_out the last event, but _IOC_ENABLE > treats ctx->is_active == F as "it is not running".
Right, same for the tick, if say we can only schedule 1 event at a time and we close the 1 event that is active, the tick will not rotate a new event in.
/me goes ponder things..
> Btw, why ctx_sched_out() checks nr_events under perf_pmu_disable() ?
hysterical-raisins or somesuch, how about the below:
--- Subject: perf: Optimize ctx_sched_out From: Peter Zijlstra <a.p.zijlstra@chello.nl> Date: Mon Mar 28 16:55:36 CEST 2011 Oleg noted that ctx_sched_out() disables the PMU even though it might not actually do something, avoid needless PMU-disabling.
Reported-by: Oleg Nesterov <oleg@redhat.com> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> LKML-Reference: <new-submission> --- kernel/perf_event.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Index: linux-2.6/kernel/perf_event.c =================================================================== --- linux-2.6.orig/kernel/perf_event.c +++ linux-2.6/kernel/perf_event.c @@ -1767,7 +1767,6 @@ static void ctx_sched_out(struct perf_ev struct perf_event *event; raw_spin_lock(&ctx->lock); - perf_pmu_disable(ctx->pmu); ctx->is_active = 0; if (likely(!ctx->nr_events)) goto out; @@ -1777,6 +1776,7 @@ static void ctx_sched_out(struct perf_ev if (!ctx->nr_active) goto out; + perf_pmu_disable(ctx->pmu); if (event_type & EVENT_PINNED) { list_for_each_entry(event, &ctx->pinned_groups, group_entry) group_sched_out(event, cpuctx, ctx); @@ -1786,8 +1786,8 @@ static void ctx_sched_out(struct perf_ev list_for_each_entry(event, &ctx->flexible_groups, group_entry) group_sched_out(event, cpuctx, ctx); } -out: perf_pmu_enable(ctx->pmu); +out: raw_spin_unlock(&ctx->lock); }
|  |