[PATCH 00/10] clockevents: Rearrange cpuhotplug operations

[PATCH 00/10] clockevents: Rearrange cpuhotplug operations

[Frederic Weisbecker]

Some recent RCU issues triggered with offline ticks uncovered
hotplug behaviour differences between oneshot and periodic ticks.
The former is disabled on the dying CPU but not the latter. Also
the clockevents hotplug code is quite chaotic. This is a proposal to
fix the situation.

Frederic Weisbecker (10):
  clockevents: Improve clockevents_notify_released() comment
  clockevents: Shutdown and unregister current clockevents at
    CPUHP_AP_TICK_DYING
  tick: Remove now unneeded low-res tick stop on CPUHP_AP_TICK_DYING
  arm/twd: Remove clockevents shutdown call on offlining
  clocksource/drivers/arm_arch_timer: Remove clockevents shutdown call
    on offlining
  clocksource/driver/arm_global_timer: Remove clockevents shutdown call
    on offlining
  drivers/clocksource/exynos_mct: Remove clockevents shutdown call on
    offlining
  clocksource/drivers/armada-370-xp: Remove clockevents shutdown call on
    offlining
  clocksource/drivers/qcom: Remove clockevents shutdown call on
    offlining
  drivers/clocksource/timer-tegra: Remove clockevents shutdown call on
    offlining

 arch/arm/kernel/smp_twd.c                 |  1 -
 drivers/clocksource/arm_arch_timer.c      |  2 -
 drivers/clocksource/arm_global_timer.c    |  1 -
 drivers/clocksource/exynos_mct.c          |  1 -
 drivers/clocksource/timer-armada-370-xp.c |  1 -
 drivers/clocksource/timer-qcom.c          |  1 -
 drivers/clocksource/timer-tegra.c         |  1 -
 include/linux/tick.h                      |  2 -
 kernel/cpu.c                              |  2 -
 kernel/time/clockevents.c                 | 45 ++++++++++++-----------
 kernel/time/tick-sched.c                  | 25 +++----------
 11 files changed, 30 insertions(+), 52 deletions(-)

-- 
2.46.0

[PATCH 01/10] clockevents: Improve clockevents_notify_released() comment

[Frederic Weisbecker]

When a new clockevent device is added and replaces a previous device,
the latter is put into the released list. Then the released list is
added back.

This may look counter-intuitive but the reason is that released device
might be suitable for other uses. For example a released CPU regular
clockevent can be a better replacement for the current broadcast event.
Similarly a released broadcast clockevent can be a better replacement
for the current regular clockevent of a given CPU.

Improve comments stating about these subtleties.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 kernel/time/clockevents.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 78c7bd64d0dd..4af27994db93 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -337,13 +337,21 @@ int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,
 }
 
 /*
- * Called after a notify add to make devices available which were
- * released from the notifier call.
+ * Called after a clockevent has been added which might
+ * have replaced a current regular or broadcast device. A
+ * released normal device might be a suitable replacement
+ * for the current broadcast device. Similarly a released
+ * broadcast device might be a suitable replacement for a
+ * normal device.
  */
 static void clockevents_notify_released(void)
 {
 	struct clock_event_device *dev;
 
+	/*
+	 * Keep iterating as long as tick_check_new_device()
+	 * replaces a device.
+	 */
 	while (!list_empty(&clockevents_released)) {
 		dev = list_entry(clockevents_released.next,
 				 struct clock_event_device, list);
-- 
2.46.0

[PATCH 02/10] clockevents: Shutdown and unregister current clockevents at CPUHP_AP_TICK_DYING

[Frederic Weisbecker]

The way the clockevent devices are finally stopped while a CPU is
offlining is currently chaotic. The layout being by order:

1) tick_sched_timer_dying() stops the tick and the underlying clockevent
  but only for oneshot case. The periodic tick and its related
  clockevent still runs.

2) tick_broadcast_offline() detaches and stops the per-cpu oneshot
  broadcast and append it to the released list.

3) Some individual clockevent drivers stop the clockevents (a second time if
  the tick is oneshot)

4) Once the CPU is dead, a control CPU remotely detaches and stops
  (a 3rd time if oneshot mode) the CPU clockevent and adds it to the
  released list.

5) The released list containing the broadcast device released on step 2)
   and the remotely detached clockevent from step 4) are unregistered.

