Skip to content

Commit

Permalink
locking/lockdep: Fix TRACE_IRQFLAGS vs. NMIs
Browse files Browse the repository at this point in the history
Prior to commit:

  859d069 ("lockdep: Prepare for NMI IRQ state tracking")

IRQ state tracking was disabled in NMIs due to nmi_enter()
doing lockdep_off() -- with the obvious requirement that NMI entry
call nmi_enter() before trace_hardirqs_off().

[ AFAICT, PowerPC and SH violate this order on their NMI entry ]

However, that commit explicitly changed lockdep_hardirqs_*() to ignore
lockdep_off() and breaks every architecture that has irq-tracing in
it's NMI entry that hasn't been fixed up (x86 being the only fixed one
at this point).

The reason for this change is that by ignoring lockdep_off() we can:

  - get rid of 'current->lockdep_recursion' in lockdep_assert_irqs*()
    which was going to to give header-recursion issues with the
    seqlock rework.

  - allow these lockdep_assert_*() macros to function in NMI context.

Restore the previous state of things and allow an architecture to
opt-in to the NMI IRQ tracking support, however instead of relying on
lockdep_off(), rely on in_nmi(), both are part of nmi_enter() and so
over-all entry ordering doesn't need to change.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20200727124852.GK119549@hirez.programming.kicks-ass.net
  • Loading branch information
peterz@infradead.org authored and Ingo Molnar committed Jul 27, 2020
1 parent ba1f2b2 commit ed00495
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 1 deletion.
3 changes: 3 additions & 0 deletions arch/x86/Kconfig.debug
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
config TRACE_IRQFLAGS_SUPPORT
def_bool y

config TRACE_IRQFLAGS_NMI_SUPPORT
def_bool y

config EARLY_PRINTK_USB
bool

Expand Down
8 changes: 7 additions & 1 deletion kernel/locking/lockdep.c
Original file line number Diff line number Diff line change
Expand Up @@ -3712,6 +3712,9 @@ void noinstr lockdep_hardirqs_on(unsigned long ip)
* and not rely on hardware state like normal interrupts.
*/
if (unlikely(in_nmi())) {
if (!IS_ENABLED(CONFIG_TRACE_IRQFLAGS_NMI))
return;

/*
* Skip:
* - recursion check, because NMI can hit lockdep;
Expand Down Expand Up @@ -3773,7 +3776,10 @@ void noinstr lockdep_hardirqs_off(unsigned long ip)
* they will restore the software state. This ensures the software
* state is consistent inside NMIs as well.
*/
if (unlikely(!in_nmi() && (current->lockdep_recursion & LOCKDEP_RECURSION_MASK)))
if (in_nmi()) {
if (!IS_ENABLED(CONFIG_TRACE_IRQFLAGS_NMI))
return;
} else if (current->lockdep_recursion & LOCKDEP_RECURSION_MASK)
return;

/*
Expand Down
6 changes: 6 additions & 0 deletions lib/Kconfig.debug
Original file line number Diff line number Diff line change
Expand Up @@ -1325,11 +1325,17 @@ config WW_MUTEX_SELFTEST
endmenu # lock debugging

config TRACE_IRQFLAGS
depends on TRACE_IRQFLAGS_SUPPORT
bool
help
Enables hooks to interrupt enabling and disabling for
either tracing or lock debugging.

config TRACE_IRQFLAGS_NMI
def_bool y
depends on TRACE_IRQFLAGS
depends on TRACE_IRQFLAGS_NMI_SUPPORT

config STACKTRACE
bool "Stack backtrace support"
depends on STACKTRACE_SUPPORT
Expand Down

0 comments on commit ed00495

Please sign in to comment.