[PATCH V2 0/5] generalize panic_print's dump function to be used by other kernel parts

[PATCH V2 0/5] generalize panic_print's dump function to be used by other kernel parts

[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 to a new file 'lib/sys_info.c',
and make it usable for other cases which also need system info for
debugging. 

Also as suggested by Petr Mladek, add 'panic_sys_info=' interface to
take human readable string like "tasks,mem,lock,timer,ftrace,....", 
and eventually obsolete the current 'panic_print' bitmap interface.

In RFC and V1 version, hung_task and SW/HW watchdog modules are enabled
with the new sys_info dump interface. In v2, they are kept out for
better review of current change, and will be posted later. 

Locally these have been used in our bug chasing for stabilility issues
and was helpful.

Please help to review, thanks!

- Feng

Changelog:

  Sinc V1:
     * Separate the 'sys_show_info' related code to new file sys_info.[ch] 
       (Petr Mladek)
     * Clean up the code for panic console replay (Petr Mladek)
     * Add 'panic_sys_info=' cmdline and sysctl interface for taking
       human readable parameters (Petr Mladek)
     * Add note about the obsoleting of 'panic_print' (Petr Mladek)
     * Hold the changes to hungtask/watchdog 

  Since RFC:
     * Don't print all cpu backtrace if 'sysctl_hung_task_all_cpu_backtracemay'
       is 'false' (Lance Yang)
     * Change the name of 2 new kernel control knob to have 'mask' inside, and
       add kernel document and code comments for them (Lance Yang)
     * Make the sys_show_info() support printk msg replay and all CPU backtrace. 


Feng Tang (5):
  panic: clean up code for console replay
  panic: generalize panic_print's function to show sys info
  sys_info: add help to translate sys_info string to bitmap
  panic: add 'panic_sys_info=' setup option for sysctl and kernel
    cmdline
  panic: add note that panic_print interface is deprecated

 .../admin-guide/kernel-parameters.txt         |  13 ++
 Documentation/admin-guide/sysctl/kernel.rst   |  18 +++
 include/linux/kernel.h                        |   1 +
 include/linux/sys_info.h                      |  27 ++++
 kernel/panic.c                                |  66 +++++----
 lib/Makefile                                  |   2 +-
 lib/sys_info.c                                | 127 ++++++++++++++++++
 7 files changed, 217 insertions(+), 37 deletions(-)
 create mode 100644 include/linux/sys_info.h
 create mode 100644 lib/sys_info.c

-- 
2.39.5 (Apple Git-154)

[PATCH V2 1/5] panic: clean up code for console replay

[Feng Tang]

Currently the panic_print_sys_info() was called twice with different
parameters to handle console replay case, which is kind of confusing.

Add panic_console_replay() explicitly to make the code straightforward.

Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
---
 kernel/panic.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index b0b9a8bf4560..af0d5206a624 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -238,14 +238,14 @@ void nmi_panic(struct pt_regs *regs, const char *msg)
 }
 EXPORT_SYMBOL(nmi_panic);
 
-static void panic_print_sys_info(bool console_flush)
+static void panic_console_replay(void)
 {
-	if (console_flush) {
-		if (panic_print & PANIC_PRINT_ALL_PRINTK_MSG)
-			console_flush_on_panic(CONSOLE_REPLAY_ALL);
-		return;
-	}
+	if (panic_print & PANIC_PRINT_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();
 
@@ -410,7 +410,7 @@ void panic(const char *fmt, ...)
 	 */
 	atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
 
-	panic_print_sys_info(false);
+	panic_print_sys_info();
 
 	kmsg_dump_desc(KMSG_DUMP_PANIC, buf);
 
@@ -439,7 +439,7 @@ void panic(const char *fmt, ...)
 	debug_locks_off();
 	console_flush_on_panic(CONSOLE_FLUSH_PENDING);
 
-	panic_print_sys_info(true);
+	panic_console_replay();
 
 	if (!panic_blink)
 		panic_blink = no_blink;
-- 
2.39.5 (Apple Git-154)

[PATCH V2 2/5] 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 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)

[PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap

[Feng Tang]

Bitmap definition is hard to remember and decode. Add sysctl interface
to translate human readable string like "tasks,mem,timer,lock,ftrace,..."
to bitmap.

Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
---
 include/linux/sys_info.h |  7 +++
 lib/sys_info.c           | 97 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+)

diff --git a/include/linux/sys_info.h b/include/linux/sys_info.h
index 79bf4a942e5f..0b49863cd414 100644
--- a/include/linux/sys_info.h
+++ b/include/linux/sys_info.h
@@ -16,5 +16,12 @@
 #define SYS_SHOW_BLOCKED_TASKS		0x00000080
 
 extern void sys_show_info(unsigned long info_mask);
+extern unsigned long sys_info_parse_param(char *str);
 
+#ifdef CONFIG_SYSCTL
+struct ctl_table;
+extern int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
+					  void *buffer, size_t *lenp,
+					  loff_t *ppos);
+#endif
 #endif	/* _LINUX_SYS_INFO_H */
diff --git a/lib/sys_info.c b/lib/sys_info.c
index 90a79b5164c9..9693542435ba 100644
--- a/lib/sys_info.c
+++ b/lib/sys_info.c
@@ -5,6 +5,103 @@
 #include <linux/console.h>
 #include <linux/nmi.h>
 
