[PATCH RFC 0/3] generalize panic_print's dump function to be

[PATCH RFC 0/3] generalize panic_print's dump function to be

[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)

[PATCH RFC 1/3] kernel/panic: generalize panic_print's function to show sys info

[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)

[PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected

[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)

[PATCH RFC 3/3] kernel/watchdog: add option to dump system info when system is locked up

[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)

Re: [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected

[Lance Yang]

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");
>   }

Re: [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected

[Feng Tang]

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");
> >   }

Re: [PATCH RFC 0/3] generalize panic_print's dump function to be

[Lance Yang]

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(-)
> 

Re: [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected

[Lance Yang]

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");
>>>    }

Re: [PATCH RFC 2/3] kernel/hung_task: add option to dump system info when hung task detected

[Feng Tang]

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");
> > > >    }