These random events can be factorized if the current clockevent is
detached and stopped by the dying CPU at the generic layer, that is
from the dying CPU:

a) Stop the tick
b) Stop/detach the underlying per-cpu oneshot broadcast clockevent
c) Stop/detach the underlying clockevent
d) Release / unregister the clockevents from b) and c)
e) Release / unregister the remaining clockevents from the dying CPU.
   This part could be performed by the dying CPU

This way the drivers and the tick layer don't need to care about
clockevent operations during cpuhotplug down. This also unifies the tick
behaviour on offline CPUs between oneshot and periodic modes, avoiding
offline ticks altogether for sanity.

Adopt the simplification and verify no further clockevent can be
registered for the dying CPU after the final release.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 include/linux/tick.h      |  2 --
 kernel/cpu.c              |  2 --
 kernel/time/clockevents.c | 33 ++++++++++++++-------------------
 3 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index 72744638c5b0..b0c74bfe0600 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -20,12 +20,10 @@ extern void __init tick_init(void);
 extern void tick_suspend_local(void);
 /* Should be core only, but XEN resume magic and ARM BL switcher require it */
 extern void tick_resume_local(void);
-extern void tick_cleanup_dead_cpu(int cpu);
 #else /* CONFIG_GENERIC_CLOCKEVENTS */
 static inline void tick_init(void) { }
 static inline void tick_suspend_local(void) { }
 static inline void tick_resume_local(void) { }
-static inline void tick_cleanup_dead_cpu(int cpu) { }
 #endif /* !CONFIG_GENERIC_CLOCKEVENTS */
 
 #if defined(CONFIG_GENERIC_CLOCKEVENTS) && defined(CONFIG_HOTPLUG_CPU)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index d293d52a3e00..895f3287e3f3 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1338,8 +1338,6 @@ static int takedown_cpu(unsigned int cpu)
 
 	cpuhp_bp_sync_dead(cpu);
 
-	tick_cleanup_dead_cpu(cpu);
-
 	/*
 	 * Callbacks must be re-integrated right away to the RCU state machine.
 	 * Otherwise an RCU callback could block a further teardown function
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 4af27994db93..4ac562ef7f40 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -452,6 +452,9 @@ void clockevents_register_device(struct clock_event_device *dev)
 {
 	unsigned long flags;
 
+	if (WARN_ON_ONCE(cpu_is_offline(raw_smp_processor_id())))
+		return;
+
 	/* Initialize state to DETACHED */
 	clockevent_set_state(dev, CLOCK_EVT_STATE_DETACHED);
 
@@ -618,39 +621,30 @@ void clockevents_resume(void)
 
 #ifdef CONFIG_HOTPLUG_CPU
 
-# ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 /**
- * tick_offline_cpu - Take CPU out of the broadcast mechanism
+ * tick_offline_cpu - Shutdown all clock events related
+ *                    to this CPU and take it out of the
+ *                    broadcast mechanism.
  * @cpu:	The outgoing CPU
  *
- * Called on the outgoing CPU after it took itself offline.
+ * Called by the dying CPU during teardown.
  */
 void tick_offline_cpu(unsigned int cpu)
-{
-	raw_spin_lock(&clockevents_lock);
-	tick_broadcast_offline(cpu);
-	raw_spin_unlock(&clockevents_lock);
-}
-# endif
-
-/**
- * tick_cleanup_dead_cpu - Cleanup the tick and clockevents of a dead cpu
- * @cpu:	The dead CPU
- */
-void tick_cleanup_dead_cpu(int cpu)
 {
 	struct clock_event_device *dev, *tmp;
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&clockevents_lock, flags);
+	raw_spin_lock(&clockevents_lock);
 
+	tick_broadcast_offline(cpu);
 	tick_shutdown(cpu);
+
 	/*
 	 * Unregister the clock event devices which were
-	 * released from the users in the notify chain.
+	 * released above.
 	 */
 	list_for_each_entry_safe(dev, tmp, &clockevents_released, list)
 		list_del(&dev->list);
+
 	/*
 	 * Now check whether the CPU has left unused per cpu devices
 	 */
@@ -662,7 +656,8 @@ void tick_cleanup_dead_cpu(int cpu)
 			list_del(&dev->list);
 		}
 	}