+struct sys_info_name {
+	unsigned long bit;
+	const char *name;
+};
+
+static const char sys_info_avail[] = "tasks,mem,timer,lock,ftrace,all_bt,blocked_tasks";
+
+static const struct sys_info_name  si_names[] = {
+	{ SYS_SHOW_TASK_INFO,	"tasks" },
+	{ SYS_SHOW_MEM_INFO,	"mem" },
+	{ SYS_SHOW_TIMER_INFO,	"timer" },
+	{ SYS_SHOW_LOCK_INFO,	"lock" },
+	{ SYS_SHOW_FTRACE_INFO, "ftrace" },
+	{ SYS_SHOW_ALL_CPU_BT,	"all_bt" },
+	{ SYS_SHOW_BLOCKED_TASKS, "blocked_tasks" },
+};
+
+/* Expecting string like "xxx_sys_info=tasks,mem,timer,lock,ftrace,..." */
+unsigned long sys_info_parse_param(char *str)
+{
+	unsigned long si_bits = 0;
+	char *s, *name;
+	int i;
+
+	s = str;
+	while ((name = strsep(&s, ",")) && *name) {
+		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
+			if (!strcmp(name, si_names[i].name)) {
+				si_bits |= si_names[i].bit;
+				break;
+			}
+		}
+	}
+
+	return si_bits;
+}
+
+#ifdef CONFIG_SYSCTL
+int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
+					  void *buffer, size_t *lenp,
+					  loff_t *ppos)
+{
+	char names[sizeof(sys_info_avail) + 1];
+	struct ctl_table table;
+	unsigned long *si_bits_global;
+	int i, ret, len;
+
+	si_bits_global = ro_table->data;
+
+	if (write) {
+		unsigned long si_bits;
+
+		table = *ro_table;
+		table.data = names;
+		table.maxlen = sizeof(names);
+		ret = proc_dostring(&table, write, buffer, lenp, ppos);
+		if (ret)
+			return ret;
+
+		si_bits = sys_info_parse_param(names);
+		/*
+		 * The access to the global value is not synchronized.
+		 */
+		WRITE_ONCE(*si_bits_global, si_bits);
+		return 0;
+	} else {
+		/* for 'read' operation */
+		bool first = true;
+		char *buf;
+
+		buf = names;
+		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
+			if (*si_bits_global & si_names[i].bit) {
+
+				if (first) {
+					first = false;
+				} else {
+					*buf = ',';
+					buf++;
+				}
+
+				len = strlen(si_names[i].name);
+				strncpy(buf, si_names[i].name, len);
+				buf += len;
+			}
+
+		}
+		*buf = '\0';
+
+		table = *ro_table;
+		table.data = names;
+		table.maxlen = sizeof(names);
+		return proc_dostring(&table, write, buffer, lenp, ppos);
+	}
+}
+#endif
+
 void sys_show_info(unsigned long info_flag)
 {
 	if (info_flag & SYS_SHOW_TASK_INFO)
-- 
2.39.5 (Apple Git-154)

[PATCH V2 4/5] panic: add 'panic_sys_info=' setup option for sysctl and kernel cmdline

[Feng Tang]

Add 'panic_sys_info=' setup which expects string like "tasks,mem,lock,...".
It supports both runtime sysctl control and boot time kernel cmdline setup,
and could be seen as human readable string version of 'panic_print'.

The detail mapping is:
	SYS_SHOW_TASK_INFO	"tasks"
	SYS_SHOW_MEM_INFO	"mem"
	SYS_SHOW_TIMER_INFO	"timer"
	SYS_SHOW_LOCK_INFO	"lock"
	SYS_SHOW_FTRACE_INFO	"ftrace"
	SYS_SHOW_ALL_CPU_BT	"all_bt"
	SYS_SHOW_BLOCKED_TASKS	"blocked_tasks"

Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
---
 .../admin-guide/kernel-parameters.txt          | 13 +++++++++++++
 Documentation/admin-guide/sysctl/kernel.rst    | 18 ++++++++++++++++++
 kernel/panic.c                                 | 16 ++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f1f2c0874da9..d714a0ebf909 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4541,6 +4541,19 @@
 			Use this option carefully, maybe worth to setup a
 			bigger log buffer with "log_buf_len" along with this.
 
+	panic_sys_info=
+			String of subsystem info to be dumped on panic.
+			It expects string of comma-separated words like
+			"tasks,mem,timer,...", which is a human readable string
+			version of 'panic_print':
+			tasks: print all tasks info
+			mem: print system memory info
+			timer: print timer info
+			lock: print locks info if CONFIG_LOCKDEP is on
+			ftrace: print ftrace buffer
+			all_bt: print all CPUs backtrace (if available in the arch)
+			blocked_tasks: print only tasks in uninterruptible (blocked) state
+
 	parkbd.port=	[HW] Parallel port number the keyboard adapter is
 			connected to, default is 0.
 			Format: <parport#>
diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index dd49a89a62d3..2013afd98605 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -899,6 +899,24 @@ So for example to print tasks and memory info on panic, user can::
   echo 3 > /proc/sys/kernel/panic_print
 
 
+panic_sys_info
+==============
+
+String of subsystem info to be dumped on panic. It expects string of
+comma-separated words like "tasks,mem,timer,...", which is a human
+readable string version of 'panic_print':
+
+=============   ===================================================
+tasks           print all tasks info
+mem             print system memory info
+timer           print timer info
+lock            print locks info if CONFIG_LOCKDEP is on
+ftrace          print ftrace buffer
+all_bt          print all CPUs backtrace (if available in the arch)
+blocked_tasks   print only tasks in uninterruptible (blocked) state
+=============   ===================================================
+
+
 panic_on_rcu_stall
 ==================
 
diff --git a/kernel/panic.c b/kernel/panic.c
index 35c98aefa39f..ea238f7d4b54 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -125,6 +125,13 @@ static const struct ctl_table kern_panic_table[] = {
 		.mode           = 0644,
 		.proc_handler   = proc_douintvec,
 	},
+	{
+		.procname	= "panic_sys_info",
+		.data		= &panic_print,
+		.maxlen         = sizeof(panic_print),
+		.mode		= 0644,
+		.proc_handler	= sysctl_sys_info_handler,
+	},
 };
 
 static __init int kernel_panic_sysctls_init(void)
@@ -135,6 +142,15 @@ static __init int kernel_panic_sysctls_init(void)
 late_initcall(kernel_panic_sysctls_init);
 #endif
 
+/* The format is "panic_sys_info=task,mem,ftrace,..." */
+static int __init setup_panic_sys_info(char *buf)
+{
+	/* There is no risk of race in kernel boot phase */
+	panic_print = sys_info_parse_param(buf);
+	return 1;
+}
+__setup("panic_sys_info=", setup_panic_sys_info);
+
 static atomic_t warn_count = ATOMIC_INIT(0);
 
 #ifdef CONFIG_SYSFS
-- 
2.39.5 (Apple Git-154)

