[PATCH V2 2/5] panic: generalize panic_print's function to show sys info

Feng Tang posted 5 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH V2 2/5] panic: generalize panic_print's function to show sys info
Posted by Feng Tang 3 months, 3 weeks ago
'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 also help debugging
cases like task-hung, soft/hard lockup, and other cases , where user
may need the snapshot of system info at that time.

Extract sys_show_info() function out of panic code to be used by other
kernel parts for debugging.

Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
---
 include/linux/kernel.h   |  1 +
 include/linux/sys_info.h | 20 ++++++++++++++++++++
 kernel/panic.c           | 35 +++--------------------------------
 lib/Makefile             |  2 +-
 lib/sys_info.c           | 30 ++++++++++++++++++++++++++++++
 5 files changed, 55 insertions(+), 33 deletions(-)
 create mode 100644 include/linux/sys_info.h
 create mode 100644 lib/sys_info.c

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 1cce1f6410a9..4788c0e4a8bd 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -27,6 +27,7 @@
 #include <linux/math.h>
 #include <linux/minmax.h>
 #include <linux/typecheck.h>
+#include <linux/sys_info.h>
 #include <linux/panic.h>
 #include <linux/printk.h>
 #include <linux/build_bug.h>
diff --git a/include/linux/sys_info.h b/include/linux/sys_info.h
new file mode 100644
index 000000000000..79bf4a942e5f
--- /dev/null
+++ b/include/linux/sys_info.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_SYS_INFO_H
+#define _LINUX_SYS_INFO_H
+
+/*
+ * SYS_SHOW_ALL_PRINTK_MSG is for panic case only, as it needs special
+ * handling which only fits panic case.
+ */
+#define SYS_SHOW_TASK_INFO		0x00000001
+#define SYS_SHOW_MEM_INFO		0x00000002
+#define SYS_SHOW_TIMER_INFO		0x00000004
+#define SYS_SHOW_LOCK_INFO		0x00000008
+#define SYS_SHOW_FTRACE_INFO		0x00000010
+#define SYS_SHOW_ALL_PRINTK_MSG		0x00000020
+#define SYS_SHOW_ALL_CPU_BT		0x00000040
+#define SYS_SHOW_BLOCKED_TASKS		0x00000080
+
+extern void sys_show_info(unsigned long info_mask);
+
+#endif	/* _LINUX_SYS_INFO_H */
diff --git a/kernel/panic.c b/kernel/panic.c
index af0d5206a624..35c98aefa39f 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);
@@ -240,31 +232,10 @@ EXPORT_SYMBOL(nmi_panic);
 
 static void panic_console_replay(void)
 {
-	if (panic_print & PANIC_PRINT_ALL_PRINTK_MSG)
+	if (panic_print & SYS_SHOW_ALL_PRINTK_MSG)
 		console_flush_on_panic(CONSOLE_REPLAY_ALL);
 }
 