-	raw_spin_unlock_irqrestore(&clockevents_lock, flags);
+
+	raw_spin_unlock(&clockevents_lock);
 }
 #endif
 
-- 
2.46.0

Re: [PATCH 02/10] clockevents: Shutdown and unregister current clockevents at CPUHP_AP_TICK_DYING

[kernel test robot]

Hi Frederic,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/timers/core]
[also build test ERROR on tip/smp/core tegra/for-next linus/master v6.12-rc4 next-20241018]
[cannot apply to tip/timers/nohz daniel-lezcano/clockevents/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Frederic-Weisbecker/clockevents-Improve-clockevents_notify_released-comment/20241018-010807
base:   tip/timers/core
patch link:    https://lore.kernel.org/r/20241017165041.6954-3-frederic%40kernel.org
patch subject: [PATCH 02/10] clockevents: Shutdown and unregister current clockevents at CPUHP_AP_TICK_DYING
config: s390-allnoconfig (https://download.01.org/0day-ci/archive/20241021/202410211134.JXHThFcy-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project bfe84f7085d82d06d61c632a7bad1e692fd159e4)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241021/202410211134.JXHThFcy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410211134.JXHThFcy-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from kernel/time/clockevents.c:10:
   In file included from include/linux/clockchips.h:14:
   In file included from include/linux/clocksource.h:22:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
         |                                                           ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
     102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
         |                                                      ^
   In file included from kernel/time/clockevents.c:10:
   In file included from include/linux/clockchips.h:14:
   In file included from include/linux/clocksource.h:22:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
         |                                                           ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
     115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
         |                                                      ^
   In file included from kernel/time/clockevents.c:10:
   In file included from include/linux/clockchips.h:14:
   In file included from include/linux/clocksource.h:22:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     693 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     701 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     709 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     718 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     727 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     736 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   In file included from kernel/time/clockevents.c:13:
   In file included from include/linux/module.h:19:
   In file included from include/linux/elf.h:6:
   In file included from arch/s390/include/asm/elf.h:181:
   In file included from arch/s390/include/asm/mmu_context.h:11:
   In file included from arch/s390/include/asm/pgalloc.h:18:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> kernel/time/clockevents.c:632:6: error: redefinition of 'tick_offline_cpu'
     632 | void tick_offline_cpu(unsigned int cpu)
         |      ^
   kernel/time/tick-internal.h:148:20: note: previous definition is here
     148 | static inline void tick_offline_cpu(unsigned int cpu) { }
         |                    ^
   13 warnings and 1 error generated.


vim +/tick_offline_cpu +632 kernel/time/clockevents.c

1b72d43237980e Thomas Gleixner     2019-03-21  623  
1b72d43237980e Thomas Gleixner     2019-03-21  624  /**
86e36067908464 Frederic Weisbecker 2024-10-17  625   * tick_offline_cpu - Shutdown all clock events related
86e36067908464 Frederic Weisbecker 2024-10-17  626   *                    to this CPU and take it out of the
86e36067908464 Frederic Weisbecker 2024-10-17  627   *                    broadcast mechanism.
1b72d43237980e Thomas Gleixner     2019-03-21  628   * @cpu:	The outgoing CPU
1b72d43237980e Thomas Gleixner     2019-03-21  629   *
86e36067908464 Frederic Weisbecker 2024-10-17  630   * Called by the dying CPU during teardown.
1b72d43237980e Thomas Gleixner     2019-03-21  631   */
1b72d43237980e Thomas Gleixner     2019-03-21 @632  void tick_offline_cpu(unsigned int cpu)
d316c57ff6bfad Thomas Gleixner     2007-02-16  633  {
bb6eddf7676e1c Thomas Gleixner     2009-12-10  634  	struct clock_event_device *dev, *tmp;
0b858e6ff9a38b Li Zefan            2008-02-08  635  
86e36067908464 Frederic Weisbecker 2024-10-17  636  	raw_spin_lock(&clockevents_lock);
d316c57ff6bfad Thomas Gleixner     2007-02-16  637  
86e36067908464 Frederic Weisbecker 2024-10-17  638  	tick_broadcast_offline(cpu);
a49b116dcb1265 Thomas Gleixner     2015-04-03  639  	tick_shutdown(cpu);
86e36067908464 Frederic Weisbecker 2024-10-17  640  
d316c57ff6bfad Thomas Gleixner     2007-02-16  641  	/*
d316c57ff6bfad Thomas Gleixner     2007-02-16  642  	 * Unregister the clock event devices which were
86e36067908464 Frederic Weisbecker 2024-10-17  643  	 * released above.
d316c57ff6bfad Thomas Gleixner     2007-02-16  644  	 */
bb6eddf7676e1c Thomas Gleixner     2009-12-10  645  	list_for_each_entry_safe(dev, tmp, &clockevents_released, list)
bb6eddf7676e1c Thomas Gleixner     2009-12-10  646  		list_del(&dev->list);
86e36067908464 Frederic Weisbecker 2024-10-17  647  
bb6eddf7676e1c Thomas Gleixner     2009-12-10  648  	/*
bb6eddf7676e1c Thomas Gleixner     2009-12-10  649  	 * Now check whether the CPU has left unused per cpu devices
bb6eddf7676e1c Thomas Gleixner     2009-12-10  650  	 */
bb6eddf7676e1c Thomas Gleixner     2009-12-10  651  	list_for_each_entry_safe(dev, tmp, &clockevent_devices, list) {
bb6eddf7676e1c Thomas Gleixner     2009-12-10  652  		if (cpumask_test_cpu(cpu, dev->cpumask) &&
ea9d8e3f45404d Xiaotian Feng       2010-01-07  653  		    cpumask_weight(dev->cpumask) == 1 &&
ea9d8e3f45404d Xiaotian Feng       2010-01-07  654  		    !tick_is_broadcast_device(dev)) {
472c4a9437d3c6 Viresh Kumar        2015-05-21  655  			BUG_ON(!clockevent_state_detached(dev));
bb6eddf7676e1c Thomas Gleixner     2009-12-10  656  			list_del(&dev->list);
bb6eddf7676e1c Thomas Gleixner     2009-12-10  657  		}
bb6eddf7676e1c Thomas Gleixner     2009-12-10  658  	}
86e36067908464 Frederic Weisbecker 2024-10-17  659  
86e36067908464 Frederic Weisbecker 2024-10-17  660  	raw_spin_unlock(&clockevents_lock);
d316c57ff6bfad Thomas Gleixner     2007-02-16  661  }
a49b116dcb1265 Thomas Gleixner     2015-04-03  662  #endif
501f867064e95f Thomas Gleixner     2013-04-25  663  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 03/10] tick: Remove now unneeded low-res tick stop on CPUHP_AP_TICK_DYING

[Frederic Weisbecker]

The generic clockevent layer now detaches and stops the underlying
clockevent from the dying CPU, unifying the tick behaviour for both
periodic and oneshot mode on offline CPUs. There is no more need for
the tick layer to care about that.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 kernel/time/tick-sched.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 753a184c7090..9f90c7333b1d 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -311,14 +311,6 @@ static enum hrtimer_restart tick_nohz_handler(struct hrtimer *timer)
 	return HRTIMER_RESTART;
 }
 
