Re: [PATCH 1/1] dynticks/preempt_rt: Fix a nohz_full entry failure in preempt_rt

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2020-12-23 04:20:36 [-0500], ramesh.thomas@intel.com wrote:
> From: Ramesh Thomas <ramesh.thomas@intel.com>
> 
> After all conditions to enter nohz_full state are met, if a timer event
> is detected in the next period, nohz_full entry is aborted in
> tick_nohz_next_event(). In the failure condition, this scenario keeps
> repeating. This has been reproduced in preempt_rt kernel, however it may
> not be limited to it.
 
Which timer is? Why does it make sense to enter NOHZ_FULL if the timer
expires shortly after (in next jiffy window if I understood it
properly).

> Not bailing out if CONFIG_NO_HZ_FULL is defined, fixes the issue. Since
> in NO_HZ_FULL mode, idle state is not entered at tick stoppage, the
> pending timer event can be handled where the next timer is programmed in
> tick_nohz_stop_tick(). This also simplifies the logic by keeping a
> common path to handle the entry/exit of nohz_full state, whether the
> tick was already stopped or not.
> 
> (The entire check for the presence of a timer event in the next period
> can possibly be skipped for NO_HZ_FULL)
> 
> Signed-off-by: Ramesh Thomas <ramesh.thomas@intel.com>
> ---
>  kernel/time/tick-sched.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 308a98c28ee8..3798dbc5f082 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -739,10 +739,12 @@ static ktime_t tick_nohz_next_event(struct tick_sched *ts, int cpu)
>  		 * We've not stopped the tick yet, and there's a timer in the
>  		 * next period, so no point in stopping it either, bail.
>  		 */
> +#ifndef CONFIG_NO_HZ_FULL
>  		if (!ts->tick_stopped) {
>  			ts->timer_expires = 0;
> -			goto out;
> +			return 0;
>  		}
> +#endif
>  	}
>  
>  	/*
> @@ -763,7 +765,6 @@ static ktime_t tick_nohz_next_event(struct tick_sched *ts, int cpu)
>  
>  	ts->timer_expires = min_t(u64, expires, next_tick);
>  
> -out:
>  	return ts->timer_expires;
>  }
>  
> -- 
> 2.26.2
> 

Sebastian