linux-kernel - Re: [RFC] [PATCH 1/1] hrtimers: Cache next hrtimer

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.00.0908280045200.2888@localhost.localdomain>
Date:	Fri, 28 Aug 2009 00:51:22 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Ashwin Chaugule <ashwinc@...eaurora.org>
cc:	linux-kernel@...r.kernel.org, mingo@...hat.com
Subject: Re: [RFC] [PATCH 1/1] hrtimers: Cache next hrtimer

On Thu, 27 Aug 2009, Ashwin Chaugule wrote:
>    Cached the hrtimer which causes the expire_next
>    value to change. This is to avoid unnecessary re-programming
>    of the clock events device.

You forgot to describe the application scenario which triggers this.
 
> @@ -843,16 +851,20 @@ static void __remove_hrtimer(struct hrtimer *timer,
>                  struct hrtimer_clock_base *base,
>                  unsigned long newstate, int reprogram)
> {
> -    if (timer->state & HRTIMER_STATE_ENQUEUED) {
> +    struct hrtimer *next_hrtimer = __get_cpu_var(hrtimer_bases).next_hrtimer;
> +
> +    if (hrtimer_is_queued(timer)) {
>         /*
>          * 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);
> +            if (next_hrtimer == timer) {
> +                /* Reprogram the clock event device. if enabled */
> +                if (reprogram && hrtimer_hres_active())
> +                    hrtimer_force_reprogram(base->cpu_base);
> +            }
>         }
>         rb_erase(&timer->node, &base->active);

So if I'm not totally on the wrong track, that's the meat of the
patch.

Any reason why we can't solve that problem with checking
cpu_base->expires_next against the timer which is deleted ? 

See the patently untested patch below.

Another question which arises is whether we should bother with the
reprogramming at all and just let the last programmed event happen
even when the corresponding timer has been removed.

Thanks,

	tglx
---
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 49da79a..91d099c 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -906,19 +906,30 @@ 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);
-		}
-		rb_erase(&timer->node, &base->active);
-	}
+	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.
+	 */
+	rb_erase(&timer->node, &base->active);
+
+	if (base->first != &timer->node)
+		goto out;
+
+	base->first = rb_next(&timer->node);
+	/* Reprogram the clock event device. if enabled */
+	if (!reprogram || !hrtimer_hres_active())
+		goto out;
+
+	expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
+	if (base->cpu_base->expires_next.tv64 == expires.tv64)
+		hrtimer_force_reprogram(base->cpu_base);
+
+out:
 	timer->state = newstate;
 }
 

--
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/