-static void panic_print_sys_info(void)
-{
-	if (panic_print & PANIC_PRINT_TASK_INFO)
-		show_state();
-
-	if (panic_print & PANIC_PRINT_MEM_INFO)
-		show_mem();
-
-	if (panic_print & PANIC_PRINT_TIMER_INFO)
-		sysrq_timer_list_show();
-
-	if (panic_print & PANIC_PRINT_LOCK_INFO)
-		debug_show_all_locks();
-
-	if (panic_print & PANIC_PRINT_FTRACE_INFO)
-		ftrace_dump(DUMP_ALL);
-
-	if (panic_print & PANIC_PRINT_BLOCKED_TASKS)
-		show_state_filter(TASK_UNINTERRUPTIBLE);
-}
-
 void check_panic_on_warn(const char *origin)
 {
 	unsigned int limit;
@@ -285,7 +256,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_SHOW_ALL_CPU_BT) {
 		/* Temporary allow non-panic CPUs to write their backtraces. */
 		panic_triggering_all_cpu_backtrace = true;
 		trigger_all_cpu_backtrace();
@@ -410,7 +381,7 @@ void panic(const char *fmt, ...)
 	 */
 	atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
 
-	panic_print_sys_info();
+	sys_show_info(panic_print);
 
 	kmsg_dump_desc(KMSG_DUMP_PANIC, buf);
 
diff --git a/lib/Makefile b/lib/Makefile
index c38582f187dd..88d6228089a8 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -40,7 +40,7 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
 	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
 	 earlycpio.o seq_buf.o siphash.o dec_and_lock.o \
 	 nmi_backtrace.o win_minmax.o memcat_p.o \
-	 buildid.o objpool.o iomem_copy.o
+	 buildid.o objpool.o iomem_copy.o sys_info.o
 
 lib-$(CONFIG_UNION_FIND) += union_find.o
 lib-$(CONFIG_PRINTK) += dump_stack.o
diff --git a/lib/sys_info.c b/lib/sys_info.c
new file mode 100644
index 000000000000..90a79b5164c9
--- /dev/null
+++ b/lib/sys_info.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/sched/debug.h>
+#include <linux/kernel.h>
+#include <linux/ftrace.h>
+#include <linux/console.h>
+#include <linux/nmi.h>
+
+void sys_show_info(unsigned long info_flag)
+{
+	if (info_flag & SYS_SHOW_TASK_INFO)
+		show_state();
+
+	if (info_flag & SYS_SHOW_MEM_INFO)
+		show_mem();
+
+	if (info_flag & SYS_SHOW_TIMER_INFO)
+		sysrq_timer_list_show();
+
+	if (info_flag & SYS_SHOW_LOCK_INFO)
+		debug_show_all_locks();
+
+	if (info_flag & SYS_SHOW_FTRACE_INFO)
+		ftrace_dump(DUMP_ALL);
+
+	if (info_flag & SYS_SHOW_ALL_CPU_BT)
+		trigger_all_cpu_backtrace();
+
+	if (info_flag & SYS_SHOW_BLOCKED_TASKS)
+		show_state_filter(TASK_UNINTERRUPTIBLE);
+}
-- 
2.39.5 (Apple Git-154)
Re: [PATCH V2 2/5] panic: generalize panic_print's function to show sys info
Posted by Petr Mladek 3 months, 3 weeks ago
On Mon 2025-06-16 09:08:37, Feng Tang wrote:
> '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 also help debugging
> cases like task-hung, soft/hard lockup, and other cases , where user
> may need the snapshot of system info at that time.
> 
> Extract sys_show_info() function out of panic code to be used by other
> kernel parts for debugging.
> 
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -27,6 +27,7 @@
>  #include <linux/math.h>
>  #include <linux/minmax.h>
>  #include <linux/typecheck.h>
> +#include <linux/sys_info.h>

There will be only few users of this API. There is no need to
include it via this generic header which is included almost
everywhere.

Some people are working hard on getting rid of this header file,
see the comment at the beginning:

 * This header has combined a lot of unrelated to each other stuff.
 * The process of splitting its content is in progress while keeping
 * backward compatibility. That's why it's highly recommended NOT to
 * include this header inside another header file, especially under
 * generic or architectural include/ directory.

Instead, please include the new linux/sys_info.h in panic.c directly.

>  #include <linux/panic.h>
>  #include <linux/printk.h>
>  #include <linux/build_bug.h>

> --- /dev/null
> +++ b/include/linux/sys_info.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_SYS_INFO_H
> +#define _LINUX_SYS_INFO_H
> +
> +/*
> + * SYS_SHOW_ALL_PRINTK_MSG is for panic case only, as it needs special
> + * handling which only fits panic case.

This flags is really special. I would even rename it to match
the function where it is used:

#define PANIC_CONSOLE_REPLAY		0x00000020

And it would be better to do the rename (ALL_PRINTK_MSG ->
CONSOLE_REPLAY) already in the 1st patch where panic_console_replay()
was introduced.

Also it would make sense to update the documentation (in 1st patch),
something like:

--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4533,7 +4533,7 @@
 			bit 2: print timer info
 			bit 3: print locks info if CONFIG_LOCKDEP is on
 			bit 4: print ftrace buffer
-			bit 5: print all printk messages in buffer
+			bit 5: replay all messages on consoles at the end of panic
 			bit 6: print all CPUs backtrace (if available in the arch)
 			bit 7: print only tasks in uninterruptible (blocked) state
 			*Be aware* that this option may print a _lot_ of lines,

> + */
> +#define SYS_SHOW_TASK_INFO		0x00000001
> +#define SYS_SHOW_MEM_INFO		0x00000002
> +#define SYS_SHOW_TIMER_INFO		0x00000004
> +#define SYS_SHOW_LOCK_INFO		0x00000008
> +#define SYS_SHOW_FTRACE_INFO		0x00000010
> +#define SYS_SHOW_ALL_PRINTK_MSG		0x00000020
> +#define SYS_SHOW_ALL_CPU_BT		0x00000040
> +#define SYS_SHOW_BLOCKED_TASKS		0x00000080
> +
> +extern void sys_show_info(unsigned long info_mask);

Please, do not use "extern" in new header files. This is from
Documentation/process/coding-style.rst:

    Do not use the ``extern`` keyword with function declarations as this makes
    lines longer and isn't strictly necessary.

Also the header file is named "sys_info" but the API is "sys_show_*info".
It would be more user friendly to consistently use the same prefix
for the entire API, for example:

#define SYS_INFO_TASK		0x00000001
#define SYS_INFO_MEM		0x00000002
#define SYS_INFO_TIMER		0x00000004

void sys_info(unsigned long si_mask);

I am sorry that I did not tell your this in the RFC.
I focused on the bigger picture at that time.

> +#endif	/* _LINUX_SYS_INFO_H */

Best Regards,
Petr
Re: [PATCH V2 2/5] panic: generalize panic_print's function to show sys info
Posted by Feng Tang 3 months, 2 weeks ago
On Fri, Jun 20, 2025 at 05:21:08PM +0200, Petr Mladek wrote:
> On Mon 2025-06-16 09:08:37, Feng Tang wrote:
> > '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 also help debugging
> > cases like task-hung, soft/hard lockup, and other cases , where user
> > may need the snapshot of system info at that time.
> > 
> > Extract sys_show_info() function out of panic code to be used by other
> > kernel parts for debugging.
> > 
> > --- a/include/linux/kernel.h
> > +++ b/include/linux/kernel.h
> > @@ -27,6 +27,7 @@
> >  #include <linux/math.h>
> >  #include <linux/minmax.h>
> >  #include <linux/typecheck.h>
> > +#include <linux/sys_info.h>
> 
> There will be only few users of this API. There is no need to
> include it via this generic header which is included almost
> everywhere.
> 
> Some people are working hard on getting rid of this header file,
> see the comment at the beginning:
> 
>  * This header has combined a lot of unrelated to each other stuff.
>  * The process of splitting its content is in progress while keeping
>  * backward compatibility. That's why it's highly recommended NOT to
>  * include this header inside another header file, especially under
>  * generic or architectural include/ directory.
> 
> Instead, please include the new linux/sys_info.h in panic.c directly.

Makes sense! WIll change.

> >  #include <linux/panic.h>
> >  #include <linux/printk.h>
> >  #include <linux/build_bug.h>
> 
> > --- /dev/null
> > +++ b/include/linux/sys_info.h
> > @@ -0,0 +1,20 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _LINUX_SYS_INFO_H
> > +#define _LINUX_SYS_INFO_H
> > +
> > +/*
> > + * SYS_SHOW_ALL_PRINTK_MSG is for panic case only, as it needs special
> > + * handling which only fits panic case.
> 
> This flags is really special. I would even rename it to match
> the function where it is used:
> 
> #define PANIC_CONSOLE_REPLAY		0x00000020
> 
> And it would be better to do the rename (ALL_PRINTK_MSG ->
> CONSOLE_REPLAY) already in the 1st patch where panic_console_replay()
> was introduced.
 
OK.

> Also it would make sense to update the documentation (in 1st patch),
> something like:
> 
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4533,7 +4533,7 @@
>  			bit 2: print timer info
>  			bit 3: print locks info if CONFIG_LOCKDEP is on
>  			bit 4: print ftrace buffer
> -			bit 5: print all printk messages in buffer
> +			bit 5: replay all messages on consoles at the end of panic
>  			bit 6: print all CPUs backtrace (if available in the arch)
>  			bit 7: print only tasks in uninterruptible (blocked) state
>  			*Be aware* that this option may print a _lot_ of lines,

Will change.

> > + */
> > +#define SYS_SHOW_TASK_INFO		0x00000001
> > +#define SYS_SHOW_MEM_INFO		0x00000002
> > +#define SYS_SHOW_TIMER_INFO		0x00000004
> > +#define SYS_SHOW_LOCK_INFO		0x00000008
> > +#define SYS_SHOW_FTRACE_INFO		0x00000010
> > +#define SYS_SHOW_ALL_PRINTK_MSG		0x00000020
> > +#define SYS_SHOW_ALL_CPU_BT		0x00000040
> > +#define SYS_SHOW_BLOCKED_TASKS		0x00000080
> > +
> > +extern void sys_show_info(unsigned long info_mask);
> 
> Please, do not use "extern" in new header files. This is from
> Documentation/process/coding-style.rst:
> 
>     Do not use the ``extern`` keyword with function declarations as this makes
>     lines longer and isn't strictly necessary.
 
Thanks for the note!

> Also the header file is named "sys_info" but the API is "sys_show_*info".
> It would be more user friendly to consistently use the same prefix
> for the entire API, for example:
> 
> #define SYS_INFO_TASK		0x00000001
> #define SYS_INFO_MEM		0x00000002
> #define SYS_INFO_TIMER		0x00000004

Yeap, shorter and cleaner.

> 
> void sys_info(unsigned long si_mask);
> 
> I am sorry that I did not tell your this in the RFC.
> I focused on the bigger picture at that time.
 
No problem. Thanks for the review!

- Feng

> > +#endif	/* _LINUX_SYS_INFO_H */
> 
> Best Regards,
> Petr