| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4AA00165.90609@codeaurora.org>
Date: Thu, 03 Sep 2009 13:48:21 -0400
From: Ashwin Chaugule <ashwinc@...eaurora.org>
To: Thomas Gleixner <tglx@...utronix.de>
CC: linux-kernel@...r.kernel.org, mingo@...hat.com
Subject: Re: [RFC] [PATCH 1/1] hrtimers: Cache next hrtimer
Thomas Gleixner wrote:
>>>
>>>
>> This looks very good ! Our results are almost similar now. However, I think
>> that with this new
>> patch we're still arming the timer needlessly at least once. This is the
>> case when we're trying to remove the first (oldest) hrtimer with
>> timer->expires = cpu->expires_next, but we forgo the reprogramming, when we
>> really shouldn't. At this point, with the current scheme of things, we will
>> needlessly wakeup and simply correct the expires_next value by
>> traversing up the rbtree, to the parent node.
>>
>
> That only happens, when that timer is the last one in both trees. Then
> the resulting reprogramming value is KTIME_MAX and we skip the
> reprogramming. That's easy to fix by removing the KTIME_MAX check.
>
>
Thinking more about this, I realized that the said case is already taken
care of by hrtimer_interrupt. We have a while loop there which will traverse
to the parents on both trees and get the correct expires_next value.
In the case in remove_hrtimer, we actually don't want to reprogram the device,
because, the base->first node on the other tree with timer->expires = cpu->expires_next
hasn't expired yet.
Attached final patch here. Is this okay with you ?
>From 867753d2300c358e2a980b70b26beaee40efaf9f Mon Sep 17 00:00:00 2001
From: Ashwin Chaugule <ashwinc@...cinc.com>
Date: Tue, 1 Sep 2009 23:03:33 -0400
hrtimer: Eliminate needless reprogramming of clock events
device.
On NOHZ systems the following timers,
- tick_nohz_restart_sched_tick (tick_sched_timer)
- hrtimer_start (tick_sched_timer)
were reprogramming the clock events device far more often than needed.
No specific test case was required to observe this effect. This
occurred because, there was no check to see if the currently removed
or restarted hrtimer was:
1) the one which previously armed the clock events device.
2) going to be replaced by another timer which has the same expiry time.
This patch avoids the extra programming and results in faster application
startup time by ~4% amongst other benefits.
modified: kernel/hrtimer.c
[tglx: simplified code]
Signed-off-by: Ashwin Chaugule <ashwinc@...cinc.com>
---
kernel/hrtimer.c | 53 +++++++++++++++++++++++++++++++++++------------------
1 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 49da79a..24c8cc3 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -4,6 +4,7 @@
* Copyright(C) 2005-2006, Thomas Gleixner <tglx@...utronix.de>
* Copyright(C) 2005-2007, Red Hat, Inc., Ingo Molnar
* Copyright(C) 2006-2007 Timesys Corp., Thomas Gleixner
+ * Copyright(C) 2009 Code Aurora Forum., Ashwin Chaugule
*
* High-resolution kernel timers
*
@@ -542,13 +543,14 @@ static inline int hrtimer_hres_active(void)
* next event
* Called with interrupts disabled and base->lock held
*/
-static void hrtimer_force_reprogram(struct hrtimer_cpu_base *cpu_base)
+static void
+hrtimer_force_reprogram(struct hrtimer_cpu_base *cpu_base, int skip_equal)
{
int i;
struct hrtimer_clock_base *base = cpu_base->clock_base;
- ktime_t expires;
+ ktime_t expires, expires_next;
- cpu_base->expires_next.tv64 = KTIME_MAX;
+ expires_next.tv64 = KTIME_MAX;
for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++, base++) {
struct hrtimer *timer;
@@ -564,10 +566,15 @@ static void hrtimer_force_reprogram(struct hrtimer_cpu_base *cpu_base)
*/
if (expires.tv64 < 0)
expires.tv64 = 0;
- if (expires.tv64 < cpu_base->expires_next.tv64)
- cpu_base->expires_next = expires;
+ if (expires.tv64 < expires_next.tv64)
+ expires_next = expires;
}
+ if (skip_equal && expires_next.tv64 == cpu_base->expires_next.tv64)
+ return;
+
+ cpu_base->expires_next.tv64 = expires_next.tv64;
+
if (cpu_base->expires_next.tv64 != KTIME_MAX)
tick_program_event(cpu_base->expires_next, 1);
}
@@ -650,7 +657,7 @@ static void retrigger_next_event(void *arg)
base->clock_base[CLOCK_REALTIME].offset =
timespec_to_ktime(realtime_offset);
- hrtimer_force_reprogram(base);
+ hrtimer_force_reprogram(base, 0);
spin_unlock(&base->lock);
}
@@ -763,7 +770,8 @@ static int hrtimer_switch_to_hres(void)
static inline int hrtimer_hres_active(void) { return 0; }
static inline int hrtimer_is_hres_enabled(void) { return 0; }
static inline int hrtimer_switch_to_hres(void) { return 0; }
-static inline void hrtimer_force_reprogram(struct hrtimer_cpu_base *base) { }
+static inline void
+hrtimer_force_reprogram(struct hrtimer_cpu_base *base, int skip_equal) { }
static inline int hrtimer_enqueue_reprogram(struct hrtimer *timer,
struct hrtimer_clock_base *base,
int wakeup)
@@ -906,19 +914,28 @@ static void __remove_hrtimer(struct hrtimer *timer,
struct hrtimer_clock_base *base,
unsigned long newstate, int reprogram)
{
- if (timer->state & HRTIMER_STATE_ENQUEUED) {
- /*
- * Remove the timer from the rbtree and replace the
- * first entry pointer if necessary.
- */
- if (base->first == &timer->node) {
- base->first = rb_next(&timer->node);
- /* Reprogram the clock event device. if enabled */
- if (reprogram && hrtimer_hres_active())
- hrtimer_force_reprogram(base->cpu_base);
+ ktime_t expires;
+
+ if (!(timer->state & HRTIMER_STATE_ENQUEUED))
+ goto out;
+
+ /*
+ * Remove the timer from the rbtree and replace the first
+ * entry pointer if necessary.
+ */
+ if (base->first == &timer->node) {
+ base->first = rb_next(&timer->node);
+ /* Reprogram the clock event device. if enabled */
+ if (reprogram && hrtimer_hres_active()) {
+ expires = ktime_sub(hrtimer_get_expires(timer),
+ base->offset);
+ if (base->cpu_base->expires_next.tv64 == expires.tv64)
+ hrtimer_force_reprogram(base->cpu_base, 1);
}
- rb_erase(&timer->node, &base->active);
}
+
+ rb_erase(&timer->node, &base->active);
+out:
timer->state = newstate;
}
--
1.5.6.3
Cheers,
Ashwin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists