Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Joel Fernandes <joel@joelfernandes.org>
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH 3/3] tick/nohz: Don't shutdown the lowres tick from itself
Date:   Sat, 15 Jul 2023 18:31:24 -0400
Message-Id: <D077B8C8-41EB-4643-99A7-CA54F314218F@joelfernandes.org>
References: <ZLLjEVxM+kf84vgI@lothringen>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        LKML <linux-kernel@vger.kernel.org>
In-Reply-To: <ZLLjEVxM+kf84vgI@lothringen>
To:     Frederic Weisbecker <frederic@kernel.org>
Precedence: bulk


> On Jul 15, 2023, at 2:19 PM, Frederic Weisbecker <frederic@kernel.org> wro=
te:
>=20
> =EF=BB=BFOn Fri, Jul 14, 2023 at 09:02:43PM -0400, Joel Fernandes wrote:
>>> On Fri, Jul 14, 2023 at 8:01=E2=80=AFPM Frederic Weisbecker <frederic@ke=
rnel.org> wrote:
>>>=20
>>> 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(), do=
n'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?
>>>>=20
>>>>        /* Skip reprogram of event if its not changed */
>>>>        if (ts->tick_stopped && (expires =3D=3D ts->next_tick)) {
>>>>                /* Sanity check: make sure clockevent is actually progra=
mmed */
>>>>                if (tick =3D=3D KTIME_MAX || ts->next_tick =3D=3D  [...]=

>>>>                        return;
>>>>              [...]
>>>>      }
>>>=20
>>> 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.
>>>=20
>>> Or am I missing something else?
>>=20
>> 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)".
>=20
> I'm a bit confused about that one-off thing. What can trigger that timer
> interrupt if it has been stopped?

It is the tick that has been stopped in this scenario.
The timer could still be armed to serve a future hrtimer?

I think the naming in the code for is a bit confusing for tick vs timer even=
t,
so I could be the confused one.

Thanks,

 - Joel


> 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 t=
imer
> interrupt is not going to reprogram another one. So it remains stopped.
>=20
> Thanks.