If an interrupt is not masked by local_irq_disable (e.g., a powerpc perf
interrupt), then it can hit in local_irq_enable() after trace_hardirqs_on()
and before raw_local_irq_enable().

If that interrupt handler calls local_irq_save(), it will call
trace_hardirqs_off() but the local_irq_restore() will not call
trace_hardirqs_on() again because raw_irqs_disabled_flags(flags) is true.

This can lead lockdep_assert_irqs_enabled() to trigger false positive
warnings.

Fix this by being careful to only enable and disable trace_hardirqs with
the outer-most irq enable/disable.

Reported-by: Alexey Kardashevskiy <a...@ozlabs.ru>
Signed-off-by: Nicholas Piggin <npig...@gmail.com>
---

I haven't tested on other architectures but I imagine NMIs in general
might cause a similar problem.

Other architectures might have to be updated for patch 2, but there's
a lot of asm around interrupt/return, so I didn't have a very good
lock. The warnings should be harmless enough and uncover most places
that need updating.

 arch/powerpc/include/asm/hw_irq.h | 11 ++++-------
 include/linux/irqflags.h          | 29 ++++++++++++++++++-----------
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/include/asm/hw_irq.h 
b/arch/powerpc/include/asm/hw_irq.h
index 3a0db7b0b46e..35060be09073 100644
--- 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(void)
 #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)                      \
diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 6384d2813ded..571ee29ecefc 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -163,26 +163,33 @@ do {                                              \
  * if !TRACE_IRQFLAGS.
  */
 #ifdef CONFIG_TRACE_IRQFLAGS
-#define local_irq_enable() \
-       do { trace_hardirqs_on(); raw_local_irq_enable(); } while (0)
-#define local_irq_disable() \
-       do { raw_local_irq_disable(); trace_hardirqs_off(); } while (0)
+#define local_irq_enable()                             \
+       do {                                            \
+               trace_hardirqs_on();                    \
+               raw_local_irq_enable();                 \
+       } while (0)
+
+#define local_irq_disable()                            \
+       do {                                            \
+               bool was_disabled = raw_irqs_disabled(); \
+               raw_local_irq_disable();                \
+               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()                            \
-- 
2.23.0

Reply via email to