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;
Message-ID: <f61fa0cc-eca7-c04a-abf6-5ca5318bd2b8@joelfernandes.org>
Date:   Fri, 14 Jul 2023 14:28:18 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH 1/3] tick/nohz: Rename the tick handlers to more
 self-explanatory names
To:     Frederic Weisbecker <frederic@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>
Cc:     LKML <linux-kernel@vger.kernel.org>
References: <20230714120852.23573-1-frederic@kernel.org>
 <20230714120852.23573-2-frederic@kernel.org>
Content-Language: en-US
From:   Joel Fernandes <joel@joelfernandes.org>
In-Reply-To: <20230714120852.23573-2-frederic@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/14/23 08:08, Frederic Weisbecker wrote:
> The current name of the tick handlers don't tell much about what differ
> between them. Use names that better reflect their role and resolution.
> 
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> ---
>   kernel/time/tick-sched.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 89055050d1ac..a7b5960258f2 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -1355,7 +1355,7 @@ void tick_nohz_idle_exit(void)
>   /*
>    * The nohz low res interrupt handler
>    */
> -static void tick_nohz_handler(struct clock_event_device *dev)
> +static void tick_lowres_handler(struct clock_event_device *dev)

Maybe tick_nohz_lowres_handler() is better?

You also have tick_periodic which is also 'lowres'.

That would also keep the "tick_nohz" prefix which is prevalent in this code.

>   {
>   	struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched);
>   	struct pt_regs *regs = get_irq_regs();
> @@ -1401,7 +1401,7 @@ static void tick_nohz_switch_to_nohz(void)
>   	if (!tick_nohz_enabled)
>   		return;
>   
> -	if (tick_switch_to_oneshot(tick_nohz_handler))
> +	if (tick_switch_to_oneshot(tick_lowres_handler))
>   		return;
>   
>   	/*
> @@ -1464,7 +1464,7 @@ void tick_irq_enter(void)
>    * We rearm the timer until we get disabled by the idle code.
>    * Called with interrupts disabled.
>    */
> -static enum hrtimer_restart tick_sched_timer(struct hrtimer *timer)
> +static enum hrtimer_restart tick_highres_handler(struct hrtimer *timer)

This one looks good.

thanks,

  - Joel


>   {
>   	struct tick_sched *ts =
>   		container_of(timer, struct tick_sched, sched_timer);
> @@ -1513,7 +1513,7 @@ void tick_setup_sched_timer(void)
>   	 * Emulate tick processing via per-CPU hrtimers:
>   	 */
>   	hrtimer_init(&ts->sched_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_HARD);
> -	ts->sched_timer.function = tick_sched_timer;
> +	ts->sched_timer.function = tick_highres_handler;
>   
>   	/* Get the next period (per-CPU) */
>   	hrtimer_set_expires(&ts->sched_timer, tick_init_jiffy_update());