From: Peter Zijlstra <peterz@infradead.org>
To: linux-kernel@vger.kernel.org, mingo@kernel.org, will@kernel.org
Cc: npiggin@gmail.com, elver@google.com, jgross@suse.com,
paulmck@kernel.org, rostedt@goodmis.org, rjw@rjwysocki.net,
joel@joelfernandes.org, svens@linux.ibm.com, tglx@linutronix.de,
peterz@infradead.org
Subject: [PATCH 8/9] lockdep: Only trace IRQ edges
Date: Thu, 20 Aug 2020 09:30:39 +0200 [thread overview]
Message-ID: <20200820074640.752402406@infradead.org> (raw)
In-Reply-To: 20200820073031.886217423@infradead.org
From: Nicholas Piggin <npiggin@gmail.com>
Problem:
raw_local_irq_save();
local_irq_save();
...
local_irq_restore();
raw_local_irq_restore();
existing instances:
- lock_acquire()
raw_local_irq_save()
__lock_acquire()
arch_spin_lock(&graph_lock)
pv_wait() := kvm_wait() (same or worse for Xen/HyperV)
local_irq_save()
- trace_clock_global()
raw_local_irq_save()
arch_spin_lock()
pv_wait() := kvm_wait()
local_irq_save()
- apic_retrigger_irq()
raw_local_irq_save()
apic->send_IPI() := default_send_IPI_single_phys()
local_irq_save()
Possible solutions:
A) make it work by enabling the tracing inside raw_*()
B) make it work by keeping tracing disabled inside raw_*()
C) call it broken and clean it up now
Now, given that the only reason to use the raw_* variant is because you don't
want tracing. Therefore A) seems like a weird option (although it can be done).
C) is tempting, but OTOH it ends up converting a _lot_ of code to raw just
because there is one raw user, this strips the validation/tracing off for all
the other users.
So we pick B) and declare any code that ends up doing:
raw_local_irq_save()
local_irq_save()
lockdep_assert_irqs_disabled();
broken. AFAICT this problem has existed forever, the only reason it came
up is because I changed IRQ tracing vs lockdep recursion and the first
instance is fairly common, the other cases hardly ever happen.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
[rewrote changelog]
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/powerpc/include/asm/hw_irq.h | 11 ++++-------
include/linux/irqflags.h | 15 +++++++--------
2 files changed, 11 insertions(+), 15 deletions(-)
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -200,17 +200,14 @@ static inline bool arch_irqs_disabled(vo
#define powerpc_local_irq_pmu_save(flags) \
do { \
raw_local_irq_pmu_save(flags); \
- trace_hardirqs_off(); \
+ if (!raw_irqs_disabled_flags(flags)) \
+ trace_hardirqs_off(); \
} while(0)
#define powerpc_local_irq_pmu_restore(flags) \
do { \
- if (raw_irqs_disabled_flags(flags)) { \
- raw_local_irq_pmu_restore(flags); \
- trace_hardirqs_off(); \
- } else { \
+ if (!raw_irqs_disabled_flags(flags)) \
trace_hardirqs_on(); \
- raw_local_irq_pmu_restore(flags); \
- } \
+ raw_local_irq_pmu_restore(flags); \
} while(0)
#else
#define powerpc_local_irq_pmu_save(flags) \
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -191,25 +191,24 @@ do { \
#define local_irq_disable() \
do { \
+ bool was_disabled = raw_irqs_disabled();\
raw_local_irq_disable(); \
- trace_hardirqs_off(); \
+ if (!was_disabled) \
+ trace_hardirqs_off(); \
} while (0)
#define local_irq_save(flags) \
do { \
raw_local_irq_save(flags); \
- trace_hardirqs_off(); \
+ if (!raw_irqs_disabled_flags(flags)) \
+ trace_hardirqs_off(); \
} while (0)
#define local_irq_restore(flags) \
do { \
- if (raw_irqs_disabled_flags(flags)) { \
- raw_local_irq_restore(flags); \
- trace_hardirqs_off(); \
- } else { \
+ if (!raw_irqs_disabled_flags(flags)) \
trace_hardirqs_on(); \
- raw_local_irq_restore(flags); \
- } \
+ raw_local_irq_restore(flags); \
} while (0)
#define safe_halt() \
next prev parent reply other threads:[~2020-08-20 7:50 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-20 7:30 [PATCH 0/9] TRACE_IRQFLAGS wreckage Peter Zijlstra
2020-08-20 7:30 ` [PATCH 1/9] lockdep: Use raw_cpu_*() for per-cpu variables Peter Zijlstra
2020-08-20 13:54 ` Steven Rostedt
2020-08-20 7:30 ` [PATCH 2/9] sched,idle,rcu: Push rcu_idle deeper into the idle path Peter Zijlstra
2020-08-20 13:58 ` Steven Rostedt
2020-08-20 7:30 ` [PATCH 3/9] cpuidle: Make CPUIDLE_FLAG_TLB_FLUSHED generic Peter Zijlstra
2020-08-20 7:30 ` [PATCH 4/9] cpuidle: Move trace_cpu_idle() into generic code Peter Zijlstra
2020-08-20 7:30 ` [PATCH 5/9] x86/entry: Remove unused THUNKs Peter Zijlstra
2020-08-20 7:30 ` [PATCH 6/9] locking/lockdep: Cleanup Peter Zijlstra
2020-08-20 7:30 ` [PATCH 7/9] nds32: Implement arch_irqs_disabled() Peter Zijlstra
2020-08-20 7:30 ` Peter Zijlstra [this message]
2020-08-20 7:30 ` [PATCH 9/9] lockdep,trace: Expose tracepoints Peter Zijlstra
2020-08-20 14:36 ` [PATCH 0/9] TRACE_IRQFLAGS wreckage Steven Rostedt
2020-08-20 14:39 ` Steven Rostedt
2020-08-20 14:49 ` Marco Elver
2020-08-20 14:58 ` peterz
2020-08-20 16:53 ` Steven Rostedt
2020-08-20 17:20 ` Marco Elver
2020-08-20 19:59 ` Steven Rostedt
2020-08-21 6:37 ` Marco Elver
2020-08-21 6:54 ` peterz
2020-08-21 7:05 ` Marco Elver
2020-08-27 7:54 ` [tip: sched/urgent] sched: Use __always_inline on is_idle_task() tip-bot2 for Marco Elver
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=20200820074640.752402406@infradead.org \
--to=peterz@infradead.org \
--cc=elver@google.com \
--cc=jgross@suse.com \
--cc=joel@joelfernandes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=npiggin@gmail.com \
--cc=paulmck@kernel.org \
--cc=rjw@rjwysocki.net \
--cc=rostedt@goodmis.org \
--cc=svens@linux.ibm.com \
--cc=tglx@linutronix.de \
--cc=will@kernel.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 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).