-static void tick_sched_timer_cancel(struct tick_sched *ts)
-{
-	if (tick_sched_flag_test(ts, TS_FLAG_HIGHRES))
-		hrtimer_cancel(&ts->sched_timer);
-	else if (tick_sched_flag_test(ts, TS_FLAG_NOHZ))
-		tick_program_event(KTIME_MAX, 1);
-}
-
 #ifdef CONFIG_NO_HZ_FULL
 cpumask_var_t tick_nohz_full_mask;
 EXPORT_SYMBOL_GPL(tick_nohz_full_mask);
@@ -1055,7 +1047,10 @@ static void tick_nohz_stop_tick(struct tick_sched *ts, int cpu)
 	 * the tick timer.
 	 */
 	if (unlikely(expires == KTIME_MAX)) {
-		tick_sched_timer_cancel(ts);
+		if (tick_sched_flag_test(ts, TS_FLAG_HIGHRES))
+			hrtimer_cancel(&ts->sched_timer);
+		else
+			tick_program_event(KTIME_MAX, 1);
 		return;
 	}
 
@@ -1604,21 +1599,13 @@ void tick_setup_sched_timer(bool hrtimer)
  */
 void tick_sched_timer_dying(int cpu)
 {
-	struct tick_device *td = &per_cpu(tick_cpu_device, cpu);
 	struct tick_sched *ts = &per_cpu(tick_cpu_sched, cpu);
-	struct clock_event_device *dev = td->evtdev;
 	ktime_t idle_sleeptime, iowait_sleeptime;
 	unsigned long idle_calls, idle_sleeps;
 
 	/* This must happen before hrtimers are migrated! */
-	tick_sched_timer_cancel(ts);
-
-	/*
-	 * If the clockevents doesn't support CLOCK_EVT_STATE_ONESHOT_STOPPED,
-	 * make sure not to call low-res tick handler.
-	 */
-	if (tick_sched_flag_test(ts, TS_FLAG_NOHZ))
-		dev->event_handler = clockevents_handle_noop;
+	if (tick_sched_flag_test(ts, TS_FLAG_HIGHRES))
+		hrtimer_cancel(&ts->sched_timer);
 
 	idle_sleeptime = ts->idle_sleeptime;
 	iowait_sleeptime = ts->iowait_sleeptime;
-- 
2.46.0

[PATCH 04/10] arm/twd: Remove clockevents shutdown call on offlining

[Frederic Weisbecker]

The clockevents core already detached and unregistered it at this stage.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 arch/arm/kernel/smp_twd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 9a14f721a2b0..42a3706e16a6 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -93,7 +93,6 @@ static void twd_timer_stop(void)
 {
 	struct clock_event_device *clk = raw_cpu_ptr(twd_evt);
 
-	twd_shutdown(clk);
 	disable_percpu_irq(clk->irq);
 }
 
-- 
2.46.0

[PATCH 05/10] clocksource/drivers/arm_arch_timer: Remove clockevents shutdown call on offlining

[Frederic Weisbecker]

The clockevents core already detached and unregistered it at this stage.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 drivers/clocksource/arm_arch_timer.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 03733101e231..2bba81e25aa2 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -1179,8 +1179,6 @@ static void arch_timer_stop(struct clock_event_device *clk)
 	disable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi]);
 	if (arch_timer_has_nonsecure_ppi())
 		disable_percpu_irq(arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI]);
