linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] tick/broadcast: Allow per-cpu timers to be used instead of broadcast
@ 2021-05-24 22:18 Will Deacon
  2021-05-24 22:18 ` [PATCH v2 1/5] tick/broadcast: Drop unneeded CONFIG_GENERIC_CLOCKEVENTS_BROADCAST guard Will Deacon
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Will Deacon @ 2021-05-24 22:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, Will Deacon, Frederic Weisbecker, Thomas Gleixner,
	Marc Zyngier, Lorenzo Colitti, John Stultz, Stephen Boyd,
	Mika Penttilä,
	kernel-team

Hi all,

This is version two of the series I posted last week:

  https://lore.kernel.org/r/20210520184705.10845-1-will@kernel.org

This patch series adds support for hardware where the per-cpu tick timer
cannot wake up from deep idle states (i.e. CLOCK_EVT_FEAT_C3STOP is set)
yet there is a secondary per-cpu timer which is generally less preferable
(i.e. slow to access) yet capable of delivering the wakeup.

Changes since v1 include:

  * Fixed module refcounting and use of clockevents_exchange_device()
  * Require CLOCK_EVT_FEAT_PERCPU for new wakeup per-cpu source
  * Fix transition to oneshot mode while idle
  * Tested on my x86 laptop

Cheers,

Will

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Lorenzo Colitti <lorenzo@google.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Mika Penttilä <mika.penttila@nextfour.com>
Cc: kernel-team@android.com

--->8

Will Deacon (5):
  tick/broadcast: Drop unneeded CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
    guard
  tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper
  tick/broadcast: Prefer per-cpu oneshot wakeup timers to broadcast
  tick/broadcast: Program wakeup timer when entering idle if required
  timer_list: Print name of per-cpu wakeup device

 kernel/time/tick-broadcast.c | 143 +++++++++++++++++++++++++++++++----
 kernel/time/tick-common.c    |   2 +-
 kernel/time/tick-internal.h  |   5 +-
 kernel/time/timer_list.c     |  10 ++-
 4 files changed, 140 insertions(+), 20 deletions(-)

-- 
2.31.1.818.g46aad6cb9e-goog


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/5] tick/broadcast: Drop unneeded CONFIG_GENERIC_CLOCKEVENTS_BROADCAST guard
  2021-05-24 22:18 [PATCH v2 0/5] tick/broadcast: Allow per-cpu timers to be used instead of broadcast Will Deacon
@ 2021-05-24 22:18 ` Will Deacon
  2021-05-31 15:07   ` [tip: timers/core] " tip-bot2 for Will Deacon
  2021-05-24 22:18 ` [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper Will Deacon
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2021-05-24 22:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, Will Deacon, Frederic Weisbecker, Thomas Gleixner,
	Marc Zyngier, Lorenzo Colitti, John Stultz, Stephen Boyd,
	Mika Penttilä,
	kernel-team

tick-broadcast.o is only built if CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
so remove the redundant #ifdef guards around the definition of
tick_receive_broadcast().

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Lorenzo Colitti <lorenzo@google.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Will Deacon <will@kernel.org>
---
 kernel/time/tick-broadcast.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index a44055228796..fb794ff4855e 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -253,7 +253,6 @@ int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu)
 	return ret;
 }
 
-#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 int tick_receive_broadcast(void)
 {
 	struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
@@ -268,7 +267,6 @@ int tick_receive_broadcast(void)
 	evt->event_handler(evt);
 	return 0;
 }
-#endif
 
 /*
  * Broadcast the event to the cpus, which are set in the mask (mangled).
-- 
2.31.1.818.g46aad6cb9e-goog


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper
  2021-05-24 22:18 [PATCH v2 0/5] tick/broadcast: Allow per-cpu timers to be used instead of broadcast Will Deacon
  2021-05-24 22:18 ` [PATCH v2 1/5] tick/broadcast: Drop unneeded CONFIG_GENERIC_CLOCKEVENTS_BROADCAST guard Will Deacon
@ 2021-05-24 22:18 ` Will Deacon
  2021-05-27  7:23   ` Xin Hao
  2021-05-31 15:06   ` [tip: timers/core] " tip-bot2 for Will Deacon
  2021-05-24 22:18 ` [PATCH v2 3/5] tick/broadcast: Prefer per-cpu oneshot wakeup timers to broadcast Will Deacon
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 19+ messages in thread
From: Will Deacon @ 2021-05-24 22:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, Will Deacon, Frederic Weisbecker, Thomas Gleixner,
	Marc Zyngier, Lorenzo Colitti, John Stultz, Stephen Boyd,
	Mika Penttilä,
	kernel-team

In preparation for adding support for per-cpu wakeup timers, split
_tick_broadcast_oneshot_control() into a helper function which deals
only with the broadcast timer management across idle transitions.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Lorenzo Colitti <lorenzo@google.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Will Deacon <will@kernel.org>
---
 kernel/time/tick-broadcast.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index fb794ff4855e..f3f2f4ba4321 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -717,24 +717,16 @@ static void broadcast_shutdown_local(struct clock_event_device *bc,
 	clockevents_switch_state(dev, CLOCK_EVT_STATE_SHUTDOWN);
 }
 
-int __tick_broadcast_oneshot_control(enum tick_broadcast_state state)
+static int ___tick_broadcast_oneshot_control(enum tick_broadcast_state state,
+					     struct tick_device *td,
+					     int cpu)
 {
-	struct clock_event_device *bc, *dev;
-	int cpu, ret = 0;
+	struct clock_event_device *bc, *dev = td->evtdev;
+	int ret = 0;
 	ktime_t now;
 
-	/*
-	 * If there is no broadcast device, tell the caller not to go
-	 * into deep idle.
-	 */
-	if (!tick_broadcast_device.evtdev)
-		return -EBUSY;
-
-	dev = this_cpu_ptr(&tick_cpu_device)->evtdev;
-
 	raw_spin_lock(&tick_broadcast_lock);
 	bc = tick_broadcast_device.evtdev;
