[PATCH v2] tick: shift tick_nohz_switch_to_nohz() from tick_check_oneshot_change() to hrtimer_run_queues()

From: Oleg Nesterov
Date: Sun Jun 02 2024 - 06:21:52 EST


hrtimer_run_queues() calls tick_check_oneshot_change() to check if we
can switch to highres or nohz mode, but the current code looks very
confusing to me. In the highres=n or CONFIG_HIGH_RES_TIMERS=n cases
tick_check_oneshot_change() itself calls tick_nohz_switch_to_nohz()
and returns zero; that is why it needs the "allow_nohz" argument,
which imo also adds confusion.

This patch turns tick_check_oneshot_change() into a "pure" function
without arguments. hrtimer_run_queues() calls hrtimer_switch_to_hres()
or tick_nohz_switch_to_nohz() depending on hrtimer_is_hres_enabled().

Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
---
v2: fix build failure without CONFIG_NO_HZ_COMMON (thanks Frederic)
---
kernel/time/hrtimer.c | 7 +++++--
kernel/time/tick-internal.h | 6 ++++--
kernel/time/tick-sched.c | 15 ++++-----------
3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 492c14aac642..806f352b095d 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1891,8 +1891,11 @@ void hrtimer_run_queues(void)
* there only sets the check bit in the tick_oneshot code,
* otherwise we might deadlock vs. xtime_lock.
*/
- if (tick_check_oneshot_change(!hrtimer_is_hres_enabled())) {
- hrtimer_switch_to_hres();
+ if (tick_check_oneshot_change()) {
+ if (hrtimer_is_hres_enabled())
+ hrtimer_switch_to_hres();
+ else
+ tick_nohz_switch_to_nohz();
return;
}

diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 5f2105e637bd..961dca323649 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -111,7 +111,7 @@ extern void tick_resume_oneshot(void);
static inline bool tick_oneshot_possible(void) { return true; }
extern int tick_oneshot_mode_active(void);
extern void tick_clock_notify(void);
-extern int tick_check_oneshot_change(int allow_nohz);
+extern int tick_check_oneshot_change(void);
extern int tick_init_highres(void);
#else /* !CONFIG_TICK_ONESHOT: */
static inline
@@ -124,7 +124,7 @@ static inline void tick_oneshot_notify(void) { }
static inline bool tick_oneshot_possible(void) { return false; }
static inline int tick_oneshot_mode_active(void) { return 0; }
static inline void tick_clock_notify(void) { }
-static inline int tick_check_oneshot_change(int allow_nohz) { return 0; }
+static inline int tick_check_oneshot_change(void) { return 0; }
#endif /* !CONFIG_TICK_ONESHOT */

/* Functions related to oneshot broadcasting */
@@ -157,6 +157,7 @@ static inline void tick_nohz_init(void) { }
#endif

#ifdef CONFIG_NO_HZ_COMMON
+extern void tick_nohz_switch_to_nohz(void);
extern unsigned long tick_nohz_active;
extern void timers_update_nohz(void);
extern u64 get_jiffies_update(unsigned long *basej);
@@ -171,6 +172,7 @@ extern bool timer_base_is_idle(void);
extern void timer_expire_remote(unsigned int cpu);
# endif
#else /* CONFIG_NO_HZ_COMMON */
+static inline void tick_nohz_switch_to_nohz(void) { }
static inline void timers_update_nohz(void) { }
#define tick_nohz_active (0)
#endif
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 71a792cd8936..4fd70be50b7f 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -1514,7 +1514,7 @@ static inline void tick_nohz_activate(struct tick_sched *ts)
/**
* tick_nohz_switch_to_nohz - switch to NOHZ mode
*/
-static void tick_nohz_switch_to_nohz(void)
+void tick_nohz_switch_to_nohz(void)
{
if (!tick_nohz_enabled)
return;
@@ -1552,7 +1552,6 @@ static inline void tick_nohz_irq_enter(void)

#else

-static inline void tick_nohz_switch_to_nohz(void) { }
static inline void tick_nohz_irq_enter(void) { }
static inline void tick_nohz_activate(struct tick_sched *ts) { }

@@ -1670,11 +1669,9 @@ void tick_oneshot_notify(void)
* Check if a change happened, which makes oneshot possible.
*
* Called cyclically from the hrtimer softirq (driven by the timer
- * softirq). 'allow_nohz' signals that we can switch into low-res NOHZ
- * mode, because high resolution timers are disabled (either compile
- * or runtime). Called with interrupts disabled.
+ * softirq). Called with interrupts disabled.
*/
-int tick_check_oneshot_change(int allow_nohz)
+int tick_check_oneshot_change(void)
{
struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched);

@@ -1687,9 +1684,5 @@ int tick_check_oneshot_change(int allow_nohz)
if (!timekeeping_valid_for_hres() || !tick_is_oneshot_available())
return 0;

- if (!allow_nohz)
- return 1;
-
- tick_nohz_switch_to_nohz();
- return 0;
+ return 1;
}
--
2.25.1.362.g51ebf55