On Tue, 15 Nov 2022 21:28:52 +0100 (CET) Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote: > 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 it 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@xxxxxxxxxxx> > Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx> > Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > Link: https://lore.kernel.org/all/20220407161745.7d6754b3@xxxxxxxxxxxxxxxxxx > Link: https://lore.kernel.org/all/20221110064101.429013735@xxxxxxxxxxx > --- > kernel/time/timer.c | 64 ++++++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 55 insertions(+), 9 deletions(-) > > --- a/kernel/time/timer.c > +++ b/kernel/time/timer.c > @@ -1293,14 +1293,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 Nit, needs a common (or "then") after true. > + * 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; > @@ -1308,9 +1315,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); > } > > @@ -1332,20 +1352,31 @@ static int __timer_delete(struct timer_l > */ > 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 Same here. > + * 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; > @@ -1357,6 +1388,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); > > @@ -1379,7 +1412,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); > > @@ -1460,12 +1493,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 Here too. -- Steve > + * 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; > > @@ -1495,7 +1541,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); > @@ -1547,7 +1593,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); >
