Messages in this thread | | | From | Joel Fernandes <> | Date | Fri, 14 Jul 2023 21:02:43 -0400 | Subject | Re: [PATCH 3/3] tick/nohz: Don't shutdown the lowres tick from itself |
| |
On Fri, Jul 14, 2023 at 8:01 PM Frederic Weisbecker <frederic@kernel.org> wrote: > > On Fri, Jul 14, 2023 at 02:44:49PM -0400, Joel Fernandes wrote: > > On 7/14/23 08:08, Frederic Weisbecker wrote: > > One slight concern here though, where in the idle loop is the removed > > statement "tick_program_event(KTIME_MAX, 1);" happening if the tick was > > already stopped before? If it is happening in tick_nohz_stop_tick(), don't > > we early return from there and avoid doing that > > "tick_program_event(KTIME_MAX, 1);" altogether, if the tick was already > > stopped and the next event has not changed? > > > > /* Skip reprogram of event if its not changed */ > > if (ts->tick_stopped && (expires == ts->next_tick)) { > > /* Sanity check: make sure clockevent is actually programmed */ > > if (tick == KTIME_MAX || ts->next_tick == [...] > > return; > > [...] > > } > > Sure, if tick_program_event(KTIME_MAX, 1) was already called in the > previous idle loop iteration, then there is no need to call that again. > > Or am I missing something else?
Just take it with a grain of salt but I think you need to still call tick_program_event(KTIME_MAX, 1) here for the case where the tick was previously stopped, and then when the next tick fires (say after a long time T), but that tick is a one-off and does not result in restarting the tick -- then there is no one to call "tick_program_event(KTIME_MAX, 1)".
I think that's the concern Nick was addressing in [1] so that it resets the tick device correctly?
[1] https://lore.kernel.org/lkml/165089105607.4207.3022534114716811208.tip-bot2@tip-bot2/
> > > > Also just a nit, here you can remove indent by doing: > > > > if (unlikely(ts->tick_stopped)) > > return; > > hrtimer_forward(&ts->sched_timer, now, TICK_NSEC); > > tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1); > > > > Which is pretty much the original code except for the tick_program_event(). > > Either I remove an indent or I remove a statement. I guess it's a matter of > personal taste. I don't mind either way :-)
Ah true, in defense of the "less indent" way, the original code was also using that style. ;-) But I am also Ok with either way. :-)
thanks,
- Joel
| |