All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org,
	linux-rt-users <linux-rt-users@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Carsten Emde <C.Emde@osadl.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	John Kacur <jkacur@redhat.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>
Subject: [PATCH RT 3/8] lockdep: Correctly annotate hardirq context in irq_exit()
Date: Wed, 15 Jan 2014 20:58:27 -0500	[thread overview]
Message-ID: <20140116020324.318895929@goodmis.org> (raw)
In-Reply-To: 20140116015824.201732155@goodmis.org

[-- Attachment #1: 0003-lockdep-Correctly-annotate-hardirq-context-in-irq_ex.patch --]
[-- Type: text/plain, Size: 4289 bytes --]

3.2.53-rt76-rc1 stable review patch.
If anyone has any objections, please let me know.

------------------

From: Peter Zijlstra <peterz@infradead.org>

There was a reported deadlock on -rt which lockdep didn't report.

It turns out that in irq_exit() we tell lockdep that the hardirq
context ends and then do all kinds of locking afterwards.

To fix it, move trace_hardirq_exit() to the very end of irq_exit(), this
ensures all locking in tick_irq_exit() and rcu_irq_exit() are properly
recorded as happening from hardirq context.

This however leads to the 'fun' little problem of running softirqs
while in hardirq context. To cure this make the softirq code a little
more complex (in the CONFIG_TRACE_IRQFLAGS case).

Due to stack swizzling arch dependent trickery we cannot pass an
argument to __do_softirq() to tell it if it was done from hardirq
context or not; so use a side-band argument.

When we do __do_softirq() from hardirq context, 'atomically' flip to
softirq context and back, so that no locking goes without being in
either hard- or soft-irq context.

I didn't find any new problems in mainline using this patch, but it
did show the -rt problem.

Cc: stable-rt@vger.kernel.org
Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/n/tip-dgwc5cdksbn0jk09vbmcc9sa@git.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/softirq.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/kernel/softirq.c b/kernel/softirq.c
index ca00a68..b261fad 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -297,6 +297,44 @@ EXPORT_SYMBOL(local_bh_enable_ip);
  */
 #define MAX_SOFTIRQ_RESTART 10
 
+#ifdef CONFIG_TRACE_IRQFLAGS
+/*
+ * Convoluted means of passing __do_softirq() a message through the various
+ * architecture execute_on_stack() bits.
+ *
+ * When we run softirqs from irq_exit() and thus on the hardirq stack we need
+ * to keep the lockdep irq context tracking as tight as possible in order to
+ * not miss-qualify lock contexts and miss possible deadlocks.
+ */
+static DEFINE_PER_CPU(int, softirq_from_hardirq);
+
+static inline void lockdep_softirq_from_hardirq(void)
+{
+	this_cpu_write(softirq_from_hardirq, 1);
+}
+
+static inline void lockdep_softirq_start(void)
+{
+	if (this_cpu_read(softirq_from_hardirq))
+		trace_hardirq_exit();
+	lockdep_softirq_enter();
+}
+
+static inline void lockdep_softirq_end(void)
+{
+	lockdep_softirq_exit();
+	if (this_cpu_read(softirq_from_hardirq)) {
+		this_cpu_write(softirq_from_hardirq, 0);
+		trace_hardirq_enter();
+	}
+}
+
+#else
+static inline void lockdep_softirq_from_hardirq(void) { }
+static inline void lockdep_softirq_start(void) { }
+static inline void lockdep_softirq_end(void) { }
+#endif
+
 asmlinkage void __do_softirq(void)
 {
 	__u32 pending;
@@ -308,7 +346,7 @@ asmlinkage void __do_softirq(void)
 
 	__local_bh_disable((unsigned long)__builtin_return_address(0),
 			   SOFTIRQ_OFFSET);
-	lockdep_softirq_enter();
+	lockdep_softirq_start();
 
 	cpu = smp_processor_id();
 restart:
@@ -324,7 +362,7 @@ restart:
 	if (pending)
 		wakeup_softirqd();
 
-	lockdep_softirq_exit();
+	lockdep_softirq_end();
 
 	account_system_vtime(current);
 	__local_bh_enable(SOFTIRQ_OFFSET);
@@ -583,6 +621,7 @@ static inline void invoke_softirq(void)
 {
 #ifndef CONFIG_PREEMPT_RT_FULL
 	if (!force_irqthreads)
+		lockdep_softirq_from_hardirq();
 		__do_softirq();
 	else {
 		__local_bh_disable((unsigned long)__builtin_return_address(0),
@@ -618,7 +657,6 @@ static inline void invoke_softirq(void)
 void irq_exit(void)
 {
 	account_system_vtime(current);
-	trace_hardirq_exit();
 	sub_preempt_count(IRQ_EXIT_OFFSET);
 	if (!in_interrupt() && local_softirq_pending())
 		invoke_softirq();
@@ -629,6 +667,7 @@ void irq_exit(void)
 	if (idle_cpu(smp_processor_id()) && !in_interrupt() && !need_resched())
 		tick_nohz_stop_sched_tick(0);
 #endif
+	trace_hardirq_exit(); /* must be last! */
 	__preempt_enable_no_resched();
 }
 
-- 
1.8.4.3



  parent reply	other threads:[~2014-01-16  2:03 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-16  1:58 [PATCH RT 0/8] Linux 3.2.53-rt76-rc1 Steven Rostedt
2014-01-16  1:58 ` [PATCH RT 1/8] cpu_down: move migrate_enable() back Steven Rostedt
2014-01-16  1:58 ` [PATCH RT 2/8] swait: Add a few more users Steven Rostedt
2014-01-16  1:58 ` Steven Rostedt [this message]
2014-01-16  1:58 ` [PATCH RT 4/8] rtmutex: use a trylock for waiter lock in trylock Steven Rostedt
2014-01-16  3:08   ` Mike Galbraith
2014-01-17  4:22     ` Steven Rostedt
2014-01-17  5:17       ` Mike Galbraith
2014-01-31 22:07         ` Sebastian Andrzej Siewior
2014-02-01  4:21           ` Mike Galbraith
2014-02-01  4:54             ` Mike Galbraith
2014-01-16  1:58 ` [PATCH RT 5/8] smp: introduce a generic on_each_cpu_mask() function Steven Rostedt
2014-01-16  1:58 ` [PATCH RT 6/8] smp: add func to IPI cpus based on parameter func Steven Rostedt
2014-01-16  1:58   ` Steven Rostedt
2014-01-16  1:58 ` [PATCH RT 7/8] fs: only send IPI to invalidate LRU BH when needed Steven Rostedt
2014-01-16  1:58 ` [PATCH RT 8/8] Linux 3.2.53-rt76-rc1 Steven Rostedt
2014-01-16  9:41 ` [PATCH RT 0/8] " Pavel Vasilyev
2014-01-16 14:18   ` Steven Rostedt
2014-01-16 14:36     ` Pavel Vasilyev
2014-01-16 15:09       ` Steven Rostedt
2014-01-16 16:08       ` Steven Rostedt
2014-01-16 17:19         ` Pavel Vasilyev
2014-01-16 16:18       ` Sebastian Andrzej Siewior
2014-01-16 16:18         ` Sebastian Andrzej Siewior
2014-01-17 13:01 ` Rolf Peukert
2014-01-17 13:21   ` Steven Rostedt
2014-01-17 15:40     ` Steven Rostedt

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=20140116020324.318895929@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=C.Emde@osadl.org \
    --cc=akpm@linux-foundation.org \
    --cc=bigeasy@linutronix.de \
    --cc=fweisbec@gmail.com \
    --cc=jkacur@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /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.