Re: [RFC][PATCH] reuse ktime in sub-functions of tick_check_idle.

[Martin Schwidefsky <schwidefsky@de.ibm.com>]

On Sat, 18 Jul 2009 16:14:56 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> 
> > 
> > before:
> > 
> >  0)               |  tick_check_idle() {
> >  0)               |    tick_nohz_stop_idle() {
> >  0)               |      ktime_get() {
> >  0)               |        read_tod_clock() {
> >  0)   0.601 us    |        }
> >  0)   1.765 us    |      }
> >  0)   3.047 us    |    }
> >  0)               |    ktime_get() {
> >  0)               |      read_tod_clock() {
> >  0)   0.570 us    |      }
> >  0)   1.727 us    |    }
> >  0)               |    tick_do_update_jiffies64() {
> >  0)   0.609 us    |    }
> >  0)   8.055 us    |  }
> > 
> > after:
> > 
> >  0)               |  tick_check_idle() {
> >  0)               |    ktime_get() {
> >  0)               |      read_tod_clock() {
> >  0)   0.617 us    |      }
> >  0)   1.773 us    |    }
> >  0)               |    tick_do_update_jiffies64() {
> >  0)   0.593 us    |    }
> >  0)   4.477 us    |  }
> 
> Nice!

Yes, isn't it? I currently looking at the cpu wakeup path and try to
make it faster. The biggest one is probably the ktime_get optimization
but this one seems worthwhile as well.

> > @@ -579,22 +574,18 @@ static void tick_nohz_switch_to_nohz(voi
> >   * timer and do not touch the other magic bits which need to be done
> >   * when idle is left.
> >   */
> > -static void tick_nohz_kick_tick(int cpu)
> > +static void tick_nohz_kick_tick(int cpu, ktime_t now)
> >  {
> >  #if 0
> 
> hm?

You mean the tick_nohz_kick_tick function? Seems like old ballast, I
have no idea who might want to uncomment the #if 0 ever again. But if
they do the function should work, no?
 
> > @@ -614,11 +605,22 @@ static inline void tick_nohz_switch_to_n
> >   */
> >  void tick_check_idle(int cpu)
> >  {
> > +#ifdef CONFIG_NO_HZ
> > +	struct tick_sched *ts;
> > +#endif
> > +
> >  	tick_check_oneshot_broadcast(cpu);
> >  #ifdef CONFIG_NO_HZ
> > -	tick_nohz_stop_idle(cpu);
> > -	tick_nohz_update_jiffies();
> > -	tick_nohz_kick_tick(cpu);
> > +	ts = &per_cpu(tick_cpu_sched, cpu);
> > +	if (ts->idle_active || ts->tick_stopped) {
> > +		ktime_t now = ktime_get();
> > +		if (ts->idle_active)
> > +			tick_nohz_stop_idle(cpu, now);
> > +		if (ts->tick_stopped) {
> > +			tick_nohz_update_jiffies(now);
> > +			tick_nohz_kick_tick(cpu, now);
> > +		}
> > +	}
> >  #endif
> 
> Those ifdefs look quite ugly, dont they?

How about another inline function then:

@@ -603,9 +594,26 @@
 #endif
 }
 
+static inline void tick_check_nohz(int cpu)
+{
+	struct tick_sched *ts = &per_cpu(tick_cpu_sched, cpu);
+	ktime_t now;
+
+	if (!ts->idle_active && !ts->tick_stopped)
+		return;
+	now = ktime_get();
+	if (ts->idle_active)
+		tick_nohz_stop_idle(cpu, now);
+	if (ts->tick_stopped) {
+		tick_nohz_update_jiffies(now);
+		tick_nohz_kick_tick(cpu, now);
+	}
+}
+
 #else
 
 static inline void tick_nohz_switch_to_nohz(void) { }
+static inline void tick_check_nohz(int cpu) { }
 
 #endif /* NO_HZ */
 
@@ -615,11 +623,7 @@
 void tick_check_idle(int cpu)
 {
 	tick_check_oneshot_broadcast(cpu);
-#ifdef CONFIG_NO_HZ
-	tick_nohz_stop_idle(cpu);
-	tick_nohz_update_jiffies();
-	tick_nohz_kick_tick(cpu);
-#endif
+	tick_check_nohz(cpu);
 }
 
 /*


-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.