-	cpu = smp_processor_id();
 
 	if (state == TICK_BROADCAST_ENTER) {
 		/*
@@ -863,6 +855,21 @@ int __tick_broadcast_oneshot_control(enum tick_broadcast_state state)
 	return ret;
 }
 
+int __tick_broadcast_oneshot_control(enum tick_broadcast_state state)
+{
+	struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
+	int cpu = smp_processor_id();
+
+	if (tick_broadcast_device.evtdev)
+		return ___tick_broadcast_oneshot_control(state, td, cpu);
+
+	/*
+	 * If there is no broadcast device, tell the caller not
+	 * to go into deep idle.
+	 */
+	return -EBUSY;
+}
+
 /*
  * Reset the one shot broadcast for a cpu
  *
-- 
2.31.1.818.g46aad6cb9e-goog


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 3/5] tick/broadcast: Prefer per-cpu oneshot wakeup timers to broadcast
  2021-05-24 22:18 [PATCH v2 0/5] tick/broadcast: Allow per-cpu timers to be used instead of broadcast Will Deacon
  2021-05-24 22:18 ` [PATCH v2 1/5] tick/broadcast: Drop unneeded CONFIG_GENERIC_CLOCKEVENTS_BROADCAST guard Will Deacon
  2021-05-24 22:18 ` [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper Will Deacon
@ 2021-05-24 22:18 ` Will Deacon
  2021-05-31 15:06   ` [tip: timers/core] " tip-bot2 for Will Deacon
  2021-05-24 22:18 ` [PATCH v2 4/5] tick/broadcast: Program wakeup timer when entering idle if required Will Deacon
  2021-05-24 22:18 ` [PATCH v2 5/5] timer_list: Print name of per-cpu wakeup device Will Deacon
  4 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2021-05-24 22:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, Will Deacon, Frederic Weisbecker, Thomas Gleixner,
	Marc Zyngier, Lorenzo Colitti, John Stultz, Stephen Boyd,
	Mika Penttilä,
	kernel-team

Some SoCs have two per-cpu timer implementations where the timer with
the higher rating stops in deep idle (i.e. suffers from
CLOCK_EVT_FEAT_C3STOP) but is otherwise preferable to the timer with the
lower rating. In such a design, we rely on a global broadcast timer and
IPIs to wake up from deep idle states.

To avoid the reliance on a global broadcast timer and also to reduce the
overhead associated with the IPI wakeups, extend
tick_install_broadcast_device() to manage per-cpu wakeup timers
separately from the broadcast device.

For now, these timers remain unused.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Lorenzo Colitti <lorenzo@google.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Will Deacon <will@kernel.org>
---
 kernel/time/tick-broadcast.c | 59 +++++++++++++++++++++++++++++++++++-
 kernel/time/tick-common.c    |  2 +-
 kernel/time/tick-internal.h  |  4 +--
 3 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index f3f2f4ba4321..0e9e06d6cc5c 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -33,6 +33,8 @@ static int tick_broadcast_forced;
 static __cacheline_aligned_in_smp DEFINE_RAW_SPINLOCK(tick_broadcast_lock);
 
 #ifdef CONFIG_TICK_ONESHOT
+static DEFINE_PER_CPU(struct clock_event_device *, tick_oneshot_wakeup_device);
+
 static void tick_broadcast_setup_oneshot(struct clock_event_device *bc);
 static void tick_broadcast_clear_oneshot(int cpu);
 static void tick_resume_broadcast_oneshot(struct clock_event_device *bc);
@@ -88,13 +90,65 @@ static bool tick_check_broadcast_device(struct clock_event_device *curdev,
 	return !curdev || newdev->rating > curdev->rating;
 }
 
+#ifdef CONFIG_TICK_ONESHOT
+static struct clock_event_device *tick_get_oneshot_wakeup_device(int cpu)
+{
+	return per_cpu(tick_oneshot_wakeup_device, cpu);
+}
+
+static bool tick_set_oneshot_wakeup_device(struct clock_event_device *newdev,
+					   int cpu)
+{
+	struct clock_event_device *curdev = tick_get_oneshot_wakeup_device(cpu);
+
+	if (!newdev)
+		goto set_device;
+
+	if ((newdev->features & CLOCK_EVT_FEAT_DUMMY) ||
+	    (newdev->features & CLOCK_EVT_FEAT_C3STOP))
+		 return false;
+
+	if (!(newdev->features & CLOCK_EVT_FEAT_PERCPU) ||
+	    !(newdev->features & CLOCK_EVT_FEAT_ONESHOT))
+		return false;
+
+	if (!cpumask_equal(newdev->cpumask, cpumask_of(cpu)))
+		return false;
+
+	if (curdev && newdev->rating <= curdev->rating)
+		return false;
+
+	if (!try_module_get(newdev->owner))
+		return false;
+
+set_device:
+	clockevents_exchange_device(curdev, newdev);
+	per_cpu(tick_oneshot_wakeup_device, cpu) = newdev;
+	return true;
+}
+#else
+static struct clock_event_device *tick_get_oneshot_wakeup_device(int cpu)
+{
+	return NULL;
+}
+
+static bool tick_set_oneshot_wakeup_device(struct clock_event_device *newdev,
+					   int cpu)
+{
+	return false;
+}
+#endif
+
 /*
  * Conditionally install/replace broadcast device
  */
-void tick_install_broadcast_device(struct clock_event_device *dev)
+void tick_install_broadcast_device(struct clock_event_device *dev, int cpu)
 {
 	struct clock_event_device *cur = tick_broadcast_device.evtdev;
 
+	if (tick_set_oneshot_wakeup_device(dev, cpu))
+		return;
+
 	if (!tick_check_broadcast_device(cur, dev))
 		return;
 
@@ -996,6 +1050,9 @@ void hotplug_cpu__broadcast_tick_pull(int deadcpu)
  */
 static void tick_broadcast_oneshot_offline(unsigned int cpu)
 {
+	if (tick_get_oneshot_wakeup_device(cpu))
+		tick_set_oneshot_wakeup_device(NULL, cpu);
+
 	/*
 	 * Clear the broadcast masks for the dead cpu, but do not stop
 	 * the broadcast device!
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index e15bc0ef1912..d663249652ef 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -373,7 +373,7 @@ void tick_check_new_device(struct clock_event_device *newdev)
 	/*
 	 * Can the new device be used as a broadcast device ?
 	 */
-	tick_install_broadcast_device(newdev);
+	tick_install_broadcast_device(newdev, cpu);
 }
 
 /**
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 7a981c9e87a4..30c89639e305 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -61,7 +61,7 @@ extern ssize_t sysfs_get_uname(const char *buf, char *dst, size_t cnt);
 /* Broadcasting support */
 # ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 extern int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu);
-extern void tick_install_broadcast_device(struct clock_event_device *dev);
+extern void tick_install_broadcast_device(struct clock_event_device *dev, int cpu);
 extern int tick_is_broadcast_device(struct clock_event_device *dev);
 extern void tick_suspend_broadcast(void);
 extern void tick_resume_broadcast(void);
@@ -72,7 +72,7 @@ extern int tick_broadcast_update_freq(struct clock_event_device *dev, u32 freq);
 extern struct tick_device *tick_get_broadcast_device(void);
 extern struct cpumask *tick_get_broadcast_mask(void);
 # else /* !CONFIG_GENERIC_CLOCKEVENTS_BROADCAST: */
-static inline void tick_install_broadcast_device(struct clock_event_device *dev) { }
+static inline void tick_install_broadcast_device(struct clock_event_device *dev, int cpu) { }
 static inline int tick_is_broadcast_device(struct clock_event_device *dev) { return 0; }
 static inline int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu) { return 0; }
 static inline void tick_do_periodic_broadcast(struct clock_event_device *d) { }
