On 05/27/14 12:13, Steven Rostedt wrote: > Hey! I was able to get to this.
Great! > > On Mon, 19 May 2014 12:30:47 -0700 > Stephen Boyd <[email protected]> wrote: > >> + if (!tracer_enabled || !tracing_is_enabled() || >> + per_cpu(timings_stopped, cpu)) >> + return; > Micro optimization. As this gets called even when not tracing, the > tracer_enabled is what determines if tracing is enabled or not. Can you > keep the first condition the same, and just add your check to the one > below: > > if (per_cpu(timings_stop, cpu) || per_cpu(tracing_cpu, cpu)) > return; Ok. > >> + >> if (per_cpu(tracing_cpu, cpu)) >> return; >> >> @@ -418,7 +421,8 @@ stop_critical_timing(unsigned long ip, unsigned long >> parent_ip) >> else >> return; >> >> - if (!tracer_enabled || !tracing_is_enabled()) >> + if (!tracer_enabled || !tracing_is_enabled() || >> + per_cpu(timings_stopped, cpu)) >> return; >> >> data = per_cpu_ptr(tr->trace_buffer.data, cpu); >> @@ -439,15 +443,19 @@ stop_critical_timing(unsigned long ip, unsigned long >> parent_ip) >> /* start and stop critical timings used to for stoppage (in idle) */ >> void start_critical_timings(void) >> { >> - if (preempt_trace() || irq_trace()) >> + if (preempt_trace() || irq_trace()) { >> + per_cpu(timings_stopped, raw_smp_processor_id()) = false; >> start_critical_timing(CALLER_ADDR0, CALLER_ADDR1); >> + } >> } >> EXPORT_SYMBOL_GPL(start_critical_timings); >> >> void stop_critical_timings(void) >> { >> - if (preempt_trace() || irq_trace()) >> + if (preempt_trace() || irq_trace()) { >> stop_critical_timing(CALLER_ADDR0, CALLER_ADDR1); >> + per_cpu(timings_stopped, raw_smp_processor_id()) = true; >> + } > Hmm, I think this is racy. If we enter idle with tracing enabled it > will set timings_stopped to true for this cpu. But if we disable > tracing while in idle, it will not turn it off. > > Well, this isn't really true, because once we enable tracing the > trace_type that is used to check preempt_trace() and irq_trace() stays > set even when tracing isn't enabled. But this may change soon and that > can make this a problem. > > I don't see any reason the setting of timings_stopped can't be set > unconditionally in these functions. I don't see any problem either. I'll send an update. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
