timers: Sanitize catchup_timer_jiffies() usage
authorThomas Gleixner <tglx@linutronix.de>
Tue, 26 May 2015 22:50:24 +0000 (22:50 +0000)
committerThomas Gleixner <tglx@linutronix.de>
Fri, 19 Jun 2015 13:18:27 +0000 (15:18 +0200)
catchup_timer_jiffies() has been applied blindly to several functions
without looking for possible better ways to do it.

1) internal_add_timer()

   Move the update to base->all_timers before we actually insert the
   timer into the wheel.

2) detach_if_pending()

   Again the update to base->all_timers allows us to explicitely do
   the timer_jiffies update in place, if this was the last timer which
   got removed.

3) __run_timers()

   We only check on entry, which is silly, because base->timer_jiffies
   can be behind - especially on NOHZ kernels - and if there is a
   single deferrable timer somewhere between base->timer_jiffies and
   jiffies we expire it and then loop until base->timer_jiffies ==
   jiffies.

   Move it into the loop.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Paul McKenney <paulmck@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Joonwoo Park <joonwoop@codeaurora.org>
Cc: Wenbo Wang <wenbo.wang@memblaze.com>
Link: http://lkml.kernel.org/r/20150526224511.662994644@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
kernel/time/timer.c

index 7775d454a204fecc1cdf31bc9dd53079638ce85e..d5e017970ac7d93a978b0abf4f60482c5912ff6e 100644 (file)
@@ -351,20 +351,6 @@ void set_timer_slack(struct timer_list *timer, int slack_hz)
 }
 EXPORT_SYMBOL_GPL(set_timer_slack);
 
-/*
- * If the list is empty, catch up ->timer_jiffies to the current time.
- * The caller must hold the tvec_base lock.  Returns true if the list
- * was empty and therefore ->timer_jiffies was updated.
- */
-static bool catchup_timer_jiffies(struct tvec_base *base)
-{
-       if (!base->all_timers) {
-               base->timer_jiffies = jiffies;
-               return true;
-       }
-       return false;
-}
-
 static void
 __internal_add_timer(struct tvec_base *base, struct timer_list *timer)
 {
@@ -411,7 +397,10 @@ __internal_add_timer(struct tvec_base *base, struct timer_list *timer)
 
 static void internal_add_timer(struct tvec_base *base, struct timer_list *timer)
 {
-       (void)catchup_timer_jiffies(base);
+       /* Advance base->jiffies, if the base is empty */
+       if (!base->all_timers++)
+               base->timer_jiffies = jiffies;
+
        __internal_add_timer(base, timer);
        /*
         * Update base->active_timers and base->next_timer
@@ -421,7 +410,6 @@ static void internal_add_timer(struct tvec_base *base, struct timer_list *timer)
                    time_before(timer->expires, base->next_timer))
                        base->next_timer = timer->expires;
        }
-       base->all_timers++;
 
        /*
         * Check whether the other CPU is in dynticks mode and needs
@@ -718,7 +706,6 @@ detach_expired_timer(struct timer_list *timer, struct tvec_base *base)
        if (!tbase_get_deferrable(timer->base))
                base->active_timers--;
        base->all_timers--;
-       (void)catchup_timer_jiffies(base);
 }
 
 static int detach_if_pending(struct timer_list *timer, struct tvec_base *base,
@@ -733,8 +720,9 @@ static int detach_if_pending(struct timer_list *timer, struct tvec_base *base,
                if (timer->expires == base->next_timer)
                        base->next_timer = base->timer_jiffies;
        }
-       base->all_timers--;
-       (void)catchup_timer_jiffies(base);
+       /* If this was the last timer, advance base->jiffies */
+       if (!--base->all_timers)
+               base->timer_jiffies = jiffies;
        return 1;
 }
 
@@ -1184,14 +1172,18 @@ static inline void __run_timers(struct tvec_base *base)
        struct timer_list *timer;
 
        spin_lock_irq(&base->lock);
-       if (catchup_timer_jiffies(base)) {
-               spin_unlock_irq(&base->lock);
-               return;
-       }
+
        while (time_after_eq(jiffies, base->timer_jiffies)) {
                struct list_head work_list;
                struct list_head *head = &work_list;
-               int index = base->timer_jiffies & TVR_MASK;
+               int index;
+
+               if (!base->all_timers) {
+                       base->timer_jiffies = jiffies;
+                       break;
+               }
+
+               index = base->timer_jiffies & TVR_MASK;
 
                /*
                 * Cascade timers: