Received-SPF: pass (google.com: domain of linux-kernel+bounces-198194-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Sun, 2 Jun 2024 12:20:07 +0200
From: Oleg Nesterov <oleg@redhat.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Anna-Maria Behnsen <anna-maria@linutronix.de>,
	Frederic Weisbecker <frederic@kernel.org>,
	Ingo Molnar <mingo@kernel.org>, linux-kernel@vger.kernel.org
Subject: [PATCH v2] tick: shift tick_nohz_switch_to_nohz() from
 tick_check_oneshot_change() to hrtimer_run_queues()
Message-ID: <20240602102007.GA27454@redhat.com>
References: <20240530124203.GA26990@redhat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240530124203.GA26990@redhat.com>
User-Agent: Mutt/1.5.24 (2015-08-30)

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@redhat.com>
---
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