Commit 6441402b authored by Thomas Gleixner's avatar Thomas Gleixner

clockevents: prevent cpu online to interfere with nohz

Impact: rare hang which can be triggered on CPU online.

tick_do_timer_cpu keeps track of the CPU which updates jiffies
via do_timer. The value -1 is used to signal, that currently no
CPU is doing this. There are two cases, where the variable can 
have this state:

 boot:
    necessary for systems where the boot cpu id can be != 0

 nohz long idle sleep:
    When the CPU which did the jiffies update last goes into
    a long idle sleep it drops the update jiffies duty so
    another CPU which is not idle can pick it up and keep
    jiffies going.

Using the same value for both situations is wrong, as the CPU online
code can see the -1 state when the timer of the newly onlined CPU is
setup. The setup for a newly onlined CPU goes through periodic mode
and can pick up the do_timer duty without being aware of the nohz /
highres mode of the already running system.

Use two separate states and make them constants to avoid magic
numbers confusion. 
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
parent 72d31053
...@@ -33,7 +33,7 @@ DEFINE_PER_CPU(struct tick_device, tick_cpu_device); ...@@ -33,7 +33,7 @@ DEFINE_PER_CPU(struct tick_device, tick_cpu_device);
*/ */
ktime_t tick_next_period; ktime_t tick_next_period;
ktime_t tick_period; ktime_t tick_period;
int tick_do_timer_cpu __read_mostly = -1; int tick_do_timer_cpu __read_mostly = TICK_DO_TIMER_BOOT;
DEFINE_SPINLOCK(tick_device_lock); DEFINE_SPINLOCK(tick_device_lock);
/* /*
...@@ -148,7 +148,7 @@ static void tick_setup_device(struct tick_device *td, ...@@ -148,7 +148,7 @@ static void tick_setup_device(struct tick_device *td,
* If no cpu took the do_timer update, assign it to * If no cpu took the do_timer update, assign it to
* this cpu: * this cpu:
*/ */
if (tick_do_timer_cpu == -1) { if (tick_do_timer_cpu == TICK_DO_TIMER_BOOT) {
tick_do_timer_cpu = cpu; tick_do_timer_cpu = cpu;
tick_next_period = ktime_get(); tick_next_period = ktime_get();
tick_period = ktime_set(0, NSEC_PER_SEC / HZ); tick_period = ktime_set(0, NSEC_PER_SEC / HZ);
...@@ -300,7 +300,8 @@ static void tick_shutdown(unsigned int *cpup) ...@@ -300,7 +300,8 @@ static void tick_shutdown(unsigned int *cpup)
if (*cpup == tick_do_timer_cpu) { if (*cpup == tick_do_timer_cpu) {
int cpu = first_cpu(cpu_online_map); int cpu = first_cpu(cpu_online_map);
tick_do_timer_cpu = (cpu != NR_CPUS) ? cpu : -1; tick_do_timer_cpu = (cpu != NR_CPUS) ? cpu :
TICK_DO_TIMER_NONE;
} }
spin_unlock_irqrestore(&tick_device_lock, flags); spin_unlock_irqrestore(&tick_device_lock, flags);
} }
......
/* /*
* tick internal variable and functions used by low/high res code * tick internal variable and functions used by low/high res code
*/ */
#define TICK_DO_TIMER_NONE -1
#define TICK_DO_TIMER_BOOT -2
DECLARE_PER_CPU(struct tick_device, tick_cpu_device); DECLARE_PER_CPU(struct tick_device, tick_cpu_device);
extern spinlock_t tick_device_lock; extern spinlock_t tick_device_lock;
extern ktime_t tick_next_period; extern ktime_t tick_next_period;
......
...@@ -221,7 +221,7 @@ void tick_nohz_stop_sched_tick(int inidle) ...@@ -221,7 +221,7 @@ void tick_nohz_stop_sched_tick(int inidle)
*/ */
if (unlikely(!cpu_online(cpu))) { if (unlikely(!cpu_online(cpu))) {
if (cpu == tick_do_timer_cpu) if (cpu == tick_do_timer_cpu)
tick_do_timer_cpu = -1; tick_do_timer_cpu = TICK_DO_TIMER_NONE;
} }
if (unlikely(ts->nohz_mode == NOHZ_MODE_INACTIVE)) if (unlikely(ts->nohz_mode == NOHZ_MODE_INACTIVE))
...@@ -303,7 +303,7 @@ void tick_nohz_stop_sched_tick(int inidle) ...@@ -303,7 +303,7 @@ void tick_nohz_stop_sched_tick(int inidle)
* invoked. * invoked.
*/ */
if (cpu == tick_do_timer_cpu) if (cpu == tick_do_timer_cpu)
tick_do_timer_cpu = -1; tick_do_timer_cpu = TICK_DO_TIMER_NONE;
ts->idle_sleeps++; ts->idle_sleeps++;
...@@ -468,7 +468,7 @@ static void tick_nohz_handler(struct clock_event_device *dev) ...@@ -468,7 +468,7 @@ static void tick_nohz_handler(struct clock_event_device *dev)
* this duty, then the jiffies update is still serialized by * this duty, then the jiffies update is still serialized by
* xtime_lock. * xtime_lock.
*/ */
if (unlikely(tick_do_timer_cpu == -1)) if (unlikely(tick_do_timer_cpu == TICK_DO_TIMER_NONE))
tick_do_timer_cpu = cpu; tick_do_timer_cpu = cpu;
/* Check, if the jiffies need an update */ /* Check, if the jiffies need an update */
...@@ -570,7 +570,7 @@ static enum hrtimer_restart tick_sched_timer(struct hrtimer *timer) ...@@ -570,7 +570,7 @@ static enum hrtimer_restart tick_sched_timer(struct hrtimer *timer)
* this duty, then the jiffies update is still serialized by * this duty, then the jiffies update is still serialized by
* xtime_lock. * xtime_lock.
*/ */
if (unlikely(tick_do_timer_cpu == -1)) if (unlikely(tick_do_timer_cpu == TICK_DO_TIMER_NONE))
tick_do_timer_cpu = cpu; tick_do_timer_cpu = cpu;
#endif #endif
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment