*** BLURB HERE ***
Steven Rostedt (Google) (3): [Backport] ARM: spear: Do not use timer namespace for timer_shutdown() function [Backport] clocksource/drivers/arm_arch_timer: Do not use timer namespace for timer_shutdown() function [Backport] clocksource/drivers/sp804: Do not use timer namespace for timer_shutdown() function
Thomas Gleixner (5): [Backport] timers: Replace BUG_ON()s [Backport] timers: Silently ignore timers with a NULL function [Backport] timers: Split [try_to_]del_timer[_sync]() to prepare for shutdown mode [Backport] timers: Add shutdown mechanism to the internal functions [Backport] timers: Provide timer_shutdown[_sync]()
arch/arm/mach-spear/time.c | 8 +- drivers/clocksource/arm_arch_timer.c | 12 +- drivers/clocksource/timer-sp804.c | 6 +- include/linux/timer.h | 2 + kernel/time/timer.c | 326 ++++++++++++++++++++++----- 5 files changed, 284 insertions(+), 70 deletions(-)
From: "Steven Rostedt (Google)" rostedt@goodmis.org
mainline inclusion from mainline-v6.2-rc1 commit 80b55772d41d8afec68dbc4ff0368a9fe5d1f390 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I7R8WG
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
A new "shutdown" timer state is being added to the generic timer code. One of the functions to change the timer into the state is called "timer_shutdown()". This means that there can not be other functions called "timer_shutdown()" as the timer code owns the "timer_*" name space.
Rename timer_shutdown() to spear_timer_shutdown() to avoid this conflict.
Signed-off-by: Steven Rostedt (Google) rostedt@goodmis.org Signed-off-by: Thomas Gleixner tglx@linutronix.de Tested-by: Guenter Roeck linux@roeck-us.net Reviewed-by: Guenter Roeck linux@roeck-us.net Reviewed-by: Jacob Keller jacob.e.keller@intel.com Reviewed-by: Anna-Maria Behnsen anna-maria@linutronix.de Acked-by: Arnd Bergmann arnd@arndb.de Acked-by: Viresh Kumar viresh.kumar@linaro.org Link: https://lkml.kernel.org/r/20221106212701.822440504@goodmis.org Link: https://lore.kernel.org/all/20221105060155.228348078@goodmis.org/ Link: https://lore.kernel.org/r/20221110064146.810953418@goodmis.org Link: https://lore.kernel.org/r/20221123201624.513863211@linutronix.de
Signed-off-by: Yu Liao liaoyu15@huawei.com --- arch/arm/mach-spear/time.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-spear/time.c b/arch/arm/mach-spear/time.c index 289e036c9c30..57ff0787bad3 100644 --- a/arch/arm/mach-spear/time.c +++ b/arch/arm/mach-spear/time.c @@ -93,7 +93,7 @@ static void __init spear_clocksource_init(void) 200, 16, clocksource_mmio_readw_up); }
-static inline void timer_shutdown(struct clock_event_device *evt) +static inline void spear_timer_shutdown(struct clock_event_device *evt) { u16 val = readw(gpt_base + CR(CLKEVT));
@@ -104,7 +104,7 @@ static inline void timer_shutdown(struct clock_event_device *evt)
static int spear_shutdown(struct clock_event_device *evt) { - timer_shutdown(evt); + spear_timer_shutdown(evt);
return 0; } @@ -114,7 +114,7 @@ static int spear_set_oneshot(struct clock_event_device *evt) u16 val;
/* stop the timer */ - timer_shutdown(evt); + spear_timer_shutdown(evt);
val = readw(gpt_base + CR(CLKEVT)); val |= CTRL_ONE_SHOT; @@ -129,7 +129,7 @@ static int spear_set_periodic(struct clock_event_device *evt) u16 val;
/* stop the timer */ - timer_shutdown(evt); + spear_timer_shutdown(evt);
period = clk_get_rate(gpt_clk) / HZ; period >>= CTRL_PRESCALER16;
From: "Steven Rostedt (Google)" rostedt@goodmis.org
mainline inclusion from mainline-v6.2-rc1 commit 73737a5833ace25a8408b0d3b783637cb6bf29d1 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I7R8WG
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
A new "shutdown" timer state is being added to the generic timer code. One of the functions to change the timer into the state is called "timer_shutdown()". This means that there can not be other functions called "timer_shutdown()" as the timer code owns the "timer_*" name space.
Rename timer_shutdown() to arch_timer_shutdown() to avoid this conflict.
Signed-off-by: Steven Rostedt (Google) rostedt@goodmis.org Signed-off-by: Thomas Gleixner tglx@linutronix.de Tested-by: Guenter Roeck linux@roeck-us.net Reviewed-by: Guenter Roeck linux@roeck-us.net Reviewed-by: Jacob Keller jacob.e.keller@intel.com Reviewed-by: Anna-Maria Behnsen anna-maria@linutronix.de Acked-by: Marc Zyngier maz@kernel.org Link: https://lkml.kernel.org/r/20221106212702.002251651@goodmis.org Link: https://lore.kernel.org/all/20221105060155.409832154@goodmis.org/ Link: https://lore.kernel.org/r/20221110064146.981725531@goodmis.org Link: https://lore.kernel.org/r/20221123201624.574672568@linutronix.de
Signed-off-by: Yu Liao liaoyu15@huawei.com --- drivers/clocksource/arm_arch_timer.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 58863fd9c91b..bdf86bf57785 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -682,8 +682,8 @@ static irqreturn_t arch_timer_handler_virt_mem(int irq, void *dev_id) return timer_handler(ARCH_TIMER_MEM_VIRT_ACCESS, evt); }
-static __always_inline int timer_shutdown(const int access, - struct clock_event_device *clk) +static __always_inline int arch_timer_shutdown(const int access, + struct clock_event_device *clk) { unsigned long ctrl;
@@ -696,22 +696,22 @@ static __always_inline int timer_shutdown(const int access,
static int arch_timer_shutdown_virt(struct clock_event_device *clk) { - return timer_shutdown(ARCH_TIMER_VIRT_ACCESS, clk); + return arch_timer_shutdown(ARCH_TIMER_VIRT_ACCESS, clk); }
static int arch_timer_shutdown_phys(struct clock_event_device *clk) { - return timer_shutdown(ARCH_TIMER_PHYS_ACCESS, clk); + return arch_timer_shutdown(ARCH_TIMER_PHYS_ACCESS, clk); }
static int arch_timer_shutdown_virt_mem(struct clock_event_device *clk) { - return timer_shutdown(ARCH_TIMER_MEM_VIRT_ACCESS, clk); + return arch_timer_shutdown(ARCH_TIMER_MEM_VIRT_ACCESS, clk); }
static int arch_timer_shutdown_phys_mem(struct clock_event_device *clk) { - return timer_shutdown(ARCH_TIMER_MEM_PHYS_ACCESS, clk); + return arch_timer_shutdown(ARCH_TIMER_MEM_PHYS_ACCESS, clk); }
static __always_inline void set_next_event(const int access, unsigned long evt,
From: "Steven Rostedt (Google)" rostedt@goodmis.org
mainline inclusion from mainline-v6.2-rc1 commit 6e1fc2591f116dfb20b65cf27356475461d61bd8 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I7R8WG
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
A new "shutdown" timer state is being added to the generic timer code. One of the functions to change the timer into the state is called "timer_shutdown()". This means that there can not be other functions called "timer_shutdown()" as the timer code owns the "timer_*" name space.
Rename timer_shutdown() to evt_timer_shutdown() to avoid this conflict.
Signed-off-by: Steven Rostedt (Google) rostedt@goodmis.org Signed-off-by: Thomas Gleixner tglx@linutronix.de Tested-by: Guenter Roeck linux@roeck-us.net Reviewed-by: Guenter Roeck linux@roeck-us.net Reviewed-by: Jacob Keller jacob.e.keller@intel.com Reviewed-by: Anna-Maria Behnsen anna-maria@linutronix.de Link: https://lkml.kernel.org/r/20221106212702.182883323@goodmis.org Link: https://lore.kernel.org/all/20221105060155.592778858@goodmis.org/ Link: https://lore.kernel.org/r/20221110064147.158230501@goodmis.org Link: https://lore.kernel.org/r/20221123201624.634354813@linutronix.de
Conflicts: drivers/clocksource/timer-sp804.c Signed-off-by: Yu Liao liaoyu15@huawei.com --- drivers/clocksource/timer-sp804.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index e01222ea888f..4af87a63ada8 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -135,14 +135,14 @@ static irqreturn_t sp804_timer_interrupt(int irq, void *dev_id) return IRQ_HANDLED; }
-static inline void timer_shutdown(struct clock_event_device *evt) +static inline void evt_timer_shutdown(struct clock_event_device *evt) { writel(0, clkevt_base + TIMER_CTRL); }
static int sp804_shutdown(struct clock_event_device *evt) { - timer_shutdown(evt); + evt_timer_shutdown(evt); return 0; }
@@ -151,7 +151,7 @@ static int sp804_set_periodic(struct clock_event_device *evt) unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE | TIMER_CTRL_PERIODIC | TIMER_CTRL_ENABLE;
- timer_shutdown(evt); + evt_timer_shutdown(evt); writel(clkevt_reload, clkevt_base + TIMER_LOAD); writel(ctrl, clkevt_base + TIMER_CTRL); return 0;
From: Thomas Gleixner tglx@linutronix.de
mainline inclusion from mainline-v6.2-rc1 commit 82ed6f7ef58f9634fe4462dd721902c580f01569 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I7R8WG
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
The timer code still has a few BUG_ON()s left which are crashing the kernel in situations where it still can recover or simply refuse to take an action.
Remove the one in the hotplug callback which checks for the CPU being offline. If that happens then the whole hotplug machinery will explode in colourful ways.
Replace the rest with WARN_ON_ONCE() and conditional returns where appropriate.
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 Reviewed-by: Anna-Maria Behnsen anna-maria@linutronix.de Link: https://lore.kernel.org/r/20221123201624.769128888@linutronix.de
Signed-off-by: Yu Liao liaoyu15@huawei.com --- kernel/time/timer.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/kernel/time/timer.c b/kernel/time/timer.c index 8b6b33b81ac4..fed6a1572fad 100644 --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -1147,7 +1147,8 @@ EXPORT_SYMBOL(timer_reduce); */ void add_timer(struct timer_list *timer) { - BUG_ON(timer_pending(timer)); + if (WARN_ON_ONCE(timer_pending(timer))) + return; __mod_timer(timer, timer->expires, MOD_TIMER_NOTPENDING); } EXPORT_SYMBOL(add_timer); @@ -1164,7 +1165,8 @@ void add_timer_on(struct timer_list *timer, int cpu) struct timer_base *new_base, *base; unsigned long flags;
- BUG_ON(timer_pending(timer) || !timer->function); + if (WARN_ON_ONCE(timer_pending(timer) || !timer->function)) + return;
new_base = get_timer_cpu_base(timer->flags, cpu);
@@ -1900,8 +1902,6 @@ int timers_dead_cpu(unsigned int cpu) struct timer_base *new_base; int b, i;
- BUG_ON(cpu_online(cpu)); - for (b = 0; b < NR_BASES; b++) { old_base = per_cpu_ptr(&timer_bases[b], cpu); new_base = get_cpu_ptr(&timer_bases[b]); @@ -1918,7 +1918,8 @@ int timers_dead_cpu(unsigned int cpu) */ forward_timer_base(new_base);
- BUG_ON(old_base->running_timer); + WARN_ON_ONCE(old_base->running_timer); + old_base->running_timer = NULL;
for (i = 0; i < WHEEL_SIZE; i++) migrate_timer_list(new_base, old_base->vectors + i);
From: Thomas Gleixner tglx@linutronix.de
mainline inclusion from mainline-v6.2-rc1 commit d02e382cef06cc73561dd32dfdc171c00dcc416d category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I7R8WG
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
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.
In preparation for that replace the warnings in the relevant code paths with checks for timer->function == NULL. If the pointer is NULL, then discard the rearm request silently.
Add debug_assert_init() instead of the WARN_ON_ONCE(!timer->function) checks so that debug objects can warn about non-initialized timers.
The warning of debug objects does not warn if timer->function == NULL. It warns when timer was not initialized using timer_setup[_on_stack]() or via DEFINE_TIMER(). If developers fail to enable debug objects and then waste lots of time to figure out why their non-initialized timer is not firing, they deserve it. Same for initializing a timer with a NULL function.
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 Reviewed-by: Anna-Maria Behnsen anna-maria@linutronix.de Link: https://lore.kernel.org/all/20220407161745.7d6754b3@gandalf.local.home Link: https://lore.kernel.org/all/20221110064101.429013735@goodmis.org Link: https://lore.kernel.org/r/87wn7kdann.ffs@tglx
Conflicts: kernel/time/timer.c Signed-off-by: Yu Liao liaoyu15@huawei.com --- kernel/time/timer.c | 47 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/kernel/time/timer.c b/kernel/time/timer.c index fed6a1572fad..93e6c7ffa15e 100644 --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -968,7 +968,7 @@ __mod_timer(struct timer_list *timer, unsigned long expires, unsigned int option unsigned long clk = 0, flags; int ret = 0;
- BUG_ON(!timer->function); + debug_assert_init(timer);
/* * This is a common optimization triggered by the networking code - if @@ -995,6 +995,14 @@ __mod_timer(struct timer_list *timer, unsigned long expires, unsigned int option * dequeue/enqueue dance. */ base = lock_timer_base(timer, &flags); + /* + * Has @timer been shutdown? This needs to be evaluated + * while holding base lock to prevent a race against the + * shutdown code. + */ + if (!timer->function) + goto out_unlock; + forward_timer_base(base);
if (timer_pending(timer) && (options & MOD_TIMER_REDUCE) && @@ -1021,6 +1029,14 @@ __mod_timer(struct timer_list *timer, unsigned long expires, unsigned int option } } else { base = lock_timer_base(timer, &flags); + /* + * Has @timer been shutdown? This needs to be evaluated + * while holding base lock to prevent a race against the + * shutdown code. + */ + if (!timer->function) + goto out_unlock; + forward_timer_base(base); }
@@ -1083,6 +1099,8 @@ __mod_timer(struct timer_list *timer, unsigned long expires, unsigned int option * but will not re-activate and modify already deleted timers. * * It is useful for unserialized use of timers. + * If @timer->function == NULL then the start operation is silently + * discarded. */ int mod_timer_pending(struct timer_list *timer, unsigned long expires) { @@ -1109,6 +1127,9 @@ EXPORT_SYMBOL(mod_timer_pending); * The function returns whether it has modified a pending timer or not. * (ie. mod_timer() of an inactive timer returns 0, mod_timer() of an * active timer returns 1.) + * + * If @timer->function == NULL then the start operation is silently + * discarded. In this case the return value is 0 and meaningless. */ int mod_timer(struct timer_list *timer, unsigned long expires) { @@ -1124,6 +1145,9 @@ EXPORT_SYMBOL(mod_timer); * timer_reduce() is very similar to mod_timer(), except that it will only * modify a running timer if that would reduce the expiration time (it will * start a timer that isn't running). + * + * If @timer->function == NULL then the start operation is silently + * discarded. */ int timer_reduce(struct timer_list *timer, unsigned long expires) { @@ -1142,6 +1166,9 @@ EXPORT_SYMBOL(timer_reduce); * The timer's ->expires, ->function fields must be set prior calling this * function. * + * If @timer->function == NULL then the start operation is silently + * discarded. + * * Timers with an ->expires field in the past will be executed in the next * timer tick. */ @@ -1165,7 +1192,9 @@ void add_timer_on(struct timer_list *timer, int cpu) struct timer_base *new_base, *base; unsigned long flags;
- if (WARN_ON_ONCE(timer_pending(timer) || !timer->function)) + debug_assert_init(timer); + + if (WARN_ON_ONCE(timer_pending(timer))) return;
new_base = get_timer_cpu_base(timer->flags, cpu); @@ -1176,6 +1205,13 @@ void add_timer_on(struct timer_list *timer, int cpu) * wrong base locked. See lock_timer_base(). */ base = lock_timer_base(timer, &flags); + /* + * Has @timer been shutdown? This needs to be evaluated while + * holding base lock to prevent a race against the shutdown code. + */ + if (!timer->function) + goto out_unlock; + if (base != new_base) { timer->flags |= TIMER_MIGRATING;
@@ -1189,6 +1225,7 @@ void add_timer_on(struct timer_list *timer, int cpu)
debug_activate(timer, timer->expires); internal_add_timer(base, timer); +out_unlock: raw_spin_unlock_irqrestore(&base->lock, flags); } EXPORT_SYMBOL_GPL(add_timer_on); @@ -1369,6 +1406,12 @@ static void expire_timers(struct timer_base *base, struct hlist_head *head)
fn = timer->function;
+ if (WARN_ON_ONCE(!fn)) { + /* Should never happen. Emphasis on should! */ + base->running_timer = NULL; + continue; + } + if (timer->flags & TIMER_IRQSAFE) { raw_spin_unlock(&base->lock); call_timer_fn(timer, fn);
From: Thomas Gleixner tglx@linutronix.de
mainline inclusion from mainline-v6.2-rc1 commit 8553b5f2774a66b1f293b7d783934210afb8f23c category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I7R8WG
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
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.
Split the inner workings of try_do_del_timer_sync(), del_timer_sync() and del_timer() into helper functions to prepare for implementing the shutdown functionality.
No functional change.
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 Reviewed-by: Anna-Maria Behnsen anna-maria@linutronix.de Link: https://lore.kernel.org/all/20220407161745.7d6754b3@gandalf.local.home Link: https://lore.kernel.org/all/20221110064101.429013735@goodmis.org Link: https://lore.kernel.org/r/20221123201625.195147423@linutronix.de
Confilicts: kernel/time/timer.c
Signed-off-by: Yu Liao liaoyu15@huawei.com --- kernel/time/timer.c | 152 ++++++++++++++++++++++++++++++-------------- 1 file changed, 104 insertions(+), 48 deletions(-)
diff --git a/kernel/time/timer.c b/kernel/time/timer.c index 93e6c7ffa15e..7461c38eb417 100644 --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -1231,17 +1231,14 @@ void add_timer_on(struct timer_list *timer, int cpu) EXPORT_SYMBOL_GPL(add_timer_on);
/** - * del_timer - deactivate a timer. - * @timer: the timer to be deactivated + * __del_timer - Internal function: Deactivate a timer + * @timer: The timer to be deactivated * - * del_timer() deactivates a timer - this works on both active and inactive - * timers. - * - * The function returns whether it has deactivated a pending timer or not. - * (ie. del_timer() of an inactive timer returns 0, del_timer() of an - * active timer returns 1.) + * Return: + * * %0 - The timer was not pending + * * %1 - The timer was pending and deactivated */ -int del_timer(struct timer_list *timer) +static int __del_timer(struct timer_list *timer) { struct timer_base *base; unsigned long flags; @@ -1257,16 +1254,37 @@ int del_timer(struct timer_list *timer)
return ret; } + +/** + * del_timer - Deactivate a timer + * @timer: The timer to be deactivated + * + * The function only deactivates a pending timer, but contrary to + * del_timer_sync() it does not take into account whether the timer's + * callback function is concurrently executed on a different CPU or not. + * It neither prevents rearming of the timer. If @timer can be rearmed + * concurrently then the return value of this function is meaningless. + * + * Return: + * * %0 - The timer was not pending + * * %1 - The timer was pending and deactivated + */ +int del_timer(struct timer_list *timer) +{ + return __del_timer(timer); +} EXPORT_SYMBOL(del_timer);
/** - * try_to_del_timer_sync - Try to deactivate a timer - * @timer: timer to delete + * __try_to_del_timer_sync - Internal function: Try to deactivate a timer + * @timer: Timer to deactivate * - * This function tries to deactivate a timer. Upon successful (ret >= 0) - * exit the timer is not queued and the handler is not running on any CPU. + * 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 */ -int try_to_del_timer_sync(struct timer_list *timer) +static int __try_to_del_timer_sync(struct timer_list *timer) { struct timer_base *base; unsigned long flags; @@ -1283,46 +1301,39 @@ int try_to_del_timer_sync(struct timer_list *timer)
return ret; } -EXPORT_SYMBOL(try_to_del_timer_sync);
-#ifdef CONFIG_SMP /** - * del_timer_sync - deactivate a timer and wait for the handler to finish. - * @timer: the timer to be deactivated - * - * This function only differs from del_timer() on SMP: besides deactivating - * the timer it also makes sure the handler has finished executing on other - * CPUs. - * - * Synchronization rules: Callers must prevent restarting of the timer, - * otherwise this function is meaningless. It must not be called from - * interrupt contexts unless the timer is an irqsafe one. The caller must - * not hold locks which would prevent completion of the timer's - * handler. The timer's handler must not call add_timer_on(). Upon exit the - * timer is not queued and the handler is not running on any CPU. + * try_to_del_timer_sync - Try to deactivate a timer + * @timer: Timer to deactivate * - * Note: For !irqsafe timers, you must not hold locks that are held in - * interrupt context while calling this function. Even if the lock has - * nothing to do with the timer in question. Here's why:: + * This function tries to deactivate a timer. On success the timer is not + * queued and the timer callback function is not running on any CPU. * - * CPU0 CPU1 - * ---- ---- - * <SOFTIRQ> - * call_timer_fn(); - * base->running_timer = mytimer; - * spin_lock_irq(somelock); - * <IRQ> - * spin_lock(somelock); - * del_timer_sync(mytimer); - * while (base->running_timer == mytimer); + * This function does not guarantee that the timer cannot be rearmed right + * after dropping the base lock. That needs to be prevented by the calling + * code if necessary. * - * Now del_timer_sync() will never return and never release somelock. - * The interrupt on the other CPU is waiting to grab somelock but - * it has interrupted the softirq that CPU0 is waiting to finish. + * 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 + */ +int try_to_del_timer_sync(struct timer_list *timer) +{ + return __try_to_del_timer_sync(timer); +} +EXPORT_SYMBOL(try_to_del_timer_sync); + +/** + * __del_timer_sync - Internal function: Deactivate a timer and wait + * for the handler to finish. + * @timer: The timer to be deactivated * - * The function returns whether it has deactivated a pending timer or not. + * Return: + * * %0 - The timer was not pending + * * %1 - The timer was pending and deactivated */ -int del_timer_sync(struct timer_list *timer) +static int __del_timer_sync(struct timer_list *timer) { #ifdef CONFIG_LOCKDEP unsigned long flags; @@ -1342,12 +1353,57 @@ int del_timer_sync(struct timer_list *timer) */ WARN_ON(in_irq() && !(timer->flags & TIMER_IRQSAFE)); for (;;) { - int ret = try_to_del_timer_sync(timer); + int ret = __try_to_del_timer_sync(timer); if (ret >= 0) return ret; cpu_relax(); } } + +#ifdef CONFIG_SMP +/** + * del_timer_sync - Deactivate a timer and wait for the handler to finish. + * @timer: The timer to be deactivated + * + * Synchronization rules: Callers must prevent restarting of the timer, + * otherwise this function is meaningless. It must not be called from + * interrupt contexts unless the timer is an irqsafe one. The caller must + * not hold locks which would prevent completion of the timer's callback + * function. The timer's handler must not call add_timer_on(). Upon exit + * the timer is not queued and the handler is not running on any CPU. + * + * For !irqsafe timers, the caller must not hold locks that are held in + * interrupt context. Even if the lock has nothing to do with the timer in + * question. Here's why:: + * + * CPU0 CPU1 + * ---- ---- + * <SOFTIRQ> + * call_timer_fn(); + * base->running_timer = mytimer; + * spin_lock_irq(somelock); + * <IRQ> + * spin_lock(somelock); + * del_timer_sync(mytimer); + * while (base->running_timer == mytimer); + * + * Now del_timer_sync() will never return and never release somelock. + * The interrupt on the other CPU is waiting to grab somelock but it has + * interrupted the softirq that CPU0 is waiting to finish. + * + * This function cannot guarantee that the timer is not rearmed again by + * some concurrent or preempting code, right after it dropped the base + * lock. If there is the possibility of a concurrent rearm then the return + * value of the function is meaningless. + * + * Return: + * * %0 - The timer was not pending + * * %1 - The timer was pending and deactivated + */ +int del_timer_sync(struct timer_list *timer) +{ + return __del_timer_sync(timer); +} EXPORT_SYMBOL(del_timer_sync); #endif
From: Thomas Gleixner tglx@linutronix.de
mainline inclusion from mainline-v6.2-rc1 commit 0cc04e80458a822300b93f82ed861a513edde194 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I7R8WG
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
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 Reviewed-by: Anna-Maria Behnsen anna-maria@linutronix.de Link: https://lore.kernel.org/all/20220407161745.7d6754b3@gandalf.local.home Link: https://lore.kernel.org/all/20221110064101.429013735@goodmis.org Link: https://lore.kernel.org/r/20221123201625.253883224@linutronix.de
Conflict: kernel/time/timer.c
Signed-off-by: Yu Liao liaoyu15@huawei.com --- kernel/time/timer.c | 62 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 8 deletions(-)
diff --git a/kernel/time/timer.c b/kernel/time/timer.c index 7461c38eb417..ab0fadb8c0b0 100644 --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -1233,12 +1233,19 @@ EXPORT_SYMBOL_GPL(add_timer_on); /** * __del_timer - 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 __del_timer(struct timer_list *timer) +static int __del_timer(struct timer_list *timer, bool shutdown) { struct timer_base *base; unsigned long flags; @@ -1246,9 +1253,22 @@ static int __del_timer(struct timer_list *timer)
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); }
@@ -1271,20 +1291,31 @@ static int __del_timer(struct timer_list *timer) */ int del_timer(struct timer_list *timer) { - return __del_timer(timer); + return __del_timer(timer, false); } EXPORT_SYMBOL(del_timer);
/** * __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; @@ -1296,6 +1327,8 @@ static int __try_to_del_timer_sync(struct timer_list *timer)
if (base->running_timer != timer) ret = detach_if_pending(timer, base, true); + if (shutdown) + timer->function = NULL;
raw_spin_unlock_irqrestore(&base->lock, flags);
@@ -1320,7 +1353,7 @@ static int __try_to_del_timer_sync(struct timer_list *timer) */ 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);
@@ -1328,12 +1361,25 @@ EXPORT_SYMBOL(try_to_del_timer_sync); * __del_timer_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 __del_timer_sync(struct timer_list *timer) +static int __del_timer_sync(struct timer_list *timer, bool shutdown) { #ifdef CONFIG_LOCKDEP unsigned long flags; @@ -1353,7 +1399,7 @@ static int __del_timer_sync(struct timer_list *timer) */ WARN_ON(in_irq() && !(timer->flags & TIMER_IRQSAFE)); for (;;) { - int ret = __try_to_del_timer_sync(timer); + int ret = __try_to_del_timer_sync(timer, shutdown); if (ret >= 0) return ret; cpu_relax(); @@ -1402,7 +1448,7 @@ static int __del_timer_sync(struct timer_list *timer) */ int del_timer_sync(struct timer_list *timer) { - return __del_timer_sync(timer); + return __del_timer_sync(timer, false); } EXPORT_SYMBOL(del_timer_sync); #endif
From: Thomas Gleixner tglx@linutronix.de
mainline inclusion from mainline-v6.2-rc1 commit f571faf6e443b6011ccb585d57866177af1f643c category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I7R8WG
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
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.
Expose new interfaces for this: timer_shutdown_sync() and timer_shutdown().
timer_shutdown_sync() has the same functionality as timer_delete_sync() plus the NULL-ification of the timer function.
timer_shutdown() has the same functionality as timer_delete() plus the NULL-ification of the timer function.
In both cases the rearming of the timer is prevented by silently discarding rearm attempts due to timer->function being NULL.
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 Reviewed-by: Anna-Maria Behnsen anna-maria@linutronix.de Link: https://lore.kernel.org/all/20220407161745.7d6754b3@gandalf.local.home Link: https://lore.kernel.org/all/20221110064101.429013735@goodmis.org Link: https://lore.kernel.org/r/20221123201625.314230270@linutronix.de Signed-off-by: Yu Liao liaoyu15@huawei.com --- include/linux/timer.h | 2 ++ kernel/time/timer.c | 66 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+)
diff --git a/include/linux/timer.h b/include/linux/timer.h index 4759f2d940ef..53d96cfc2de9 100644 --- a/include/linux/timer.h +++ b/include/linux/timer.h @@ -167,6 +167,8 @@ extern int del_timer(struct timer_list * timer); extern int mod_timer(struct timer_list *timer, unsigned long expires); extern int mod_timer_pending(struct timer_list *timer, unsigned long expires); extern int timer_reduce(struct timer_list *timer, unsigned long expires); +extern int timer_shutdown_sync(struct timer_list *timer); +extern int timer_shutdown(struct timer_list *timer);
/* * The jiffies value which is added to now, when there is no timer diff --git a/kernel/time/timer.c b/kernel/time/timer.c index ab0fadb8c0b0..defe6514ba42 100644 --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -1295,6 +1295,27 @@ int del_timer(struct timer_list *timer) } EXPORT_SYMBOL(del_timer);
+/** + * timer_shutdown - Deactivate a timer and prevent rearming + * @timer: The timer to be deactivated + * + * The function does not wait for an eventually running timer callback on a + * different CPU but it prevents rearming of the timer. Any attempt to arm + * @timer after this function returns will be silently ignored. + * + * This function is useful for teardown code and should only be used when + * timer_shutdown_sync() cannot be invoked due to locking or context constraints. + * + * Return: + * * %0 - The timer was not pending + * * %1 - The timer was pending + */ +int timer_shutdown(struct timer_list *timer) +{ + return __del_timer(timer, true); +} +EXPORT_SYMBOL_GPL(timer_shutdown); + /** * __try_to_del_timer_sync - Internal function: Try to deactivate a timer * @timer: Timer to deactivate @@ -1442,6 +1463,9 @@ static int __del_timer_sync(struct timer_list *timer, bool shutdown) * lock. If there is the possibility of a concurrent rearm then the return * value of the function is meaningless. * + * If such a guarantee is needed, e.g. for teardown situations then use + * timer_shutdown_sync() instead. + * * Return: * * %0 - The timer was not pending * * %1 - The timer was pending and deactivated @@ -1453,6 +1477,48 @@ int del_timer_sync(struct timer_list *timer) EXPORT_SYMBOL(del_timer_sync); #endif
+/** + * timer_shutdown_sync - Shutdown a timer and prevent rearming + * @timer: The timer to be shutdown + * + * When the function returns it is guaranteed that: + * - @timer is not queued + * - The callback function of @timer is not running + * - @timer cannot be enqueued again. Any attempt to rearm + * @timer is silently ignored. + * + * See timer_delete_sync() for synchronization rules. + * + * This function is useful for final teardown of an infrastructure where + * the timer is subject to a circular dependency problem. + * + * A common pattern for this is a timer and a workqueue where the timer can + * schedule work and work can arm the timer. On shutdown the workqueue must + * be destroyed and the timer must be prevented from rearming. Unless the + * code has conditionals like 'if (mything->in_shutdown)' to prevent that + * there is no way to get this correct with timer_delete_sync(). + * + * timer_shutdown_sync() is solving the problem. The correct ordering of + * calls in this case is: + * + * timer_shutdown_sync(&mything->timer); + * workqueue_destroy(&mything->workqueue); + * + * After this 'mything' can be safely freed. + * + * This obviously implies that the timer is not required to be functional + * for the rest of the shutdown operation. + * + * Return: + * * %0 - The timer was not pending + * * %1 - The timer was pending + */ +int timer_shutdown_sync(struct timer_list *timer) +{ + return __del_timer_sync(timer, true); +} +EXPORT_SYMBOL_GPL(timer_shutdown_sync); + static void call_timer_fn(struct timer_list *timer, void (*fn)(struct timer_list *)) { int count = preempt_count();
反馈: 您发送到kernel@openeuler.org的补丁/补丁集,已成功转换为PR! PR链接地址: https://gitee.com/openeuler/kernel/pulls/1701 邮件列表地址:https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/W...
FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/1701 Mailing list address: https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/W...