* Re: [PATCH v2] tracing: Don't account for cpu idle time with irqsoff tracers
2014-05-27 23:30 ` Steven Rostedt
@ 2014-05-28 0:11 ` Stephen Boyd
2014-05-28 0:23 ` Stephen Boyd
2014-05-28 0:42 ` Steven Rostedt
2014-05-28 0:25 ` Paul E. McKenney
` (3 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: Stephen Boyd @ 2014-05-28 0:11 UTC (permalink / raw)
To: Steven Rostedt
Cc: Arnd Bergmann, Frederic Weisbecker, Ingo Molnar, linux-kernel,
Corey Minyard, Stanislav Meduna, Paul E. McKenney, Peter Zijlstra
On 05/27/14 16:30, Steven Rostedt wrote:
> On Tue, 27 May 2014 15:21:39 -0700
> Stephen Boyd <sboyd@codeaurora.org> wrote:
>
>
>>> Arnd brings up a good point.
>> Hrm.. still not getting Arnd's mails.
> Strange. What mail service do you have. Could they be blocking him?
>
>>> If we disable irqs off tracing completely,
>>> we may be missing places in the idle path that disable interrupts for
>>> long periods of time. We may want to move the stop down further.
>>>
>>> The way it works (IIRC), and why tracing can start again is that it can
>>> nest. Perhaps we need to stop it further down if we can't move it
>>> completely.
>>>
>> I'm not sure how much deeper it can go and I'm afraid it will become a
>> game of whack-a-mole. I already see two places that disable and reenable
>> irqs after stop_critical_timings() is called (first in rcu_idle_enter()
>> and second in clockevents_notify()). Should rcu_idle_enter() move to
>> raw_local_irq_save()? It looks like that path calls rcu_sched_qs() and
>> on tiny RCU that again needs the raw_ treatement. We can probably call
>> stop_critical_timings() after rcu_idle_enter() to fix this.
> I don't think we need to whack-a-mole. The start stop should be around
> where it goes to sleep.
>
>> What about clockevents_notify? __raw_spin_lock_irqsave() should probably
>> use raw_local_irqsave().
> No that solution is even worse. We need lockdep working here.
>
>> If we go the raw route, do we even need stop/start_critical_timings()?
>> Can't we just use raw accessors in the idle paths
>> (tick_nohz_idle_{enter,exit}(), cpuidle_enter(), etc.) and get rid of
>> the stop/start stuff completely? I admit this patch is pretty much a big
>> sledge hammer that tries to make things simple, but if there is some
>> benefit to the raw accessors I'm willing to send patches to fix all the
>> call sites.
>>
> How about the following. I don't see any reason stop_critical_timings()
> can't be called from within rcu_idle code, as it doesn't use any rcu.
>
> Paul, Peter, see anything wrong with this?
>
cpuidle_enter_state() calls ktime_get() which on lockdep enabled builds
calls seqcount_lockdep_reader_access() which calls local_irq_save() that
then turns on the tracer again. Perhaps the problem is that irqsoff
tracer is triggered even when we aren't transitioning between irqs on
and irqs off? What about this patch? I assume there is a reason that
this is wrong, but I don't know what it is.
---8<-----
diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index d176d658fe25..ac8e0a4968bd 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -93,7 +93,8 @@
#define local_irq_save(flags) \
do { \
raw_local_irq_save(flags); \
- trace_hardirqs_off(); \
+ if (!raw_irqs_disabled_flags(flags)) \
+ trace_hardirqs_off(); \
} while (0)
@@ -101,7 +102,6 @@
do { \
if (raw_irqs_disabled_flags(flags)) { \
raw_local_irq_restore(flags); \
- trace_hardirqs_off(); \
} else { \
trace_hardirqs_on(); \
raw_local_irq_restore(flags); \
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2] tracing: Don't account for cpu idle time with irqsoff tracers
2014-05-28 0:11 ` Stephen Boyd
@ 2014-05-28 0:23 ` Stephen Boyd
2014-05-28 0:42 ` Steven Rostedt
1 sibling, 0 replies; 15+ messages in thread
From: Stephen Boyd @ 2014-05-28 0:23 UTC (permalink / raw)
To: Steven Rostedt
Cc: Arnd Bergmann, Frederic Weisbecker, Ingo Molnar, linux-kernel,
Corey Minyard, Stanislav Meduna, Paul E. McKenney, Peter Zijlstra
On 05/27/14 17:11, Stephen Boyd wrote:
> On 05/27/14 16:30, Steven Rostedt wrote:
>> On Tue, 27 May 2014 15:21:39 -0700
>> Stephen Boyd <sboyd@codeaurora.org> wrote:
>>
>>
>>>> Arnd brings up a good point.
>>> Hrm.. still not getting Arnd's mails.
>> Strange. What mail service do you have. Could they be blocking him?
>>
>>>> If we disable irqs off tracing completely,
>>>> we may be missing places in the idle path that disable interrupts for
>>>> long periods of time. We may want to move the stop down further.
>>>>
>>>> The way it works (IIRC), and why tracing can start again is that it can
>>>> nest. Perhaps we need to stop it further down if we can't move it
>>>> completely.
>>>>
>>> I'm not sure how much deeper it can go and I'm afraid it will become a
>>> game of whack-a-mole. I already see two places that disable and reenable
>>> irqs after stop_critical_timings() is called (first in rcu_idle_enter()
>>> and second in clockevents_notify()). Should rcu_idle_enter() move to
>>> raw_local_irq_save()? It looks like that path calls rcu_sched_qs() and
>>> on tiny RCU that again needs the raw_ treatement. We can probably call
>>> stop_critical_timings() after rcu_idle_enter() to fix this.
>> I don't think we need to whack-a-mole. The start stop should be around
>> where it goes to sleep.
>>
>>> What about clockevents_notify? __raw_spin_lock_irqsave() should probably
>>> use raw_local_irqsave().
>> No that solution is even worse. We need lockdep working here.
>>
>>> If we go the raw route, do we even need stop/start_critical_timings()?
>>> Can't we just use raw accessors in the idle paths
>>> (tick_nohz_idle_{enter,exit}(), cpuidle_enter(), etc.) and get rid of
>>> the stop/start stuff completely? I admit this patch is pretty much a big
>>> sledge hammer that tries to make things simple, but if there is some
>>> benefit to the raw accessors I'm willing to send patches to fix all the
>>> call sites.
>>>
>> How about the following. I don't see any reason stop_critical_timings()
>> can't be called from within rcu_idle code, as it doesn't use any rcu.
>>
>> Paul, Peter, see anything wrong with this?
>>
> cpuidle_enter_state() calls ktime_get() which on lockdep enabled builds
> calls seqcount_lockdep_reader_access() which calls local_irq_save() that
> then turns on the tracer again. Perhaps the problem is that irqsoff
> tracer is triggered even when we aren't transitioning between irqs on
> and irqs off? What about this patch? I assume there is a reason that
> this is wrong, but I don't know what it is.
>
> ---8<-----
>
> diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
> index d176d658fe25..ac8e0a4968bd 100644
> --- a/include/linux/irqflags.h
> +++ b/include/linux/irqflags.h
> @@ -93,7 +93,8 @@
> #define local_irq_save(flags) \
> do { \
> raw_local_irq_save(flags); \
> - trace_hardirqs_off(); \
> + if (!raw_irqs_disabled_flags(flags)) \
> + trace_hardirqs_off(); \
> } while (0)
>
>
> @@ -101,7 +102,6 @@
> do { \
> if (raw_irqs_disabled_flags(flags)) { \
> raw_local_irq_restore(flags); \
> - trace_hardirqs_off(); \
> } else { \
> trace_hardirqs_on(); \
> raw_local_irq_restore(flags); \
>
Aha, looks like lockdep wants to know about redundant hardirqs with the
redundant_hardirqs_off field. Can we use the same field in the irqsoff
tracer to monitor the hardirq on/off state more accurately?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] tracing: Don't account for cpu idle time with irqsoff tracers
2014-05-28 0:11 ` Stephen Boyd
2014-05-28 0:23 ` Stephen Boyd
@ 2014-05-28 0:42 ` Steven Rostedt
2014-05-28 7:24 ` Peter Zijlstra
1 sibling, 1 reply; 15+ messages in thread
From: Steven Rostedt @ 2014-05-28 0:42 UTC (permalink / raw)
To: Stephen Boyd
Cc: Arnd Bergmann, Frederic Weisbecker, Ingo Molnar, linux-kernel,
Corey Minyard, Stanislav Meduna, Paul E. McKenney, Peter Zijlstra
On Tue, 2014-05-27 at 17:11 -0700, Stephen Boyd wrote:
> cpuidle_enter_state() calls ktime_get() which on lockdep enabled builds
> calls seqcount_lockdep_reader_access() which calls local_irq_save() that
seqcount_lockdep_reader_access()?? Ug, I wonder if that should call
raw_local_irq_save/restore() as it's a lockdep helper to begin with. If
it's wrong then it's the lockdep infrastructure that broke, not the core
kernel.
Peter?
-- Steve
> then turns on the tracer again. Perhaps the problem is that irqsoff
> tracer is triggered even when we aren't transitioning between irqs on
> and irqs off? What about this patch? I assume there is a reason that
> this is wrong, but I don't know what it is.
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] tracing: Don't account for cpu idle time with irqsoff tracers
2014-05-28 0:42 ` Steven Rostedt
@ 2014-05-28 7:24 ` Peter Zijlstra
2014-05-28 17:22 ` John Stultz
0 siblings, 1 reply; 15+ messages in thread
From: Peter Zijlstra @ 2014-05-28 7:24 UTC (permalink / raw)
To: Steven Rostedt
Cc: Stephen Boyd, Arnd Bergmann, Frederic Weisbecker, Ingo Molnar,
linux-kernel, Corey Minyard, Stanislav Meduna, Paul E. McKenney,
john.stultz
[-- Attachment #1: Type: text/plain, Size: 844 bytes --]
On Tue, May 27, 2014 at 08:42:14PM -0400, Steven Rostedt wrote:
> On Tue, 2014-05-27 at 17:11 -0700, Stephen Boyd wrote:
>
> > cpuidle_enter_state() calls ktime_get() which on lockdep enabled builds
> > calls seqcount_lockdep_reader_access() which calls local_irq_save() that
>
> seqcount_lockdep_reader_access()?? Ug, I wonder if that should call
> raw_local_irq_save/restore() as it's a lockdep helper to begin with. If
> it's wrong then it's the lockdep infrastructure that broke, not the core
> kernel.
>
> Peter?
Hurm,.. don't know actually.. so from a lockdep pov it doesn't need to
do that and we can simply remove the local_irq_{save,restore}() from
that function.
It could be John did it to avoid some IRQ recursion warning, but if so,
he failed to mention it.
John, remember why you typed those characters?
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] tracing: Don't account for cpu idle time with irqsoff tracers
2014-05-28 7:24 ` Peter Zijlstra
@ 2014-05-28 17:22 ` John Stultz
0 siblings, 0 replies; 15+ messages in thread
From: John Stultz @ 2014-05-28 17:22 UTC (permalink / raw)
To: Peter Zijlstra, Steven Rostedt
Cc: Stephen Boyd, Arnd Bergmann, Frederic Weisbecker, Ingo Molnar,
linux-kernel, Corey Minyard, Stanislav Meduna, Paul E. McKenney
On 05/28/2014 12:24 AM, Peter Zijlstra wrote:
> On Tue, May 27, 2014 at 08:42:14PM -0400, Steven Rostedt wrote:
>> On Tue, 2014-05-27 at 17:11 -0700, Stephen Boyd wrote:
>>
>>> cpuidle_enter_state() calls ktime_get() which on lockdep enabled builds
>>> calls seqcount_lockdep_reader_access() which calls local_irq_save() that
>>
>> seqcount_lockdep_reader_access()?? Ug, I wonder if that should call
>> raw_local_irq_save/restore() as it's a lockdep helper to begin with. If
>> it's wrong then it's the lockdep infrastructure that broke, not the core
>> kernel.
>>
>> Peter?
>
> Hurm,.. don't know actually.. so from a lockdep pov it doesn't need to
> do that and we can simply remove the local_irq_{save,restore}() from
> that function.
>
> It could be John did it to avoid some IRQ recursion warning, but if so,
> he failed to mention it.
>
> John, remember why you typed those characters?
So.. With seqlocks, we're trying to just make sure reads and writes
don't nest under a write. However we don't care if a write nests in a
read, because the read will be restarted. An example is someone hitting
gettimeofday over and over taking a read, and then an IRQ lands mid-read
and we take the write and update the data. This is expected normal
behavior. So this was trying to make the read side lockdep
aquire/release combo atomic, so we don't create false warnings if an IRQ
landed right in-between.
Does that make sense?
thanks
-john
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] tracing: Don't account for cpu idle time with irqsoff tracers
2014-05-27 23:30 ` Steven Rostedt
2014-05-28 0:11 ` Stephen Boyd
@ 2014-05-28 0:25 ` Paul E. McKenney
2014-05-28 6:40 ` Arnd Bergmann
` (2 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Paul E. McKenney @ 2014-05-28 0:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: Stephen Boyd, Arnd Bergmann, Frederic Weisbecker, Ingo Molnar,
linux-kernel, Corey Minyard, Stanislav Meduna, Peter Zijlstra
On Tue, May 27, 2014 at 07:30:50PM -0400, Steven Rostedt wrote:
> On Tue, 27 May 2014 15:21:39 -0700
> Stephen Boyd <sboyd@codeaurora.org> wrote:
>
>
> > > Arnd brings up a good point.
> >
> > Hrm.. still not getting Arnd's mails.
>
> Strange. What mail service do you have. Could they be blocking him?
>
> >
> > > If we disable irqs off tracing completely,
> > > we may be missing places in the idle path that disable interrupts for
> > > long periods of time. We may want to move the stop down further.
> > >
> > > The way it works (IIRC), and why tracing can start again is that it can
> > > nest. Perhaps we need to stop it further down if we can't move it
> > > completely.
> > >
> >
> > I'm not sure how much deeper it can go and I'm afraid it will become a
> > game of whack-a-mole. I already see two places that disable and reenable
> > irqs after stop_critical_timings() is called (first in rcu_idle_enter()
> > and second in clockevents_notify()). Should rcu_idle_enter() move to
> > raw_local_irq_save()? It looks like that path calls rcu_sched_qs() and
> > on tiny RCU that again needs the raw_ treatement. We can probably call
> > stop_critical_timings() after rcu_idle_enter() to fix this.
>
> I don't think we need to whack-a-mole. The start stop should be around
> where it goes to sleep.
>
> >
> > What about clockevents_notify? __raw_spin_lock_irqsave() should probably
> > use raw_local_irqsave().
>
> No that solution is even worse. We need lockdep working here.
>
> >
> > If we go the raw route, do we even need stop/start_critical_timings()?
> > Can't we just use raw accessors in the idle paths
> > (tick_nohz_idle_{enter,exit}(), cpuidle_enter(), etc.) and get rid of
> > the stop/start stuff completely? I admit this patch is pretty much a big
> > sledge hammer that tries to make things simple, but if there is some
> > benefit to the raw accessors I'm willing to send patches to fix all the
> > call sites.
> >
>
> How about the following. I don't see any reason stop_critical_timings()
> can't be called from within rcu_idle code, as it doesn't use any rcu.
>
> Paul, Peter, see anything wrong with this?
I don't immediately see any RCU use by stop_critical_timings(), but could
easily have missed something. But CONFIG_PROVE_RCU=y will normally yell
if something using RCU showed up.
Looks plausible, but clearly needs testing across the usual array of
configs and arches.
Thanx, Paul
> -- Steve
>
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index 8f4390a..f5e6a64 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -88,12 +88,6 @@ static int 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
> @@ -144,6 +138,12 @@ static int cpuidle_idle_call(void)
> trace_cpu_idle_rcuidle(next_state, dev->cpu);
>
> /*
> + * During the idle period, stop measuring the
> + * disabled irqs critical sections latencies
> + */
> + stop_critical_timings();
> +
> + /*
> * Enter the idle state previously
> * returned by the governor
> * decision. This function will block
> @@ -154,6 +154,8 @@ static int cpuidle_idle_call(void)
> entered_state = cpuidle_enter(drv, dev,
> next_state);
>
> + start_critical_timings();
> +
> trace_cpu_idle_rcuidle(PWR_EVENT_EXIT,
> dev->cpu);
>
> @@ -175,8 +177,11 @@ static int cpuidle_idle_call(void)
> * We can't use the cpuidle framework, let's use the default
> * idle routine
> */
> - if (ret)
> + if (ret) {
> + stop_critical_timings();
> arch_cpu_idle();
> + start_critical_timings();
> + }
>
> __current_set_polling();
>
> @@ -188,7 +193,6 @@ static int cpuidle_idle_call(void)
> local_irq_enable();
>
> rcu_idle_exit();
> - start_critical_timings();
>
> return 0;
> }
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] tracing: Don't account for cpu idle time with irqsoff tracers
2014-05-27 23:30 ` Steven Rostedt
2014-05-28 0:11 ` Stephen Boyd
2014-05-28 0:25 ` Paul E. McKenney
@ 2014-05-28 6:40 ` Arnd Bergmann
2014-05-28 6:44 ` Arnd Bergmann
2014-05-28 7:28 ` Peter Zijlstra
4 siblings, 0 replies; 15+ messages in thread
From: Arnd Bergmann @ 2014-05-28 6:40 UTC (permalink / raw)
To: Steven Rostedt
Cc: Stephen Boyd, Frederic Weisbecker, Ingo Molnar, linux-kernel,
Corey Minyard, Stanislav Meduna, Paul E. McKenney, Peter Zijlstra
On Tuesday 27 May 2014 19:30:50 Steven Rostedt wrote:
> On Tue, 27 May 2014 15:21:39 -0700
> Stephen Boyd <sboyd@codeaurora.org> wrote:
>
>
> > > Arnd brings up a good point.
> >
> > Hrm.. still not getting Arnd's mails.
>
> Strange. What mail service do you have. Could they be blocking him?
>
I think it's United Internet again getting blocked by some blacklist,
I has had bounces from arm.com addresses today.
Arnd
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] tracing: Don't account for cpu idle time with irqsoff tracers
2014-05-27 23:30 ` Steven Rostedt
` (2 preceding siblings ...)
2014-05-28 6:40 ` Arnd Bergmann
@ 2014-05-28 6:44 ` Arnd Bergmann
2014-05-28 7:28 ` Peter Zijlstra
4 siblings, 0 replies; 15+ messages in thread
From: Arnd Bergmann @ 2014-05-28 6:44 UTC (permalink / raw)
To: Steven Rostedt
Cc: Stephen Boyd, Frederic Weisbecker, Ingo Molnar, linux-kernel,
Corey Minyard, Stanislav Meduna, Paul E. McKenney, Peter Zijlstra
On Tuesday 27 May 2014 19:30:50 Steven Rostedt wrote:
> @@ -144,6 +138,12 @@ static int cpuidle_idle_call(void)
> trace_cpu_idle_rcuidle(next_state, dev->cpu);
>
> /*
> + * During the idle period, stop measuring the
> + * disabled irqs critical sections latencies
> + */
> + stop_critical_timings();
> +
> + /*
> * Enter the idle state previously
> * returned by the governor
> * decision. This function will block
> @@ -154,6 +154,8 @@ static int cpuidle_idle_call(void)
> entered_state = cpuidle_enter(drv, dev,
> next_state);
>
> + start_critical_timings();
> +
> trace_cpu_idle_rcuidle(PWR_EVENT_EXIT,
> dev->cpu);
>
cpuidle_enter() can have quite complex implementations in drivers/cpuidle/,
it's quite possible we have to push down the
stop_critical_timings/start_critical_timings further down here into
the individual drivers.
> @@ -175,8 +177,11 @@ static int cpuidle_idle_call(void)
> * We can't use the cpuidle framework, let's use the default
> * idle routine
> */
> - if (ret)
> + if (ret) {
> + stop_critical_timings();
> arch_cpu_idle();
> + start_critical_timings();
> + }
>
> __current_set_polling();
>
>
This one seems fine on all architectures I've looked at.
Arnd
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] tracing: Don't account for cpu idle time with irqsoff tracers
2014-05-27 23:30 ` Steven Rostedt
` (3 preceding siblings ...)
2014-05-28 6:44 ` Arnd Bergmann
@ 2014-05-28 7:28 ` Peter Zijlstra
2014-05-28 14:09 ` Steven Rostedt
4 siblings, 1 reply; 15+ messages in thread
From: Peter Zijlstra @ 2014-05-28 7:28 UTC (permalink / raw)
To: Steven Rostedt
Cc: Stephen Boyd, Arnd Bergmann, Frederic Weisbecker, Ingo Molnar,
linux-kernel, Corey Minyard, Stanislav Meduna, Paul E. McKenney,
rjw
[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]
On Tue, May 27, 2014 at 07:30:50PM -0400, Steven Rostedt wrote:
> Paul, Peter, see anything wrong with this?
You mean, aside from that it won't actually apply due to that code
having been massively overhauled?
I suppose the same thing Arnd mentioned, the cpuidle_enter() thing can
be huge, but is we're going to have to push this down into all cpuidle
drivers we're not going to be happy.
So I think we're going to have to draw a line and impose restrictions on
what cpuidle_enter() implementations can and can not do.
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index 8f4390a..f5e6a64 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -88,12 +88,6 @@ static int 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
> @@ -144,6 +138,12 @@ static int cpuidle_idle_call(void)
> trace_cpu_idle_rcuidle(next_state, dev->cpu);
>
> /*
> + * During the idle period, stop measuring the
> + * disabled irqs critical sections latencies
> + */
> + stop_critical_timings();
> +
> + /*
> * Enter the idle state previously
> * returned by the governor
> * decision. This function will block
> @@ -154,6 +154,8 @@ static int cpuidle_idle_call(void)
> entered_state = cpuidle_enter(drv, dev,
> next_state);
>
> + start_critical_timings();
> +
> trace_cpu_idle_rcuidle(PWR_EVENT_EXIT,
> dev->cpu);
>
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] tracing: Don't account for cpu idle time with irqsoff tracers
2014-05-28 7:28 ` Peter Zijlstra
@ 2014-05-28 14:09 ` Steven Rostedt
0 siblings, 0 replies; 15+ messages in thread
From: Steven Rostedt @ 2014-05-28 14:09 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Stephen Boyd, Arnd Bergmann, Frederic Weisbecker, Ingo Molnar,
linux-kernel, Corey Minyard, Stanislav Meduna, Paul E. McKenney,
rjw
On Wed, 28 May 2014 09:28:52 +0200
Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, May 27, 2014 at 07:30:50PM -0400, Steven Rostedt wrote:
> > Paul, Peter, see anything wrong with this?
>
> You mean, aside from that it won't actually apply due to that code
> having been massively overhauled?
>
> I suppose the same thing Arnd mentioned, the cpuidle_enter() thing can
> be huge, but is we're going to have to push this down into all cpuidle
> drivers we're not going to be happy.
>
I believe that the stop/start_critical_timings() can nest. If not, I
can make them do so. Thus, we could start adding them in the depths of
the cpuidle drivers. If we miss one, we may not notice until someone
reports a large latency due to it. And that is only if the driver
itself calls local_irq_save/restore().
-- Steve
^ permalink raw reply [flat|nested] 15+ messages in thread