Re: [PATCH 0/2] nohz: Deal with clock reprogram skipping issues v3

[Frederic Weisbecker]

On Wed, May 24, 2017 at 09:16:28AM +0200, Ingo Molnar wrote:
> So the interdiff between your two patches and the 3 commits already queued up is:
> 
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index e3043873fcdc..30253ed0380b 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -150,12 +150,6 @@ static void tick_sched_handle(struct tick_sched *ts, struct pt_regs *regs)
>  		touch_softlockup_watchdog_sched();
>  		if (is_idle_task(current))
>  			ts->idle_jiffies++;
> -		/*
> -		 * In case the current tick fired too early past its expected
> -		 * expiration, make sure we don't bypass the next clock reprogramming
> -		 * to the same deadline.
> -		 */
> -		ts->next_tick = 0;
>  	}
>  #endif
>  	update_process_times(user_mode(regs));
> @@ -1103,8 +1097,15 @@ static void tick_nohz_handler(struct clock_event_device *dev)
>  	tick_sched_handle(ts, regs);
>  
>  	/* No need to reprogram if we are running tickless  */
> -	if (unlikely(ts->tick_stopped))
> +	if (unlikely(ts->tick_stopped)) {
> +		/*
> +		 * In case the current tick fired too early past its expected
> +		 * expiration, make sure we don't bypass the next clock reprogramming
> +		 * to the same deadline.
> +		 */
> +		ts->next_tick = 0;
>  		return;
> +	}
>  
>  	hrtimer_forward(&ts->sched_timer, now, tick_period);
>  	tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
> @@ -1202,12 +1203,17 @@ static enum hrtimer_restart tick_sched_timer(struct hrtimer *timer)
>  	 */
>  	if (regs)
>  		tick_sched_handle(ts, regs);
> -	else
> -		ts->next_tick = 0;
>  
>  	/* No need to reprogram if we are in idle or full dynticks mode */
> -	if (unlikely(ts->tick_stopped))
> +	if (unlikely(ts->tick_stopped)) {
> +		/*
> +		 * In case the current tick fired too early past its expected
> +		 * expiration, make sure we don't bypass the next clock reprogramming
> +		 * to the same deadline.
> +		 */
> +		ts->next_tick = 0;
>  		return HRTIMER_NORESTART;
> +	}
>  
>  	hrtimer_forward(timer, now, tick_period);
>  
> 
> ... so the two are not the same - I'd rather not rebase it, I'd like to keep what 
> is working, we had problems with these changes before ...
> 
> If you'd like the changes in this interdiff to be applied as well, please add a 
> changelog to it and post it as a fourth patch.
> 
> Thanks,
> 
> 	Ingo

So if you like, you can replace the top patch with the following. It's exactly
the same code, I've only added a comment and a changelog:

---