2017-08-08 18:37 GMT+08:00 Peter Zijlstra <pet...@infradead.org>: > On Tue, Aug 08, 2017 at 06:00:45PM +0800, 石祤 wrote: > >> diff --git a/kernel/events/core.c b/kernel/events/core.c >> index 426c2ff..3d86695 100644 >> --- a/kernel/events/core.c >> +++ b/kernel/events/core.c >> @@ -3180,6 +3180,13 @@ static void perf_event_context_sched_in(struct >> perf_event_context *ctx, >> return; >> >> perf_ctx_lock(cpuctx, ctx); >> + /* >> + * We must check ctx->nr_events while holding ctx->lock, such >> + * that we serialize against perf_install_in_context(). >> + */ >> + if (!cpuctx->task_ctx && !ctx->nr_events) >> + goto unlock; > > Do we really need the cpuctx->task_ctx test? I think that task_ctx is > 'tight' these days. We never have it set unless there are events > scheduled for that context. > > I even think the cpuctx->task_ctx == ctx test right above here is > superfluous these days. That could only happen when the > perf_install_in_context() IPI came before perf_event_task_sched_in(), > but we removed the arch option to do context switches with IRQs enabled. >
It looks that cpuctx->task_ctx exists somewhere else, so I thought it was conservative making this patch. For a centain, during my process of debugging I didn't figure out any value of cpuctx->task_ctx. I shall make a v3. Thanks >> + >> perf_pmu_disable(ctx->pmu); >> /* >> * We want to keep the following priority order: >> @@ -3193,6 +3200,8 @@ static void perf_event_context_sched_in(struct >> perf_event_context *ctx, >> cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE); >> perf_event_sched_in(cpuctx, ctx, task); >> perf_pmu_enable(ctx->pmu); >> + >> +unlock: >> perf_ctx_unlock(cpuctx, ctx); >> } >> >> -- >> 2.8.4.31.g9ed660f >>