-
-	clk->set_state_shutdown(clk);
 }
 
 static int arch_timer_dying_cpu(unsigned int cpu)
-- 
2.46.0

[PATCH 06/10] clocksource/driver/arm_global_timer: Remove clockevents shutdown call on offlining

[Frederic Weisbecker]

The clockevents core already detached and unregistered it at this stage.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 drivers/clocksource/arm_global_timer.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
index a05cfaab5f84..2d86bbc2764a 100644
--- a/drivers/clocksource/arm_global_timer.c
+++ b/drivers/clocksource/arm_global_timer.c
@@ -195,7 +195,6 @@ static int gt_dying_cpu(unsigned int cpu)
 {
 	struct clock_event_device *clk = this_cpu_ptr(gt_evt);
 
-	gt_clockevent_shutdown(clk);
 	disable_percpu_irq(clk->irq);
 	return 0;
 }
-- 
2.46.0

[PATCH 07/10] drivers/clocksource/exynos_mct: Remove clockevents shutdown call on offlining

[Frederic Weisbecker]

The clockevents core already detached and unregistered it at this stage.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 drivers/clocksource/exynos_mct.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index ef8cb1b71be4..e6a02e351d77 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -496,7 +496,6 @@ static int exynos4_mct_dying_cpu(unsigned int cpu)
 		per_cpu_ptr(&percpu_mct_tick, cpu);
 	struct clock_event_device *evt = &mevt->evt;
 
