On Thu, Aug 09, 2018 at 12:45:49PM +0200, Peter Zijlstra wrote:
> On Thu, Aug 09, 2018 at 01:47:27PM +0800, Leo Yan wrote:
> > diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> > index 1a3e9bd..802286e 100644
> > --- a/kernel/sched/idle.c
> > +++ b/kernel/sched/idle.c
> > @@ -190,10 +190,18 @@ static void cpuidle_idle_call(void)
> > */
> > next_state = cpuidle_select(drv, dev, &stop_tick);
> >
> > - if (stop_tick)
> > + if (stop_tick) {
> > tick_nohz_idle_stop_tick();
> > - else
> > + } else {
> > + /*
> > + * The cpuidle framework says to not stop tick but
> > + * the tick has been stopped yet, so restart it.
> > + */
> > + if (tick_nohz_tick_stopped())
> > + tick_nohz_idle_restart_tick();
> > +
>
> I suspect you want an 'else' here. restart_tick already calls
> timer_clear_idle().
No, from the testing I found must call retain_tick, otherwise the
kernel compliants the warning from tick_nohz_idle_exit() when exit
from idle state:
WARN_ON_ONCE(ts->timer_expires_base);
> > tick_nohz_idle_retain_tick();
> > + }
> >
>
> However, I would rather we stuff all this into retain_tick.
Ah, yes; I tested below change and it also have same improvement for
idle state with my preivous change; please review if it's okay?
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index da9455a..fd2bfad 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -962,6 +962,10 @@ void tick_nohz_idle_stop_tick(void)
void tick_nohz_idle_retain_tick(void)
{
+ /* Restart the tikc if it has been stopped yet. */
+ if (tick_nohz_tick_stopped())
+ tick_nohz_idle_restart_tick();
+
tick_nohz_retain_tick(this_cpu_ptr(&tick_cpu_sched));
/*
* Undo the effect of get_next_timer_interrupt() called from
Thanks,
Leo Yan
