Thread (24 messages) 24 messages, 6 authors, 2014-05-02

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help