Skip to content

Commit 044d0d6

Browse files
npigginPeter Zijlstra
authored andcommitted
lockdep: Only trace IRQ edges
Problem: raw_local_irq_save(); // software state on local_irq_save(); // software state off ... local_irq_restore(); // software state still off, because we don't enable IRQs raw_local_irq_restore(); // software state still off, *whoopsie* 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 commit: 859d069 ("lockdep: Prepare for NMI IRQ state tracking") changed IRQ tracing vs lockdep recursion and the first instance is fairly common, the other cases hardly ever happen. Signed-off-by: Nicholas Piggin <[email protected]> [rewrote changelog] Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Reviewed-by: Steven Rostedt (VMware) <[email protected]> Reviewed-by: Thomas Gleixner <[email protected]> Acked-by: Rafael J. Wysocki <[email protected]> Tested-by: Marco Elver <[email protected]> Link: https://lkml.kernel.org/r/[email protected]
1 parent 99dc56f commit 044d0d6

File tree

2 files changed

+11
-15
lines changed

2 files changed

+11
-15
lines changed

arch/powerpc/include/asm/hw_irq.h

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -200,17 +200,14 @@ static inline bool arch_irqs_disabled(void)
200200
#define powerpc_local_irq_pmu_save(flags) \
201201
do { \
202202
raw_local_irq_pmu_save(flags); \
203-
trace_hardirqs_off(); \
203+
if (!raw_irqs_disabled_flags(flags)) \
204+
trace_hardirqs_off(); \
204205
} while(0)
205206
#define powerpc_local_irq_pmu_restore(flags) \
206207
do { \
207-
if (raw_irqs_disabled_flags(flags)) { \
208-
raw_local_irq_pmu_restore(flags); \
209-
trace_hardirqs_off(); \
210-
} else { \
208+
if (!raw_irqs_disabled_flags(flags)) \
211209
trace_hardirqs_on(); \
212-
raw_local_irq_pmu_restore(flags); \
213-
} \
210+
raw_local_irq_pmu_restore(flags); \
214211
} while(0)
215212
#else
216213
#define powerpc_local_irq_pmu_save(flags) \

include/linux/irqflags.h

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -191,25 +191,24 @@ do { \
191191

192192
#define local_irq_disable() \
193193
do { \
194+
bool was_disabled = raw_irqs_disabled();\
194195
raw_local_irq_disable(); \
195-
trace_hardirqs_off(); \
196+
if (!was_disabled) \
197+
trace_hardirqs_off(); \
196198
} while (0)
197199

198200
#define local_irq_save(flags) \
199201
do { \
200202
raw_local_irq_save(flags); \
201-
trace_hardirqs_off(); \
203+
if (!raw_irqs_disabled_flags(flags)) \
204+
trace_hardirqs_off(); \
202205
} while (0)
203206

204207
#define local_irq_restore(flags) \
205208
do { \
206-
if (raw_irqs_disabled_flags(flags)) { \
207-
raw_local_irq_restore(flags); \
208-
trace_hardirqs_off(); \
209-
} else { \
209+
if (!raw_irqs_disabled_flags(flags)) \
210210
trace_hardirqs_on(); \
211-
raw_local_irq_restore(flags); \
212-
} \
211+
raw_local_irq_restore(flags); \
213212
} while (0)
214213

215214
#define safe_halt() \

0 commit comments

Comments
 (0)