2016-12-25 0:15 GMT+08:00 Frederic Weisbecker <fweis...@gmail.com>:
> When the tick is stopped and an interrupt occurs afterward, we check on
> that interrupt exit if the next tick needs to be rescheduled. If it
> doesn't need any update, we don't want to do anything.
>
> In order to check if the tick needs an update, we compare it against the
> clockevent device deadline. Now that's a problem because the clockevent
> device is at a lower level than the tick itself if it is implemented
> on top of hrtimer.
>
> Every hrtimer share this clockevent device. So comparing the next tick
> deadline against the clockevent device deadline is wrong because the
> device may be programmed for another hrtimer whose deadline collides
> with the tick. As a result we may end up not reprogramming the tick
> accidentally.
>
> In a worst case scenario under full dynticks mode, the tick stops firing
> as it is supposed to every 1hz, leaving /proc/stat stalled:
>
> Task in a full dynticks CPU
> ----------------------------
>
> * hrtimer A is queued 2 seconds ahead
> * the tick is stopped, scheduled 1 second ahead
> * tick fires 1 second later
> * on tick exit, nohz schedules the tick 1 second ahead but sees
> the clockevent device is already programmed to that deadline,
> fooled by hrtimer A, the tick isn't rescheduled.
> * hrtimer A is cancelled before its deadline
> * tick never fires again until an interrupt happens...
>
> In order to fix this, store the next tick deadline to the tick_sched
> local structure and reuse that value later to check whether we need to
> reprogram the clock after an interrupt.
>
> Reported-by: James Hartsock <hart...@redhat.com>
> Cc: Ingo Molnar <mi...@kernel.org>
> Cc: Rik van Riel <r...@redhat.com>
> Cc: Peter Zijlstra <pet...@infradead.org>
> Cc: Thomas Gleixner <t...@linutronix.de>
> Cc: James Hartsock <hart...@redhat.com>
> Signed-off-by: Frederic Weisbecker <fweis...@gmail.com>
Reviewed-by: Wanpeng Li <wanpeng...@hotmail.com>
> ---
> kernel/time/tick-sched.c | 7 ++++---
> kernel/time/tick-sched.h | 2 ++
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 71496a2..85d0688 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -661,7 +661,6 @@ static void tick_nohz_restart(struct tick_sched *ts,
> ktime_t now)
> static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
> ktime_t now, int cpu)
> {
> - struct clock_event_device *dev =
> __this_cpu_read(tick_cpu_device.evtdev);
> u64 basemono, next_tick, next_tmr, next_rcu, delta, expires;
> unsigned long seq, basejiff;
> ktime_t tick;
> @@ -767,7 +766,7 @@ static ktime_t tick_nohz_stop_sched_tick(struct
> tick_sched *ts,
> tick.tv64 = expires;
>
> /* Skip reprogram of event if its not changed */
> - if (ts->tick_stopped && (expires == dev->next_event.tv64))
> + if (ts->tick_stopped && (expires == ts->next_tick.tv64))
> goto out;
>
> /*
> @@ -787,6 +786,8 @@ static ktime_t tick_nohz_stop_sched_tick(struct
> tick_sched *ts,
> trace_tick_stop(1, TICK_DEP_MASK_NONE);
> }
>
> + ts->next_tick = tick;
> +
> /*
> * If the expiration time == KTIME_MAX, then we simply stop
> * the tick timer.
> @@ -803,7 +804,7 @@ static ktime_t tick_nohz_stop_sched_tick(struct
> tick_sched *ts,
> tick_program_event(tick, 1);
> out:
> /* Update the estimated sleep length */
> - ts->sleep_length = ktime_sub(dev->next_event, now);
> + ts->sleep_length = ktime_sub(ts->next_tick, now);
> return tick;
> }
>
> diff --git a/kernel/time/tick-sched.h b/kernel/time/tick-sched.h
> index bf38226..075444e 100644
> --- a/kernel/time/tick-sched.h
> +++ b/kernel/time/tick-sched.h
> @@ -27,6 +27,7 @@ enum tick_nohz_mode {
> * timer is modified for nohz sleeps. This is necessary
> * to resume the tick timer operation in the timeline
> * when the CPU returns from nohz sleep.
> + * @next_tick: Next tick to be fired when in dynticks mode.
> * @tick_stopped: Indicator that the idle tick has been stopped
> * @idle_jiffies: jiffies at the entry to idle for idle time accounting
> * @idle_calls: Total number of idle calls
> @@ -44,6 +45,7 @@ struct tick_sched {
> unsigned long check_clocks;
> enum tick_nohz_mode nohz_mode;
> ktime_t last_tick;
> + ktime_t next_tick;
> int inidle;
> int tick_stopped;
> unsigned long idle_jiffies;
> --
> 2.7.4
>
