linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Linus Torvalds <torvalds@linuxfoundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Anna-Maria Behnsen <anna-maria@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Stephen Boyd <sboyd@kernel.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	Julia Lawall <Julia.Lawall@inria.fr>,
	Arnd Bergmann <arnd@arndb.de>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Marc Zyngier <maz@kernel.org>,
	Marcel Holtmann <marcel@holtmann.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
	linux-bluetooth@vger.kernel.org,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, Jacob Keller <jacob.e.keller@intel.com>
Subject: [patch V3 14/17] timers: Add shutdown mechanism to the internal functions
Date: Wed, 23 Nov 2022 21:18:52 +0100 (CET)	[thread overview]
Message-ID: <20221123201625.253883224@linutronix.de> (raw)
In-Reply-To: 20221123201306.823305113@linutronix.de

Tearing down timers which have circular dependencies to other
functionality, e.g. workqueues, where the timer can schedule work and work
can arm timers, is not trivial.

In those cases it is desired to shutdown the timer in a way which prevents
rearming of the timer. The mechanism to do so is to set timer->function to
NULL and use this as an indicator for the timer arming functions to ignore
the (re)arm request.

Add a shutdown argument to the relevant internal functions which makes the
actual deactivation code set timer->function to NULL which in turn prevents
rearming of the timer.

Co-developed-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://lore.kernel.org/all/20220407161745.7d6754b3@gandalf.local.home
Link: https://lore.kernel.org/all/20221110064101.429013735@goodmis.org
---
V2: Add missing commata (Steven)
V3: Changelog updates (Anna-Maria)
---
 kernel/time/timer.c |   64 ++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 55 insertions(+), 9 deletions(-)

