public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <frederic@kernel.org>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Frederic Weisbecker <frederic@kernel.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Anna-Maria Behnsen <anna-maria@linutronix.de>
Subject: [PATCH 02/10] clockevents: Shutdown and unregister current clockevents at CPUHP_AP_TICK_DYING
Date: Tue, 29 Oct 2024 13:54:43 +0100	[thread overview]
Message-ID: <20241029125451.54574-3-frederic@kernel.org> (raw)
In-Reply-To: <20241029125451.54574-1-frederic@kernel.org>

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


  parent reply	other threads:[~2024-10-29 12:55 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-29 12:54 [PATCH 00/10] clockevents: Rearrange cpuhotplug operations v2 Frederic Weisbecker
2024-10-29 12:54 ` [PATCH 01/10] clockevents: Improve clockevents_notify_released() comment Frederic Weisbecker
2024-10-30 20:13   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2024-10-31  9:51   ` tip-bot2 for Frederic Weisbecker
2024-10-29 12:54 ` Frederic Weisbecker [this message]
2024-10-30 20:13   ` [tip: timers/core] clockevents: Shutdown and unregister current clockevents at CPUHP_AP_TICK_DYING tip-bot2 for Frederic Weisbecker
2024-10-31  9:51   ` tip-bot2 for Frederic Weisbecker
2024-10-31 12:16   ` [PATCH 02/10] " Marek Szyprowski
2024-10-31 13:09     ` Thomas Gleixner
2024-10-29 12:54 ` [PATCH 03/10] tick: Remove now unneeded low-res tick stop on CPUHP_AP_TICK_DYING Frederic Weisbecker
2024-10-30 20:13   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2024-10-31  9:51   ` tip-bot2 for Frederic Weisbecker
2024-10-29 12:54 ` [PATCH 04/10] ARM: smp_twd: Remove clockevents shutdown call on offlining Frederic Weisbecker
2024-10-30 20:13   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2024-10-31  9:51   ` tip-bot2 for Frederic Weisbecker
2024-10-29 12:54 ` [PATCH 05/10] clocksource/drivers/arm_arch_timer: " Frederic Weisbecker
2024-10-30 20:13   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2024-10-31  9:51   ` tip-bot2 for Frederic Weisbecker
2024-10-29 12:54 ` [PATCH 06/10] clocksource/drivers/arm_global_timer: " Frederic Weisbecker
2024-10-30 20:13   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2024-10-31  9:51   ` tip-bot2 for Frederic Weisbecker
2024-10-29 12:54 ` [PATCH 07/10] clocksource/drivers/exynos_mct: " Frederic Weisbecker
2024-10-30 20:13   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2024-10-31  9:51   ` tip-bot2 for Frederic Weisbecker
2024-10-29 12:54 ` [PATCH 08/10] clocksource/drivers/armada-370-xp: " Frederic Weisbecker
2024-10-30 20:13   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2024-10-31  9:51   ` tip-bot2 for Frederic Weisbecker
2024-10-29 12:54 ` [PATCH 09/10] clocksource/drivers/qcom: " Frederic Weisbecker
2024-10-30 20:13   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2024-10-31  9:51   ` tip-bot2 for Frederic Weisbecker
2024-10-29 12:54 ` [PATCH 10/10] clocksource/drivers/timer-tegra: " Frederic Weisbecker
2024-10-30 20:13   ` [tip: timers/core] " tip-bot2 for Frederic Weisbecker
2024-10-31  9:51   ` tip-bot2 for Frederic Weisbecker
  -- strict thread matches above, loose matches on Subject: below --
2024-10-17 16:50 [PATCH 00/10] clockevents: Rearrange cpuhotplug operations Frederic Weisbecker
2024-10-17 16:50 ` [PATCH 02/10] clockevents: Shutdown and unregister current clockevents at CPUHP_AP_TICK_DYING Frederic Weisbecker
2024-10-21  5:00   ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241029125451.54574-3-frederic@kernel.org \
    --to=frederic@kernel.org \
    --cc=anna-maria@linutronix.de \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox