Re: [PATCH] Call get_time() only when necessary in run_hrtimer_queue

Dimitri Sivanich
2006-03-28T17:55:33Z

On Tue, Mar 28, 2006 at 07:39:08PM +0200, Eric Dumazet wrote:
> >
> >Index: linux/kernel/hrtimer.c
> >===================================================================
> >--- linux.orig/kernel/hrtimer.c 2006-03-27 09:43:40.000000000 -0600
> >+++ linux/kernel/hrtimer.c 2006-03-27 12:35:47.416054373 -0600
> >@@ -604,14 +604,17 @@ int hrtimer_get_res(const clockid_t whic
> > */
> > static inline void run_hrtimer_queue(struct hrtimer_base *base)
> > {
> >- struct rb_node *node;
> >+ struct rb_node *node = base->first;
> >+
> >+ if (!node)
> >+ return;
> >
> > if (base->get_softirq_time)
> > base->softirq_time = base->get_softirq_time();
> >
> > spin_lock_irq(&base->lock);
> >
> >- while ((node = base->first)) {
> >+ while (node) {
>
> Are you sure of this change ?
>
> base->first may have changed just before you locked the base->lock
> (spin_lock_irq(&base->lock);)
>

We could the following simpler change instead:

Signed-off-by: Dimitri Sivanich <sivanich@sgi.com>

Index: linux/kernel/hrtimer.c
===================================================================
--- linux.orig/kernel/hrtimer.c 2006-03-28 11:46:45.279722496 -0600
+++ linux/kernel/hrtimer.c 2006-03-28 11:51:36.722469752 -0600
@@ -606,6 +606,9 @@ static inline void run_hrtimer_queue(str
{
struct rb_node *node;

+ if (!base->first)
+ return;
+
if (base->get_softirq_time)
base->softirq_time = base->get_softirq_time();