On Tue, Mar 16, 2021 at 03:35:37PM +0100, Peter Zijlstra wrote: > On Tue, Mar 16, 2021 at 02:37:03PM +0100, Frederic Weisbecker wrote: > > On Tue, Mar 16, 2021 at 01:21:29PM +0100, Peter Zijlstra wrote: > > > On Thu, Mar 11, 2021 at 01:36:59PM +0100, Frederic Weisbecker wrote: > > > > From: "Zhou Ti (x2019cwm)" <x2019...@stfx.ca> > > > > > > > > If the hardware clock happens to fire its interrupts late, two possible > > > > issues can happen while calling tick_nohz_get_sleep_length(). Either: > > > > > > > > 1) The next clockevent device event is due past the last idle entry > > > > time. > > > > > > > > or: > > > > > > > > 2) The last timekeeping update happened before the last idle entry time > > > > and the next timer callback expires before the last idle entry time. > > > > > > > > Make sure that both cases are handled to avoid returning a negative > > > > duration to the cpuidle governors. > > > > > > Why? ... and wouldn't it be cheaper the fix the caller to > > > check negative once, instead of adding two branches here? > > > > There are already two callers and potentially two return values to check > > for each because the function returns two values. > > > > I'd rather make the API more robust instead of fixing each callers and > > worrying > > about future ones. > > But what's the actual problem? The Changelog doesn't say why returning a > negative value is a problem, and in fact the return value is explicitly > signed. > > Anyway, I don't terribly mind the patch, I was just confused by the lack > of actual justification.
And you're right, the motivation is pure FUD: I don't know exactly how the cpuidle governors may react to such negative values and so this is just to prevent from potential accident. Rafael, does that look harmless to you?
