Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Medium] DEPT fails to stop when lock debugging is globally disabled via kernel tainting. - [Low] dept_stop_emerg() is unnecessarily exported via EXPORT_SYMBOL_GPL despite having no module callers. - [Low] Implicit reliance on transitive header inclusion for dept_stop_emerg(). --
commit 3772379470ad3a639343d641bfb17cbd105fc9a5 Author: Byungchul Park <[email protected]> dept: make dept stop from working on debug_locks_off() The commit adds a dept_stop_emerg() call inside debug_locks_off() to stop the DEPT feature when panic or other lock debugging suspensions occur. > diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c > index 048348ea64d29..007e1bc7d201e 100644 > --- a/kernel/dependency/dept.c > +++ b/kernel/dependency/dept.c [ ... ] > @@ -187,6 +187,12 @@ static void dept_unlock(void) > arch_spin_unlock(&dept_spin); > } > > +void dept_stop_emerg(void) > +{ > + WRITE_ONCE(dept_stop, 1); > +} > +EXPORT_SYMBOL_GPL(dept_stop_emerg); [Severity: Low] Does this function need to be exported to loadable modules? CONFIG_DEPT is a boolean configuration, so kernel/dependency/dept.c is always built-in. Its only caller in lib/debug_locks.c is also always built-in, making this export unnecessary. > + > enum bfs_ret { > BFS_CONTINUE, > BFS_DONE, > diff --git a/lib/debug_locks.c b/lib/debug_locks.c > index a75ee30b77cb8..14a965914a8fb 100644 > --- a/lib/debug_locks.c > +++ b/lib/debug_locks.c [ ... ] > @@ -38,6 +38,8 @@ EXPORT_SYMBOL_GPL(debug_locks_silent); > */ > int debug_locks_off(void) > { > + dept_stop_emerg(); [Severity: Low] Is it possible to explicitly include <linux/dept.h> in lib/debug_locks.c? Relying on implicit transitive header inclusion for dept_stop_emerg() could break the build if inner header structures like spinlock.h or lockdep.h change in the future. > + > if (debug_locks && __debug_locks_off()) { [Severity: Medium] Will this miss DEPT shutdown when the system is tainted? For example, when a system fault occurs, add_taint() disables lockdep via __debug_locks_off(): kernel/panic.c:add_taint() { ... if (lockdep_ok == LOCKDEP_NOW_UNRELIABLE && __debug_locks_off()) pr_warn("Disabling lock debugging due to kernel taint\n"); ... } Because this bypasses debug_locks_off(), DEPT remains enabled when the system lock state is already unreliable, potentially leading to false positive dependency splats or further crashes. > if (!debug_locks_silent) { > console_verbose(); > return 1; > } > } > return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=30
