Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Hides missing assembly trace annotations, causing DEPT and Lockdep states to diverge and allowing NMIs to silently revert the workaround. - [Medium] Exporting `dept_hardirqs_off` as GPL-only breaks the build/loading of non-GPL modules that use `local_irq_save()` or `local_irq_disable()`. --
commit 3f12c8d53e9de3037ef421690b95c2ca6667e3a1 Author: Byungchul Park <[email protected]> dept: call dept_hardirqs_off() in local_irq_*() regardless of irq state For dept to function properly, dept_task()->hardirqs_enabled must be set correctly. If it fails to set this value to false, for example, dept may mistakenly think irq is still enabled even when it's not. > diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h > index d8b9cf093f83d..586f5bad4da78 100644 > --- a/include/linux/irqflags.h > +++ b/include/linux/irqflags.h > @@ -214,6 +214,13 @@ extern void warn_bogus_irq_restore(void); > raw_local_irq_disable(); \ > if (!was_disabled) \ > trace_hardirqs_off(); \ > + /* \ > + * Just in case that C code has missed \ > + * trace_hardirqs_off() at the first \ > + * place e.g. disabling irq at asm code.\ > + */ \ > + else \ > + dept_hardirqs_off(); \ > } while (0) [Severity: High] Can NMIs silently revert this workaround? If assembly code disables IRQs but misses the trace_hardirqs_off() annotation, and then C code calls local_irq_disable(), this skips trace_hardirqs_off() but executes the new dept_hardirqs_off() branch. Lockdep still thinks IRQs are enabled, while DEPT thinks they are disabled. If an NMI occurs while in this state, irqentry_nmi_enter() saves the Lockdep state (which is incorrectly 'enabled'). Upon NMI exit, irqentry_nmi_exit() restores the saved Lockdep state by calling lockdep_hardirqs_on(), which unconditionally calls dept_hardirqs_on(). Will this force DEPT back into an incorrect 'enabled' state while hardware IRQs are actually disabled, potentially leading to false-positive circular dependency reports? [ ... ] > diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c > index 007e1bc7d201e..35a3667ac8b3c 100644 > --- a/kernel/dependency/dept.c > +++ b/kernel/dependency/dept.c > @@ -2095,6 +2095,7 @@ void noinstr dept_hardirqs_off(void) > */ > dept_task()->hardirqs_enabled = false; > } > +EXPORT_SYMBOL_GPL(dept_hardirqs_off); [Severity: Medium] Will this break the build or loading of non-GPL modules? Because dept_hardirqs_off() is now called directly from universal inline macros like local_irq_disable() and local_irq_save(), any out-of-tree module marked with a non-GPL license that invokes these macros will fail to link or load when CONFIG_DEPT is enabled. Standard trace_hardirqs_off() avoids this issue by being exported via EXPORT_SYMBOL. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=33