-	evt->set_state_shutdown(evt);
 	if (mct_int_type == MCT_INT_SPI) {
 		if (evt->irq != -1)
 			disable_irq_nosync(evt->irq);
-- 
2.46.0

[PATCH 08/10] clocksource/drivers/armada-370-xp: Remove clockevents shutdown call on offlining

[Frederic Weisbecker]

The clockevents core already detached and unregistered it at this stage.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 drivers/clocksource/timer-armada-370-xp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clocksource/timer-armada-370-xp.c b/drivers/clocksource/timer-armada-370-xp.c
index 6ec565d6939a..54284c1c0651 100644
--- a/drivers/clocksource/timer-armada-370-xp.c
+++ b/drivers/clocksource/timer-armada-370-xp.c
@@ -201,7 +201,6 @@ static int armada_370_xp_timer_dying_cpu(unsigned int cpu)
 {
 	struct clock_event_device *evt = per_cpu_ptr(armada_370_xp_evt, cpu);
 
-	evt->set_state_shutdown(evt);
 	disable_percpu_irq(evt->irq);
 	return 0;
 }
-- 
2.46.0

[PATCH 09/10] clocksource/drivers/qcom: Remove clockevents shutdown call on offlining

[Frederic Weisbecker]

The clockevents core already detached and unregistered it at this stage.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 drivers/clocksource/timer-qcom.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clocksource/timer-qcom.c b/drivers/clocksource/timer-qcom.c
index eac4c95c6127..ddb1debe6a6b 100644
--- a/drivers/clocksource/timer-qcom.c
+++ b/drivers/clocksource/timer-qcom.c
@@ -130,7 +130,6 @@ static int msm_local_timer_dying_cpu(unsigned int cpu)
 {
 	struct clock_event_device *evt = per_cpu_ptr(msm_evt, cpu);
 
-	evt->set_state_shutdown(evt);
 	disable_percpu_irq(evt->irq);
 	return 0;
 }
-- 
2.46.0

[PATCH 10/10] drivers/clocksource/timer-tegra: Remove clockevents shutdown call on offlining

[Frederic Weisbecker]

The clockevents core already detached and unregistered it at this stage.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 drivers/clocksource/timer-tegra.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clocksource/timer-tegra.c b/drivers/clocksource/timer-tegra.c
index e9635c25eef4..35b6ce9deffa 100644
--- a/drivers/clocksource/timer-tegra.c
+++ b/drivers/clocksource/timer-tegra.c
@@ -158,7 +158,6 @@ static int tegra_timer_stop(unsigned int cpu)
 {
 	struct timer_of *to = per_cpu_ptr(&tegra_to, cpu);
 
-	to->clkevt.set_state_shutdown(&to->clkevt);
 	disable_irq_nosync(to->clkevt.irq);
 
 	return 0;
-- 
2.46.0

Re: [PATCH 00/10] clockevents: Rearrange cpuhotplug operations

[Thomas Gleixner]

On Thu, Oct 17 2024 at 18:50, Frederic Weisbecker wrote:
> Some recent RCU issues triggered with offline ticks uncovered
> hotplug behaviour differences between oneshot and periodic ticks.
> The former is disabled on the dying CPU but not the latter. Also
> the clockevents hotplug code is quite chaotic. This is a proposal to
> fix the situation.

I like it very much!

> Frederic Weisbecker (10):
>   clockevents: Improve clockevents_notify_released() comment
>   clockevents: Shutdown and unregister current clockevents at
>     CPUHP_AP_TICK_DYING

This has a build fail.

>   tick: Remove now unneeded low-res tick stop on CPUHP_AP_TICK_DYING
>   arm/twd: Remove clockevents shutdown call on offlining

The ARM prefix preference is:

    ARM: smp_twd:

>   clocksource/drivers/arm_arch_timer: Remove clockevents shutdown call
>     on offlining
>   clocksource/driver/arm_global_timer: Remove clockevents shutdown call
>     on offlining
>   drivers/clocksource/exynos_mct: Remove clockevents shutdown call on
>     offlining
>   clocksource/drivers/armada-370-xp: Remove clockevents shutdown call on
>     offlining
>   clocksource/drivers/qcom: Remove clockevents shutdown call on
>     offlining
>   drivers/clocksource/timer-tegra: Remove clockevents shutdown call on
>     offlining

Can you please make the subsystem prefixes for the clocksource drivers
consistent 'clocksource/drivers/foo:' ?

Thanks,

        tglx

Re: [PATCH 00/10] clockevents: Rearrange cpuhotplug operations

[Frederic Weisbecker]

Le Thu, Oct 24, 2024 at 10:34:34AM +0200, Thomas Gleixner a écrit :
> On Thu, Oct 17 2024 at 18:50, Frederic Weisbecker wrote:
> > Some recent RCU issues triggered with offline ticks uncovered
> > hotplug behaviour differences between oneshot and periodic ticks.
> > The former is disabled on the dying CPU but not the latter. Also
> > the clockevents hotplug code is quite chaotic. This is a proposal to
> > fix the situation.
> 
> I like it very much!
> 
> > Frederic Weisbecker (10):
> >   clockevents: Improve clockevents_notify_released() comment
> >   clockevents: Shutdown and unregister current clockevents at
> >     CPUHP_AP_TICK_DYING
> 
> This has a build fail.

Right.

> 
> >   tick: Remove now unneeded low-res tick stop on CPUHP_AP_TICK_DYING
> >   arm/twd: Remove clockevents shutdown call on offlining
> 
> The ARM prefix preference is:
> 
>     ARM: smp_twd:
> 
> >   clocksource/drivers/arm_arch_timer: Remove clockevents shutdown call
> >     on offlining
> >   clocksource/driver/arm_global_timer: Remove clockevents shutdown call
> >     on offlining
> >   drivers/clocksource/exynos_mct: Remove clockevents shutdown call on
> >     offlining
> >   clocksource/drivers/armada-370-xp: Remove clockevents shutdown call on
> >     offlining
> >   clocksource/drivers/qcom: Remove clockevents shutdown call on
> >     offlining
> >   drivers/clocksource/timer-tegra: Remove clockevents shutdown call on
> >     offlining
> 
> Can you please make the subsystem prefixes for the clocksource drivers
> consistent 'clocksource/drivers/foo:' ?

Sure, fixing that.

Thanks!

> 
> Thanks,
> 
>         tglx

[PATCH 00/10] clockevents: Rearrange cpuhotplug operations v2

[Frederic Weisbecker]

Some recent RCU issues triggered with offline ticks uncovered
hotplug behaviour differences between oneshot and periodic ticks.
The former is disabled on the dying CPU but not the latter. Also
the clockevents hotplug code is quite chaotic. This is a proposal to
fix the situation.

Changes since v1:

* Fix build error
* Fix commits prefixes

Thanks.

Frederic Weisbecker (10):
  clockevents: Improve clockevents_notify_released() comment
  clockevents: Shutdown and unregister current clockevents at
    CPUHP_AP_TICK_DYING
  tick: Remove now unneeded low-res tick stop on CPUHP_AP_TICK_DYING
  ARM: smp_twd: Remove clockevents shutdown call on offlining
  clocksource/drivers/arm_arch_timer: Remove clockevents shutdown call
    on offlining
  clocksource/drivers/arm_global_timer: Remove clockevents shutdown call
    on offlining
  clocksource/drivers/exynos_mct: Remove clockevents shutdown call on
    offlining
  clocksource/drivers/armada-370-xp: Remove clockevents shutdown call on
    offlining
  clocksource/drivers/qcom: Remove clockevents shutdown call on
    offlining
  clocksource/drivers/timer-tegra: Remove clockevents shutdown call on
    offlining

 arch/arm/kernel/smp_twd.c                 |  1 -
 drivers/clocksource/arm_arch_timer.c      |  2 -
 drivers/clocksource/arm_global_timer.c    |  1 -
 drivers/clocksource/exynos_mct.c          |  1 -
 drivers/clocksource/timer-armada-370-xp.c |  1 -
 drivers/clocksource/timer-qcom.c          |  1 -
 drivers/clocksource/timer-tegra.c         |  1 -
 include/linux/tick.h                      |  2 -
 kernel/cpu.c                              |  2 -
 kernel/time/clockevents.c                 | 45 ++++++++++++-----------
 kernel/time/tick-internal.h               |  3 +-
 kernel/time/tick-sched.c                  | 25 +++----------
 12 files changed, 31 insertions(+), 54 deletions(-)

-- 
2.46.0

[PATCH 05/10] clocksource/drivers/arm_arch_timer: Remove clockevents shutdown call on offlining

[Frederic Weisbecker]

The clockevents core already detached and unregistered it at this stage.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 drivers/clocksource/arm_arch_timer.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 03733101e231..2bba81e25aa2 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -1179,8 +1179,6 @@ static void arch_timer_stop(struct clock_event_device *clk)
 	disable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi]);
 	if (arch_timer_has_nonsecure_ppi())
 		disable_percpu_irq(arch_timer_ppi[ARCH_TIMER_PHYS_NONSECURE_PPI]);
-
-	clk->set_state_shutdown(clk);
 }
 
 static int arch_timer_dying_cpu(unsigned int cpu)
-- 
2.46.0