diff mbox series

[02/15] tick/nohz: Remove duplicate between lowres and highres handlers

Message ID 20240124170459.24850-3-frederic@kernel.org
State New
Headers
Series timers/nohz cleanups and hotplug reorganization |

Commit Message

Frederic Weisbecker Jan. 24, 2024, 5:04 p.m. UTC 
  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
make the former reuse the latter.

Signed-off-by: Peng Liu <liupeng17@lenovo.com>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 kernel/time/tick-sched.c | 96 ++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 58 deletions(-)

Comments

Thomas Gleixner Jan. 25, 2024, 9:32 a.m. UTC | #1 
On Wed, Jan 24 2024 at 18:04, Frederic Weisbecker wrote:
> +/*
> + * We rearm the timer until we get disabled by the idle code.
> + * Called with interrupts disabled.
> + */
> +static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer)

Isn't that a misnomer now?

> +{
> +	struct tick_sched *ts =
> +		container_of(timer, struct tick_sched, sched_timer);

Let it stick out please.

> +	struct pt_regs *regs = get_irq_regs();
> +	ktime_t now = ktime_get();
> +	if (likely(tick_nohz_highres_handler(&ts->sched_timer) == HRTIMER_RESTART))
>  		tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);

Thanks,

        tglx
Frederic Weisbecker Jan. 25, 2024, 11:58 a.m. UTC | #2 
On Thu, Jan 25, 2024 at 10:32:09AM +0100, Thomas Gleixner wrote:
> On Wed, Jan 24 2024 at 18:04, Frederic Weisbecker wrote:
> > +/*
> > + * We rearm the timer until we get disabled by the idle code.
> > + * Called with interrupts disabled.
> > + */
> > +static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer)
> 
> Isn't that a misnomer now?

Would tick_nohz_hrtimer_handler() make more sense then? Because it's clearly
an hrtimer handler, just emulated in low-res mode.

> 
> > +{
> > +	struct tick_sched *ts =
> > +		container_of(timer, struct tick_sched, sched_timer);
> 
> Let it stick out please.

Ok.

Thanks.
> 
> > +	struct pt_regs *regs = get_irq_regs();
> > +	ktime_t now = ktime_get();
> > +	if (likely(tick_nohz_highres_handler(&ts->sched_timer) == HRTIMER_RESTART))
> >  		tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
> 
> Thanks,
> 
>         tglx
Thomas Gleixner Jan. 25, 2024, 1:30 p.m. UTC | #3 
On Thu, Jan 25 2024 at 12:58, Frederic Weisbecker wrote:

> On Thu, Jan 25, 2024 at 10:32:09AM +0100, Thomas Gleixner wrote:
>> On Wed, Jan 24 2024 at 18:04, Frederic Weisbecker wrote:
>> > +/*
>> > + * We rearm the timer until we get disabled by the idle code.
>> > + * Called with interrupts disabled.
>> > + */
>> > +static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer)
>> 
>> Isn't that a misnomer now?
>
> Would tick_nohz_hrtimer_handler() make more sense then? Because it's clearly
> an hrtimer handler, just emulated in low-res mode.

Kinda. tick_nohz_handler() would be sufficient too.
diff mbox series

Patch

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index e90dbb7ae70a..6f69ae88e47e 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -255,6 +255,41 @@  static void tick_sched_handle(struct tick_sched *ts, struct pt_regs *regs)
 	update_process_times(user_mode(regs));
 	profile_tick(CPU_PROFILING);
 }
+
+/*
+ * We rearm the timer until we get disabled by the idle code.
+ * Called with interrupts disabled.
+ */
+static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer)
+{
+	struct tick_sched *ts =
+		container_of(timer, struct tick_sched, sched_timer);
+	struct pt_regs *regs = get_irq_regs();
+	ktime_t now = ktime_get();
+
+	tick_sched_do_timer(ts, now);
+
+	/*
+	 * Do not call when we are not in IRQ context and have
+	 * no valid 'regs' pointer
+	 */
+	if (regs)
+		tick_sched_handle(ts, regs);
+	else
+		ts->next_tick = 0;
+
+	/*
+	 * In dynticks mode, tick reprogram is deferred:
+	 * - to the idle task if in dynticks-idle
+	 * - to IRQ exit if in full-dynticks.
+	 */
+	if (unlikely(ts->tick_stopped))
+		return HRTIMER_NORESTART;
+
+	hrtimer_forward(timer, now, TICK_NSEC);
+
+	return HRTIMER_RESTART;
+}
 #endif
 
 #ifdef CONFIG_NO_HZ_FULL
@@ -1392,30 +1427,17 @@  void tick_nohz_idle_exit(void)
  * 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 from 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)
@@ -1484,46 +1506,6 @@  void tick_irq_enter(void)
 	tick_nohz_irq_enter();
 }
 
-/*
- * High resolution timer specific code
- */
-#ifdef CONFIG_HIGH_RES_TIMERS
-/*
- * We rearm the timer until we get disabled by the idle code.
- * Called with interrupts disabled.
- */
-static enum hrtimer_restart tick_nohz_highres_handler(struct hrtimer *timer)
-{
-	struct tick_sched *ts =
-		container_of(timer, struct tick_sched, sched_timer);
-	struct pt_regs *regs = get_irq_regs();
-	ktime_t now = ktime_get();
-
-	tick_sched_do_timer(ts, now);
-
-	/*
-	 * Do not call when we are not in IRQ context and have
-	 * no valid 'regs' pointer
-	 */
-	if (regs)
-		tick_sched_handle(ts, regs);
-	else
-		ts->next_tick = 0;
-
-	/*
-	 * In dynticks mode, tick reprogram is deferred:
-	 * - to the idle task if in dynticks-idle
-	 * - to IRQ exit if in full-dynticks.
-	 */
-	if (unlikely(ts->tick_stopped))
-		return HRTIMER_NORESTART;
-
-	hrtimer_forward(timer, now, TICK_NSEC);
-
-	return HRTIMER_RESTART;
-}
-#endif /* HIGH_RES_TIMERS */
-
 #if defined CONFIG_NO_HZ_COMMON || defined CONFIG_HIGH_RES_TIMERS
 static int sched_skew_tick;
 
@@ -1545,10 +1527,8 @@  void tick_setup_sched_timer(int mode)
 
 	/* Emulate tick processing via per-CPU hrtimers: */
 	hrtimer_init(&ts->sched_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_HARD);
-#ifdef CONFIG_HIGH_RES_TIMERS
 	if (mode == NOHZ_MODE_HIGHRES)
 		ts->sched_timer.function = tick_nohz_highres_handler;
-#endif
 
 	/* Get the next period (per-CPU) */
 	hrtimer_set_expires(&ts->sched_timer, tick_init_jiffy_update());