[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221121151850.635d3883@gandalf.local.home>
Date: Mon, 21 Nov 2022 15:18:50 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: LKML <linux-kernel@...r.kernel.org>,
Linus Torvalds <torvalds@...uxfoundation.org>,
Anna-Maria Behnsen <anna-maria@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
Stephen Boyd <sboyd@...nel.org>,
Guenter Roeck <linux@...ck-us.net>,
Andrew Morton <akpm@...ux-foundation.org>,
Julia Lawall <Julia.Lawall@...ia.fr>,
Arnd Bergmann <arnd@...db.de>,
Viresh Kumar <viresh.kumar@...aro.org>,
Marc Zyngier <maz@...nel.org>,
Marcel Holtmann <marcel@...tmann.org>,
Johan Hedberg <johan.hedberg@...il.com>,
Luiz Augusto von Dentz <luiz.dentz@...il.com>,
linux-bluetooth@...r.kernel.org,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Subject: Re: [patch 05/15] timers: Replace BUG_ON()s
On Tue, 15 Nov 2022 21:28:41 +0100 (CET)
Thomas Gleixner <tglx@...utronix.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@...utronix.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@...dmis.org>
-- Steve
>
> for (i = 0; i < WHEEL_SIZE; i++)
> migrate_timer_list(new_base, old_base->vectors + i);
Powered by blists - more mailing lists