Re: [PATCH] sched: idle: Avoid retaining the tick when it has been stopped

On Fri, Aug 10, 2018 at 8:19 AM,  [off-list ref] wrote:

On Thu, Aug 09, 2018 at 07:08:34PM +0200, Rafael J . Wysocki wrote:

quoted

From: Rafael J. Wysocki <redacted>

If the tick has been stopped already, but the governor has not asked to
stop it (which it can do sometimes), the idle loop should invoke
tick_nohz_idle_stop_tick(), to let tick_nohz_stop_tick() take care
of this case properly.

IMHO, I don't think this patch is on the right way;

So we disagree here, quite obviously.

from the idle loop side, it needs to provide sane fundamental supports,
for example, it can stop or restart the tick per idle governor's request.

No, if the tick is stopped, restarting it is pointless until we exit
the loop in do_idle().

On the other hand, the idle governors can decide their own policy for how to
use the tick in idle loop.  This patch seems mixes two things and
finally it's possible to couple the implementation between idle loop
and 'menu' governor for sched tick usage.

I'm not following this, sorry.

I still think my patch to restart the tick is valid :)

It changes the behavior significantly, though, and it is not clear if
the new behavior is desirable.

The patch here simply fixes a problem while leaving the overall behavior as is.

quoted

Fixes: 554c8aa8ecad (sched: idle: Select idle state before stopping the tick)
Signed-off-by: Rafael J. Wysocki <redacted>
---
 kernel/sched/idle.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-pm/kernel/sched/idle.c
===================================================================

--- linux-pm.orig/kernel/sched/idle.c
+++ linux-pm/kernel/sched/idle.c

@@ -190,7 +190,7 @@ static void cpuidle_idle_call(void)
               */
              next_state = cpuidle_select(drv, dev, &stop_tick);

-             if (stop_tick)
+             if (stop_tick || tick_nohz_tick_stopped())
                      tick_nohz_idle_stop_tick();
              else
                      tick_nohz_idle_retain_tick();