Messages in this thread |  | | Date | Sat, 14 Jan 2023 10:36:00 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH v2 (repost)] locking/lockdep: add debug_show_all_lock_holders() |
| |
* Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
> --- a/include/linux/debug_locks.h > +++ b/include/linux/debug_locks.h > @@ -48,7 +48,18 @@ extern int debug_locks_off(void); > #endif > > #ifdef CONFIG_LOCKDEP > -extern void debug_show_all_locks(void); > +extern void __debug_show_all_locks(bool show_stack); > + > +static inline void debug_show_all_locks(void) > +{ > + __debug_show_all_locks(false); > +} > + > +static inline void debug_show_all_lock_holders(void) > +{ > + __debug_show_all_locks(true); > +} > + > extern void debug_show_held_locks(struct task_struct *task); > extern void debug_check_no_locks_freed(const void *from, unsigned long len); > extern void debug_check_no_locks_held(void); > @@ -61,6 +72,10 @@ static inline void debug_show_held_locks(struct task_struct *task) > { > } > > +static inline void debug_show_all_lock_holders(void) > +{ > +} > +
> - debug_show_all_locks(); > + debug_show_all_lock_holders();
> -void debug_show_all_locks(void) > +void __debug_show_all_locks(bool show_stack) > { > struct task_struct *g, *p; > > @@ -6495,12 +6496,19 @@ void debug_show_all_locks(void) > pr_warn("INFO: lockdep is turned off.\n"); > return; > } > - pr_warn("\nShowing all locks held in the system:\n"); > + if (show_stack) > + pr_warn("\nShowing all threads with locks held in the system:\n"); > + else > + pr_warn("\nShowing all locks held in the system:\n"); > > rcu_read_lock(); > for_each_process_thread(g, p) { > if (!p->lockdep_depth) > continue; > + if (p == current && p->lockdep_depth == 1) > + continue; > + if (show_stack) > + sched_show_task(p); > lockdep_print_held_locks(p); > touch_nmi_watchdog(); > touch_all_softlockup_watchdogs();
Yeah, so note how you introduce a function with a parameter:
void __debug_show_all_locks(bool show_stack)
... only to then *hide* the new parameter via helper functions:
static inline void debug_show_all_locks(void) { __debug_show_all_locks(false); }
static inline void debug_show_all_lock_holders(void) { __debug_show_all_locks(true); }
... which is a *strong* hint by our universe that the new parameter was probably a bad idea to begin with.
Given how small debug_show_all_locks() is to begin with, I'd suggest simply duplicating the loop into debug_show_all_lock_holders() or so.
Thanks,
Ingo
|  |