-- 
2.31.1.818.g46aad6cb9e-goog


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 4/5] tick/broadcast: Program wakeup timer when entering idle if required
  2021-05-24 22:18 [PATCH v2 0/5] tick/broadcast: Allow per-cpu timers to be used instead of broadcast Will Deacon
                   ` (2 preceding siblings ...)
  2021-05-24 22:18 ` [PATCH v2 3/5] tick/broadcast: Prefer per-cpu oneshot wakeup timers to broadcast Will Deacon
@ 2021-05-24 22:18 ` Will Deacon
  2021-05-31 15:06   ` [tip: timers/core] " tip-bot2 for Will Deacon
  2021-05-24 22:18 ` [PATCH v2 5/5] timer_list: Print name of per-cpu wakeup device Will Deacon
  4 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2021-05-24 22:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, Will Deacon, Frederic Weisbecker, Thomas Gleixner,
	Marc Zyngier, Lorenzo Colitti, John Stultz, Stephen Boyd,
	Mika Penttilä,
	kernel-team

When configuring the broadcast timer on entry to and exit from deep idle
states, prefer a per-CPU wakeup timer if one exists.

On entry to idle, stop the tick device and transfer the next event into
the oneshot wakeup device, which will serve as the wakeup from idle. To
avoid the overhead of additional hardware accesses on exit from idle,
leave the timer armed and treat the inevitable interrupt as a (possibly
spurious) tick event.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Lorenzo Colitti <lorenzo@google.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Will Deacon <will@kernel.org>
---
 kernel/time/tick-broadcast.c | 44 +++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 0e9e06d6cc5c..9b845212430b 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -96,6 +96,15 @@ static struct clock_event_device *tick_get_oneshot_wakeup_device(int cpu)
 	return per_cpu(tick_oneshot_wakeup_device, cpu);
 }
 
+static void tick_oneshot_wakeup_handler(struct clock_event_device *wd)
+{
+	/*
+	 * If we woke up early and the tick was reprogrammed in the
+	 * meantime then this may be spurious but harmless.
+	 */
+	tick_receive_broadcast();
+}
+
 static bool tick_set_oneshot_wakeup_device(struct clock_event_device *newdev,
 					   int cpu)
 {
@@ -121,6 +130,7 @@ static bool tick_set_oneshot_wakeup_device(struct clock_event_device *newdev,
 	if (!try_module_get(newdev->owner))
 		return false;
 
+	newdev->event_handler = tick_oneshot_wakeup_handler;
 set_device:
 	clockevents_exchange_device(curdev, newdev);
 	per_cpu(tick_oneshot_wakeup_device, cpu) = newdev;
@@ -909,16 +919,48 @@ static int ___tick_broadcast_oneshot_control(enum tick_broadcast_state state,
 	return ret;
 }
 
+static int tick_oneshot_wakeup_control(enum tick_broadcast_state state,
+				       struct tick_device *td,
+				       int cpu)
+{
+	struct clock_event_device *dev, *wd;
+
+	dev = td->evtdev;
+	if (td->mode != TICKDEV_MODE_ONESHOT)
+		return -EINVAL;
+
+	wd = tick_get_oneshot_wakeup_device(cpu);
+	if (!wd)
+		return -ENODEV;
+
+	switch (state) {
+	case TICK_BROADCAST_ENTER:
+		clockevents_switch_state(dev, CLOCK_EVT_STATE_ONESHOT_STOPPED);
+		clockevents_switch_state(wd, CLOCK_EVT_STATE_ONESHOT);
+		clockevents_program_event(wd, dev->next_event, 1);
+		break;
+	case TICK_BROADCAST_EXIT:
+		/* We may have transitioned to oneshot mode while idle */
+		if (clockevent_get_state(wd) != CLOCK_EVT_STATE_ONESHOT)
+			return -ENODEV;
+	}
+
+	return 0;
+}
+
 int __tick_broadcast_oneshot_control(enum tick_broadcast_state state)
 {
 	struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
 	int cpu = smp_processor_id();
 
+	if (!tick_oneshot_wakeup_control(state, td, cpu))
+		return 0;
+
 	if (tick_broadcast_device.evtdev)
 		return ___tick_broadcast_oneshot_control(state, td, cpu);
 
 	/*
-	 * If there is no broadcast device, tell the caller not
+	 * If there is no broadcast or wakeup device, tell the caller not
 	 * to go into deep idle.
 	 */
 	return -EBUSY;
-- 
2.31.1.818.g46aad6cb9e-goog


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 5/5] timer_list: Print name of per-cpu wakeup device
  2021-05-24 22:18 [PATCH v2 0/5] tick/broadcast: Allow per-cpu timers to be used instead of broadcast Will Deacon
                   ` (3 preceding siblings ...)
  2021-05-24 22:18 ` [PATCH v2 4/5] tick/broadcast: Program wakeup timer when entering idle if required Will Deacon
@ 2021-05-24 22:18 ` Will Deacon
  2021-05-31 15:06   ` [tip: timers/core] " tip-bot2 for Will Deacon
  4 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2021-05-24 22:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, Will Deacon, Frederic Weisbecker, Thomas Gleixner,
	Marc Zyngier, Lorenzo Colitti, John Stultz, Stephen Boyd,
	Mika Penttilä,
	kernel-team

With the introduction of per-cpu wakeup devices that can be used in
preference to the broadcast timer, print the name of such devices when
they are available.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Lorenzo Colitti <lorenzo@google.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Will Deacon <will@kernel.org>
---
 kernel/time/tick-broadcast.c |  7 +++++++
 kernel/time/tick-internal.h  |  1 +
 kernel/time/timer_list.c     | 10 +++++++++-
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 9b845212430b..f7fe6fe36173 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -63,6 +63,13 @@ struct cpumask *tick_get_broadcast_mask(void)
 	return tick_broadcast_mask;
 }
 
+static struct clock_event_device *tick_get_oneshot_wakeup_device(int cpu);
+
+const struct clock_event_device *tick_get_wakeup_device(int cpu)
+{
+	return tick_get_oneshot_wakeup_device(cpu);
+}
+
 /*
  * Start the device in periodic mode
  */
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 30c89639e305..6a742a29e545 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -71,6 +71,7 @@ extern void tick_set_periodic_handler(struct clock_event_device *dev, int broadc
 extern int tick_broadcast_update_freq(struct clock_event_device *dev, u32 freq);
 extern struct tick_device *tick_get_broadcast_device(void);
 extern struct cpumask *tick_get_broadcast_mask(void);
+extern const struct clock_event_device *tick_get_wakeup_device(int cpu);
 # else /* !CONFIG_GENERIC_CLOCKEVENTS_BROADCAST: */
 static inline void tick_install_broadcast_device(struct clock_event_device *dev, int cpu) { }
 static inline int tick_is_broadcast_device(struct clock_event_device *dev) { return 0; }
diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c
index 6939140ab7c5..cca611edfd65 100644
--- a/kernel/time/timer_list.c
+++ b/kernel/time/timer_list.c
@@ -228,6 +228,14 @@ print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu)
 	SEQ_printf(m, " event_handler:  %ps\n", dev->event_handler);
 	SEQ_printf(m, "\n");
 	SEQ_printf(m, " retries:        %lu\n", dev->retries);
+
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
+	if (cpu >= 0) {
+		const struct clock_event_device *wd =
+			tick_get_wakeup_device(cpu);
+		SEQ_printf(m, "Wakeup Device: %s\n", wd ? wd->name : "<NULL>");
+	}
+#endif
 	SEQ_printf(m, "\n");
 }
 
@@ -248,7 +256,7 @@ static void timer_list_show_tickdevices_header(struct seq_file *m)
 
 static inline void timer_list_header(struct seq_file *m, u64 now)
 {
-	SEQ_printf(m, "Timer List Version: v0.8\n");
+	SEQ_printf(m, "Timer List Version: v0.9\n");
 	SEQ_printf(m, "HRTIMER_MAX_CLOCK_BASES: %d\n", HRTIMER_MAX_CLOCK_BASES);
 	SEQ_printf(m, "now at %Ld nsecs\n", (unsigned long long)now);
 	SEQ_printf(m, "\n");
-- 
2.31.1.818.g46aad6cb9e-goog


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper
  2021-05-24 22:18 ` [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper Will Deacon
@ 2021-05-27  7:23   ` Xin Hao
  2021-05-27  8:22     ` Will Deacon
  2021-05-31 15:06   ` [tip: timers/core] " tip-bot2 for Will Deacon
  1 sibling, 1 reply; 19+ messages in thread
From: Xin Hao @ 2021-05-27  7:23 UTC (permalink / raw)
  To: will
  Cc: fweisbec, john.stultz, kernel-team, linux-arm-kernel,
	linux-kernel, lorenzo, maz, mika.penttila, sboyd, tglx

Hi  will:

      I  had backport you  tick/broadcast: Prefer per-cpu relatives 
patches,

but i did not get the true result,  the Wakeup Devices are all null, why?

my machine is armv8-a arm64

#cat current_clocksource
arch_sys_counter

my local clock event is

Tick Device: mode:     1
Per CPU device: 95
Clock Event Device: arch_sys_timer
  max_delta_ns:   21474836451
  min_delta_ns:   1000
  mult:           429496730
  shift:          32
  mode:           3
  next_event:     14951080000000 nsecs
  set_next_event: arch_timer_set_next_event_phys
  shutdown: arch_timer_shutdown_phys
  oneshot stopped: arch_timer_shutdown_phys
  event_handler:  hrtimer_interrupt
  retries:        70
Wakeup Device: <NULL>

cat /proc/timer_list  | grep "Wakeup Device:"

Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>
Wakeup Device: <NULL>

Wakeup Device: <NULL>

-- 
Best Regards!
Xin Hao


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper
  2021-05-27  7:23   ` Xin Hao
@ 2021-05-27  8:22     ` Will Deacon
  2021-05-27 11:35       ` Xin Hao
  0 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2021-05-27  8:22 UTC (permalink / raw)
  To: Xin Hao
  Cc: fweisbec, john.stultz, kernel-team, linux-arm-kernel,
	linux-kernel, lorenzo, maz, mika.penttila, sboyd, tglx

On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote:
>      I  had backport you  tick/broadcast: Prefer per-cpu relatives patches,
> 
> but i did not get the true result,  the Wakeup Devices are all null, why?

Probably because you don't have any suitable per-cpu timers to act as a
wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU
and CLOCK_EVT_FEAT_ONESHOT but not CLOCK_EVT_FEAT_C3STOP?

Will

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper
  2021-05-27  8:22     ` Will Deacon
@ 2021-05-27 11:35       ` Xin Hao
  2021-05-27 11:55         ` Will Deacon
  2021-05-27 11:56         ` Will Deacon
  0 siblings, 2 replies; 19+ messages in thread
From: Xin Hao @ 2021-05-27 11:35 UTC (permalink / raw)
  To: Will Deacon
  Cc: fweisbec, john.stultz, kernel-team, linux-arm-kernel,
	linux-kernel, lorenzo, maz, mika.penttila, sboyd, tglx


在 2021/5/27 下午4:22, Will Deacon 写道:
> On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote:
>>       I  had backport you  tick/broadcast: Prefer per-cpu relatives patches,
>>
>> but i did not get the true result,  the Wakeup Devices are all null, why?
> Probably because you don't have any suitable per-cpu timers to act as a
> wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU

Yes, you are right, but i want to know why the timer do not support  
CLOCK_EVT_FEAT_PERCPU.

> and CLOCK_EVT_FEAT_ONESHOT but not CLOCK_EVT_FEAT_C3STOP?
>
> Will

-- 
Best Regards!
Xin Hao


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper
  2021-05-27 11:35       ` Xin Hao
@ 2021-05-27 11:55         ` Will Deacon
  2021-05-27 11:56         ` Will Deacon
  1 sibling, 0 replies; 19+ messages in thread
From: Will Deacon @ 2021-05-27 11:55 UTC (permalink / raw)
  To: Xin Hao
  Cc: fweisbec, john.stultz, kernel-team, linux-arm-kernel,
	linux-kernel, lorenzo, maz, mika.penttila, sboyd, tglx

On Thu, May 27, 2021 at 07:35:03PM +0800, Xin Hao wrote:
> 
> 在 2021/5/27 下午4:22, Will Deacon 写道:
> > On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote:
> > >       I  had backport you  tick/broadcast: Prefer per-cpu relatives patches,
> > > 
> > > but i did not get the true result,  the Wakeup Devices are all null, why?
> > Probably because you don't have any suitable per-cpu timers to act as a
> > wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU
> 
> Yes, you are right, but i want to know why the timer do not support 
> CLOCK_EVT_FEAT_PERCPU.

I suspect there may be drivers with the feature missing simply because it
wasn't really used for much until now (I think it just prevented use for
broadcast). However, before adding that to a timer, you do need to make
sure that it really is banked per-cpu (or at least handles racing accesses)
as well as having the per-cpu irq.

Will

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper
  2021-05-27 11:35       ` Xin Hao
  2021-05-27 11:55         ` Will Deacon
@ 2021-05-27 11:56         ` Will Deacon
  2021-05-31 14:29           ` Thomas Gleixner
  1 sibling, 1 reply; 19+ messages in thread
From: Will Deacon @ 2021-05-27 11:56 UTC (permalink / raw)
  To: Xin Hao
  Cc: fweisbec, john.stultz, kernel-team, linux-arm-kernel,
	linux-kernel, lorenzo, maz, mika.penttila, sboyd, tglx

On Thu, May 27, 2021 at 07:35:03PM +0800, Xin Hao wrote:
> 
> 在 2021/5/27 下午4:22, Will Deacon 写道:
> > On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote:
> > >       I  had backport you  tick/broadcast: Prefer per-cpu relatives patches,
> > > 
> > > but i did not get the true result,  the Wakeup Devices are all null, why?
> > Probably because you don't have any suitable per-cpu timers to act as a
> > wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU
> 
> Yes, you are right, but i want to know why the timer do not support 
> CLOCK_EVT_FEAT_PERCPU.

I defer to Thomas on this one. The approach I have taken here (of printing
<NULL>) follows what is done elsewhere for the timer readout and probably
helps with parsing this stuff.

Will

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper
  2021-05-27 11:56         ` Will Deacon
@ 2021-05-31 14:29           ` Thomas Gleixner
  2021-06-01 12:13             ` Will Deacon
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Gleixner @ 2021-05-31 14:29 UTC (permalink / raw)
  To: Will Deacon, Xin Hao
  Cc: fweisbec, john.stultz, kernel-team, linux-arm-kernel,
	linux-kernel, lorenzo, maz, mika.penttila, sboyd

On Thu, May 27 2021 at 12:56, Will Deacon wrote:

> On Thu, May 27, 2021 at 07:35:03PM +0800, Xin Hao wrote:
>> 
>> 在 2021/5/27 下午4:22, Will Deacon 写道:
>> > On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote:
>> > >       I  had backport you  tick/broadcast: Prefer per-cpu relatives patches,
>> > > 
>> > > but i did not get the true result,  the Wakeup Devices are all null, why?
>> > Probably because you don't have any suitable per-cpu timers to act as a
>> > wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU
>> 
>> Yes, you are right, but i want to know why the timer do not support 
>> CLOCK_EVT_FEAT_PERCPU.
>
> I defer to Thomas on this one.

How should I know what kind of timers this hardware has?

Thanks,

        tglx


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [tip: timers/core] timer_list: Print name of per-cpu wakeup device
  2021-05-24 22:18 ` [PATCH v2 5/5] timer_list: Print name of per-cpu wakeup device Will Deacon
@ 2021-05-31 15:06   ` tip-bot2 for Will Deacon
  0 siblings, 0 replies; 19+ messages in thread
From: tip-bot2 for Will Deacon @ 2021-05-31 15:06 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Will Deacon, Thomas Gleixner, x86, linux-kernel

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     245a057fee18be08d6ac12357463579d06bea077
Gitweb:        https://git.kernel.org/tip/245a057fee18be08d6ac12357463579d06bea077
Author:        Will Deacon <will@kernel.org>
AuthorDate:    Mon, 24 May 2021 23:18:18 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 31 May 2021 17:04:49 +02:00

timer_list: Print name of per-cpu wakeup device

With the introduction of per-cpu wakeup devices that can be used in
preference to the broadcast timer, print the name of such devices when
they are available.

Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20210524221818.15850-6-will@kernel.org

---
 kernel/time/tick-broadcast.c |  7 +++++++
 kernel/time/tick-internal.h  |  1 +
 kernel/time/timer_list.c     | 10 +++++++++-
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 9b84521..f7fe6fe 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -63,6 +63,13 @@ struct cpumask *tick_get_broadcast_mask(void)
 	return tick_broadcast_mask;
 }
 
