* [PATCH RFC 0/3] generalize panic_print's dump function to be
@ 2025-05-07 10:43 Feng Tang
2025-05-07 10:43 ` [PATCH RFC 1/3] kernel/panic: generalize panic_print's function to show sys info Feng Tang
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Feng Tang @ 2025-05-07 10:43 UTC (permalink / raw)
To: Andrew Morton, Petr Mladek, Steven Rostedt, Lance Yang,
linux-kernel
Cc: Feng Tang
When working on kernel stability issues, panic, task-hung and
software/hardware lockup are frequently met. And to debug them, user
may need lots of system information at that time, like task call stacks,
lock info, memory info etc.
panic case already has panic_print_sys_info() for this purpose, and has
a 'panic_print' bitmask to control what kinds of information is needed,
which is also helpful to debug other task-hung and lockup cases.
So this patchset extract the function out, and make it usable for other
cases which also need system info for debugging.
Locally these have been used in our bug chasing for stablility issues
and was helpful.
Please help to review, thanks!
- Feng
Feng Tang (3):
kernel/panic: generalize panic_print's function to show sys info
kernel/hung_task: add option to dump system info when hung task
detected
kernel/watchdog: add option to dump system info when system is locked
up
include/linux/panic.h | 12 ++++++++++++
kernel/hung_task.c | 29 +++++++++++++++-------------
kernel/panic.c | 44 +++++++++++++++++++++----------------------
kernel/watchdog.c | 11 +++++++++++
4 files changed, 61 insertions(+), 35 deletions(-)
--
2.39.5 (Apple Git-154)
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH RFC 1/3] kernel/panic: generalize panic_print's function to show sys info 2025-05-07 10:43 [PATCH RFC 0/3] generalize panic_print's dump function to be Feng Tang @ 2025-05-07 10:43 ` Feng Tang 2025-05-07 10:43 ` [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected Feng Tang ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Feng Tang @ 2025-05-07 10:43 UTC (permalink / raw) To: Andrew Morton, Petr Mladek, Steven Rostedt, Lance Yang, linux-kernel Cc: Feng Tang panic_print was introduced to help debugging kernel panic by dumping different kinds of system information like tasks' call stack, memory, ftrace buffer etc. Acutually this function could help debugging cases like task-hung, soft/hard lockup too, where user may need the snapshot of system info at that time. Extract sys_show_info() function out to be used by other kernel parts for debugging. Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com> --- include/linux/panic.h | 12 ++++++++++++ kernel/panic.c | 44 +++++++++++++++++++++---------------------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/include/linux/panic.h b/include/linux/panic.h index 2494d51707ef..a6b538936a67 100644 --- a/include/linux/panic.h +++ b/include/linux/panic.h @@ -16,6 +16,18 @@ extern void oops_enter(void); extern void oops_exit(void); extern bool oops_may_print(void); +/* Currently SYS_PRINT_ALL_PRINTK_MSG is only used for panic case */ +#define SYS_PRINT_TASK_INFO 0x00000001 +#define SYS_PRINT_MEM_INFO 0x00000002 +#define SYS_PRINT_TIMER_INFO 0x00000004 +#define SYS_PRINT_LOCK_INFO 0x00000008 +#define SYS_PRINT_FTRACE_INFO 0x00000010 +#define SYS_PRINT_ALL_PRINTK_MSG 0x00000020 +#define SYS_PRINT_ALL_CPU_BT 0x00000040 +#define SYS_PRINT_BLOCKED_TASKS 0x00000080 + +extern void sys_show_info(unsigned long info_mask); + extern bool panic_triggering_all_cpu_backtrace; extern int panic_timeout; extern unsigned long panic_print; diff --git a/kernel/panic.c b/kernel/panic.c index a3889f38153d..4fd9499f6505 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -69,14 +69,6 @@ bool panic_triggering_all_cpu_backtrace; int panic_timeout = CONFIG_PANIC_TIMEOUT; EXPORT_SYMBOL_GPL(panic_timeout); -#define PANIC_PRINT_TASK_INFO 0x00000001 -#define PANIC_PRINT_MEM_INFO 0x00000002 -#define PANIC_PRINT_TIMER_INFO 0x00000004 -#define PANIC_PRINT_LOCK_INFO 0x00000008 -#define PANIC_PRINT_FTRACE_INFO 0x00000010 -#define PANIC_PRINT_ALL_PRINTK_MSG 0x00000020 -#define PANIC_PRINT_ALL_CPU_BT 0x00000040 -#define PANIC_PRINT_BLOCKED_TASKS 0x00000080 unsigned long panic_print; ATOMIC_NOTIFIER_HEAD(panic_notifier_list); @@ -208,31 +200,39 @@ void nmi_panic(struct pt_regs *regs, const char *msg) } EXPORT_SYMBOL(nmi_panic); -static void panic_print_sys_info(bool console_flush) +void sys_show_info(unsigned long info_mask) { - if (console_flush) { - if (panic_print & PANIC_PRINT_ALL_PRINTK_MSG) - console_flush_on_panic(CONSOLE_REPLAY_ALL); - return; - } - - if (panic_print & PANIC_PRINT_TASK_INFO) + if (info_mask & SYS_PRINT_TASK_INFO) show_state(); - if (panic_print & PANIC_PRINT_MEM_INFO) + if (info_mask & SYS_PRINT_MEM_INFO) show_mem(); - if (panic_print & PANIC_PRINT_TIMER_INFO) + if (info_mask & SYS_PRINT_TIMER_INFO) sysrq_timer_list_show(); - if (panic_print & PANIC_PRINT_LOCK_INFO) + if (info_mask & SYS_PRINT_LOCK_INFO) debug_show_all_locks(); - if (panic_print & PANIC_PRINT_FTRACE_INFO) + if (info_mask & SYS_PRINT_FTRACE_INFO) ftrace_dump(DUMP_ALL); - if (panic_print & PANIC_PRINT_BLOCKED_TASKS) + if (info_mask & SYS_PRINT_BLOCKED_TASKS) show_state_filter(TASK_UNINTERRUPTIBLE); + + if (info_mask & SYS_PRINT_ALL_CPU_BT) + trigger_all_cpu_backtrace(); +} + +static void panic_print_sys_info(bool console_flush) +{ + if (console_flush) { + if (panic_print & SYS_PRINT_ALL_PRINTK_MSG) + console_flush_on_panic(CONSOLE_REPLAY_ALL); + return; + } + + sys_show_info(panic_print); } void check_panic_on_warn(const char *origin) @@ -255,7 +255,7 @@ void check_panic_on_warn(const char *origin) */ static void panic_other_cpus_shutdown(bool crash_kexec) { - if (panic_print & PANIC_PRINT_ALL_CPU_BT) { + if (panic_print & SYS_PRINT_ALL_CPU_BT) { /* Temporary allow non-panic CPUs to write their backtraces. */ panic_triggering_all_cpu_backtrace = true; trigger_all_cpu_backtrace(); -- 2.39.5 (Apple Git-154) ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected 2025-05-07 10:43 [PATCH RFC 0/3] generalize panic_print's dump function to be Feng Tang 2025-05-07 10:43 ` [PATCH RFC 1/3] kernel/panic: generalize panic_print's function to show sys info Feng Tang @ 2025-05-07 10:43 ` Feng Tang 2025-05-08 3:02 ` Lance Yang 2025-05-07 10:43 ` [PATCH RFC 3/3] kernel/watchdog: add option to dump system info when system is locked up Feng Tang 2025-05-09 2:26 ` [PATCH RFC 0/3] generalize panic_print's dump function to be Lance Yang 3 siblings, 1 reply; 9+ messages in thread From: Feng Tang @ 2025-05-07 10:43 UTC (permalink / raw) To: Andrew Morton, Petr Mladek, Steven Rostedt, Lance Yang, linux-kernel Cc: Feng Tang Kernel panic code utilizes sys_show_info() to dump needed system information to help debugging. Similarly, add this debug option for task hung case, and 'hungtask_print' is the knob to control what information should be printed out. Also clean up the code about dumping locks and triggering backtrace for all CPUs. One todo may be to merge this 'hungtask_print' with some sysctl knobs in hung_task.c. Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com> --- kernel/hung_task.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/kernel/hung_task.c b/kernel/hung_task.c index dc898ec93463..8229637be2c7 100644 --- a/kernel/hung_task.c +++ b/kernel/hung_task.c @@ -58,12 +58,20 @@ static unsigned long __read_mostly sysctl_hung_task_check_interval_secs; static int __read_mostly sysctl_hung_task_warnings = 10; static int __read_mostly did_panic; -static bool hung_task_show_lock; static bool hung_task_call_panic; -static bool hung_task_show_all_bt; static struct task_struct *watchdog_task; +/* + * A bitmask to control what kinds of system info to be printed when a + * hung task is detected, it could be task, memory, lock etc. Refer panic.h + * for details of bit definition. + */ +unsigned long hungtask_print; +core_param(hungtask_print, hungtask_print, ulong, 0644); + +static unsigned long cur_hungtask_print; + #ifdef CONFIG_SMP /* * Should we dump all CPUs backtraces in a hung task event? @@ -163,11 +171,12 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout) */ sysctl_hung_task_detect_count++; + cur_hungtask_print = hungtask_print; trace_sched_process_hang(t); if (sysctl_hung_task_panic) { console_verbose(); - hung_task_show_lock = true; + cur_hungtask_print |= SYS_PRINT_LOCK_INFO; hung_task_call_panic = true; } @@ -190,10 +199,10 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout) " disables this message.\n"); sched_show_task(t); debug_show_blocker(t); - hung_task_show_lock = true; + cur_hungtask_print |= SYS_PRINT_LOCK_INFO; if (sysctl_hung_task_all_cpu_backtrace) - hung_task_show_all_bt = true; + cur_hungtask_print |= SYS_PRINT_ALL_CPU_BT; if (!sysctl_hung_task_warnings) pr_info("Future hung task reports are suppressed, see sysctl kernel.hung_task_warnings\n"); } @@ -242,7 +251,7 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) if (test_taint(TAINT_DIE) || did_panic) return; - hung_task_show_lock = false; + cur_hungtask_print = 0; rcu_read_lock(); for_each_process_thread(g, t) { unsigned int state; @@ -266,14 +275,8 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) } unlock: rcu_read_unlock(); - if (hung_task_show_lock) - debug_show_all_locks(); - - if (hung_task_show_all_bt) { - hung_task_show_all_bt = false; - trigger_all_cpu_backtrace(); - } + sys_show_info(cur_hungtask_print); if (hung_task_call_panic) panic("hung_task: blocked tasks"); } -- 2.39.5 (Apple Git-154) ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected 2025-05-07 10:43 ` [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected Feng Tang @ 2025-05-08 3:02 ` Lance Yang 2025-05-08 5:45 ` Feng Tang 0 siblings, 1 reply; 9+ messages in thread From: Lance Yang @ 2025-05-08 3:02 UTC (permalink / raw) To: Feng Tang; +Cc: Andrew Morton, Petr Mladek, Steven Rostedt, linux-kernel Hi Feng, Thanks for the patch series! On 2025/5/7 18:43, Feng Tang wrote: > Kernel panic code utilizes sys_show_info() to dump needed system > information to help debugging. Similarly, add this debug option for > task hung case, and 'hungtask_print' is the knob to control what > information should be printed out. > > Also clean up the code about dumping locks and triggering backtrace > for all CPUs. One todo may be to merge this 'hungtask_print' with > some sysctl knobs in hung_task.c. > > Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com> > --- > kernel/hung_task.c | 29 ++++++++++++++++------------- > 1 file changed, 16 insertions(+), 13 deletions(-) > > diff --git a/kernel/hung_task.c b/kernel/hung_task.c > index dc898ec93463..8229637be2c7 100644 > --- a/kernel/hung_task.c > +++ b/kernel/hung_task.c > @@ -58,12 +58,20 @@ static unsigned long __read_mostly sysctl_hung_task_check_interval_secs; > static int __read_mostly sysctl_hung_task_warnings = 10; > > static int __read_mostly did_panic; > -static bool hung_task_show_lock; > static bool hung_task_call_panic; > -static bool hung_task_show_all_bt; > > static struct task_struct *watchdog_task; > > +/* > + * A bitmask to control what kinds of system info to be printed when a > + * hung task is detected, it could be task, memory, lock etc. Refer panic.h > + * for details of bit definition. > + */ > +unsigned long hungtask_print; > +core_param(hungtask_print, hungtask_print, ulong, 0644); how about lockup_debug_print_mask? It could be useful for debugging, but there are a few concerns: 1) SYS_PRINT_* vs. hung_task_* priority conflict - If SYS_PRINT_ALL_CPU_BT is set on the command line but hung_task_all_cpu_backtrace is disabled, which one wins? - Or should SYS_PRINT_ALL_CPU_BT force-enable hung_task_all_cpu_backtrace? 2) Duplicate prints With SYS_PRINT_BLOCKED_TASKS enabled, processes in D state will be printed twice, right? Also, we really should document how those command-line parameters work ;) Thansk, Lance > + > +static unsigned long cur_hungtask_print; > + > #ifdef CONFIG_SMP > /* > * Should we dump all CPUs backtraces in a hung task event? > @@ -163,11 +171,12 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout) > */ > sysctl_hung_task_detect_count++; > > + cur_hungtask_print = hungtask_print; > trace_sched_process_hang(t); > > if (sysctl_hung_task_panic) { > console_verbose(); > - hung_task_show_lock = true; > + cur_hungtask_print |= SYS_PRINT_LOCK_INFO; > hung_task_call_panic = true; > } > > @@ -190,10 +199,10 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout) > " disables this message.\n"); > sched_show_task(t); > debug_show_blocker(t); > - hung_task_show_lock = true; > + cur_hungtask_print |= SYS_PRINT_LOCK_INFO; > > if (sysctl_hung_task_all_cpu_backtrace) > - hung_task_show_all_bt = true; > + cur_hungtask_print |= SYS_PRINT_ALL_CPU_BT; > if (!sysctl_hung_task_warnings) > pr_info("Future hung task reports are suppressed, see sysctl kernel.hung_task_warnings\n"); > } > @@ -242,7 +251,7 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) > if (test_taint(TAINT_DIE) || did_panic) > return; > > - hung_task_show_lock = false; > + cur_hungtask_print = 0; > rcu_read_lock(); > for_each_process_thread(g, t) { > unsigned int state; > @@ -266,14 +275,8 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) > } > unlock: > rcu_read_unlock(); > - if (hung_task_show_lock) > - debug_show_all_locks(); > - > - if (hung_task_show_all_bt) { > - hung_task_show_all_bt = false; > - trigger_all_cpu_backtrace(); > - } > > + sys_show_info(cur_hungtask_print); > if (hung_task_call_panic) > panic("hung_task: blocked tasks"); > } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected 2025-05-08 3:02 ` Lance Yang @ 2025-05-08 5:45 ` Feng Tang 2025-05-09 4:44 ` Lance Yang 0 siblings, 1 reply; 9+ messages in thread From: Feng Tang @ 2025-05-08 5:45 UTC (permalink / raw) To: Lance Yang; +Cc: Andrew Morton, Petr Mladek, Steven Rostedt, linux-kernel Hi Lance, Many thanks for the review! On Thu, May 08, 2025 at 11:02:22AM +0800, Lance Yang wrote: > Hi Feng, > > Thanks for the patch series! > > On 2025/5/7 18:43, Feng Tang wrote: > > Kernel panic code utilizes sys_show_info() to dump needed system > > information to help debugging. Similarly, add this debug option for > > task hung case, and 'hungtask_print' is the knob to control what > > information should be printed out. > > > > Also clean up the code about dumping locks and triggering backtrace > > for all CPUs. One todo may be to merge this 'hungtask_print' with > > some sysctl knobs in hung_task.c. > > > > Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com> > > --- > > kernel/hung_task.c | 29 ++++++++++++++++------------- > > 1 file changed, 16 insertions(+), 13 deletions(-) > > > > diff --git a/kernel/hung_task.c b/kernel/hung_task.c > > index dc898ec93463..8229637be2c7 100644 > > --- a/kernel/hung_task.c > > +++ b/kernel/hung_task.c > > @@ -58,12 +58,20 @@ static unsigned long __read_mostly sysctl_hung_task_check_interval_secs; > > static int __read_mostly sysctl_hung_task_warnings = 10; > > static int __read_mostly did_panic; > > -static bool hung_task_show_lock; > > static bool hung_task_call_panic; > > -static bool hung_task_show_all_bt; > > static struct task_struct *watchdog_task; > > +/* > > + * A bitmask to control what kinds of system info to be printed when a > > + * hung task is detected, it could be task, memory, lock etc. Refer panic.h > > + * for details of bit definition. > > + */ > > +unsigned long hungtask_print; > > +core_param(hungtask_print, hungtask_print, ulong, 0644); > > how about lockup_debug_print_mask? The 3/3 patch has a 'lockup_print' as it is for soft/hard lockup :). The name follows the existing 'panic_print', and indeed it's actually a bitmask, how about 'hung_print_mask'? > > It could be useful for debugging, but there are a few concerns: > > 1) SYS_PRINT_* vs. hung_task_* priority conflict > - If SYS_PRINT_ALL_CPU_BT is set on the command line but > hung_task_all_cpu_backtrace is disabled, which one wins? > - Or should SYS_PRINT_ALL_CPU_BT force-enable hung_task_all_cpu_backtrace? With this patch, the 'hungtask_print' and hung_task_all_cpu_backtrace will be ORed, so yes, if user sets SYS_PRINT_ALL_CPU_BT explicitly, the all-cpu-backtrace will be printed. While the default value for hungtask_print is 0, and no system info will be dumped by default. Long term wise, I'm not sure if sysctl_hung_task_all_cpu_backtracemay could be removed as its function can be covered by this print_mask knob. > 2) Duplicate prints > With SYS_PRINT_BLOCKED_TASKS enabled, processes in D state will be printed > twice, right? Good point. As sys_show_info() is a general API helper, the user may chose not to set SYS_PRINT_BLOCKED_TASKS when debugging task hung. In one recent bug we debugged with this patch, when the first "task hung" was shown, there were already dozens of tasks were in D state, which just hadn't hit the 120 seconds limit yet, and dumping them all helped in that case. > Also, we really should document how those command-line parameters work ;) Exactly! It currently just said 'refer panic.h' in code comment, maybe I should copy those definitions here as comments. How do you think? Thanks, Feng > Thansk, > Lance > > > + > > +static unsigned long cur_hungtask_print; > > + > > #ifdef CONFIG_SMP > > /* > > * Should we dump all CPUs backtraces in a hung task event? > > @@ -163,11 +171,12 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout) > > */ > > sysctl_hung_task_detect_count++; > > + cur_hungtask_print = hungtask_print; > > trace_sched_process_hang(t); > > if (sysctl_hung_task_panic) { > > console_verbose(); > > - hung_task_show_lock = true; > > + cur_hungtask_print |= SYS_PRINT_LOCK_INFO; > > hung_task_call_panic = true; > > } > > @@ -190,10 +199,10 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout) > > " disables this message.\n"); > > sched_show_task(t); > > debug_show_blocker(t); > > - hung_task_show_lock = true; > > + cur_hungtask_print |= SYS_PRINT_LOCK_INFO; > > if (sysctl_hung_task_all_cpu_backtrace) > > - hung_task_show_all_bt = true; > > + cur_hungtask_print |= SYS_PRINT_ALL_CPU_BT; > > if (!sysctl_hung_task_warnings) > > pr_info("Future hung task reports are suppressed, see sysctl kernel.hung_task_warnings\n"); > > } > > @@ -242,7 +251,7 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) > > if (test_taint(TAINT_DIE) || did_panic) > > return; > > - hung_task_show_lock = false; > > + cur_hungtask_print = 0; > > rcu_read_lock(); > > for_each_process_thread(g, t) { > > unsigned int state; > > @@ -266,14 +275,8 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) > > } > > unlock: > > rcu_read_unlock(); > > - if (hung_task_show_lock) > > - debug_show_all_locks(); > > - > > - if (hung_task_show_all_bt) { > > - hung_task_show_all_bt = false; > > - trigger_all_cpu_backtrace(); > > - } > > + sys_show_info(cur_hungtask_print); > > if (hung_task_call_panic) > > panic("hung_task: blocked tasks"); > > } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected 2025-05-08 5:45 ` Feng Tang @ 2025-05-09 4:44 ` Lance Yang 2025-05-09 6:35 ` Feng Tang 0 siblings, 1 reply; 9+ messages in thread From: Lance Yang @ 2025-05-09 4:44 UTC (permalink / raw) To: Feng Tang Cc: Andrew Morton, Petr Mladek, Steven Rostedt, linux-kernel, llong, mhiramat, amaindex On 2025/5/8 13:45, Feng Tang wrote: > Hi Lance, > > Many thanks for the review! > > On Thu, May 08, 2025 at 11:02:22AM +0800, Lance Yang wrote: >> Hi Feng, >> >> Thanks for the patch series! >> >> On 2025/5/7 18:43, Feng Tang wrote: >>> Kernel panic code utilizes sys_show_info() to dump needed system >>> information to help debugging. Similarly, add this debug option for >>> task hung case, and 'hungtask_print' is the knob to control what >>> information should be printed out. >>> >>> Also clean up the code about dumping locks and triggering backtrace >>> for all CPUs. One todo may be to merge this 'hungtask_print' with >>> some sysctl knobs in hung_task.c. >>> >>> Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com> >>> --- >>> kernel/hung_task.c | 29 ++++++++++++++++------------- >>> 1 file changed, 16 insertions(+), 13 deletions(-) >>> >>> diff --git a/kernel/hung_task.c b/kernel/hung_task.c >>> index dc898ec93463..8229637be2c7 100644 >>> --- a/kernel/hung_task.c >>> +++ b/kernel/hung_task.c >>> @@ -58,12 +58,20 @@ static unsigned long __read_mostly sysctl_hung_task_check_interval_secs; >>> static int __read_mostly sysctl_hung_task_warnings = 10; >>> static int __read_mostly did_panic; >>> -static bool hung_task_show_lock; >>> static bool hung_task_call_panic; >>> -static bool hung_task_show_all_bt; >>> static struct task_struct *watchdog_task; >>> +/* >>> + * A bitmask to control what kinds of system info to be printed when a >>> + * hung task is detected, it could be task, memory, lock etc. Refer panic.h >>> + * for details of bit definition. >>> + */ >>> +unsigned long hungtask_print; >>> +core_param(hungtask_print, hungtask_print, ulong, 0644); >> >> how about lockup_debug_print_mask? Oops, typo: hungtask_* (not lockup_*) > > The 3/3 patch has a 'lockup_print' as it is for soft/hard lockup :). > The name follows the existing 'panic_print', and indeed it's actually > a bitmask, how about 'hung_print_mask'? Yep, we should be following ’panic_print‘ pattern like 'hungtask_print', but I‘d rather go with 'hungtask_print_mask' ;) > >> >> It could be useful for debugging, but there are a few concerns: >> >> 1) SYS_PRINT_* vs. hung_task_* priority conflict >> - If SYS_PRINT_ALL_CPU_BT is set on the command line but >> hung_task_all_cpu_backtrace is disabled, which one wins? >> - Or should SYS_PRINT_ALL_CPU_BT force-enable hung_task_all_cpu_backtrace? > > With this patch, the 'hungtask_print' and hung_task_all_cpu_backtrace > will be ORed, so yes, if user sets SYS_PRINT_ALL_CPU_BT explicitly, the > all-cpu-backtrace will be printed. > > While the default value for hungtask_print is 0, and no system info will > be dumped by default. > > Long term wise, I'm not sure if sysctl_hung_task_all_cpu_backtracemay > could be removed as its function can be covered by this print_mask knob. Afraid we cannot remove that knob — it would break user-space. Note that hungtask_print_mask should act as an extension (to provide more details when investigating hangs), and it must still follow hung-task detector's rules, IIUC. Hmm... SYS_PRINT_ALL_CPU_BT is a bit tricky here. Maybe we can directly enable hung_task_all_cpu_backtrace when SYS_PRINT_ALL_CPU_BT is set in hungtask_print_mask, while still allowing manual disabling to dump backtraces for all CPUs via hung_task_all_cpu_backtrace. This way, we keep its original semantics ;) > >> 2) Duplicate prints >> With SYS_PRINT_BLOCKED_TASKS enabled, processes in D state will be printed >> twice, right? > > Good point. As sys_show_info() is a general API helper, the user may chose > not to set SYS_PRINT_BLOCKED_TASKS when debugging task hung. > > In one recent bug we debugged with this patch, when the first "task hung" was > shown, there were already dozens of tasks were in D state, which just hadn't > hit the 120 seconds limit yet, and dumping them all helped in that case. Makes sense to me. Right, SYS_PRINT_BLOCKED_TASKS doesn’t duplicate prints, and catches all D-state tasks - even the ones not yet timed out. > >> Also, we really should document how those command-line parameters work ;) > > Exactly! It currently just said 'refer panic.h' in code comment, maybe I > should copy those definitions here as comments. How do you think? Yes. Both comments and kernel-doc are needed ;) Thanks, Lance > > Thanks, > Feng > >> Thansk, >> Lance >> >>> + >>> +static unsigned long cur_hungtask_print; >>> + >>> #ifdef CONFIG_SMP >>> /* >>> * Should we dump all CPUs backtraces in a hung task event? >>> @@ -163,11 +171,12 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout) >>> */ >>> sysctl_hung_task_detect_count++; >>> + cur_hungtask_print = hungtask_print; >>> trace_sched_process_hang(t); >>> if (sysctl_hung_task_panic) { >>> console_verbose(); >>> - hung_task_show_lock = true; >>> + cur_hungtask_print |= SYS_PRINT_LOCK_INFO; >>> hung_task_call_panic = true; >>> } >>> @@ -190,10 +199,10 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout) >>> " disables this message.\n"); >>> sched_show_task(t); >>> debug_show_blocker(t); >>> - hung_task_show_lock = true; >>> + cur_hungtask_print |= SYS_PRINT_LOCK_INFO; >>> if (sysctl_hung_task_all_cpu_backtrace) >>> - hung_task_show_all_bt = true; >>> + cur_hungtask_print |= SYS_PRINT_ALL_CPU_BT; >>> if (!sysctl_hung_task_warnings) >>> pr_info("Future hung task reports are suppressed, see sysctl kernel.hung_task_warnings\n"); >>> } >>> @@ -242,7 +251,7 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) >>> if (test_taint(TAINT_DIE) || did_panic) >>> return; >>> - hung_task_show_lock = false; >>> + cur_hungtask_print = 0; >>> rcu_read_lock(); >>> for_each_process_thread(g, t) { >>> unsigned int state; >>> @@ -266,14 +275,8 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) >>> } >>> unlock: >>> rcu_read_unlock(); >>> - if (hung_task_show_lock) >>> - debug_show_all_locks(); >>> - >>> - if (hung_task_show_all_bt) { >>> - hung_task_show_all_bt = false; >>> - trigger_all_cpu_backtrace(); >>> - } >>> + sys_show_info(cur_hungtask_print); >>> if (hung_task_call_panic) >>> panic("hung_task: blocked tasks"); >>> } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected 2025-05-09 4:44 ` Lance Yang @ 2025-05-09 6:35 ` Feng Tang 0 siblings, 0 replies; 9+ messages in thread From: Feng Tang @ 2025-05-09 6:35 UTC (permalink / raw) To: Lance Yang Cc: Andrew Morton, Petr Mladek, Steven Rostedt, linux-kernel, llong, mhiramat, amaindex On Fri, May 09, 2025 at 12:44:14PM +0800, Lance Yang wrote: > > > On 2025/5/8 13:45, Feng Tang wrote: > > Hi Lance, > > > > Many thanks for the review! > > > > On Thu, May 08, 2025 at 11:02:22AM +0800, Lance Yang wrote: > > > Hi Feng, > > > > > > Thanks for the patch series! > > > > > > On 2025/5/7 18:43, Feng Tang wrote: > > > > Kernel panic code utilizes sys_show_info() to dump needed system > > > > information to help debugging. Similarly, add this debug option for > > > > task hung case, and 'hungtask_print' is the knob to control what > > > > information should be printed out. > > > > > > > > Also clean up the code about dumping locks and triggering backtrace > > > > for all CPUs. One todo may be to merge this 'hungtask_print' with > > > > some sysctl knobs in hung_task.c. > > > > > > > > Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com> > > > > --- > > > > kernel/hung_task.c | 29 ++++++++++++++++------------- > > > > 1 file changed, 16 insertions(+), 13 deletions(-) > > > > > > > > diff --git a/kernel/hung_task.c b/kernel/hung_task.c > > > > index dc898ec93463..8229637be2c7 100644 > > > > --- a/kernel/hung_task.c > > > > +++ b/kernel/hung_task.c > > > > @@ -58,12 +58,20 @@ static unsigned long __read_mostly sysctl_hung_task_check_interval_secs; > > > > static int __read_mostly sysctl_hung_task_warnings = 10; > > > > static int __read_mostly did_panic; > > > > -static bool hung_task_show_lock; > > > > static bool hung_task_call_panic; > > > > -static bool hung_task_show_all_bt; > > > > static struct task_struct *watchdog_task; > > > > +/* > > > > + * A bitmask to control what kinds of system info to be printed when a > > > > + * hung task is detected, it could be task, memory, lock etc. Refer panic.h > > > > + * for details of bit definition. > > > > + */ > > > > +unsigned long hungtask_print; > > > > +core_param(hungtask_print, hungtask_print, ulong, 0644); > > > > > > how about lockup_debug_print_mask? > > Oops, typo: hungtask_* (not lockup_*) > > > > > The 3/3 patch has a 'lockup_print' as it is for soft/hard lockup :). > > The name follows the existing 'panic_print', and indeed it's actually > > a bitmask, how about 'hung_print_mask'? > > Yep, we should be following ’panic_print‘ pattern like 'hungtask_print', > but I‘d rather go with 'hungtask_print_mask' ;) OK, will change to it. > > > > > > > > > It could be useful for debugging, but there are a few concerns: > > > > > > 1) SYS_PRINT_* vs. hung_task_* priority conflict > > > - If SYS_PRINT_ALL_CPU_BT is set on the command line but > > > hung_task_all_cpu_backtrace is disabled, which one wins? > > > - Or should SYS_PRINT_ALL_CPU_BT force-enable hung_task_all_cpu_backtrace? > > > > With this patch, the 'hungtask_print' and hung_task_all_cpu_backtrace > > will be ORed, so yes, if user sets SYS_PRINT_ALL_CPU_BT explicitly, the > > all-cpu-backtrace will be printed. > > > > While the default value for hungtask_print is 0, and no system info will > > be dumped by default. > > > > Long term wise, I'm not sure if sysctl_hung_task_all_cpu_backtracemay > > could be removed as its function can be covered by this print_mask knob. > > Afraid we cannot remove that knob — it would break user-space. Note that > hungtask_print_mask should act as an extension (to provide more details > when investigating hangs), and it must still follow hung-task detector's > rules, IIUC. Right. > Hmm... SYS_PRINT_ALL_CPU_BT is a bit tricky here. Maybe we can directly > enable hung_task_all_cpu_backtrace when SYS_PRINT_ALL_CPU_BT is set in > hungtask_print_mask, while still allowing manual disabling to dump > backtraces for all CPUs via hung_task_all_cpu_backtrace. This way, we > keep its original semantics ;) Sounds goot to me, thanks for the suggestion! > > > > > 2) Duplicate prints > > > With SYS_PRINT_BLOCKED_TASKS enabled, processes in D state will be printed > > > twice, right? > > > > Good point. As sys_show_info() is a general API helper, the user may chose > > not to set SYS_PRINT_BLOCKED_TASKS when debugging task hung. > > > > In one recent bug we debugged with this patch, when the first "task hung" was > > shown, there were already dozens of tasks were in D state, which just hadn't > > hit the 120 seconds limit yet, and dumping them all helped in that case. > > Makes sense to me. Right, SYS_PRINT_BLOCKED_TASKS doesn’t duplicate prints, > and > catches all D-state tasks - even the ones not yet timed out. > > > > > > Also, we really should document how those command-line parameters work ;) > > > > Exactly! It currently just said 'refer panic.h' in code comment, maybe I > > should copy those definitions here as comments. How do you think? > > Yes. Both comments and kernel-doc are needed ;) Sure. Seems kernel-parameters.txt is the good place for core parameters. Thanks, Feng > > Thanks, > Lance > > > > > Thanks, > > Feng > > > > > Thansk, > > > Lance > > > > > > > + > > > > +static unsigned long cur_hungtask_print; > > > > + > > > > #ifdef CONFIG_SMP > > > > /* > > > > * Should we dump all CPUs backtraces in a hung task event? > > > > @@ -163,11 +171,12 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout) > > > > */ > > > > sysctl_hung_task_detect_count++; > > > > + cur_hungtask_print = hungtask_print; > > > > trace_sched_process_hang(t); > > > > if (sysctl_hung_task_panic) { > > > > console_verbose(); > > > > - hung_task_show_lock = true; > > > > + cur_hungtask_print |= SYS_PRINT_LOCK_INFO; > > > > hung_task_call_panic = true; > > > > } > > > > @@ -190,10 +199,10 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout) > > > > " disables this message.\n"); > > > > sched_show_task(t); > > > > debug_show_blocker(t); > > > > - hung_task_show_lock = true; > > > > + cur_hungtask_print |= SYS_PRINT_LOCK_INFO; > > > > if (sysctl_hung_task_all_cpu_backtrace) > > > > - hung_task_show_all_bt = true; > > > > + cur_hungtask_print |= SYS_PRINT_ALL_CPU_BT; > > > > if (!sysctl_hung_task_warnings) > > > > pr_info("Future hung task reports are suppressed, see sysctl kernel.hung_task_warnings\n"); > > > > } > > > > @@ -242,7 +251,7 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) > > > > if (test_taint(TAINT_DIE) || did_panic) > > > > return; > > > > - hung_task_show_lock = false; > > > > + cur_hungtask_print = 0; > > > > rcu_read_lock(); > > > > for_each_process_thread(g, t) { > > > > unsigned int state; > > > > @@ -266,14 +275,8 @@ static void check_hung_uninterruptible_tasks(unsigned long timeout) > > > > } > > > > unlock: > > > > rcu_read_unlock(); > > > > - if (hung_task_show_lock) > > > > - debug_show_all_locks(); > > > > - > > > > - if (hung_task_show_all_bt) { > > > > - hung_task_show_all_bt = false; > > > > - trigger_all_cpu_backtrace(); > > > > - } > > > > + sys_show_info(cur_hungtask_print); > > > > if (hung_task_call_panic) > > > > panic("hung_task: blocked tasks"); > > > > } ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH RFC 3/3] kernel/watchdog: add option to dump system info when system is locked up 2025-05-07 10:43 [PATCH RFC 0/3] generalize panic_print's dump function to be Feng Tang 2025-05-07 10:43 ` [PATCH RFC 1/3] kernel/panic: generalize panic_print's function to show sys info Feng Tang 2025-05-07 10:43 ` [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected Feng Tang @ 2025-05-07 10:43 ` Feng Tang 2025-05-09 2:26 ` [PATCH RFC 0/3] generalize panic_print's dump function to be Lance Yang 3 siblings, 0 replies; 9+ messages in thread From: Feng Tang @ 2025-05-07 10:43 UTC (permalink / raw) To: Andrew Morton, Petr Mladek, Steven Rostedt, Lance Yang, linux-kernel Cc: Feng Tang Kernel panic code utilizes sys_show_info() to dump needed system information to help debugging. Similarly, add this debug option for software/hardware lockup cases, and 'lockup_print' is the knob to control what information should be printed out. Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com> --- kernel/watchdog.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 9fa2af9dbf2c..60afcb0247ab 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -52,6 +52,14 @@ static int __read_mostly watchdog_hardlockup_available; struct cpumask watchdog_cpumask __read_mostly; unsigned long *watchdog_cpumask_bits = cpumask_bits(&watchdog_cpumask); +/* + * A bitmask to control what kinds of system info to be printed when a + * software/hardware lockup is detected, it could be task, memory, lock etc. + * Refer panic.h for details of bit definition. + */ +unsigned long lockup_print; +core_param(lockup_print, lockup_print, ulong, 0644); + #ifdef CONFIG_HARDLOCKUP_DETECTOR # ifdef CONFIG_SMP @@ -212,6 +220,7 @@ void watchdog_hardlockup_check(unsigned int cpu, struct pt_regs *regs) clear_bit_unlock(0, &hard_lockup_nmi_warn); } + sys_show_info(lockup_print); if (hardlockup_panic) nmi_panic(regs, "Hard LOCKUP"); @@ -774,6 +783,8 @@ static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer) } add_taint(TAINT_SOFTLOCKUP, LOCKDEP_STILL_OK); + + sys_show_info(lockup_print); if (softlockup_panic) panic("softlockup: hung tasks"); } -- 2.39.5 (Apple Git-154) ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH RFC 0/3] generalize panic_print's dump function to be 2025-05-07 10:43 [PATCH RFC 0/3] generalize panic_print's dump function to be Feng Tang ` (2 preceding siblings ...) 2025-05-07 10:43 ` [PATCH RFC 3/3] kernel/watchdog: add option to dump system info when system is locked up Feng Tang @ 2025-05-09 2:26 ` Lance Yang 3 siblings, 0 replies; 9+ messages in thread From: Lance Yang @ 2025-05-09 2:26 UTC (permalink / raw) To: mhiramat, llong Cc: Feng Tang, Petr Mladek, Steven Rostedt, linux-kernel, Andrew Morton Cc Masami and Waiman On 2025/5/7 18:43, Feng Tang wrote: > When working on kernel stability issues, panic, task-hung and > software/hardware lockup are frequently met. And to debug them, user > may need lots of system information at that time, like task call stacks, > lock info, memory info etc. > > panic case already has panic_print_sys_info() for this purpose, and has > a 'panic_print' bitmask to control what kinds of information is needed, > which is also helpful to debug other task-hung and lockup cases. > > So this patchset extract the function out, and make it usable for other > cases which also need system info for debugging. > > Locally these have been used in our bug chasing for stablility issues > and was helpful. > > Please help to review, thanks! > > - Feng > > Feng Tang (3): > kernel/panic: generalize panic_print's function to show sys info > kernel/hung_task: add option to dump system info when hung task > detected > kernel/watchdog: add option to dump system info when system is locked > up > > include/linux/panic.h | 12 ++++++++++++ > kernel/hung_task.c | 29 +++++++++++++++------------- > kernel/panic.c | 44 +++++++++++++++++++++---------------------- > kernel/watchdog.c | 11 +++++++++++ > 4 files changed, 61 insertions(+), 35 deletions(-) > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-05-09 6:35 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-05-07 10:43 [PATCH RFC 0/3] generalize panic_print's dump function to be Feng Tang 2025-05-07 10:43 ` [PATCH RFC 1/3] kernel/panic: generalize panic_print's function to show sys info Feng Tang 2025-05-07 10:43 ` [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected Feng Tang 2025-05-08 3:02 ` Lance Yang 2025-05-08 5:45 ` Feng Tang 2025-05-09 4:44 ` Lance Yang 2025-05-09 6:35 ` Feng Tang 2025-05-07 10:43 ` [PATCH RFC 3/3] kernel/watchdog: add option to dump system info when system is locked up Feng Tang 2025-05-09 2:26 ` [PATCH RFC 0/3] generalize panic_print's dump function to be Lance Yang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.