On Friday, July 03, 2015 04:19:40 PM Lucas Stach wrote:
> Make sure to stop tracing only once we are past a point where all
> latency tracing events have been processed (irqs are not enabled
> again). This has the slight advantage of capturing more latency
> related events in the idle path, but most importantly it makes sure
> that latency tracing doesn't get re-enabled inadvertently when
> new events are coming in.
>
> This makes the irqsoff latency tracer useful again, as we stop
> capturing CPU sleep time as IRQ latency.
>
> Signed-off-by: Lucas Stach <[email protected]>
> ---
> drivers/cpuidle/cpuidle.c | 2 ++
> kernel/sched/idle.c | 14 +++++---------
> 2 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 61c417b9e53f..d78514ac3f5d 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -173,7 +173,9 @@ int cpuidle_enter_state(struct cpuidle_device *dev,
> struct cpuidle_driver *drv,
> trace_cpu_idle_rcuidle(index, dev->cpu);
> time_start = ktime_get();
>
> + stop_critical_timings();
> entered_state = target_state->enter(dev, drv, index);
> + start_critical_timings();
>
> time_end = ktime_get();
> trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index fefcb1fa5160..30b799bfc3c6 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -93,12 +93,6 @@ static void cpuidle_idle_call(void)
> }
>
> /*
> - * During the idle period, stop measuring the disabled irqs
> - * critical sections latencies
> - */
> - stop_critical_timings();
> -
> - /*
> * Tell the RCU framework we are entering an idle section,
> * so no more rcu read side critical sections and one more
> * step to the grace period
> @@ -181,7 +175,6 @@ exit_idle:
> local_irq_enable();
>
> rcu_idle_exit();
> - start_critical_timings();
Are you sure we can safely reorder stop/start_critical_timings() with respect
to rcu_idle_enter/exit()?
> return;
>
> use_default:
> @@ -189,10 +182,13 @@ use_default:
> * We can't use the cpuidle framework, let's use the default
> * idle routine.
> */
> - if (current_clr_polling_and_test())
> + if (current_clr_polling_and_test()) {
> local_irq_enable();
> - else
> + } else {
> + stop_critical_timings();
> arch_cpu_idle();
> + start_critical_timings();
> + }
>
> goto exit_idle;
> }
And what about enter_freeze_proper()? Don't we need those in there too?
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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/
