[PATCH v2 2/2] tick/nohz: Remove duplicate between tick_nohz_lowres_handler() and tick_nohz_highres_handler()

[PATCH v2 2/2] tick/nohz: Remove duplicate between tick_nohz_lowres_handler() and tick_nohz_highres_handler()

[Peng Liu]

From: Peng Liu <liupeng17@lenovo.com>

tick_nohz_lowres_handler() does the same work as tick_nohz_highres_handler()
plus the clockevent device reprogramming, so should reuse the latter.

Signed-off-by: Peng Liu <liupeng17@lenovo.com>
---
Changes in v2:
- Fix build warning: Function parameter or member 'mode' not described in 'tick_setup_sched_timer'
- Fix build error: use of undeclared identifier 'tick_nohz_highres_handler'
- Fix build error: use of undeclared identifier 'sched_skew_tick'
---
 kernel/time/tick-sched.c | 28 ++++++----------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 96fcf5cb1b49..c2142b38c31d 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -1383,36 +1383,25 @@ void tick_nohz_idle_exit(void)
 	local_irq_enable();
 }
 
+static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer);
+
 /*
  * In low-resolution mode, the tick handler must be implemented directly
  * at the clockevent level. hrtimer can't be used instead, because its
  * infrastructure actually relies on the tick itself as a backend in
  * low-resolution mode (see hrtimer_run_queues()).
  *
- * This low-resolution handler still makes use of some hrtimer APIs meanwhile
- * for convenience with expiration calculation and forwarding.
+ * This low-resolution handler still reuse tick_nohz_highres_handler() since
+ * most of the work is independent of the clockevent level.
  */
 static void tick_nohz_lowres_handler(struct clock_event_device *dev)
 {
 	struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched);
-	struct pt_regs *regs = get_irq_regs();
-	ktime_t now = ktime_get();
 
 	dev->next_event = KTIME_MAX;
 
-	tick_sched_do_timer(ts, now);
-	tick_sched_handle(ts, regs);
-
-	/*
-	 * In dynticks mode, tick reprogram is deferred:
-	 * - to the idle task if in dynticks-idle
-	 * - to IRQ exit if in full-dynticks.
-	 */
-	if (likely(!ts->tick_stopped)) {
-		hrtimer_forward(&ts->sched_timer, now, TICK_NSEC);
+	if (likely(tick_nohz_highres_handler(&ts->sched_timer) == HRTIMER_RESTART))
 		tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
-	}
-
 }
 
 static inline void tick_nohz_activate(struct tick_sched *ts, int mode)
@@ -1481,10 +1470,7 @@ void tick_irq_enter(void)
 	tick_nohz_irq_enter();
 }
 
-/*
- * High resolution timer specific code
- */
-#ifdef CONFIG_HIGH_RES_TIMERS
+#if defined CONFIG_NO_HZ_COMMON || defined CONFIG_HIGH_RES_TIMERS
 /*
  * We rearm the timer until we get disabled by the idle code.
  * Called with interrupts disabled.
@@ -1519,9 +1505,7 @@ static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer)
 
 	return HRTIMER_RESTART;
 }
-#endif /* HIGH_RES_TIMERS */
 
-#if defined CONFIG_NO_HZ_COMMON || defined CONFIG_HIGH_RES_TIMERS
 static int sched_skew_tick;
 
 static int __init skew_tick(char *str)
-- 
2.34.1

Re: [PATCH v2 2/2] tick/nohz: Remove duplicate between tick_nohz_lowres_handler() and tick_nohz_highres_handler()

[Frederic Weisbecker]

Le Tue, Nov 28, 2023 at 05:24:13PM +0800, Peng Liu a écrit :
> From: Peng Liu <liupeng17@lenovo.com>
> 
> tick_nohz_lowres_handler() does the same work as tick_nohz_highres_handler()
> plus the clockevent device reprogramming, so should reuse the latter.
> 
> Signed-off-by: Peng Liu <liupeng17@lenovo.com>
> ---
> Changes in v2:
> - Fix build warning: Function parameter or member 'mode' not described in 'tick_setup_sched_timer'
> - Fix build error: use of undeclared identifier 'tick_nohz_highres_handler'
> - Fix build error: use of undeclared identifier 'sched_skew_tick'
> ---
>  kernel/time/tick-sched.c | 28 ++++++----------------------
>  1 file changed, 6 insertions(+), 22 deletions(-)
> 
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 96fcf5cb1b49..c2142b38c31d 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -1383,36 +1383,25 @@ void tick_nohz_idle_exit(void)
>  	local_irq_enable();
>  }
>  
> +static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer);

Then please move the entire function in this case. You'll need to move it to
a place where it is available both for CONFIG_NO_HZ_COMMON and
CONFIG_HIGH_RES_TIMERS. Below tick_sched_handle() for example.

Thanks!