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

[Ingo Molnar <mingo@elte.hu>]

* 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