Received-SPF: pass (google.com: domain of linux-kernel+bounces-6955-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 20 Dec 2023 14:27:43 +0100
From: Frederic Weisbecker <frederic@kernel.org>
To: Anna-Maria Behnsen <anna-maria@linutronix.de>
Cc: linux-kernel@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>,
	John Stultz <jstultz@google.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Eric Dumazet <edumazet@google.com>,
	"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
	Arjan van de Ven <arjan@infradead.org>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	Rik van Riel <riel@surriel.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sebastian Siewior <bigeasy@linutronix.de>,
	Giovanni Gherdovich <ggherdovich@suse.cz>,
	Lukasz Luba <lukasz.luba@arm.com>,
	"Gautham R . Shenoy" <gautham.shenoy@amd.com>,
	Srinivas Pandruvada <srinivas.pandruvada@intel.com>,
	K Prateek Nayak <kprateek.nayak@amd.com>
Subject: Re: [PATCH v9 03/32] tick-sched: Warn when next tick seems to be in
 the past
Message-ID: <ZYLrz24qc8VQ_BIc@localhost.localdomain>
References: <20231201092654.34614-1-anna-maria@linutronix.de>
 <20231201092654.34614-4-anna-maria@linutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20231201092654.34614-4-anna-maria@linutronix.de>

Le Fri, Dec 01, 2023 at 10:26:25AM +0100, Anna-Maria Behnsen a ?crit :
> When the next tick is in the past, the delta between basemono and the next
> tick gets negativ. But the next tick should never be in the past. The
> negative effect of a wrong next tick might be a stop of the tick and timers
> might expire late.
> 
> To prevent expensive debugging when changing underlying code, add a
> WARN_ON_ONCE into this code path. To prevent complete misbehaviour, also
> reset next_tick to basemono in this case.
> 
> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
> ---
> v9: Add reset of next_tick to basemono
> ---
>  kernel/time/tick-sched.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 89517cfb6510..b1b591de781e 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -839,6 +839,10 @@ static ktime_t tick_nohz_next_event(struct tick_sched *ts, int cpu)
>  		ts->next_timer = next_tick;
>  	}
>  
> +	/* Make sure next_tick is never before basemono! */
> +	if (WARN_ON_ONCE(basemono > next_tick))
> +		next_tick = basemono;
> +

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

And some food for thoughts:

1) Is it possible for hrtimer_get_next_event() to return values in the past?
2) Is hrtimer_get_next_event() unconditionally locking &cpu_base->lock even
   high resolution is active? Can we avoid that?

Thanks.


>  	/*
>  	 * If the tick is due in the next period, keep it ticking or
>  	 * force prod the timer.
> -- 
> 2.39.2
>