* [PATCH RESEND] softirq: reorder trace_softirqs_on to prevent lockdep splat
@ 2018-06-07 20:11 Joel Fernandes
2018-06-21 13:08 ` Ingo Molnar
0 siblings, 1 reply; 3+ messages in thread
From: Joel Fernandes @ 2018-06-07 20:11 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google),
Steven Rostedt, Peter Zijlstra, Ingo Molnar, Linus Torvalds,
Mathieu Desnoyers, Tom Zanussi, Namhyung Kim, Thomas Glexiner,
Boqun Feng, Paul McKenney, Masami Hiramatsu, Todd Kjos,
Erick Reyes, Julia Cartwright, Byungchul Park, stable
From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
I'm able to reproduce a lockdep splat with config options:
CONFIG_PROVE_LOCKING=y,
CONFIG_DEBUG_LOCK_ALLOC=y and
CONFIG_PREEMPTIRQ_EVENTS=y
$ echo 1 > /d/tracing/events/preemptirq/preempt_enable/enable
[ 26.112609] DEBUG_LOCKS_WARN_ON(current->softirqs_enabled)
[ 26.112636] WARNING: CPU: 0 PID: 118 at kernel/locking/lockdep.c:3854
[...]
[ 26.144229] Call Trace:
[ 26.144926] <IRQ>
[ 26.145506] lock_acquire+0x55/0x1b0
[ 26.146499] ? __do_softirq+0x46f/0x4d9
[ 26.147571] ? __do_softirq+0x46f/0x4d9
[ 26.148646] trace_preempt_on+0x8f/0x240
[ 26.149744] ? trace_preempt_on+0x4d/0x240
[ 26.150862] ? __do_softirq+0x46f/0x4d9
[ 26.151930] preempt_count_sub+0x18a/0x1a0
[ 26.152985] __do_softirq+0x46f/0x4d9
[ 26.153937] irq_exit+0x68/0xe0
[ 26.154755] smp_apic_timer_interrupt+0x271/0x280
[ 26.156056] apic_timer_interrupt+0xf/0x20
[ 26.157105] </IRQ>
The issue was this:
preempt_count = 1 << SOFTIRQ_SHIFT
__local_bh_enable(cnt = 1 << SOFTIRQ_SHIFT) {
if (softirq_count() == (cnt && SOFTIRQ_MASK)) {
trace_softirqs_on() {
current->softirqs_enabled = 1;
}
}
preempt_count_sub(cnt) {
trace_preempt_on() {
tracepoint() {
rcu_read_lock_sched() {
// jumps into lockdep
Where preempt_count still has softirqs disabled, but
current->softirqs_enabled is true, and we get a splat.
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Thomas Glexiner <tglx@linutronix.de>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Paul McKenney <paulmck@linux.vnet.ibm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Todd Kjos <tkjos@google.com>
Cc: Erick Reyes <erickreyes@google.com>
Cc: Julia Cartwright <julia@ni.com>
Cc: Byungchul Park <byungchul.park@lge.com>
Cc: stable@vger.kernel.org
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Fixes: d59158162e032 ("tracing: Add support for preempt and irq enable/disable events")
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
kernel/softirq.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 177de3640c78..8a040bcaa033 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -139,9 +139,13 @@ static void __local_bh_enable(unsigned int cnt)
{
lockdep_assert_irqs_disabled();
+ if (preempt_count() == cnt)
+ trace_preempt_on(CALLER_ADDR0, get_lock_parent_ip());
+
if (softirq_count() == (cnt & SOFTIRQ_MASK))
trace_softirqs_on(_RET_IP_);
- preempt_count_sub(cnt);
+
+ __preempt_count_sub(cnt);
}
/*
--
2.17.1.1185.g55be947832-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH RESEND] softirq: reorder trace_softirqs_on to prevent lockdep splat
2018-06-07 20:11 [PATCH RESEND] softirq: reorder trace_softirqs_on to prevent lockdep splat Joel Fernandes
@ 2018-06-21 13:08 ` Ingo Molnar
2018-06-21 13:25 ` Steven Rostedt
0 siblings, 1 reply; 3+ messages in thread
From: Ingo Molnar @ 2018-06-21 13:08 UTC (permalink / raw)
To: Joel Fernandes, Steven Rostedt
Cc: linux-kernel, Joel Fernandes (Google),
Steven Rostedt, Peter Zijlstra, Ingo Molnar, Linus Torvalds,
Mathieu Desnoyers, Tom Zanussi, Namhyung Kim, Thomas Glexiner,
Boqun Feng, Paul McKenney, Masami Hiramatsu, Todd Kjos,
Erick Reyes, Julia Cartwright, Byungchul Park, stable
* Joel Fernandes <joelaf@google.com> wrote:
> From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
>
> I'm able to reproduce a lockdep splat with config options:
> CONFIG_PROVE_LOCKING=y,
> CONFIG_DEBUG_LOCK_ALLOC=y and
> CONFIG_PREEMPTIRQ_EVENTS=y
>
> $ echo 1 > /d/tracing/events/preemptirq/preempt_enable/enable
>
> [ 26.112609] DEBUG_LOCKS_WARN_ON(current->softirqs_enabled)
> [ 26.112636] WARNING: CPU: 0 PID: 118 at kernel/locking/lockdep.c:3854
> [...]
> [ 26.144229] Call Trace:
> [ 26.144926] <IRQ>
> [ 26.145506] lock_acquire+0x55/0x1b0
> [ 26.146499] ? __do_softirq+0x46f/0x4d9
> [ 26.147571] ? __do_softirq+0x46f/0x4d9
> [ 26.148646] trace_preempt_on+0x8f/0x240
> [ 26.149744] ? trace_preempt_on+0x4d/0x240
> [ 26.150862] ? __do_softirq+0x46f/0x4d9
> [ 26.151930] preempt_count_sub+0x18a/0x1a0
> [ 26.152985] __do_softirq+0x46f/0x4d9
> [ 26.153937] irq_exit+0x68/0xe0
> [ 26.154755] smp_apic_timer_interrupt+0x271/0x280
> [ 26.156056] apic_timer_interrupt+0xf/0x20
> [ 26.157105] </IRQ>
>
> The issue was this:
>
> preempt_count = 1 << SOFTIRQ_SHIFT
>
> __local_bh_enable(cnt = 1 << SOFTIRQ_SHIFT) {
> if (softirq_count() == (cnt && SOFTIRQ_MASK)) {
> trace_softirqs_on() {
> current->softirqs_enabled = 1;
> }
> }
> preempt_count_sub(cnt) {
> trace_preempt_on() {
> tracepoint() {
> rcu_read_lock_sched() {
> // jumps into lockdep
>
> Where preempt_count still has softirqs disabled, but
> current->softirqs_enabled is true, and we get a splat.
>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Thomas Glexiner <tglx@linutronix.de>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Paul McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Todd Kjos <tkjos@google.com>
> Cc: Erick Reyes <erickreyes@google.com>
> Cc: Julia Cartwright <julia@ni.com>
> Cc: Byungchul Park <byungchul.park@lge.com>
> Cc: stable@vger.kernel.org
> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Fixes: d59158162e032 ("tracing: Add support for preempt and irq enable/disable events")
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> kernel/softirq.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index 177de3640c78..8a040bcaa033 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -139,9 +139,13 @@ static void __local_bh_enable(unsigned int cnt)
> {
> lockdep_assert_irqs_disabled();
>
> + if (preempt_count() == cnt)
> + trace_preempt_on(CALLER_ADDR0, get_lock_parent_ip());
> +
> if (softirq_count() == (cnt & SOFTIRQ_MASK))
> trace_softirqs_on(_RET_IP_);
> - preempt_count_sub(cnt);
> +
> + __preempt_count_sub(cnt);
> }
Acked-by: Ingo Molnar <mingo@kernel.org>
Steve, wanna queue this up? Splat should only occur with tracing AFAICT.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH RESEND] softirq: reorder trace_softirqs_on to prevent lockdep splat
2018-06-21 13:08 ` Ingo Molnar
@ 2018-06-21 13:25 ` Steven Rostedt
0 siblings, 0 replies; 3+ messages in thread
From: Steven Rostedt @ 2018-06-21 13:25 UTC (permalink / raw)
To: Ingo Molnar
Cc: Joel Fernandes, linux-kernel, Joel Fernandes (Google),
Peter Zijlstra, Ingo Molnar, Linus Torvalds, Mathieu Desnoyers,
Tom Zanussi, Namhyung Kim, Thomas Glexiner, Boqun Feng,
Paul McKenney, Masami Hiramatsu, Todd Kjos, Erick Reyes,
Julia Cartwright, Byungchul Park, stable
On Thu, 21 Jun 2018 15:08:48 +0200
Ingo Molnar <mingo@kernel.org> wrote:
> Acked-by: Ingo Molnar <mingo@kernel.org>
Thanks a lot Ingo!
>
> Steve, wanna queue this up? Splat should only occur with tracing AFAICT.
Yeah, I'll take this in my tree.
-- Steve
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-06-21 13:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-07 20:11 [PATCH RESEND] softirq: reorder trace_softirqs_on to prevent lockdep splat Joel Fernandes
2018-06-21 13:08 ` Ingo Molnar
2018-06-21 13:25 ` Steven Rostedt
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).