+static struct clock_event_device *tick_get_oneshot_wakeup_device(int cpu);
+
+const struct clock_event_device *tick_get_wakeup_device(int cpu)
+{
+	return tick_get_oneshot_wakeup_device(cpu);
+}
+
 /*
  * Start the device in periodic mode
  */
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 30c8963..6a742a2 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -71,6 +71,7 @@ extern void tick_set_periodic_handler(struct clock_event_device *dev, int broadc
 extern int tick_broadcast_update_freq(struct clock_event_device *dev, u32 freq);
 extern struct tick_device *tick_get_broadcast_device(void);
 extern struct cpumask *tick_get_broadcast_mask(void);
+extern const struct clock_event_device *tick_get_wakeup_device(int cpu);
 # else /* !CONFIG_GENERIC_CLOCKEVENTS_BROADCAST: */
 static inline void tick_install_broadcast_device(struct clock_event_device *dev, int cpu) { }
 static inline int tick_is_broadcast_device(struct clock_event_device *dev) { return 0; }
diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c
index 6939140..ed7d6ad 100644
--- a/kernel/time/timer_list.c
+++ b/kernel/time/timer_list.c
@@ -228,6 +228,14 @@ print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu)
 	SEQ_printf(m, " event_handler:  %ps\n", dev->event_handler);
 	SEQ_printf(m, "\n");
 	SEQ_printf(m, " retries:        %lu\n", dev->retries);
