Am Samstag, den 04.07.2015, 02:08 +0200 schrieb Rafael J. Wysocki:
> 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()?
>
I'm no RCU expert, but I'm reasonably sure that this is a safe thing to
do, as stop/start_critical_timings() does not use any RCU facilities.
I've ran this patch quite some time with PROVE_RCU enabled and it didn't
indicate any suspicious RCU usage.
> > 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?
>
You are right on this one. I'll spin a V2 with this fixed.
Thanks,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
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/
