[PATCH 05/13] hrtimer: optimize hrtimer_run_queues

[PATCH 05/13] hrtimer: optimize hrtimer_run_queues

[Roman Zippel]

Every time hrtimer_run_queues() is called, get_time() is called twice,
which can be quite expensive, just reading xtime is much cheaper and
does the same job (at least for the current low resolution timer, for
high resolution timer we can something different later).
Cache the expiry time in last_expired, so run_hrtimer_queue() doesn't
has to calculate it (clock sources usually know when their expired).

Signed-off-by: Roman Zippel <zippel@linux-m68k.org>

---

 include/linux/hrtimer.h |    1 +
 kernel/hrtimer.c        |   17 +++++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

Index: linux-2.6-git/include/linux/hrtimer.h
===================================================================
--- linux-2.6-git.orig/include/linux/hrtimer.h	2006-02-12 18:33:07.000000000 +0100
+++ linux-2.6-git/include/linux/hrtimer.h	2006-02-12 18:33:21.000000000 +0100
@@ -89,6 +89,7 @@ struct hrtimer_base {
 	ktime_t			resolution;
 	ktime_t			(*get_time)(void);
 	struct hrtimer		*curr_timer;
+	ktime_t			last_expired;
 };
 
 /*
Index: linux-2.6-git/kernel/hrtimer.c
===================================================================
--- linux-2.6-git.orig/kernel/hrtimer.c	2006-02-12 18:33:16.000000000 +0100
+++ linux-2.6-git/kernel/hrtimer.c	2006-02-12 18:33:21.000000000 +0100
@@ -541,7 +541,7 @@ int hrtimer_get_res(clockid_t which_cloc
  */
 static inline void run_hrtimer_queue(struct hrtimer_base *base)
 {
-	ktime_t now = base->get_time();
+	ktime_t now = base->last_expired;
 	struct rb_node *node;
 
 	spin_lock_irq(&base->lock);
@@ -594,10 +594,19 @@ static inline void run_hrtimer_queue(str
 void hrtimer_run_queues(void)
 {
 	struct hrtimer_base *base = __get_cpu_var(hrtimer_bases);
-	int i;
+	ktime_t now, mono;
+	int seq;
 
-	for (i = 0; i < MAX_HRTIMER_BASES; i++)
-		run_hrtimer_queue(&base[i]);
+	do {
+		seq = read_seqbegin(&xtime_lock);
+		now = timespec_to_ktime(xtime);
+		mono = timespec_to_ktime(wall_to_monotonic);
+	} while (read_seqretry(&xtime_lock, seq));
+
+	base[CLOCK_REALTIME].last_expired = now;
+	run_hrtimer_queue(&base[CLOCK_REALTIME]);
+	base[CLOCK_MONOTONIC].last_expired = ktime_add(now, mono);
+	run_hrtimer_queue(&base[CLOCK_MONOTONIC]);
 }
 
 /*

Re: [PATCH 05/13] hrtimer: optimize hrtimer_run_queues

[Ingo Molnar]

* Roman Zippel <zippel@linux-m68k.org> wrote:

> Every time hrtimer_run_queues() is called, get_time() is called twice, 
> which can be quite expensive, just reading xtime is much cheaper and 
> does the same job (at least for the current low resolution timer, for 
> high resolution timer we can something different later). Cache the 
> expiry time in last_expired, so run_hrtimer_queue() doesn't has to 
> calculate it (clock sources usually know when their expired).
> 
> Signed-off-by: Roman Zippel <zippel@linux-m68k.org>
> 
> ---
> 
>  include/linux/hrtimer.h |    1 +
>  kernel/hrtimer.c        |   17 +++++++++++++----
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> Index: linux-2.6-git/include/linux/hrtimer.h
> ===================================================================
> --- linux-2.6-git.orig/include/linux/hrtimer.h	2006-02-12 18:33:07.000000000 +0100
> +++ linux-2.6-git/include/linux/hrtimer.h	2006-02-12 18:33:21.000000000 +0100
> @@ -89,6 +89,7 @@ struct hrtimer_base {
>  	ktime_t			resolution;
>  	ktime_t			(*get_time)(void);
>  	struct hrtimer		*curr_timer;
> +	ktime_t			last_expired;
>  };
>  
>  /*
> Index: linux-2.6-git/kernel/hrtimer.c
> ===================================================================
> --- linux-2.6-git.orig/kernel/hrtimer.c	2006-02-12 18:33:16.000000000 +0100
> +++ linux-2.6-git/kernel/hrtimer.c	2006-02-12 18:33:21.000000000 +0100
> @@ -541,7 +541,7 @@ int hrtimer_get_res(clockid_t which_cloc
>   */
>  static inline void run_hrtimer_queue(struct hrtimer_base *base)
>  {
> -	ktime_t now = base->get_time();
> +	ktime_t now = base->last_expired;
>  	struct rb_node *node;
>  
>  	spin_lock_irq(&base->lock);
> @@ -594,10 +594,19 @@ static inline void run_hrtimer_queue(str
>  void hrtimer_run_queues(void)
>  {
>  	struct hrtimer_base *base = __get_cpu_var(hrtimer_bases);
> -	int i;
> +	ktime_t now, mono;
> +	int seq;
>  
> -	for (i = 0; i < MAX_HRTIMER_BASES; i++)
> -		run_hrtimer_queue(&base[i]);
> +	do {
> +		seq = read_seqbegin(&xtime_lock);
> +		now = timespec_to_ktime(xtime);
> +		mono = timespec_to_ktime(wall_to_monotonic);
> +	} while (read_seqretry(&xtime_lock, seq));
> +
> +	base[CLOCK_REALTIME].last_expired = now;
> +	run_hrtimer_queue(&base[CLOCK_REALTIME]);
> +	base[CLOCK_MONOTONIC].last_expired = ktime_add(now, mono);
> +	run_hrtimer_queue(&base[CLOCK_MONOTONIC]);

hm, we can do this - although the open-coded loop looks ugly. In any 
case, this is an optimization, and not necessary for v2.6.16. It is 
certainly ok for -mm.

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: [PATCH 05/13] hrtimer: optimize hrtimer_run_queues

[Roman Zippel]

Hi,

On Mon, 13 Feb 2006, Ingo Molnar wrote:

> hm, we can do this - although the open-coded loop looks ugly. In any 
> case, this is an optimization, and not necessary for v2.6.16. It is 
> certainly ok for -mm.

I could also call this perfomance regressions to 2.6.15, unless there is 
a good reason not to include them, I'd prefer to see it in 2.6.16.

bye, Roman

Re: [PATCH 05/13] hrtimer: optimize hrtimer_run_queues

[Ingo Molnar]

* Roman Zippel <zippel@linux-m68k.org> wrote:

> > hm, we can do this - although the open-coded loop looks ugly. In any 
> > case, this is an optimization, and not necessary for v2.6.16. It is 
> > certainly ok for -mm.
> 
> I could also call this perfomance regressions to 2.6.15, unless there 
> is a good reason not to include them, I'd prefer to see it in 2.6.16.

can you measure it? This is tricky code, we definitely dont want to 
change it this late in the v2.6.16 cycles, execpt if it's some 
measurable performance issue that users will see. (or if it's some 
regression, which it isnt.)

	Ingo

Re: [PATCH 05/13] hrtimer: optimize hrtimer_run_queues

[Roman Zippel]

Hi,

On Mon, 13 Feb 2006, Ingo Molnar wrote:

> > I could also call this perfomance regressions to 2.6.15, unless there 
> > is a good reason not to include them, I'd prefer to see it in 2.6.16.
> 
> can you measure it? This is tricky code, we definitely dont want to 
> change it this late in the v2.6.16 cycles, execpt if it's some 
> measurable performance issue that users will see. (or if it's some 
> regression, which it isnt.)

Why is not a regression?
I'm busy enough with m68k as is just to catch up and you're not making it 
easier. Such repetitive task have a tendency to show up pretty high when I 
occasionally run an profile test run, e.g. a much simpler vertical blank 
interrupt at 50Hz is noticable. The new hrtimer code is much heavier and 
runs twice as much.

bye, Roman