diff mbox series

[05/15] timers: Replace BUG_ON()s

Message ID 20221115202117.267934419@linutronix.de
State Superseded
Headers show
Series timers: Provide timer_shutdown[_sync]() | expand

Commit Message

Thomas Gleixner Nov. 15, 2022, 8:28 p.m. UTC
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>
---
 kernel/time/timer.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Steven Rostedt Nov. 21, 2022, 8:18 p.m. UTC | #1
On Tue, 15 Nov 2022 21:28:41 +0100 (CET)
Thomas Gleixner <tglx@linutronix.de> wrote:

> 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>
> ---
>  kernel/time/timer.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -1193,7 +1193,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);
> @@ -1210,7 +1211,8 @@ void add_timer_on(struct timer_list *tim
>  	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);
>  

I wonder if this patch should be split up into two patches, as the above is
trivial and the below is a bit more "complex". Although it's probably moot
as none of these should ever trigger.

> @@ -2017,8 +2019,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]);
> @@ -2035,7 +2035,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;

I guess interesting things could happen if running_timer was not NULL, but
again, WARN_ON_ONCE() should never be triggered in a correctly running
kernel.

Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve

>  
>  		for (i = 0; i < WHEEL_SIZE; i++)
>  			migrate_timer_list(new_base, old_base->vectors + i);
diff mbox series

Patch

--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1193,7 +1193,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);
@@ -1210,7 +1211,8 @@  void add_timer_on(struct timer_list *tim
 	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);
 
@@ -2017,8 +2019,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]);
@@ -2035,7 +2035,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);