+
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
+	if (cpu >= 0) {
+		const struct clock_event_device *wd = tick_get_wakeup_device(cpu);
+
+		SEQ_printf(m, "Wakeup Device: %s\n", wd ? wd->name : "<NULL>");
+	}
+#endif
 	SEQ_printf(m, "\n");
 }
 
@@ -248,7 +256,7 @@ static void timer_list_show_tickdevices_header(struct seq_file *m)
 
 static inline void timer_list_header(struct seq_file *m, u64 now)
 {
-	SEQ_printf(m, "Timer List Version: v0.8\n");
+	SEQ_printf(m, "Timer List Version: v0.9\n");
 	SEQ_printf(m, "HRTIMER_MAX_CLOCK_BASES: %d\n", HRTIMER_MAX_CLOCK_BASES);
 	SEQ_printf(m, "now at %Ld nsecs\n", (unsigned long long)now);
 	SEQ_printf(m, "\n");

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [tip: timers/core] tick/broadcast: Prefer per-cpu oneshot wakeup timers to broadcast
  2021-05-24 22:18 ` [PATCH v2 3/5] tick/broadcast: Prefer per-cpu oneshot wakeup timers to broadcast Will Deacon
@ 2021-05-31 15:06   ` tip-bot2 for Will Deacon
  0 siblings, 0 replies; 19+ messages in thread
From: tip-bot2 for Will Deacon @ 2021-05-31 15:06 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Will Deacon, Thomas Gleixner, x86, linux-kernel

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     c94a8537df12708cc03da9120c3c3561ae744ce1
Gitweb:        https://git.kernel.org/tip/c94a8537df12708cc03da9120c3c3561ae744ce1
Author:        Will Deacon <will@kernel.org>
AuthorDate:    Mon, 24 May 2021 23:18:16 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 31 May 2021 17:04:45 +02:00

tick/broadcast: Prefer per-cpu oneshot wakeup timers to broadcast

Some SoCs have two per-cpu timer implementations where the timer with the
higher rating stops in deep idle (i.e. suffers from CLOCK_EVT_FEAT_C3STOP)
but is otherwise preferable to the timer with the lower rating. In such a
design, selecting the higher rated devices relies on a global broadcast
timer and IPIs to wake up from deep idle states.

To avoid the reliance on a global broadcast timer and also to reduce the
overhead associated with the IPI wakeups, extend
tick_install_broadcast_device() to manage per-cpu wakeup timers separately
from the broadcast device.

For now, these timers remain unused.

Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20210524221818.15850-4-will@kernel.org

---
 kernel/time/tick-broadcast.c | 59 ++++++++++++++++++++++++++++++++++-
 kernel/time/tick-common.c    |  2 +-
 kernel/time/tick-internal.h  |  4 +-
 3 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index f3f2f4b..0e9e06d 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -33,6 +33,8 @@ static int tick_broadcast_forced;
 static __cacheline_aligned_in_smp DEFINE_RAW_SPINLOCK(tick_broadcast_lock);
 
 #ifdef CONFIG_TICK_ONESHOT
+static DEFINE_PER_CPU(struct clock_event_device *, tick_oneshot_wakeup_device);
+
 static void tick_broadcast_setup_oneshot(struct clock_event_device *bc);
 static void tick_broadcast_clear_oneshot(int cpu);
 static void tick_resume_broadcast_oneshot(struct clock_event_device *bc);
@@ -88,13 +90,65 @@ static bool tick_check_broadcast_device(struct clock_event_device *curdev,
 	return !curdev || newdev->rating > curdev->rating;
 }
 
+#ifdef CONFIG_TICK_ONESHOT
+static struct clock_event_device *tick_get_oneshot_wakeup_device(int cpu)
+{
+	return per_cpu(tick_oneshot_wakeup_device, cpu);
+}
+
+static bool tick_set_oneshot_wakeup_device(struct clock_event_device *newdev,
+					   int cpu)
+{
+	struct clock_event_device *curdev = tick_get_oneshot_wakeup_device(cpu);
+
+	if (!newdev)
+		goto set_device;
+
+	if ((newdev->features & CLOCK_EVT_FEAT_DUMMY) ||
+	    (newdev->features & CLOCK_EVT_FEAT_C3STOP))
+		 return false;
+
+	if (!(newdev->features & CLOCK_EVT_FEAT_PERCPU) ||
+	    !(newdev->features & CLOCK_EVT_FEAT_ONESHOT))
+		return false;
+
+	if (!cpumask_equal(newdev->cpumask, cpumask_of(cpu)))
+		return false;
+
+	if (curdev && newdev->rating <= curdev->rating)
+		return false;
+
+	if (!try_module_get(newdev->owner))
+		return false;
+
+set_device:
+	clockevents_exchange_device(curdev, newdev);
+	per_cpu(tick_oneshot_wakeup_device, cpu) = newdev;
+	return true;
+}
+#else
+static struct clock_event_device *tick_get_oneshot_wakeup_device(int cpu)
+{
+	return NULL;
+}
+
+static bool tick_set_oneshot_wakeup_device(struct clock_event_device *newdev,
+					   int cpu)
+{
+	return false;
+}
+#endif
+
 /*
  * Conditionally install/replace broadcast device
  */
-void tick_install_broadcast_device(struct clock_event_device *dev)
+void tick_install_broadcast_device(struct clock_event_device *dev, int cpu)
 {
 	struct clock_event_device *cur = tick_broadcast_device.evtdev;
 
+	if (tick_set_oneshot_wakeup_device(dev, cpu))
+		return;
+
 	if (!tick_check_broadcast_device(cur, dev))
 		return;
 
@@ -996,6 +1050,9 @@ void hotplug_cpu__broadcast_tick_pull(int deadcpu)
  */
 static void tick_broadcast_oneshot_offline(unsigned int cpu)
 {
+	if (tick_get_oneshot_wakeup_device(cpu))
+		tick_set_oneshot_wakeup_device(NULL, cpu);
+
 	/*
 	 * Clear the broadcast masks for the dead cpu, but do not stop
 	 * the broadcast device!
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index e15bc0e..d663249 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -373,7 +373,7 @@ out_bc:
 	/*
 	 * Can the new device be used as a broadcast device ?
 	 */
-	tick_install_broadcast_device(newdev);
+	tick_install_broadcast_device(newdev, cpu);
 }
 
 /**
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 7a981c9..30c8963 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -61,7 +61,7 @@ extern ssize_t sysfs_get_uname(const char *buf, char *dst, size_t cnt);
 /* Broadcasting support */
 # ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 extern int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu);
-extern void tick_install_broadcast_device(struct clock_event_device *dev);
+extern void tick_install_broadcast_device(struct clock_event_device *dev, int cpu);
 extern int tick_is_broadcast_device(struct clock_event_device *dev);
 extern void tick_suspend_broadcast(void);
 extern void tick_resume_broadcast(void);
@@ -72,7 +72,7 @@ extern int tick_broadcast_update_freq(struct clock_event_device *dev, u32 freq);
 extern struct tick_device *tick_get_broadcast_device(void);
 extern struct cpumask *tick_get_broadcast_mask(void);
 # else /* !CONFIG_GENERIC_CLOCKEVENTS_BROADCAST: */
-static inline void tick_install_broadcast_device(struct clock_event_device *dev) { }
+static inline void tick_install_broadcast_device(struct clock_event_device *dev, int cpu) { }
 static inline int tick_is_broadcast_device(struct clock_event_device *dev) { return 0; }
 static inline int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu) { return 0; }
 static inline void tick_do_periodic_broadcast(struct clock_event_device *d) { }

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [tip: timers/core] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper
  2021-05-24 22:18 ` [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper Will Deacon
  2021-05-27  7:23   ` Xin Hao
@ 2021-05-31 15:06   ` tip-bot2 for Will Deacon
  1 sibling, 0 replies; 19+ messages in thread
From: tip-bot2 for Will Deacon @ 2021-05-31 15:06 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Will Deacon, Thomas Gleixner, x86, linux-kernel

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     e5007c288e7981e0b0cf8ea3dea443f0b8c34345
Gitweb:        https://git.kernel.org/tip/e5007c288e7981e0b0cf8ea3dea443f0b8c34345
Author:        Will Deacon <will@kernel.org>
AuthorDate:    Mon, 24 May 2021 23:18:15 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 31 May 2021 17:04:45 +02:00

tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper

In preparation for adding support for per-cpu wakeup timers, split
_tick_broadcast_oneshot_control() into a helper function which deals
only with the broadcast timer management across idle transitions.

Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20210524221818.15850-3-will@kernel.org

---
 kernel/time/tick-broadcast.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index fb794ff..f3f2f4b 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -717,24 +717,16 @@ static void broadcast_shutdown_local(struct clock_event_device *bc,
 	clockevents_switch_state(dev, CLOCK_EVT_STATE_SHUTDOWN);
 }
 
-int __tick_broadcast_oneshot_control(enum tick_broadcast_state state)
+static int ___tick_broadcast_oneshot_control(enum tick_broadcast_state state,
+					     struct tick_device *td,
+					     int cpu)
 {
-	struct clock_event_device *bc, *dev;
-	int cpu, ret = 0;
+	struct clock_event_device *bc, *dev = td->evtdev;
+	int ret = 0;
 	ktime_t now;
 
-	/*
-	 * If there is no broadcast device, tell the caller not to go
-	 * into deep idle.
-	 */
-	if (!tick_broadcast_device.evtdev)
-		return -EBUSY;
-
-	dev = this_cpu_ptr(&tick_cpu_device)->evtdev;
-
 	raw_spin_lock(&tick_broadcast_lock);
 	bc = tick_broadcast_device.evtdev;
-	cpu = smp_processor_id();
 
 	if (state == TICK_BROADCAST_ENTER) {
 		/*
@@ -863,6 +855,21 @@ out:
 	return ret;
 }
 
+int __tick_broadcast_oneshot_control(enum tick_broadcast_state state)
+{
+	struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
+	int cpu = smp_processor_id();
+
+	if (tick_broadcast_device.evtdev)
+		return ___tick_broadcast_oneshot_control(state, td, cpu);
+
+	/*
+	 * If there is no broadcast device, tell the caller not
+	 * to go into deep idle.
+	 */
+	return -EBUSY;
+}
+
 /*
  * Reset the one shot broadcast for a cpu
  *

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [tip: timers/core] tick/broadcast: Program wakeup timer when entering idle if required
  2021-05-24 22:18 ` [PATCH v2 4/5] tick/broadcast: Program wakeup timer when entering idle if required Will Deacon
@ 2021-05-31 15:06   ` tip-bot2 for Will Deacon
  0 siblings, 0 replies; 19+ messages in thread
From: tip-bot2 for Will Deacon @ 2021-05-31 15:06 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Will Deacon, Thomas Gleixner, x86, linux-kernel

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     ea5c7f1b9aa1a7c9d1bb9440084ac1256789fadb
Gitweb:        https://git.kernel.org/tip/ea5c7f1b9aa1a7c9d1bb9440084ac1256789fadb
Author:        Will Deacon <will@kernel.org>
AuthorDate:    Mon, 24 May 2021 23:18:17 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 31 May 2021 17:04:46 +02:00

tick/broadcast: Program wakeup timer when entering idle if required

When configuring the broadcast timer on entry to and exit from deep idle
states, prefer a per-CPU wakeup timer if one exists.

On entry to idle, stop the tick device and transfer the next event into
the oneshot wakeup device, which will serve as the wakeup from idle. To
avoid the overhead of additional hardware accesses on exit from idle,
leave the timer armed and treat the inevitable interrupt as a (possibly
spurious) tick event.

Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20210524221818.15850-5-will@kernel.org

---
 kernel/time/tick-broadcast.c | 44 ++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 0e9e06d..9b84521 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -96,6 +96,15 @@ static struct clock_event_device *tick_get_oneshot_wakeup_device(int cpu)
 	return per_cpu(tick_oneshot_wakeup_device, cpu);
 }
 
+static void tick_oneshot_wakeup_handler(struct clock_event_device *wd)
+{
+	/*
+	 * If we woke up early and the tick was reprogrammed in the
+	 * meantime then this may be spurious but harmless.
+	 */
+	tick_receive_broadcast();
+}
+
 static bool tick_set_oneshot_wakeup_device(struct clock_event_device *newdev,
 					   int cpu)
 {
@@ -121,6 +130,7 @@ static bool tick_set_oneshot_wakeup_device(struct clock_event_device *newdev,
 	if (!try_module_get(newdev->owner))
 		return false;
 
+	newdev->event_handler = tick_oneshot_wakeup_handler;
 set_device:
 	clockevents_exchange_device(curdev, newdev);
 	per_cpu(tick_oneshot_wakeup_device, cpu) = newdev;
@@ -909,16 +919,48 @@ out:
 	return ret;
 }
 
+static int tick_oneshot_wakeup_control(enum tick_broadcast_state state,
+				       struct tick_device *td,
+				       int cpu)
+{
+	struct clock_event_device *dev, *wd;
+
+	dev = td->evtdev;
+	if (td->mode != TICKDEV_MODE_ONESHOT)
+		return -EINVAL;
+
+	wd = tick_get_oneshot_wakeup_device(cpu);
+	if (!wd)
+		return -ENODEV;
+
+	switch (state) {
+	case TICK_BROADCAST_ENTER:
+		clockevents_switch_state(dev, CLOCK_EVT_STATE_ONESHOT_STOPPED);
+		clockevents_switch_state(wd, CLOCK_EVT_STATE_ONESHOT);
+		clockevents_program_event(wd, dev->next_event, 1);
+		break;
+	case TICK_BROADCAST_EXIT:
+		/* We may have transitioned to oneshot mode while idle */
+		if (clockevent_get_state(wd) != CLOCK_EVT_STATE_ONESHOT)
+			return -ENODEV;
+	}
+
+	return 0;
+}
+
 int __tick_broadcast_oneshot_control(enum tick_broadcast_state state)
 {
 	struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
 	int cpu = smp_processor_id();
 
+	if (!tick_oneshot_wakeup_control(state, td, cpu))
+		return 0;
+
 	if (tick_broadcast_device.evtdev)
 		return ___tick_broadcast_oneshot_control(state, td, cpu);
 
 	/*
-	 * If there is no broadcast device, tell the caller not
+	 * If there is no broadcast or wakeup device, tell the caller not
 	 * to go into deep idle.
 	 */
 	return -EBUSY;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [tip: timers/core] tick/broadcast: Drop unneeded CONFIG_GENERIC_CLOCKEVENTS_BROADCAST guard
  2021-05-24 22:18 ` [PATCH v2 1/5] tick/broadcast: Drop unneeded CONFIG_GENERIC_CLOCKEVENTS_BROADCAST guard Will Deacon
@ 2021-05-31 15:07   ` tip-bot2 for Will Deacon
  0 siblings, 0 replies; 19+ messages in thread
From: tip-bot2 for Will Deacon @ 2021-05-31 15:07 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Will Deacon, Thomas Gleixner, x86, linux-kernel

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     c2d4fee3f6d170dee5ee7c337a0ba5e92fad7a64
Gitweb:        https://git.kernel.org/tip/c2d4fee3f6d170dee5ee7c337a0ba5e92fad7a64
Author:        Will Deacon <will@kernel.org>
AuthorDate:    Mon, 24 May 2021 23:18:14 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 31 May 2021 17:04:44 +02:00

tick/broadcast: Drop unneeded CONFIG_GENERIC_CLOCKEVENTS_BROADCAST guard

tick-broadcast.o is only built if CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
so remove the redundant #ifdef guards around the definition of
tick_receive_broadcast().

Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20210524221818.15850-2-will@kernel.org

---
 kernel/time/tick-broadcast.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index a440552..fb794ff 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -253,7 +253,6 @@ int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu)
 	return ret;
 }
 
-#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 int tick_receive_broadcast(void)
 {
 	struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
@@ -268,7 +267,6 @@ int tick_receive_broadcast(void)
 	evt->event_handler(evt);
 	return 0;
 }
-#endif
 
 /*
  * Broadcast the event to the cpus, which are set in the mask (mangled).

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper
  2021-05-31 14:29           ` Thomas Gleixner
@ 2021-06-01 12:13             ` Will Deacon
  2021-06-01 18:14               ` Thomas Gleixner
  0 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2021-06-01 12:13 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Xin Hao, fweisbec, john.stultz, kernel-team, linux-arm-kernel,
	linux-kernel, lorenzo, maz, mika.penttila, sboyd

Hi Thomas,

On Mon, May 31, 2021 at 04:29:20PM +0200, Thomas Gleixner wrote:
> On Thu, May 27 2021 at 12:56, Will Deacon wrote:
> 
> > On Thu, May 27, 2021 at 07:35:03PM +0800, Xin Hao wrote:
> >> 
> >> 在 2021/5/27 下午4:22, Will Deacon 写道:
> >> > On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote:
> >> > >       I  had backport you  tick/broadcast: Prefer per-cpu relatives patches,
> >> > > 
> >> > > but i did not get the true result,  the Wakeup Devices are all null, why?
> >> > Probably because you don't have any suitable per-cpu timers to act as a
> >> > wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU
> >> 
> >> Yes, you are right, but i want to know why the timer do not support 
> >> CLOCK_EVT_FEAT_PERCPU.
> >
> > I defer to Thomas on this one.
> 
> How should I know what kind of timers this hardware has?

Duh, sorry, I replied to the wrong question. I meant to defer the decision
about whether to print "<NULL>" if the wakeup timer is absent, or whether to
omit the line entirely.

I went with the former in the patches you queued as it's both consistent
with the rest of the code and probably (?) easier to parse.

Will

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper
  2021-06-01 12:13             ` Will Deacon
@ 2021-06-01 18:14               ` Thomas Gleixner
  0 siblings, 0 replies; 19+ messages in thread
From: Thomas Gleixner @ 2021-06-01 18:14 UTC (permalink / raw)
  To: Will Deacon
  Cc: Xin Hao, fweisbec, john.stultz, kernel-team, linux-arm-kernel,
	linux-kernel, lorenzo, maz, mika.penttila, sboyd

On Tue, Jun 01 2021 at 13:13, Will Deacon wrote:
> On Mon, May 31, 2021 at 04:29:20PM +0200, Thomas Gleixner wrote:
>> On Thu, May 27 2021 at 12:56, Will Deacon wrote:
>> 
>> > On Thu, May 27, 2021 at 07:35:03PM +0800, Xin Hao wrote:
>> >> 
>> >> 在 2021/5/27 下午4:22, Will Deacon 写道:
>> >> > On Thu, May 27, 2021 at 03:23:06PM +0800, Xin Hao wrote:
>> >> > >       I  had backport you  tick/broadcast: Prefer per-cpu relatives patches,
>> >> > > 
>> >> > > but i did not get the true result,  the Wakeup Devices are all null, why?
>> >> > Probably because you don't have any suitable per-cpu timers to act as a
>> >> > wakeup. Do you have a per-cpu timer registered with CLOCK_EVT_FEAT_PERCPU
>> >> 
>> >> Yes, you are right, but i want to know why the timer do not support 
>> >> CLOCK_EVT_FEAT_PERCPU.
>> >
>> > I defer to Thomas on this one.
>> 
>> How should I know what kind of timers this hardware has?
>
> Duh, sorry, I replied to the wrong question. I meant to defer the decision
> about whether to print "<NULL>" if the wakeup timer is absent, or whether to
> omit the line entirely.
>
> I went with the former in the patches you queued as it's both consistent
> with the rest of the code and probably (?) easier to parse.

That makes more sense. I just kept it as is. The <NULL> is fine.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-06-01 18:14 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-24 22:18 [PATCH v2 0/5] tick/broadcast: Allow per-cpu timers to be used instead of broadcast Will Deacon
2021-05-24 22:18 ` [PATCH v2 1/5] tick/broadcast: Drop unneeded CONFIG_GENERIC_CLOCKEVENTS_BROADCAST guard Will Deacon
2021-05-31 15:07   ` [tip: timers/core] " tip-bot2 for Will Deacon
2021-05-24 22:18 ` [PATCH v2 2/5] tick/broadcast: Split __tick_broadcast_oneshot_control() into a helper Will Deacon
2021-05-27  7:23   ` Xin Hao
2021-05-27  8:22     ` Will Deacon
2021-05-27 11:35       ` Xin Hao
2021-05-27 11:55         ` Will Deacon
2021-05-27 11:56         ` Will Deacon
2021-05-31 14:29           ` Thomas Gleixner
2021-06-01 12:13             ` Will Deacon
2021-06-01 18:14               ` Thomas Gleixner
2021-05-31 15:06   ` [tip: timers/core] " tip-bot2 for Will Deacon
2021-05-24 22:18 ` [PATCH v2 3/5] tick/broadcast: Prefer per-cpu oneshot wakeup timers to broadcast Will Deacon
2021-05-31 15:06   ` [tip: timers/core] " tip-bot2 for Will Deacon
2021-05-24 22:18 ` [PATCH v2 4/5] tick/broadcast: Program wakeup timer when entering idle if required Will Deacon
2021-05-31 15:06   ` [tip: timers/core] " tip-bot2 for Will Deacon
2021-05-24 22:18 ` [PATCH v2 5/5] timer_list: Print name of per-cpu wakeup device Will Deacon
2021-05-31 15:06   ` [tip: timers/core] " tip-bot2 for Will Deacon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).