[PATCH V2 5/5] panic: add note that panic_print interface is deprecated

[Feng Tang]

Long term wise, the 'panic_sys_info' should be the only controlling
interface, which can be referred by other modules.

Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
---
 kernel/panic.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index ea238f7d4b54..e8a05fc6b733 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -76,6 +76,13 @@ ATOMIC_NOTIFIER_HEAD(panic_notifier_list);
 EXPORT_SYMBOL(panic_notifier_list);
 
 #ifdef CONFIG_SYSCTL
+static int sysctl_panic_print_handler(const struct ctl_table *table, int write,
+			   void *buffer, size_t *lenp, loff_t *ppos)
+{
+	printk_once("panic: 'panic_print' sysctl interface will be obsoleted by 'panic_sys_info' interface.\n");
+	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
+}
+
 static const struct ctl_table kern_panic_table[] = {
 #ifdef CONFIG_SMP
 	{
@@ -107,7 +114,7 @@ static const struct ctl_table kern_panic_table[] = {
 		.data		= &panic_print,
 		.maxlen		= sizeof(unsigned long),
 		.mode		= 0644,
-		.proc_handler	= proc_doulongvec_minmax,
+		.proc_handler	= sysctl_panic_print_handler,
 	},
 	{
 		.procname	= "panic_on_warn",
-- 
2.39.5 (Apple Git-154)

Re: [PATCH V2 5/5] panic: add note that panic_print interface is deprecated

[Lance Yang]

On 2025/6/16 09:08, Feng Tang wrote:
> Long term wise, the 'panic_sys_info' should be the only controlling
> interface, which can be referred by other modules.
> 
> Suggested-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
> ---
>   kernel/panic.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/panic.c b/kernel/panic.c
> index ea238f7d4b54..e8a05fc6b733 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -76,6 +76,13 @@ ATOMIC_NOTIFIER_HEAD(panic_notifier_list);
>   EXPORT_SYMBOL(panic_notifier_list);
>   
>   #ifdef CONFIG_SYSCTL
> +static int sysctl_panic_print_handler(const struct ctl_table *table, int write,
> +			   void *buffer, size_t *lenp, loff_t *ppos)
> +{
> +	printk_once("panic: 'panic_print' sysctl interface will be obsoleted by 'panic_sys_info' interface.\n");

Hmm... I would get scared for a second when I read messages prefixed 
with "panic:"
from dmesg. That prefix should have a very specific meaning ;)

Well, we can just change the prefix to something more neutral:

printk_once("Kernel: 'panic_print' sysctl interface will be obsoleted by 
'panic_sys_info' interface.\n");

Thanks,
Lance

> +	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
> +}
> +
>   static const struct ctl_table kern_panic_table[] = {
>   #ifdef CONFIG_SMP
>   	{
> @@ -107,7 +114,7 @@ static const struct ctl_table kern_panic_table[] = {
>   		.data		= &panic_print,
>   		.maxlen		= sizeof(unsigned long),
>   		.mode		= 0644,
> -		.proc_handler	= proc_doulongvec_minmax,
> +		.proc_handler	= sysctl_panic_print_handler,
>   	},
>   	{
>   		.procname	= "panic_on_warn",

Re: [PATCH V2 5/5] panic: add note that panic_print interface is deprecated

[Feng Tang]

On Mon, Jun 16, 2025 at 09:45:16AM +0800, Lance Yang wrote:
> 
> 
> On 2025/6/16 09:08, Feng Tang wrote:
> > Long term wise, the 'panic_sys_info' should be the only controlling
> > interface, which can be referred by other modules.
> > 
> > Suggested-by: Petr Mladek <pmladek@suse.com>
> > Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
> > ---
> >   kernel/panic.c | 9 ++++++++-
> >   1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/panic.c b/kernel/panic.c
> > index ea238f7d4b54..e8a05fc6b733 100644
> > --- a/kernel/panic.c
> > +++ b/kernel/panic.c
> > @@ -76,6 +76,13 @@ ATOMIC_NOTIFIER_HEAD(panic_notifier_list);
> >   EXPORT_SYMBOL(panic_notifier_list);
> >   #ifdef CONFIG_SYSCTL
> > +static int sysctl_panic_print_handler(const struct ctl_table *table, int write,
> > +			   void *buffer, size_t *lenp, loff_t *ppos)
> > +{
> > +	printk_once("panic: 'panic_print' sysctl interface will be obsoleted by 'panic_sys_info' interface.\n");
> 
> Hmm... I would get scared for a second when I read messages prefixed with
> "panic:"
> from dmesg. That prefix should have a very specific meaning ;)
> 
> Well, we can just change the prefix to something more neutral:
> 
> printk_once("Kernel: 'panic_print' sysctl interface will be obsoleted by
> 'panic_sys_info' interface.\n");
 
Yes. I tried to give the module name, but forgot the name is too scary
for normal users :)

Thanks,
Feng

> Thanks,
> Lance

Re: [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap

[kernel test robot]

Hi Feng,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-nonmm-unstable]
[also build test WARNING on akpm-mm/mm-everything linus/master v6.16-rc2 next-20250613]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Feng-Tang/panic-clean-up-code-for-console-replay/20250616-091042
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-nonmm-unstable
patch link:    https://lore.kernel.org/r/20250616010840.38258-4-feng.tang%40linux.alibaba.com
patch subject: [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap
config: x86_64-buildonly-randconfig-001-20250616 (https://download.01.org/0day-ci/archive/20250616/202506161234.wRZSzo5v-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250616/202506161234.wRZSzo5v-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506161234.wRZSzo5v-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> lib/sys_info.c:13:19: warning: unused variable 'sys_info_avail' [-Wunused-const-variable]
      13 | static const char sys_info_avail[] = "tasks,mem,timer,lock,ftrace,all_bt,blocked_tasks";
         |                   ^~~~~~~~~~~~~~
   1 warning generated.


vim +/sys_info_avail +13 lib/sys_info.c

    12	
  > 13	static const char sys_info_avail[] = "tasks,mem,timer,lock,ftrace,all_bt,blocked_tasks";
    14	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap

[Feng Tang]

On Mon, Jun 16, 2025 at 12:25:29PM +0800, kernel test robot wrote:
> Hi Feng,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on akpm-mm/mm-nonmm-unstable]
> [also build test WARNING on akpm-mm/mm-everything linus/master v6.16-rc2 next-20250613]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Feng-Tang/panic-clean-up-code-for-console-replay/20250616-091042
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-nonmm-unstable
> patch link:    https://lore.kernel.org/r/20250616010840.38258-4-feng.tang%40linux.alibaba.com
> patch subject: [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap
> config: x86_64-buildonly-randconfig-001-20250616 (https://download.01.org/0day-ci/archive/20250616/202506161234.wRZSzo5v-lkp@intel.com/config)
> compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
> rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250616/202506161234.wRZSzo5v-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202506161234.wRZSzo5v-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
> >> lib/sys_info.c:13:19: warning: unused variable 'sys_info_avail' [-Wunused-const-variable]
>       13 | static const char sys_info_avail[] = "tasks,mem,timer,lock,ftrace,all_bt,blocked_tasks";
>          |                   ^~~~~~~~~~~~~~
>    1 warning generated.

Thanks for the reporting! This is related with CONFIG_SYSCTL=n, and
should be fixed by below patch:

---
diff --git a/lib/sys_info.c b/lib/sys_info.c
index 9693542435ba..ca289b4871b4 100644
--- a/lib/sys_info.c
+++ b/lib/sys_info.c
@@ -10,8 +10,6 @@ struct sys_info_name {
 	const char *name;
 };
 
-static const char sys_info_avail[] = "tasks,mem,timer,lock,ftrace,all_bt,blocked_tasks";
-
 static const struct sys_info_name  si_names[] = {
 	{ SYS_SHOW_TASK_INFO,	"tasks" },
 	{ SYS_SHOW_MEM_INFO,	"mem" },
@@ -43,6 +41,8 @@ unsigned long sys_info_parse_param(char *str)
 }
 
 #ifdef CONFIG_SYSCTL
+static const char sys_info_avail[] = "tasks,mem,timer,lock,ftrace,all_bt,blocked_tasks";
+
 int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
 					  void *buffer, size_t *lenp,
 					  loff_t *ppos)

Re: [PATCH V2 1/5] panic: clean up code for console replay

[Petr Mladek]

On Mon 2025-06-16 09:08:36, Feng Tang wrote:
> Currently the panic_print_sys_info() was called twice with different
> parameters to handle console replay case, which is kind of confusing.
> 
> Add panic_console_replay() explicitly to make the code straightforward.
> 
> Suggested-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>

Looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

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

[Petr Mladek]

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

[Feng Tang]

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

Re: [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap

[Petr Mladek]

On Mon 2025-06-16 09:08:38, Feng Tang wrote:
> Bitmap definition is hard to remember and decode. Add sysctl interface
> to translate human readable string like "tasks,mem,timer,lock,ftrace,..."
> to bitmap.
> 
> Suggested-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
> ---
>  include/linux/sys_info.h |  7 +++
>  lib/sys_info.c           | 97 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 104 insertions(+)
> 
> diff --git a/include/linux/sys_info.h b/include/linux/sys_info.h
> index 79bf4a942e5f..0b49863cd414 100644
> --- a/include/linux/sys_info.h
> +++ b/include/linux/sys_info.h
> @@ -16,5 +16,12 @@
>  #define SYS_SHOW_BLOCKED_TASKS		0x00000080
>  
>  extern void sys_show_info(unsigned long info_mask);
> +extern unsigned long sys_info_parse_param(char *str);
>  
> +#ifdef CONFIG_SYSCTL
> +struct ctl_table;
> +extern int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
> +					  void *buffer, size_t *lenp,
> +					  loff_t *ppos);
> +#endif
>  #endif	/* _LINUX_SYS_INFO_H */
> diff --git a/lib/sys_info.c b/lib/sys_info.c
> index 90a79b5164c9..9693542435ba 100644
> --- a/lib/sys_info.c
> +++ b/lib/sys_info.c
> @@ -5,6 +5,103 @@
>  #include <linux/console.h>
>  #include <linux/nmi.h>
>  
> +struct sys_info_name {
> +	unsigned long bit;
> +	const char *name;
> +};
> +
> +static const char sys_info_avail[] = "tasks,mem,timer,lock,ftrace,all_bt,blocked_tasks";
> +
> +static const struct sys_info_name  si_names[] = {
> +	{ SYS_SHOW_TASK_INFO,	"tasks" },
> +	{ SYS_SHOW_MEM_INFO,	"mem" },
> +	{ SYS_SHOW_TIMER_INFO,	"timer" },

I see that the naming is a bit inconsistent in using singulars
vs. plurals. I suggest to use plulars when it makes sense.
It means here:

	{ SYS_SHOW_TIMERS_INFO,	"timers" },

> +	{ SYS_SHOW_LOCK_INFO,	"lock" },

and here

	{ SYS_SHOW_LOCKS_INFO,	"locks" },

> +	{ SYS_SHOW_FTRACE_INFO, "ftrace" },
> +	{ SYS_SHOW_ALL_CPU_BT,	"all_bt" },
> +	{ SYS_SHOW_BLOCKED_TASKS, "blocked_tasks" },
> +};

[...]

> +#ifdef CONFIG_SYSCTL
> +int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
> +					  void *buffer, size_t *lenp,
> +					  loff_t *ppos)
> +{
> +	char names[sizeof(sys_info_avail) + 1];
> +	struct ctl_table table;
> +	unsigned long *si_bits_global;
> +	int i, ret, len;
> +
> +	si_bits_global = ro_table->data;
> +
> +	if (write) {
> +		unsigned long si_bits;
> +
> +		table = *ro_table;
> +		table.data = names;
> +		table.maxlen = sizeof(names);
> +		ret = proc_dostring(&table, write, buffer, lenp, ppos);
> +		if (ret)
> +			return ret;
> +
> +		si_bits = sys_info_parse_param(names);
> +		/*
> +		 * The access to the global value is not synchronized.
> +		 */

Nit, the comment fits on a single line. I would use:

		/* The access to the global value is not synchronized. */

> +		WRITE_ONCE(*si_bits_global, si_bits);
> +		return 0;
> +	} else {
> +		/* for 'read' operation */
> +		bool first = true;
> +		char *buf;
> +
> +		buf = names;
> +		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
> +			if (*si_bits_global & si_names[i].bit) {
> +
> +				if (first) {
> +					first = false;
> +				} else {
> +					*buf = ',';
> +					buf++;
> +				}
> +
> +				len = strlen(si_names[i].name);
> +				strncpy(buf, si_names[i].name, len);

I am afraid of a buffer overflow when people add new entry into
si_names[] table but they forget to update sys_info_avail[] string.
I would prefer to limit the write to the buffer by the size of the buffer.

Something like (on top of this patch):

--- a/lib/sys_info.c
+++ b/lib/sys_info.c
@@ -50,7 +50,7 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
 	char names[sizeof(sys_info_avail) + 1];
 	struct ctl_table table;
 	unsigned long *si_bits_global;
-	int i, ret, len;
+	int i, ret;
 
 	si_bits_global = ro_table->data;
 
@@ -72,27 +72,18 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
 		return 0;
 	} else {
 		/* for 'read' operation */
-		bool first = true;
-		char *buf;
+		char *delim = "";
+		int len = 0;
 
-		buf = names;
+		names[0] = '\0';
 		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
 			if (*si_bits_global & si_names[i].bit) {
-
-				if (first) {
-					first = false;
-				} else {
-					*buf = ',';
-					buf++;
-				}
-
-				len = strlen(si_names[i].name);
-				strncpy(buf, si_names[i].name, len);
-				buf += len;
+				len += scnprintf(names + len, sizeof(names) - len,
+						 "%s%s", delim, si_names[i].name);
+				delim = ",";
 			}
 
 		}
-		*buf = '\0';
 
 		table = *ro_table;
 		table.data = names;

Best Regards,
Petr

Re: [PATCH V2 4/5] panic: add 'panic_sys_info=' setup option for sysctl and kernel cmdline

[Petr Mladek]

On Mon 2025-06-16 09:08:39, Feng Tang wrote:
> Add 'panic_sys_info=' setup which expects string like "tasks,mem,lock,...".

This patch actually adds also the sysctl interface. It should be
mentioned in the "Subject" and here.

That said, it might be better to add the sysctl interface in
the previous patch and add just the setup() here.

> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4541,6 +4541,19 @@
>  			Use this option carefully, maybe worth to setup a
>  			bigger log buffer with "log_buf_len" along with this.
>  
> +	panic_sys_info=


> +			String of subsystem info to be dumped on panic.

I am not a native speaker but I have troubles to parse the above
sentence. See below.

> +			It expects string of comma-separated words like
> +			"tasks,mem,timer,...", which is a human readable string
> +			version of 'panic_print':
> +			tasks: print all tasks info
> +			mem: print system memory info
> +			timer: print timer info
> +			lock: print locks info if CONFIG_LOCKDEP is on
> +			ftrace: print ftrace buffer
> +			all_bt: print all CPUs backtrace (if available in the arch)
> +			blocked_tasks: print only tasks in uninterruptible (blocked) state

This blob is hard to parse. I suggest to replace it with something
like:

<proposal>
	panic_sys_info= A comma separated list of extra information to be dumped
			on panic.
			Format: val[,val...]
			Where @val can be any of the following:

			tasks:		print all tasks info
			mem:		print system memory info
			timers:		print timers info
			locks:		print locks info if CONFIG_LOCKDEP is on
			ftrace:		print ftrace buffer
			all_bt:		print all CPUs backtrace (if available in the arch)
			blocked_tasks:	print only tasks in uninterruptible (blocked) state

			This is a human readable alternative to the 'panic_print' option.
</proposal>

> +
>  	parkbd.port=	[HW] Parallel port number the keyboard adapter is
>  			connected to, default is 0.
>  			Format: <parport#>
> diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
> index dd49a89a62d3..2013afd98605 100644
> --- a/Documentation/admin-guide/sysctl/kernel.rst
> +++ b/Documentation/admin-guide/sysctl/kernel.rst
> @@ -899,6 +899,24 @@ So for example to print tasks and memory info on panic, user can::
>    echo 3 > /proc/sys/kernel/panic_print
>  
>  
> +panic_sys_info
> +==============
> +
> +String of subsystem info to be dumped on panic. It expects string of

Same here.

> +comma-separated words like "tasks,mem,timer,...", which is a human
> +readable string version of 'panic_print':

I would replace it with:

<proposal>
A comma separated list of extra information to be dumped on panic,
for example, "tasks,mem,timers,...".  It is a human readable alternative
to 'panic_print'. Possible values are:
</proposal>

> +
> +=============   ===================================================
> +tasks           print all tasks info
> +mem             print system memory info
> +timer           print timer info
> +lock            print locks info if CONFIG_LOCKDEP is on
> +ftrace          print ftrace buffer
> +all_bt          print all CPUs backtrace (if available in the arch)
> +blocked_tasks   print only tasks in uninterruptible (blocked) state
> +=============   ===================================================
> +
> +
>  panic_on_rcu_stall
>  ==================

The rest looks good.

Best Regards,
Petr

Re: [PATCH V2 5/5] panic: add note that panic_print interface is deprecated

[Petr Mladek]

On Mon 2025-06-16 09:45:16, Lance Yang wrote:
> 
> 
> On 2025/6/16 09:08, Feng Tang wrote:
> > Long term wise, the 'panic_sys_info' should be the only controlling
> > interface, which can be referred by other modules.
> > 
> > Suggested-by: Petr Mladek <pmladek@suse.com>
> > Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
> > ---
> >   kernel/panic.c | 9 ++++++++-
> >   1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/panic.c b/kernel/panic.c
> > index ea238f7d4b54..e8a05fc6b733 100644
> > --- a/kernel/panic.c
> > +++ b/kernel/panic.c
> > @@ -76,6 +76,13 @@ ATOMIC_NOTIFIER_HEAD(panic_notifier_list);
> >   EXPORT_SYMBOL(panic_notifier_list);
> >   #ifdef CONFIG_SYSCTL
> > +static int sysctl_panic_print_handler(const struct ctl_table *table, int write,
> > +			   void *buffer, size_t *lenp, loff_t *ppos)
> > +{
> > +	printk_once("panic: 'panic_print' sysctl interface will be obsoleted by 'panic_sys_info' interface.\n");
> 
> Hmm... I would get scared for a second when I read messages prefixed with
> "panic:"
> from dmesg. That prefix should have a very specific meaning ;)
> 
> Well, we can just change the prefix to something more neutral:
> 
> printk_once("Kernel: 'panic_print' sysctl interface will be obsoleted by
> 'panic_sys_info' interface.\n");

I agree that this looks better.

That said, I am not a native speaker but I think the "will be
obsoleted" is not correct. I would use "has been obsoleted" instead.

Also the messages should use a particular loglevel, e.g. KERN_INFO.

I would do:

	pr_info_once("Kernel: 'panic_print' sysctl interface has been obsoleted by 'panic_sys_info' interface.\n");


> 
> > +	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
> > +}
> > +

Best Regards,
Petr

Re: [PATCH V2 5/5] panic: add note that panic_print interface is deprecated

[Petr Mladek]

On Mon 2025-06-16 09:08:40, Feng Tang wrote:
> Long term wise, the 'panic_sys_info' should be the only controlling
> interface, which can be referred by other modules.

Strictly speaking, 'panic_sys_info' is not a complete
replacement for 'panic_print' because it does not allow
replaying the log buffer during panic.

I suggest to add a separate parameter "panic_console_replay"
for the missing functionality first. And 'panic_print' will
be obsoleted by both 'panic_sys_info' and 'panic_console_replay'.

> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -76,6 +76,13 @@ ATOMIC_NOTIFIER_HEAD(panic_notifier_list);
>  EXPORT_SYMBOL(panic_notifier_list);
>  
>  #ifdef CONFIG_SYSCTL
> +static int sysctl_panic_print_handler(const struct ctl_table *table, int write,
> +			   void *buffer, size_t *lenp, loff_t *ppos)
> +{
> +	printk_once("panic: 'panic_print' sysctl interface will be obsoleted by 'panic_sys_info' interface.\n");
> +	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
> +}
> +
>  static const struct ctl_table kern_panic_table[] = {
>  #ifdef CONFIG_SMP
>  	{
> @@ -107,7 +114,7 @@ static const struct ctl_table kern_panic_table[] = {
>  		.data		= &panic_print,
>  		.maxlen		= sizeof(unsigned long),
>  		.mode		= 0644,
> -		.proc_handler	= proc_doulongvec_minmax,
> +		.proc_handler	= sysctl_panic_print_handler,

This handles only the sysctl interface. We should do something similar
also for the "panic_print" command line parameter. It would require
using core_param_cb() instead of core_param().

>  	},
>  	{
>  		.procname	= "panic_on_warn",

Best Regards,
Petr

Re: [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap

[Feng Tang]

On Mon, Jun 23, 2025 at 04:04:56PM +0200, Petr Mladek wrote:
> On Mon 2025-06-16 09:08:38, Feng Tang wrote:
> > Bitmap definition is hard to remember and decode. Add sysctl interface
> > to translate human readable string like "tasks,mem,timer,lock,ftrace,..."
> > to bitmap.
> > 
> > Suggested-by: Petr Mladek <pmladek@suse.com>
> > Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
> > ---
> >  include/linux/sys_info.h |  7 +++
> >  lib/sys_info.c           | 97 ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 104 insertions(+)
> > 
> > diff --git a/include/linux/sys_info.h b/include/linux/sys_info.h
> > index 79bf4a942e5f..0b49863cd414 100644
> > --- a/include/linux/sys_info.h
> > +++ b/include/linux/sys_info.h
> > @@ -16,5 +16,12 @@
> >  #define SYS_SHOW_BLOCKED_TASKS		0x00000080
> >  
> >  extern void sys_show_info(unsigned long info_mask);
> > +extern unsigned long sys_info_parse_param(char *str);
> >  
> > +#ifdef CONFIG_SYSCTL
> > +struct ctl_table;
> > +extern int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
> > +					  void *buffer, size_t *lenp,
> > +					  loff_t *ppos);
> > +#endif
> >  #endif	/* _LINUX_SYS_INFO_H */
> > diff --git a/lib/sys_info.c b/lib/sys_info.c
> > index 90a79b5164c9..9693542435ba 100644
> > --- a/lib/sys_info.c
> > +++ b/lib/sys_info.c
> > @@ -5,6 +5,103 @@
> >  #include <linux/console.h>
> >  #include <linux/nmi.h>
> >  
> > +struct sys_info_name {
> > +	unsigned long bit;
> > +	const char *name;
> > +};
> > +
> > +static const char sys_info_avail[] = "tasks,mem,timer,lock,ftrace,all_bt,blocked_tasks";
> > +
> > +static const struct sys_info_name  si_names[] = {
> > +	{ SYS_SHOW_TASK_INFO,	"tasks" },
> > +	{ SYS_SHOW_MEM_INFO,	"mem" },
> > +	{ SYS_SHOW_TIMER_INFO,	"timer" },
> 
> I see that the naming is a bit inconsistent in using singulars
> vs. plurals. I suggest to use plulars when it makes sense.
> It means here:
> 
> 	{ SYS_SHOW_TIMERS_INFO,	"timers" },
> 
> > +	{ SYS_SHOW_LOCK_INFO,	"lock" },
> 
> and here
> 
> 	{ SYS_SHOW_LOCKS_INFO,	"locks" },

Will change.

> > +	{ SYS_SHOW_FTRACE_INFO, "ftrace" },
> > +	{ SYS_SHOW_ALL_CPU_BT,	"all_bt" },
> > +	{ SYS_SHOW_BLOCKED_TASKS, "blocked_tasks" },
> > +};
> 
> [...]
> 
> > +#ifdef CONFIG_SYSCTL
> > +int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
> > +					  void *buffer, size_t *lenp,
> > +					  loff_t *ppos)
> > +{
> > +	char names[sizeof(sys_info_avail) + 1];
> > +	struct ctl_table table;
> > +	unsigned long *si_bits_global;
> > +	int i, ret, len;
> > +
> > +	si_bits_global = ro_table->data;
> > +
> > +	if (write) {
> > +		unsigned long si_bits;
> > +
> > +		table = *ro_table;
> > +		table.data = names;
> > +		table.maxlen = sizeof(names);
> > +		ret = proc_dostring(&table, write, buffer, lenp, ppos);
> > +		if (ret)
> > +			return ret;
> > +
> > +		si_bits = sys_info_parse_param(names);
> > +		/*
> > +		 * The access to the global value is not synchronized.
> > +		 */
> 
> Nit, the comment fits on a single line. I would use:
> 
> 		/* The access to the global value is not synchronized. */

Yes.

> > +		WRITE_ONCE(*si_bits_global, si_bits);
> > +		return 0;
> > +	} else {
> > +		/* for 'read' operation */
> > +		bool first = true;
> > +		char *buf;
> > +
> > +		buf = names;
> > +		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
> > +			if (*si_bits_global & si_names[i].bit) {
> > +
> > +				if (first) {
> > +					first = false;
> > +				} else {
> > +					*buf = ',';
> > +					buf++;
> > +				}
> > +
> > +				len = strlen(si_names[i].name);
> > +				strncpy(buf, si_names[i].name, len);
> 
> I am afraid of a buffer overflow when people add new entry into
> si_names[] table but they forget to update sys_info_avail[] string.
> I would prefer to limit the write to the buffer by the size of the buffer.

Indeed. In my v3 branch, I have also added a line to warn about it:

	/*
	 * When 'si_names' gets updated,  please make sure the 'sys_info_avail'
	 * below is updated accordingly.
	 */
	static const struct sys_info_name  si_names[] = {

But still, better to do some handling in code as you suggested.

> Something like (on top of this patch):
> 
> --- a/lib/sys_info.c
> +++ b/lib/sys_info.c
> @@ -50,7 +50,7 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
>  	char names[sizeof(sys_info_avail) + 1];
>  	struct ctl_table table;
>  	unsigned long *si_bits_global;
> -	int i, ret, len;
> +	int i, ret;
>  
>  	si_bits_global = ro_table->data;
>  
> @@ -72,27 +72,18 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
>  		return 0;
>  	} else {
>  		/* for 'read' operation */
> -		bool first = true;
> -		char *buf;
> +		char *delim = "";
> +		int len = 0;
>  
> -		buf = names;
> +		names[0] = '\0';
>  		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
>  			if (*si_bits_global & si_names[i].bit) {
> -
> -				if (first) {
> -					first = false;
> -				} else {
> -					*buf = ',';
> -					buf++;
> -				}
> -
> -				len = strlen(si_names[i].name);
> -				strncpy(buf, si_names[i].name, len);
> -				buf += len;
> +				len += scnprintf(names + len, sizeof(names) - len,
> +						 "%s%s", delim, si_names[i].name);
> +				delim = ",";
>  			}
>  
>  		}
> -		*buf = '\0';
>  
>  		table = *ro_table;
>  		table.data = names;

Thanks!

- Feng
 
> Best Regards,
> Petr

Re: [PATCH V2 4/5] panic: add 'panic_sys_info=' setup option for sysctl and kernel cmdline

[Feng Tang]

On Mon, Jun 23, 2025 at 05:04:46PM +0200, Petr Mladek wrote:
> On Mon 2025-06-16 09:08:39, Feng Tang wrote:
> > Add 'panic_sys_info=' setup which expects string like "tasks,mem,lock,...".
> 
> This patch actually adds also the sysctl interface. It should be
> mentioned in the "Subject" and here.
 
I mentioned 'sysctl' in the subjec line, but maybe it's not obvious :)

> That said, it might be better to add the sysctl interface in
> the previous patch and add just the setup() here.

OK.

> 
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -4541,6 +4541,19 @@
> >  			Use this option carefully, maybe worth to setup a
> >  			bigger log buffer with "log_buf_len" along with this.
> >  
> > +	panic_sys_info=
> 
> 
> > +			String of subsystem info to be dumped on panic.
> 
> I am not a native speaker but I have troubles to parse the above
> sentence. See below.
> > +			It expects string of comma-separated words like
> > +			"tasks,mem,timer,...", which is a human readable string
> > +			version of 'panic_print':
> > +			tasks: print all tasks info
> > +			mem: print system memory info
> > +			timer: print timer info
> > +			lock: print locks info if CONFIG_LOCKDEP is on
> > +			ftrace: print ftrace buffer
> > +			all_bt: print all CPUs backtrace (if available in the arch)
> > +			blocked_tasks: print only tasks in uninterruptible (blocked) state
> 
> This blob is hard to parse. I suggest to replace it with something
> like:
> 
> <proposal>
> 	panic_sys_info= A comma separated list of extra information to be dumped
> 			on panic.
> 			Format: val[,val...]
> 			Where @val can be any of the following:
> 
> 			tasks:		print all tasks info
> 			mem:		print system memory info
> 			timers:		print timers info
> 			locks:		print locks info if CONFIG_LOCKDEP is on
> 			ftrace:		print ftrace buffer
> 			all_bt:		print all CPUs backtrace (if available in the arch)
> 			blocked_tasks:	print only tasks in uninterruptible (blocked) state
> 
> 			This is a human readable alternative to the 'panic_print' option.
> </proposal>

Thanks! It's much better.

> > +
> >  	parkbd.port=	[HW] Parallel port number the keyboard adapter is
> >  			connected to, default is 0.
> >  			Format: <parport#>
> > diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
> > index dd49a89a62d3..2013afd98605 100644
> > --- a/Documentation/admin-guide/sysctl/kernel.rst
> > +++ b/Documentation/admin-guide/sysctl/kernel.rst
> > @@ -899,6 +899,24 @@ So for example to print tasks and memory info on panic, user can::
> >    echo 3 > /proc/sys/kernel/panic_print
> >  
> >  
> > +panic_sys_info
> > +==============
> > +
> > +String of subsystem info to be dumped on panic. It expects string of
> 
> Same here.
> 
> > +comma-separated words like "tasks,mem,timer,...", which is a human
> > +readable string version of 'panic_print':
> 
> I would replace it with:
> 
> <proposal>
> A comma separated list of extra information to be dumped on panic,
> for example, "tasks,mem,timers,...".  It is a human readable alternative
> to 'panic_print'. Possible values are:
> </proposal>

Will change.

> > +
> > +=============   ===================================================
> > +tasks           print all tasks info
> > +mem             print system memory info
> > +timer           print timer info
> > +lock            print locks info if CONFIG_LOCKDEP is on
> > +ftrace          print ftrace buffer
> > +all_bt          print all CPUs backtrace (if available in the arch)
> > +blocked_tasks   print only tasks in uninterruptible (blocked) state
> > +=============   ===================================================
> > +
> > +
> >  panic_on_rcu_stall
> >  ==================
> 
> The rest looks good.
 
Thanks!

- Feng

> Best Regards,
> Petr

Re: [PATCH V2 5/5] panic: add note that panic_print interface is deprecated

[Feng Tang]

On Mon, Jun 23, 2025 at 05:22:20PM +0200, Petr Mladek wrote:
> On Mon 2025-06-16 09:08:40, Feng Tang wrote:
> > Long term wise, the 'panic_sys_info' should be the only controlling
> > interface, which can be referred by other modules.
> 
> Strictly speaking, 'panic_sys_info' is not a complete
> replacement for 'panic_print' because it does not allow
> replaying the log buffer during panic.

Indeed, I forgot this part.

> I suggest to add a separate parameter "panic_console_replay"
> for the missing functionality first. And 'panic_print' will
> be obsoleted by both 'panic_sys_info' and 'panic_console_replay'.
 
Will do.

> > --- a/kernel/panic.c
> > +++ b/kernel/panic.c
> > @@ -76,6 +76,13 @@ ATOMIC_NOTIFIER_HEAD(panic_notifier_list);
> >  EXPORT_SYMBOL(panic_notifier_list);
> >  
> >  #ifdef CONFIG_SYSCTL
> > +static int sysctl_panic_print_handler(const struct ctl_table *table, int write,
> > +			   void *buffer, size_t *lenp, loff_t *ppos)
> > +{
> > +	printk_once("panic: 'panic_print' sysctl interface will be obsoleted by 'panic_sys_info' interface.\n");
> > +	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
> > +}
> > +
> >  static const struct ctl_table kern_panic_table[] = {
> >  #ifdef CONFIG_SMP
> >  	{
> > @@ -107,7 +114,7 @@ static const struct ctl_table kern_panic_table[] = {
> >  		.data		= &panic_print,
> >  		.maxlen		= sizeof(unsigned long),
> >  		.mode		= 0644,
> > -		.proc_handler	= proc_doulongvec_minmax,
> > +		.proc_handler	= sysctl_panic_print_handler,
> 
> This handles only the sysctl interface. We should do something similar
> also for the "panic_print" command line parameter. It would require
> using core_param_cb() instead of core_param().

OK, thanks!

- Feng

> >  	},
> >  	{
> >  		.procname	= "panic_on_warn",
> 
> Best Regards,
> Petr

Re: [PATCH V2 5/5] panic: add note that panic_print interface is deprecated

[Feng Tang]

On Mon, Jun 23, 2025 at 05:22:20PM +0200, Petr Mladek wrote:
> On Mon 2025-06-16 09:08:40, Feng Tang wrote:
> > Long term wise, the 'panic_sys_info' should be the only controlling
> > interface, which can be referred by other modules.
> 
> Strictly speaking, 'panic_sys_info' is not a complete
> replacement for 'panic_print' because it does not allow
> replaying the log buffer during panic.
> 
> I suggest to add a separate parameter "panic_console_replay"
> for the missing functionality first. And 'panic_print' will
> be obsoleted by both 'panic_sys_info' and 'panic_console_replay'.
> 
> > --- a/kernel/panic.c
> > +++ b/kernel/panic.c
> > @@ -76,6 +76,13 @@ ATOMIC_NOTIFIER_HEAD(panic_notifier_list);
> >  EXPORT_SYMBOL(panic_notifier_list);
> >  
> >  #ifdef CONFIG_SYSCTL
> > +static int sysctl_panic_print_handler(const struct ctl_table *table, int write,
> > +			   void *buffer, size_t *lenp, loff_t *ppos)
> > +{
> > +	printk_once("panic: 'panic_print' sysctl interface will be obsoleted by 'panic_sys_info' interface.\n");
> > +	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
> > +}
> > +
> >  static const struct ctl_table kern_panic_table[] = {
> >  #ifdef CONFIG_SMP
> >  	{
> > @@ -107,7 +114,7 @@ static const struct ctl_table kern_panic_table[] = {
> >  		.data		= &panic_print,
> >  		.maxlen		= sizeof(unsigned long),
> >  		.mode		= 0644,
> > -		.proc_handler	= proc_doulongvec_minmax,
> > +		.proc_handler	= sysctl_panic_print_handler,
> 
> This handles only the sysctl interface. We should do something similar
> also for the "panic_print" command line parameter. It would require
> using core_param_cb() instead of core_param().

When testing the change, I found  a problem:  'core_param_cb' is not
the real counterpart of 'core_param', that it is a module parameter
instead of kernel/core parameter, and adds the module.prefix to the
parameter, say, the effective cmdline parameter is changed to
'panic.panic_print=' instead of 'panic_print='.

While below patch of 'kernel_param_cb' works fine, but I'm not sure if
it is worth the change:

---
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index bfb85fd13e1f..71053d078cea 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -194,6 +194,9 @@ struct kparam_array
 #define core_param_cb(name, ops, arg, perm)		\
 	__level_param_cb(name, ops, arg, perm, 1)
 
+#define kernel_param_cb(name, ops, arg, perm) \
+	__module_param_call("", name, ops, arg, perm, -1, 0)
+
 /**
  * postcore_param_cb - general callback for a module/cmdline parameter
  *                     to be evaluated before postcore initcall level


Or should we change the 'core_param_cb' to what 'kernel_param_cb' is
doing.

Thanks,
Feng