Re: [PATCH 3/3] tick/nohz: Don't shutdown the lowres tick from itself

[Frederic Weisbecker <frederic@kernel.org>]

On Fri, Jul 14, 2023 at 09:02:43PM -0400, Joel Fernandes wrote:
> 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'm a bit confused about that one-off thing. What can trigger that timer
interrupt if it has been stopped?

One thing can happen though: a pending timer IRQ while we are stopping the
tick (IRQs are disabled in that idle loop portion). But then that pending timer
interrupt is not going to reprogram another one. So it remains stopped.

Thanks.