--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1297,14 +1297,21 @@ void add_timer_on(struct timer_list *tim
 EXPORT_SYMBOL_GPL(add_timer_on);
 
 /**
- * __timer_delete - Internal function: Deactivate a timer.
+ * __timer_delete - Internal function: Deactivate a timer
  * @timer:	The timer to be deactivated
+ * @shutdown:	If true, this indicates that the timer is about to be
+ *		shutdown permanently.
+ *
+ * If @shutdown is true then @timer->function is set to NULL under the
+ * timer base lock which prevents further rearming of the time. In that
+ * case any attempt to rearm @timer after this function returns will be
+ * silently ignored.
  *
  * Return:
  * * %0 - The timer was not pending
  * * %1 - The timer was pending and deactivated
  */
-static int __timer_delete(struct timer_list *timer)
+static int __timer_delete(struct timer_list *timer, bool shutdown)
 {
 	struct timer_base *base;
 	unsigned long flags;
@@ -1312,9 +1319,22 @@ static int __timer_delete(struct timer_l
 
 	debug_assert_init(timer);
 
-	if (timer_pending(timer)) {
+	/*
+	 * If @shutdown is set then the lock has to be taken whether the
+	 * timer is pending or not to protect against a concurrent rearm
+	 * which might hit between the lockless pending check and the lock
+	 * aquisition. By taking the lock it is ensured that such a newly
+	 * enqueued timer is dequeued and cannot end up with
+	 * timer->function == NULL in the expiry code.
+	 *
+	 * If timer->function is currently executed, then this makes sure
+	 * that the callback cannot requeue the timer.
+	 */
+	if (timer_pending(timer) || shutdown) {
 		base = lock_timer_base(timer, &flags);
 		ret = detach_if_pending(timer, base, true);
+		if (shutdown)
+			timer->function = NULL;
 		raw_spin_unlock_irqrestore(&base->lock, flags);
 	}
 
@@ -1338,20 +1358,31 @@ EXPORT_SYMBOL(timer_delete);
  */
 int timer_delete(struct timer_list *timer)
 {
-	return __timer_delete(timer);
+	return __timer_delete(timer, false);
 }
 EXPORT_SYMBOL(timer_delete);
 
 /**
  * __try_to_del_timer_sync - Internal function: Try to deactivate a timer
  * @timer:	Timer to deactivate
+ * @shutdown:	If true, this indicates that the timer is about to be
+ *		shutdown permanently.
+ *
+ * If @shutdown is true then @timer->function is set to NULL under the
+ * timer base lock which prevents further rearming of the timer. Any
+ * attempt to rearm @timer after this function returns will be silently
+ * ignored.
+ *
+ * This function cannot guarantee that the timer cannot be rearmed
+ * right after dropping the base lock if @shutdown is false. That
+ * needs to be prevented by the calling code if necessary.
  *
  * Return:
  * * %0  - The timer was not pending
  * * %1  - The timer was pending and deactivated
  * * %-1 - The timer callback function is running on a different CPU
  */
-static int __try_to_del_timer_sync(struct timer_list *timer)
+static int __try_to_del_timer_sync(struct timer_list *timer, bool shutdown)
 {
 	struct timer_base *base;
 	unsigned long flags;
@@ -1363,6 +1394,8 @@ static int __try_to_del_timer_sync(struc
 
 	if (base->running_timer != timer)
 		ret = detach_if_pending(timer, base, true);
+	if (shutdown)
+		timer->function = NULL;
 
 	raw_spin_unlock_irqrestore(&base->lock, flags);
 
@@ -1387,7 +1420,7 @@ static int __try_to_del_timer_sync(struc
  */
 int try_to_del_timer_sync(struct timer_list *timer)
 {
-	return __try_to_del_timer_sync(timer);
+	return __try_to_del_timer_sync(timer, false);
 }
 EXPORT_SYMBOL(try_to_del_timer_sync);
 
@@ -1468,12 +1501,25 @@ static inline void del_timer_wait_runnin
  * __timer_delete_sync - Internal function: Deactivate a timer and wait
  *			 for the handler to finish.
  * @timer:	The timer to be deactivated
+ * @shutdown:	If true, @timer->function will be set to NULL under the
+ *		timer base lock which prevents rearming of @timer
+ *
+ * If @shutdown is not set the timer can be rearmed later. If the timer can
+ * be rearmed concurrently, i.e. after dropping the base lock then the
+ * return value is meaningless.
+ *
+ * If @shutdown is set then @timer->function is set to NULL under timer
+ * base lock which prevents rearming of the timer. Any attempt to rearm
+ * a shutdown timer is silently ignored.
+ *
+ * If the timer should be reused after shutdown it has to be initialized
+ * again.
  *
  * Return:
  * * %0	- The timer was not pending
  * * %1	- The timer was pending and deactivated
  */
-static int __timer_delete_sync(struct timer_list *timer)
+static int __timer_delete_sync(struct timer_list *timer, bool shutdown)
 {
 	int ret;
 
@@ -1503,7 +1549,7 @@ static int __timer_delete_sync(struct ti
 		lockdep_assert_preemption_enabled();
 
 	do {
-		ret = __try_to_del_timer_sync(timer);
+		ret = __try_to_del_timer_sync(timer, shutdown);
 
 		if (unlikely(ret < 0)) {
 			del_timer_wait_running(timer);
@@ -1555,7 +1601,7 @@ static int __timer_delete_sync(struct ti
  */
 int timer_delete_sync(struct timer_list *timer)
 {
-	return __timer_delete_sync(timer);
+	return __timer_delete_sync(timer, false);
 }
 EXPORT_SYMBOL(timer_delete_sync);
 


  parent reply	other threads:[~2022-11-23 20:22 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-23 20:18 [patch V3 00/17] timers: Provide timer_shutdown[_sync]() Thomas Gleixner
2022-11-23 20:18 ` [patch V3 01/17] Documentation: Remove bogus claim about del_timer_sync() Thomas Gleixner
2022-11-24 14:16   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2022-11-23 20:18 ` [patch V3 02/17] ARM: spear: Do not use timer namespace for timer_shutdown() function Thomas Gleixner
2022-11-23 20:18 ` [patch V3 03/17] clocksource/drivers/arm_arch_timer: " Thomas Gleixner
2022-11-23 20:18 ` [patch V3 04/17] clocksource/drivers/sp804: " Thomas Gleixner
2022-11-23 20:18 ` [patch V3 05/17] timers: Get rid of del_singleshot_timer_sync() Thomas Gleixner
2022-11-24 14:16   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2022-11-23 20:18 ` [patch V3 06/17] timers: Replace BUG_ON()s Thomas Gleixner
2022-11-24 14:16   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2022-11-23 20:18 ` [patch V3 07/17] timers: Update kernel-doc for various functions Thomas Gleixner
2022-11-24 14:16   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2022-11-23 20:18 ` [patch V3 08/17] timers: Use del_timer_sync() even on UP Thomas Gleixner
2022-11-23 20:18 ` [patch V3 09/17] timers: Rename del_timer_sync() to timer_delete_sync() Thomas Gleixner
2022-11-24 14:16   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2022-11-23 20:18 ` [patch V3 10/17] timers: Rename del_timer() to timer_delete() Thomas Gleixner
2022-11-24 14:16   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2022-11-23 20:18 ` [patch V3 11/17] Documentation: Replace del_timer/del_timer_sync() Thomas Gleixner
2022-11-24 14:16   ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2022-11-23 20:18 ` [patch V3 12/17] timers: Silently ignore timers with a NULL function Thomas Gleixner
2022-11-24  7:37   ` Anna-Maria Behnsen
2022-11-24  8:18     ` Thomas Gleixner
2022-11-24  8:22       ` [patch V3.1 " Thomas Gleixner
2022-11-23 20:18 ` [patch V3 13/17] timers: Split [try_to_]del_timer[_sync]() to prepare for shutdown mode Thomas Gleixner
2022-11-23 20:18 ` Thomas Gleixner [this message]
2022-11-24 13:48   ` [patch V3 14/17] timers: Add shutdown mechanism to the internal functions Anna-Maria Behnsen
2022-11-23 20:18 ` [patch V3 15/17] timers: Provide timer_shutdown[_sync]() Thomas Gleixner
2022-11-23 20:18 ` [patch V3 16/17] timers: Update the documentation to reflect on the new timer_shutdown() API Thomas Gleixner
2022-11-23 20:18 ` [patch V3 17/17] Bluetooth: hci_qca: Fix the teardown problem for real Thomas Gleixner
2022-11-24 13:56   ` Anna-Maria Behnsen
2022-11-24 14:00 ` [patch V3 00/17] timers: Provide timer_shutdown[_sync]() Anna-Maria Behnsen

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=20221123201625.253883224@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=Julia.Lawall@inria.fr \
    --cc=akpm@linux-foundation.org \
    --cc=anna-maria@linutronix.de \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jacob.e.keller@intel.com \
    --cc=johan.hedberg@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.org \
    --cc=maz@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=sboyd@kernel.org \
    --cc=torvalds@linuxfoundation.org \
    --cc=viresh.kumar@linaro.org \
    /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;
as well as URLs for NNTP newsgroup(s).