[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

[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 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

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 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 ++++++++++++++-------------------
 kernel/time/tick-internal.h |  3 +--
 4 files changed, 15 insertions(+), 25 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
 
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 5f2105e637bd..faac36de35b9 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -25,6 +25,7 @@ extern int tick_do_timer_cpu __read_mostly;
 extern void tick_setup_periodic(struct clock_event_device *dev, int broadcast);
 extern void tick_handle_periodic(struct clock_event_device *dev);
 extern void tick_check_new_device(struct clock_event_device *dev);
+extern void tick_offline_cpu(unsigned int cpu);
 extern void tick_shutdown(unsigned int cpu);
 extern void tick_suspend(void);
 extern void tick_resume(void);
@@ -142,10 +143,8 @@ static inline bool tick_broadcast_oneshot_available(void) { return tick_oneshot_
 #endif /* !(BROADCAST && ONESHOT) */
 
 #if defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) && defined(CONFIG_HOTPLUG_CPU)
-extern void tick_offline_cpu(unsigned int cpu);
 extern void tick_broadcast_offline(unsigned int cpu);
 #else
-static inline void tick_offline_cpu(unsigned int cpu) { }
 static inline void tick_broadcast_offline(unsigned int cpu) { }
 #endif
 
-- 
2.46.0

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

[Marek Szyprowski]

On 29.10.2024 13:54, Frederic Weisbecker wrote:
> 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>

This patch landed in today's linux-next as commit c80e6e9de6ae 
("clockevents: Shutdown and unregister current clockevents at 
CPUHP_AP_TICK_DYING"). Unfortunately it breaks booting most of my test 
systems (ARM 32bit, ARM 64bit and RISC-V 64bit). Reverting $subject on 
top of linux-next (and fixing the merge conflict) seems to be restoring 
proper boot sequence. Here is a log observed on QEMU's ARM64 'virt' machine:

--->8---

smp: Bringing up secondary CPUs ...
Detected PIPT I-cache on CPU1
------------[ cut here ]------------
WARNING: CPU: 1 PID: 0 at kernel/time/clockevents.c:455 
clockevents_register_device+0x170/0x180
Modules linked in:
CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Not tainted 6.12.0-rc1+ #9324
Hardware name: linux,dummy-virt (DT)
pstate: 200003c5 (nzCv DAIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : clockevents_register_device+0x170/0x180
lr : clockevents_config_and_register+0x2c/0x3c
...
Call trace:
  clockevents_register_device+0x170/0x180
  clockevents_config_and_register+0x2c/0x3c
  arch_timer_starting_cpu+0x170/0x294
  cpuhp_invoke_callback+0x16c/0x2b0
  __cpuhp_invoke_callback_range+0x90/0x118
  notify_cpu_starting+0x80/0xac
  secondary_start_kernel+0xe0/0x15c
  __secondary_switched+0xb8/0xbc
irq event stamp: 0
hardirqs last  enabled at (0): [<0000000000000000>] 0x0
hardirqs last disabled at (0): [<ffff800080050f2c>] 
copy_process+0x67c/0x198c
softirqs last  enabled at (0): [<ffff800080050f2c>] 
copy_process+0x67c/0x198c
softirqs last disabled at (0): [<0000000000000000>] 0x0
---[ end trace 0000000000000000 ]---
CPU1: Booted secondary processor 0x0000000001 [0x411fd070]
smp: Brought up 1 node, 2 CPUs
SMP: Total of 2 processors activated.
CPU: All CPU(s) started at EL1
CPU features: detected: 32-bit EL0 Support
CPU features: detected: CRC32 instructions
alternatives: applying system-wide alternatives
Memory: 855068K/1048576K available (19008K kernel code, 6526K rwdata, 
15852K rodata, 13824K init, 11422K bss, 90156K reserved, 98304K 
cma-reserved)
devtmpfs: initialized
Callback from call_rcu_tasks() invoked.
Running RCU synchronous self tests
Running RCU synchronous self tests
clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, 
max_idle_ns: 7645041785100000 ns
futex hash table entries: 512 (order: 4, 65536 bytes, linear)
16048 pages in range for non-PLT usage
507568 pages in range for PLT usage
pinctrl core: initialized pinctrl subsystem
NET: Registered PF_NETLINK/PF_ROUTE protocol family
DMA: preallocated 128 KiB GFP_KERNEL pool for atomic allocations
DMA: preallocated 128 KiB GFP_KERNEL|GFP_DMA pool for atomic allocations
DMA: preallocated 128 KiB GFP_KERNEL|GFP_DMA32 pool for atomic allocations
audit: initializing netlink subsys (disabled)
audit: type=2000 audit(0.580:1): state=initialized audit_enabled=0 res=1
thermal_sys: Registered thermal governor 'step_wise'
thermal_sys: Registered thermal governor 'power_allocator'
cpuidle: using governor menu
hw-breakpoint: found 6 breakpoint and 4 watchpoint registers.
ASID allocator initialised with 65536 entries
Serial: AMBA PL011 UART driver
9000000.pl011: ttyAMA0 at MMIO 0x9000000 (irq = 13, base_baud = 0) is a 
PL011 rev1
printk: legacy console [ttyAMA0] enabled
HugeTLB: registered 1.00 GiB page size, pre-allocated 0 pages
HugeTLB: 0 KiB vmemmap can be freed for a 1.00 GiB page
HugeTLB: registered 32.0 MiB page size, pre-allocated 0 pages
HugeTLB: 0 KiB vmemmap can be freed for a 32.0 MiB page
HugeTLB: registered 2.00 MiB page size, pre-allocated 0 pages
HugeTLB: 0 KiB vmemmap can be freed for a 2.00 MiB page
HugeTLB: registered 64.0 KiB page size, pre-allocated 0 pages
HugeTLB: 0 KiB vmemmap can be freed for a 64.0 KiB page
iommu: Default domain type: Translated
iommu: DMA domain TLB invalidation policy: strict mode
SCSI subsystem initialized
usbcore: registered new interface driver usbfs
usbcore: registered new interface driver hub
usbcore: registered new device driver usb
pps_core: LinuxPPS API ver. 1 registered
pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti 
<giometti@linux.it>
PTP clock support registered
EDAC MC: Ver: 3.0.0
scmi_core: SCMI protocol bus registered
FPGA manager framework
Advanced Linux Sound Architecture Driver Initialized.
vgaarb: loaded
clocksource: Switched to clocksource arch_sys_counter
VFS: Disk quotas dquot_6.6.0
VFS: Dquot-cache hash table entries: 512 (order 0, 4096 bytes)
NET: Registered PF_INET protocol family
IP idents hash table entries: 16384 (order: 5, 131072 bytes, linear)
tcp_listen_portaddr_hash hash table entries: 512 (order: 3, 36864 bytes, 
linear)
Table-perturb hash table entries: 65536 (order: 6, 262144 bytes, linear)
TCP established hash table entries: 8192 (order: 4, 65536 bytes, linear)
TCP bind hash table entries: 8192 (order: 8, 1179648 bytes, linear)
TCP: Hash tables configured (established 8192 bind 8192)
UDP hash table entries: 512 (order: 4, 81920 bytes, linear)
UDP-Lite hash table entries: 512 (order: 4, 81920 bytes, linear)
NET: Registered PF_UNIX/PF_LOCAL protocol family
RPC: Registered named UNIX socket transport module.
RPC: Registered udp transport module.
RPC: Registered tcp transport module.
RPC: Registered tcp-with-tls transport module.
RPC: Registered tcp NFSv4.1 backchannel transport module.
PCI: CLS 0 bytes, default 64
Initialise system trusted keyrings
workingset: timestamp_bits=42 max_order=18 bucket_order=0
squashfs: version 4.0 (2009/01/31) Phillip Lougher
NFS: Registering the id_resolver key type
Key type id_resolver registered
Key type id_legacy registered
nfs4filelayout_init: NFSv4 File Layout Driver Registering...
nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver Registering...
9p: Installing v9fs 9p2000 file system support
Key type asymmetric registered
Asymmetric key parser 'x509' registered
Block layer SCSI generic (bsg) driver version 0.4 loaded (major 244)
io scheduler mq-deadline registered
io scheduler kyber registered
io scheduler bfq registered
pl061_gpio 9030000.pl061: PL061 GPIO chip registered
ledtrig-cpu: registered to indicate activity on CPUs
pci-host-generic 4010000000.pcie: host bridge /pcie@10000000 ranges:
pci-host-generic 4010000000.pcie:       IO 0x003eff0000..0x003effffff -> 
0x0000000000
pci-host-generic 4010000000.pcie:      MEM 0x0010000000..0x003efeffff -> 
0x0010000000
pci-host-generic 4010000000.pcie:      MEM 0x8000000000..0xffffffffff -> 
0x8000000000
pci-host-generic 4010000000.pcie: Memory resource size exceeds max for 
32 bits
pci-host-generic 4010000000.pcie: ECAM at [mem 
0x4010000000-0x401fffffff] for [bus 00-ff]
pci-host-generic 4010000000.pcie: PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [bus 00-ff]
pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
pci_bus 0000:00: root bus resource [mem 0x10000000-0x3efeffff]
pci_bus 0000:00: root bus resource [mem 0x8000000000-0xffffffffff]
pci 0000:00:00.0: [1b36:0008] type 00 class 0x060000 conventional PCI 
endpoint
pci_bus 0000:00: resource 4 [io  0x0000-0xffff]
pci_bus 0000:00: resource 5 [mem 0x10000000-0x3efeffff]
pci_bus 0000:00: resource 6 [mem 0x8000000000-0xffffffffff]
Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
msm_serial: driver initialized
SuperH (H)SCI(F) driver initialized
STM32 USART driver initialized
brd: module loaded
loop: module loaded
virtio_blk virtio2: 1/0/0 default/read/poll queues
virtio_blk virtio2: [vda] 15106048 512-byte logical blocks (7.73 GB/7.20 
GiB)
virtio_blk virtio3: 1/0/0 default/read/poll queues
virtio_blk virtio3: [vdb] 154688 512-byte logical blocks (79.2 MB/75.5 MiB)
megasas: 07.727.03.00-rc1
physmap-flash 0.flash: physmap platform flash device: [mem 
0x00000000-0x03ffffff]
0.flash: Found 2 x16 devices at 0x0 in 32-bit bank. Manufacturer ID 
0x000000 Chip ID 0x000000
Intel/Sharp Extended Query Table at 0x0031
Using buffer write method
physmap-flash 0.flash: physmap platform flash device: [mem 
0x04000000-0x07ffffff]
0.flash: Found 2 x16 devices at 0x0 in 32-bit bank. Manufacturer ID 
0x000000 Chip ID 0x000000
Intel/Sharp Extended Query Table at 0x0031
Using buffer write method
Concatenating MTD devices:
(0): "0.flash"
(1): "0.flash"
into device "0.flash"
rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
rcu:     1-...!: (0 ticks this GP) idle=81c8/0/0x0 softirq=0/0 fqs=0 
(false positive?)
rcu:     (detected by 0, t=6502 jiffies, g=-1167, q=164 ncpus=2)
Sending NMI from CPU 0 to CPUs 1:
NMI backtrace for cpu 1 skipped: idling at default_idle_call+0xa8/0x1f0
rcu: rcu_preempt kthread timer wakeup didn't happen for 6501 jiffies! 
g-1167 f0x0 RCU_GP_WAIT_FQS(5) ->state=0x402
rcu:     Possible timer handling issue on cpu=1 timer-softirq=0
rcu: rcu_preempt kthread starved for 6502 jiffies! g-1167 f0x0 
RCU_GP_WAIT_FQS(5) ->state=0x402 ->cpu=1
rcu:     Unless rcu_preempt kthread gets sufficient CPU time, OOM is now 
expected behavior.
rcu: RCU grace-period kthread stack dump:
task:rcu_preempt     state:I stack:0     pid:16    tgid:16 ppid:2      
flags:0x00000008
Call trace:
  __switch_to+0xf4/0x168
  __schedule+0x2ec/0xb90
  schedule+0x50/0x15c
  schedule_timeout+0x7c/0x100
  rcu_gp_fqs_loop+0x170/0x8b8
  rcu_gp_kthread+0x27c/0x310
  kthread+0x124/0x128
  ret_from_fork+0x10/0x20

--->8---


> ---
>   include/linux/tick.h        |  2 --
>   kernel/cpu.c                |  2 --
>   kernel/time/clockevents.c   | 33 ++++++++++++++-------------------
>   kernel/time/tick-internal.h |  3 +--
>   4 files changed, 15 insertions(+), 25 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
>   
> diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
> index 5f2105e637bd..faac36de35b9 100644
> --- a/kernel/time/tick-internal.h
> +++ b/kernel/time/tick-internal.h
> @@ -25,6 +25,7 @@ extern int tick_do_timer_cpu __read_mostly;
>   extern void tick_setup_periodic(struct clock_event_device *dev, int broadcast);
>   extern void tick_handle_periodic(struct clock_event_device *dev);
>   extern void tick_check_new_device(struct clock_event_device *dev);
> +extern void tick_offline_cpu(unsigned int cpu);
>   extern void tick_shutdown(unsigned int cpu);
>   extern void tick_suspend(void);
>   extern void tick_resume(void);
> @@ -142,10 +143,8 @@ static inline bool tick_broadcast_oneshot_available(void) { return tick_oneshot_
>   #endif /* !(BROADCAST && ONESHOT) */
>   
>   #if defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) && defined(CONFIG_HOTPLUG_CPU)
> -extern void tick_offline_cpu(unsigned int cpu);
>   extern void tick_broadcast_offline(unsigned int cpu);
>   #else
> -static inline void tick_offline_cpu(unsigned int cpu) { }
>   static inline void tick_broadcast_offline(unsigned int cpu) { }
>   #endif
>   

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

[Thomas Gleixner]

On Thu, Oct 31 2024 at 13:16, Marek Szyprowski wrote:
> On 29.10.2024 13:54, Frederic Weisbecker wrote:
> This patch landed in today's linux-next as commit c80e6e9de6ae 
> ("clockevents: Shutdown and unregister current clockevents at 
> CPUHP_AP_TICK_DYING"). Unfortunately it breaks booting most of my test 
> systems (ARM 32bit, ARM 64bit and RISC-V 64bit). Reverting $subject on 
> top of linux-next (and fixing the merge conflict) seems to be restoring 
> proper boot sequence. Here is a log observed on QEMU's ARM64 'virt' machine:

It's fixed already. Refetch the branch (force updated).

Thanks,

        tglx