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: 29+ 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-09 16:33 ` [PATCH] tile: comment assumption about __insn_mtspr for <asm/irqflags.h> 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).