From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
To: <tglx@linutronix.de>
Cc: Vineet Gupta <Vineet.Gupta1@synopsys.com>,
Christian Ruppert <christian.ruppert@abilis.com>,
Pierrick Hascoet <pierrick.hascoet@abilis.com>,
Robert Love <rml@tech9.net>,
<kpreempt-tech@lists.sourceforge.net>,
Frederic Weisbecker <fweisbec@gmail.com>,
Steven Rostedt <srostedt@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>, <linux-kernel@vger.kernel.org>
Subject: [PATCH] [PATCH] Gaurantee spinlocks implicit barrier for !PREEMPT_COUNT
Date: Wed, 3 Apr 2013 19:41:22 +0530 [thread overview]
Message-ID: <1364998282-21437-1-git-send-email-vgupta@synopsys.com> (raw)
In-Reply-To: <alpine.LFD.2.02.1304031433350.21884@ionos>
spinlocks built in a !PREEMPT_COUNT config don't have the compiler
barrier provided by preempt_* routines. This can break lot of code which
relies on barrier semantics.
This manifested as random crashes in timer code when stress testing
ARC Linux (3.9-rc3): !SMP && !PREEMPT_COUNT
Here's the exact sequence which caused this:
(0). tv1[x] <----> t1 <---> t2
(1). mod_timer(t1) interrupted after it calls timer_pending()
(2). mod_timer(t2) completes
(3). mod_timer(t1) resumes but messes up the list.
(4). __runt_timers( ) uses bogus timer_list entry / crashes in
timer->function
when mod_timer() races against itself, the spinlock rightly serializes
the tv1[] timer link list, however timer_pending() called outside the
spinlock accesses timer's link list element, cached in a register.
With low register pressure (and a deep register file), there's nothing
forcing gcc to reload the element across the spinlock, causing a stale
value in register in case of race - ensuing a list corruption.
And the ARcompact disassembly which shows the culprit generated code:
mod_timer:
push_s blink
mov_s r13,r0 # timer, timer
..
###### timer_pending( )
ld_s r3,[r13] # <------ <variable>.entry.next LOADED
brne r3, 0, @.L163
.L163:
..
###### spin_lock_irq( )
lr r5, [status32] # flags
bic r4, r5, 6 # temp, flags,
and.f 0, r5, 6 # flags,
flag.nz r4
###### detach_if_pending( ) begins
tst_s r3,r3 <--------------
# timer_pending( ) checks timer->entry.next
# r3 is NOT reloaded by gcc, using stale value
beq.d @.L169
mov.eq r0,0
##### detach_timer( ): __list_del( )
ld r4,[r13,4] # <variable>.entry.prev, D.31439
st r4,[r3,4] # <variable>.prev, D.31439
st r3,[r4] # <variable>.next, D.30246
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
Reported-by: Christian Ruppert <christian.ruppert@abilis.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Christian Ruppert <christian.ruppert@abilis.com>
Cc: Pierrick Hascoet <pierrick.hascoet@abilis.com>
Cc: Robert Love <rml@tech9.net>
Cc: kpreempt-tech@lists.sourceforge.net
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Steven Rostedt <srostedt@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org
---
include/linux/preempt.h | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/include/linux/preempt.h b/include/linux/preempt.h
index 5a710b9..354d6e3 100644
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -93,14 +93,19 @@ do { \
#else /* !CONFIG_PREEMPT_COUNT */
-#define preempt_disable() do { } while (0)
-#define sched_preempt_enable_no_resched() do { } while (0)
-#define preempt_enable_no_resched() do { } while (0)
-#define preempt_enable() do { } while (0)
-
-#define preempt_disable_notrace() do { } while (0)
-#define preempt_enable_no_resched_notrace() do { } while (0)
-#define preempt_enable_notrace() do { } while (0)
+/*
+ * compiler barrier needed to ensure that spinlocks provide the barrier
+ * semantics despite !CONFIG_PREEMPT_COUNT.
+ * See commit log for actual bug which forced this change
+ */
+#define preempt_disable() do { barrier(); } while (0)
+#define sched_preempt_enable_no_resched() do { barrier(); } while (0)
+#define preempt_enable_no_resched() do { barrier(); } while (0)
+#define preempt_enable() do { barrier(); } while (0)
+
+#define preempt_disable_notrace() do { barrier(); } while (0)
+#define preempt_enable_no_resched_notrace() do { barrier(); } while (0)
+#define preempt_enable_notrace() do { barrier(); } while (0)
#endif /* CONFIG_PREEMPT_COUNT */
--
1.7.10.4
next prev parent reply other threads:[~2013-04-03 14:13 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-29 10:33 [PATCH] timer: Fix possible issues with non serialized timer_pending( ) Vineet Gupta
2013-04-03 7:20 ` Vineet Gupta
2013-04-03 8:53 ` Christian Ruppert
2013-04-03 12:36 ` Thomas Gleixner
2013-04-03 13:03 ` Christian Ruppert
2013-04-03 13:10 ` [RFC] Add implicit barriers to irqsave/restore class of functions Christian Ruppert
2013-04-03 13:29 ` Vineet Gupta
2013-04-04 8:26 ` Christian Ruppert
2013-04-04 16:13 ` Peter Zijlstra
2013-04-05 4:27 ` Vineet Gupta
2013-04-03 14:11 ` Vineet Gupta [this message]
2013-04-04 15:28 ` [PATCH] [PATCH] Gaurantee spinlocks implicit barrier for !PREEMPT_COUNT Christian Ruppert
2013-04-05 4:36 ` Vineet Gupta
2013-04-06 13:34 ` Vineet Gupta
2013-04-06 16:13 ` Linus Torvalds
2013-04-06 18:01 ` Linus Torvalds
2013-04-06 19:54 ` Jacquiot, Aurelien
2013-04-06 19:54 ` Jacquiot, Aurelien
2013-04-09 16:33 ` [PATCH] tile: comment assumption about __insn_mtspr for <asm/irqflags.h> Chris Metcalf
2013-04-09 16:33 ` Chris Metcalf
2013-04-08 4:20 ` [PATCH] [PATCH] Gaurantee spinlocks implicit barrier for !PREEMPT_COUNT Vineet Gupta
2013-04-08 4:48 ` Linus Torvalds
2013-04-08 13:37 ` Peter Zijlstra
2013-04-08 14:31 ` Steven Rostedt
2013-04-08 14:50 ` Linus Torvalds
2013-04-08 14:59 ` Steven Rostedt
2013-04-08 15:07 ` Linus Torvalds
2013-04-09 14:32 ` Linus Torvalds
2013-04-10 7:12 ` Peter Zijlstra
2013-04-08 14:05 ` Steven Rostedt
2013-04-08 4:49 ` Linus Torvalds
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1364998282-21437-1-git-send-email-vgupta@synopsys.com \
--to=vineet.gupta1@synopsys.com \
--cc=christian.ruppert@abilis.com \
--cc=fweisbec@gmail.com \
--cc=kpreempt-tech@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=pierrick.hascoet@abilis.com \
--cc=rml@tech9.net \
--cc=srostedt@redhat.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.