Re: BUG: spinlock trylock failure on UP, i.MX28 3.12.15-rt25
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: 2014-05-02 18:38:37
Also in:
linux-arm-kernel, lkml
* Steven Rostedt | 2014-04-22 14:16:50 [-0400]:
quoted hunk ↗ jump to hunk
On Tue, 22 Apr 2014 13:48:02 -0400 Steven Rostedt [off-list ref] wrote:quoted
I need to take a deeper look into the actual code. But as trylocks on UP are nops (always succeed), and if it expects to be able to do something in a critical section that is protected by spinlocks (again nops on UP), this would be broken for UP.Reading the code, I see it's broken. We should add something like this: Signed-off-by: Steven Rostedt <rostedt@goodmis.org> ---diff --git a/kernel/timer.c b/kernel/timer.c index cc34e42..a03164a 100644 --- a/kernel/timer.c +++ b/kernel/timer.c@@ -1447,6 +1447,12 @@ static void run_timer_softirq(struct softirq_action *h)__run_timers(base); } +#ifdef CONFIG_SMP +#define timer_should_raise_softirq(lock) !spin_do_trylock(lock) +#else +#define timer_should_raise_softirq(lock) 1 +#endif + /* * Called by the local, per-CPU timer interrupt on SMP. */@@ -1467,7 +1473,7 @@ void run_local_timers(void)return; } - if (!spin_do_trylock(&base->lock)) { + if (timer_should_raise_softirq(&base->lock)) { raise_softirq(TIMER_SOFTIRQ); return; }
Okay. So Peter said that it is okay to apply this since FULL_NO_HZ users wouldn't complain on UP. I still wouldn't say it is broken but that is a different story. We have two users of this trylock. run_local_timers() which pops up quite often (and you patched here) and the other is get_next_timer_interrupt(). What do you suggest we do here? It is basically the same thing. Sebastian