Re: The tick is active on idle adaptive-tick CPUs when /dev/cpu_dma_latency is used

[Thomas Lindroth]

On 7/10/19 12:22 PM, Rafael J. Wysocki wrote:
> On Saturday, July 6, 2019 3:02:11 PM CEST Thomas Lindroth wrote:
> > On 7/6/19 1:06 PM, Rafael J. Wysocki wrote:
> > > The patch is below, but note that it adds the tick stopping overhead to the idle loop
> > > for CPUs that are not adaptive-tick and when PM QoS latency constraints are used
> > > which is not desirable in general.
> > >
> > > Please test it, but as I said above, the real solution appears to be to treat adaptive-tick
> > > CPUs in a special way in the idle loop.
> > >
> > > ---
> > >    drivers/cpuidle/governors/menu.c |   16 +++++-----------
> > >    1 file changed, 5 insertions(+), 11 deletions(-)
> > >
> > > Index: linux-pm/drivers/cpuidle/governors/menu.c
> > > ===================================================================
> > > --- linux-pm.orig/drivers/cpuidle/governors/menu.c
> > > +++ linux-pm/drivers/cpuidle/governors/menu.c
> > > @@ -302,9 +302,10 @@ static int menu_select(struct cpuidle_dr
> > >    	     !drv->states[0].disabled && !dev->states_usage[0].disable)) {
> > >    		/*
> > >    		 * In this case state[0] will be used no matter what, so return
> > > -		 * it right away and keep the tick running.
> > > +		 * it right away and keep the tick running if state[0] is a
> > > +		 * polling one.
> > >    		 */
> > > -		*stop_tick = false;
> > > +		*stop_tick = !!(drv->states[0].flags & CPUIDLE_FLAG_POLLING);
> > >    		return 0;
> > >    	}
> > >    
> > > @@ -395,16 +396,9 @@ static int menu_select(struct cpuidle_dr
> > >    
> > >    			return idx;
> > >    		}
> > > -		if (s->exit_latency > latency_req) {
> > > -			/*
> > > -			 * If we break out of the loop for latency reasons, use
> > > -			 * the target residency of the selected state as the
> > > -			 * expected idle duration so that the tick is retained
> > > -			 * as long as that target residency is low enough.
> > > -			 */
> > > -			predicted_us = drv->states[idx].target_residency;
> > > +		if (s->exit_latency > latency_req)
> > >    			break;
> > > -		}
> > > +
> > >    		idx = i;
> > >    	}
> >
> > I tested the patch and it appears to work. Idle CPUs now have ticks disabled even
> > when /dev/cpu_dma_latency is used.
>
> OK, thanks, but as I said previously, you'd see the problem again with the PM QoS
> latency constraint set to 0, which is somewhat inconsistent.  Also, this fix is
> specific to the menu governor and the behavior should not depend on the
> governor here IMO, so I have another patch to try (appended).
>
> Please test it (instead of the previous one) and report back.
>
> ---
>   kernel/sched/idle.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux-pm/kernel/sched/idle.c
> ===================================================================
> --- linux-pm.orig/kernel/sched/idle.c
> +++ linux-pm/kernel/sched/idle.c
> @@ -191,7 +191,8 @@ static void cpuidle_idle_call(void)
>   		 */
>   		next_state = cpuidle_select(drv, dev, &stop_tick);
>   
> -		if (stop_tick || tick_nohz_tick_stopped())
> +		if (stop_tick || tick_nohz_tick_stopped() ||
> +		    !housekeeping_cpu(dev->cpu, HK_FLAG_TICK))
>   			tick_nohz_idle_stop_tick();
>   		else
>   			tick_nohz_idle_retain_tick();

I tested this patch and it seems to work fine using the menu governor
and PM QoS latency constraints matching each C-state including 0.
I didn't test the TEO governor.