On Thu, Dec 29, 2016 at 05:42:48PM +0100, Thomas Gleixner wrote:
> On Sat, 24 Dec 2016, Frederic Weisbecker wrote:
> > 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;
>
> Good catch!
>
> >
> > /*
> > @@ -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);
>
> This is wrong. If the next event is earlier than the next estimated tick
> then tick_nohz_get_sleep_length() will return crap and the idle governor
> will go into a deeper C-state than sensible.
Ah I see, the governor wants to know about the next timer, whether it is the
tick
or not, right? I'll fix that and improve the comment along.
Thanks.
