* [PATCH] clockevents: avoid unnecessary reprograming of event timer
@ 2009-02-12 22:23 Michael Davidson
2009-02-13 8:22 ` Ingo Molnar
0 siblings, 1 reply; 5+ messages in thread
From: Michael Davidson @ 2009-02-12 22:23 UTC (permalink / raw)
To: mbligh, tglx; +Cc: linux-kernel
From: Michael Davidson <md@google.com>
Don't reprogram the event timer if it is already set to expire
at the correct time.
Signed-off-by: Michael Davidson <md@google.com>
---
--- linux-2.6.29-rc4.orig/kernel/time/clockevents.c 2009-02-12 13:13:24.000000000 -0800
+++ linux-2.6.29-rc4/kernel/time/clockevents.c 2009-02-12 13:25:42.525558000 -0800
@@ -103,6 +103,9 @@
if (delta <= 0)
return -ETIME;
+ if (ktime_equal(dev->next_event, expires))
+ return 0;
+
dev->next_event = expires;
if (dev->mode == CLOCK_EVT_MODE_SHUTDOWN)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clockevents: avoid unnecessary reprograming of event timer
2009-02-12 22:23 [PATCH] clockevents: avoid unnecessary reprograming of event timer Michael Davidson
@ 2009-02-13 8:22 ` Ingo Molnar
2009-02-13 9:39 ` Thomas Gleixner
2009-02-13 18:50 ` Michael Davidson
0 siblings, 2 replies; 5+ messages in thread
From: Ingo Molnar @ 2009-02-13 8:22 UTC (permalink / raw)
To: Michael Davidson; +Cc: mbligh, tglx, linux-kernel
* Michael Davidson <md@google.com> wrote:
> From: Michael Davidson <md@google.com>
>
> Don't reprogram the event timer if it is already set to expire
> at the correct time.
>
> Signed-off-by: Michael Davidson <md@google.com>
> ---
> --- linux-2.6.29-rc4.orig/kernel/time/clockevents.c 2009-02-12 13:13:24.000000000 -0800
> +++ linux-2.6.29-rc4/kernel/time/clockevents.c 2009-02-12 13:25:42.525558000 -0800
> @@ -103,6 +103,9 @@
> if (delta <= 0)
> return -ETIME;
>
> + if (ktime_equal(dev->next_event, expires))
> + return 0;
> +
Hm, given that a good high-res source has nanoseconds resolution,
what's the chance of this optimization triggering in practice?
Near zero i think - unless we trigger useless reprogramming without
having added or removed any new timers - but then we should
concentrate on analyzing the reason for that redundant reprogramming.
Does it trigger often for you?
Ingo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clockevents: avoid unnecessary reprograming of event timer
2009-02-13 8:22 ` Ingo Molnar
@ 2009-02-13 9:39 ` Thomas Gleixner
2009-02-13 9:55 ` Ingo Molnar
2009-02-13 18:50 ` Michael Davidson
1 sibling, 1 reply; 5+ messages in thread
From: Thomas Gleixner @ 2009-02-13 9:39 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Michael Davidson, mbligh, linux-kernel
On Fri, 13 Feb 2009, Ingo Molnar wrote:
> > + if (ktime_equal(dev->next_event, expires))
> > + return 0;
> > +
>
> Hm, given that a good high-res source has nanoseconds resolution,
> what's the chance of this optimization triggering in practice?
> Near zero i think - unless we trigger useless reprogramming without
> having added or removed any new timers - but then we should
> concentrate on analyzing the reason for that redundant reprogramming.
>
> Does it trigger often for you?
The only point where this should trigger is when we come out of C2/C3
and switch the local APIC timer back on. There we reload the
next_event which was discarded in the hardware before we entered C2/C3
and switched to the broadcast timer. So this change would prevent the
rearming of the local APIC after coming out of C2/C3 and probably give
us some new C-State related headaches.
Thanks,
tglx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clockevents: avoid unnecessary reprograming of event timer
2009-02-13 9:39 ` Thomas Gleixner
@ 2009-02-13 9:55 ` Ingo Molnar
0 siblings, 0 replies; 5+ messages in thread
From: Ingo Molnar @ 2009-02-13 9:55 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Michael Davidson, mbligh, linux-kernel
* Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 13 Feb 2009, Ingo Molnar wrote:
> > > + if (ktime_equal(dev->next_event, expires))
> > > + return 0;
> > > +
> >
> > Hm, given that a good high-res source has nanoseconds resolution,
> > what's the chance of this optimization triggering in practice?
> > Near zero i think - unless we trigger useless reprogramming without
> > having added or removed any new timers - but then we should
> > concentrate on analyzing the reason for that redundant reprogramming.
> >
> > Does it trigger often for you?
>
> The only point where this should trigger is when we come out of C2/C3
> and switch the local APIC timer back on. There we reload the
> next_event which was discarded in the hardware before we entered C2/C3
> and switched to the broadcast timer. So this change would prevent the
> rearming of the local APIC after coming out of C2/C3 and probably give
> us some new C-State related headaches.
yes, that was my other thought. Albeit arguably the CE code should
set next_event to -1 or so when suspending, out of caution, right?
Ingo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clockevents: avoid unnecessary reprograming of event timer
2009-02-13 8:22 ` Ingo Molnar
2009-02-13 9:39 ` Thomas Gleixner
@ 2009-02-13 18:50 ` Michael Davidson
1 sibling, 0 replies; 5+ messages in thread
From: Michael Davidson @ 2009-02-13 18:50 UTC (permalink / raw)
To: Ingo Molnar; +Cc: mbligh, tglx, linux-kernel
On Fri, Feb 13, 2009 at 12:22 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> Hm, given that a good high-res source has nanoseconds resolution,
> what's the chance of this optimization triggering in practice?
> Near zero i think - unless we trigger useless reprogramming without
> having added or removed any new timers - but then we should
> concentrate on analyzing the reason for that redundant reprogramming.
>
> Does it trigger often for you?
Yes - running 2.6.26 built with CONFIG_NO_HZ on a 4 core AMD system,
depending on the workload I see it trigger between 40 and 100 times a second.
I will take a closer look at why this is happening.
md
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-02-13 18:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-12 22:23 [PATCH] clockevents: avoid unnecessary reprograming of event timer Michael Davidson
2009-02-13 8:22 ` Ingo Molnar
2009-02-13 9:39 ` Thomas Gleixner
2009-02-13 9:55 ` Ingo Molnar
2009-02-13 18:50 ` Michael Davidson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox