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

[PATCH v3 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 available for other cases which also need to dump system info
for debugging. 

Also as suggested by Petr Mladek, add 'panic_sys_info=' interface to
take human readable string like "tasks,mem,locks,timers,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 stability issues
and was proven helpful.

Many thanks to Petr Mladek for great suggestions on both the code and
architectures!

- Feng

One to do left is about adding note for obsoleting 'panic_print' cmdline
as discussed in https://lore.kernel.org/lkml/aFvBuOnD0cAEWJfl@U-2FWC9VHC-2323.local/
and will be posted later.

Changelog:

  Since v2:
     * Rename to PANIC_CONSOLE_REPLAY (Petr Mladek) 
     * Don't let kernel.h include sys_info.h (Petr Mladek)
     * Improve documents and coding style (Petr Mladek/Lance Yang)
     * Add 'panic_console_replay' parameter (Petr Mladek)
     * Fix compiling problem (0Day bot)
     * Add reviewed-by tag from Petr for patch 1/5

  Since 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
  panic: add 'panic_sys_info' sysctl to take human readable string
    parameter
  panic: add 'panic_sys_info=' setup option for kernel cmdline
  panic: add note that panic_print sysctl interface is deprecated

 .../admin-guide/kernel-parameters.txt         |  21 ++-
 Documentation/admin-guide/sysctl/kernel.rst   |  20 ++-
 include/linux/sys_info.h                      |  27 ++++
 kernel/panic.c                                |  71 +++++-----
 lib/Makefile                                  |   2 +-
 lib/sys_info.c                                | 122 ++++++++++++++++++
 6 files changed, 221 insertions(+), 42 deletions(-)
 create mode 100644 include/linux/sys_info.h
 create mode 100644 lib/sys_info.c

-- 
2.43.5

[PATCH v3 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 and rename 'PANIC_PRINT_ALL_PRINTK_MSG'
to 'PANIC_CONSOLE_REPLAY', to make the code straightforward. The
related kernel document is also updated.

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

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f1f2c0874da9..abb2ade021ee 100644
--- 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,
diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index dd49a89a62d3..0d08b7a2db2d 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -889,7 +889,7 @@ bit 1  print system memory info
 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
 =====  ============================================
diff --git a/kernel/panic.c b/kernel/panic.c
index b0b9a8bf4560..9b6c5dc28a65 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -74,7 +74,7 @@ EXPORT_SYMBOL_GPL(panic_timeout);
 #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_CONSOLE_REPLAY		0x00000020
 #define PANIC_PRINT_ALL_CPU_BT		0x00000040
 #define PANIC_PRINT_BLOCKED_TASKS	0x00000080
 unsigned long panic_print;
@@ -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_CONSOLE_REPLAY)
+		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.43.5

[PATCH v3 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. Actually this function could also be used to help
debugging other cases like task-hung, soft/hard lockup, etc. where user
may need the snapshot of system info at that time.

Extract system info dump function related code from panic.c to separate
file sys_info.[ch], for wider usage by other kernel parts for debugging.

Also modify the macro names about singulars/plurals.

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

diff --git a/include/linux/sys_info.h b/include/linux/sys_info.h
new file mode 100644
index 000000000000..53b7e27dbf2a
--- /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_INFO_PANIC_CONSOLE_REPLAY is for panic case only, as it needs special
+ * handling which only fits panic case.
+ */
+#define SYS_INFO_TASKS			0x00000001
+#define SYS_INFO_MEM			0x00000002
+#define SYS_INFO_TIMERS			0x00000004
+#define SYS_INFO_LOCKS			0x00000008
+#define SYS_INFO_FTRACE			0x00000010
+#define SYS_INFO_PANIC_CONSOLE_REPLAY	0x00000020
+#define SYS_INFO_ALL_CPU_BT		0x00000040
+#define SYS_INFO_BLOCKED_TASKS		0x00000080
+
+void sys_info(unsigned long si_mask);
+
+#endif	/* _LINUX_SYS_INFO_H */
diff --git a/kernel/panic.c b/kernel/panic.c
index 9b6c5dc28a65..cbb0681177b3 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -36,6 +36,7 @@
 #include <linux/sysfs.h>
 #include <linux/context_tracking.h>
 #include <linux/seq_buf.h>
+#include <linux/sys_info.h>
 #include <trace/events/error_report.h>
 #include <asm/sections.h>
 
@@ -69,14 +70,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_CONSOLE_REPLAY		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 +233,10 @@ EXPORT_SYMBOL(nmi_panic);
 
 static void panic_console_replay(void)
 {
-	if (panic_print & PANIC_CONSOLE_REPLAY)
+	if (panic_print & SYS_INFO_PANIC_CONSOLE_REPLAY)
 		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 +257,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_INFO_ALL_CPU_BT) {
 		/* Temporary allow non-panic CPUs to write their backtraces. */
 		panic_triggering_all_cpu_backtrace = true;
 		trigger_all_cpu_backtrace();
@@ -410,7 +382,7 @@ void panic(const char *fmt, ...)
 	 */
 	atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
 
-	panic_print_sys_info();
+	sys_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..53031e5cb98e
--- /dev/null
+++ b/lib/sys_info.c
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/sched/debug.h>
+#include <linux/console.h>
+#include <linux/kernel.h>
+#include <linux/ftrace.h>
+#include <linux/nmi.h>
+
+#include <linux/sys_info.h>
+
+void sys_info(unsigned long si_mask)
+{
+	if (si_mask & SYS_INFO_TASKS)
+		show_state();
+
+	if (si_mask & SYS_INFO_MEM)
+		show_mem();
+
+	if (si_mask & SYS_INFO_TIMERS)
+		sysrq_timer_list_show();
+
+	if (si_mask & SYS_INFO_LOCKS)
+		debug_show_all_locks();
+
+	if (si_mask & SYS_INFO_FTRACE)
+		ftrace_dump(DUMP_ALL);
+
+	if (si_mask & SYS_INFO_ALL_CPU_BT)
+		trigger_all_cpu_backtrace();
+
+	if (si_mask & SYS_INFO_BLOCKED_TASKS)
+		show_state_filter(TASK_UNINTERRUPTIBLE);
+}
-- 
2.43.5

[PATCH 3/5] panic: add 'panic_sys_info' sysctl to take human readable string parameter

[Feng Tang]

Bitmap definition for 'panic_print' is hard to remember and decode.
Add 'panic_sys_info='sysctl to take human readable string like
"tasks,mem,timers,locks,ftrace,..." and translate it into bitmap.

The detailed mapping is:
	SYS_INFO_TASKS		"tasks"
	SYS_INFO_MEM		"mem"
	SYS_INFO_TIMERS		"timers"
	SYS_INFO_LOCKS		"locks"
	SYS_INFO_FTRACE		"ftrace"
	SYS_INFO_ALL_CPU_BT	"all_bt"
	SYS_INFO_BLOCKED_TASKS	"blocked_tasks"

Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
---
 Documentation/admin-guide/sysctl/kernel.rst | 18 +++++
 include/linux/sys_info.h                    |  8 ++
 kernel/panic.c                              |  7 ++
 lib/sys_info.c                              | 90 +++++++++++++++++++++
 4 files changed, 123 insertions(+)

diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index 0d08b7a2db2d..cccb06d1a6bf 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
+==============
+
+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:
+
+=============   ===================================================
+tasks           print all tasks info
+mem             print system memory info
+timer           print timers 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/include/linux/sys_info.h b/include/linux/sys_info.h
index 53b7e27dbf2a..89d77dc4f2ed 100644
--- a/include/linux/sys_info.h
+++ b/include/linux/sys_info.h
@@ -2,6 +2,8 @@
 #ifndef _LINUX_SYS_INFO_H
 #define _LINUX_SYS_INFO_H
 
+#include <linux/sysctl.h>
+
 /*
  * SYS_INFO_PANIC_CONSOLE_REPLAY is for panic case only, as it needs special
  * handling which only fits panic case.
@@ -16,5 +18,11 @@
 #define SYS_INFO_BLOCKED_TASKS		0x00000080
 
 void sys_info(unsigned long si_mask);
+unsigned long sys_info_parse_param(char *str);
 
+#ifdef CONFIG_SYSCTL
+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/kernel/panic.c b/kernel/panic.c
index cbb0681177b3..d7aa427dc23c 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -126,6 +126,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)
diff --git a/lib/sys_info.c b/lib/sys_info.c
index 53031e5cb98e..46d6f4f1ad2a 100644
--- a/lib/sys_info.c
+++ b/lib/sys_info.c
@@ -3,10 +3,100 @@
 #include <linux/console.h>
 #include <linux/kernel.h>
 #include <linux/ftrace.h>
+#include <linux/sysctl.h>
 #include <linux/nmi.h>
 
 #include <linux/sys_info.h>
 
+struct sys_info_name {
+	unsigned long bit;
+	const char *name;
+};
+
+/*
+ * When 'si_names' gets updated,  please make sure the 'sys_info_avail'
+ * below is updated accordingly.
+ */
+static const struct sys_info_name  si_names[] = {
+	{ SYS_INFO_TASKS,		"tasks" },
+	{ SYS_INFO_MEM,			"mem" },
+	{ SYS_INFO_TIMERS,		"timers" },
+	{ SYS_INFO_LOCKS,		"locks" },
+	{ SYS_INFO_FTRACE,		"ftrace" },
+	{ SYS_INFO_ALL_CPU_BT,		"all_bt" },
+	{ SYS_INFO_BLOCKED_TASKS,	"blocked_tasks" },
+};
+
+/* Expecting string like "xxx_sys_info=tasks,mem,timers,locks,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
+
+static const char sys_info_avail[] = "tasks,mem,timers,locks,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)
+{
+	char names[sizeof(sys_info_avail) + 1];
+	struct ctl_table table;
+	unsigned long *si_bits_global;
+
+	si_bits_global = ro_table->data;
+
+	if (write) {
+		unsigned long si_bits;
+		int ret;
+
+		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 */
+		char *delim = "";
+		int i, len = 0;
+
+		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
+			if (*si_bits_global & si_names[i].bit) {
+				len += scnprintf(names + len, sizeof(names) - len,
+					"%s%s", delim, si_names[i].name);
+				delim = ",";
+			}
+		}
+
+		table = *ro_table;
+		table.data = names;
+		table.maxlen = sizeof(names);
+		return proc_dostring(&table, write, buffer, lenp, ppos);
+	}
+}
+#endif
+
 void sys_info(unsigned long si_mask)
 {
 	if (si_mask & SYS_INFO_TASKS)
-- 
2.43.5

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

[Feng Tang]

'panic_sys_info=' sysctl interface is already added for runtime setting.
Add counterpart kernel cmdline option for boottime setting.

Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 15 +++++++++++++++
 kernel/panic.c                                  |  9 +++++++++
 2 files changed, 24 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index abb2ade021ee..39ddef7c5857 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4541,6 +4541,21 @@
 			Use this option carefully, maybe worth to setup a
 			bigger log buffer with "log_buf_len" along with this.
 
+	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.
+
 	parkbd.port=	[HW] Parallel port number the keyboard adapter is
 			connected to, default is 0.
 			Format: <parport#>
diff --git a/kernel/panic.c b/kernel/panic.c
index d7aa427dc23c..d9d4fcd5e318 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -143,6 +143,15 @@ static __init int kernel_panic_sysctls_init(void)
 late_initcall(kernel_panic_sysctls_init);
 #endif
 
+/* The format is "panic_sys_info=tasks,mem,locks,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.43.5

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

[Feng Tang]

Add a dedicated core parameter 'panic_console_replay' for controlling
console replay, and add note that 'panic_print' sysctl interface  will
be obsoleted by 'panic_sys_info' and 'panic_console_replay'.  When it
happens, the SYS_INFO_PANIC_CONSOLE_REPLAY can be removed as well.

Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
---
 .../admin-guide/kernel-parameters.txt         |  4 ++++
 kernel/panic.c                                | 21 ++++++++++++-------
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 39ddef7c5857..f34de9978a91 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4556,6 +4556,10 @@
 
                         This is a human readable alternative to the 'panic_print' option.
 
+	panic_console_replay
+			When panic happens, replay all kernel messages on
+			consoles at the end of panic.
+
 	parkbd.port=	[HW] Parallel port number the keyboard adapter is
 			connected to, default is 0.
 			Format: <parport#>
diff --git a/kernel/panic.c b/kernel/panic.c
index d9d4fcd5e318..bb16f254cd02 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -64,6 +64,7 @@ int panic_on_warn __read_mostly;
 unsigned long panic_on_taint;
 bool panic_on_taint_nousertaint = false;
 static unsigned int warn_limit __read_mostly;
+static bool panic_console_replay;
 
 bool panic_triggering_all_cpu_backtrace;
 
@@ -77,6 +78,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)
+{
+	pr_info_once("Kernel: 'panic_print' sysctl interface will be obsoleted by both 'panic_sys_info' and 'panic_console_replay'\n");
+	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
+}
+
 static const struct ctl_table kern_panic_table[] = {
 #ifdef CONFIG_SMP
 	{
@@ -108,7 +116,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",
@@ -247,12 +255,6 @@ void nmi_panic(struct pt_regs *regs, const char *msg)
 }
 EXPORT_SYMBOL(nmi_panic);
 
-static void panic_console_replay(void)
-{
-	if (panic_print & SYS_INFO_PANIC_CONSOLE_REPLAY)
-		console_flush_on_panic(CONSOLE_REPLAY_ALL);
-}
-
 void check_panic_on_warn(const char *origin)
 {
 	unsigned int limit;
@@ -427,7 +429,9 @@ void panic(const char *fmt, ...)
 	debug_locks_off();
 	console_flush_on_panic(CONSOLE_FLUSH_PENDING);
 
-	panic_console_replay();
+	if ((panic_print & SYS_INFO_PANIC_CONSOLE_REPLAY) ||
+		panic_console_replay)
+		console_flush_on_panic(CONSOLE_REPLAY_ALL);
 
 	if (!panic_blink)
 		panic_blink = no_blink;
@@ -869,6 +873,7 @@ core_param(panic_print, panic_print, ulong, 0644);
 core_param(pause_on_oops, pause_on_oops, int, 0644);
 core_param(panic_on_warn, panic_on_warn, int, 0644);
 core_param(crash_kexec_post_notifiers, crash_kexec_post_notifiers, bool, 0644);
+core_param(panic_console_replay, panic_console_replay, bool, 0644);
 
 static int __init oops_setup(char *s)
 {
-- 
2.43.5

Re: [PATCH 3/5] panic: add 'panic_sys_info' sysctl to take human readable string parameter

[Lance Yang]

[PATCH 3/5] panic: add 'panic_sys_info' sysctl to take human readable 
string parameter

Just a minor nitpick on the subject line: it seems to be missing the
"v3" tag, unlike the other patches in this series.

Thanks,
Lance

Re: [PATCH 3/5] panic: add 'panic_sys_info' sysctl to take human readable string parameter

[Feng Tang]

On Thu, Jul 03, 2025 at 10:56:36AM +0800, Lance Yang wrote:
> [PATCH 3/5] panic: add 'panic_sys_info' sysctl to take human readable string
> parameter
> 
> Just a minor nitpick on the subject line: it seems to be missing the
> "v3" tag, unlike the other patches in this series.

Thanks for the note!

Yes, I did some last minutes change and forgot adding the 'v3' :)


> Thanks,
> Lance

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

[Lance Yang]

Just hit a build failure with this patch series when building for arm64
with a minimal configuration:

kernel/panic.c: In function ‘setup_panic_sys_info’:
kernel/panic.c:151:23: error: implicit declaration of function 
‘sys_info_parse_param’ [-Wimplicit-function-declaration]
151 |         panic_print = sys_info_parse_param(buf);
|                       ^~~~~~~~~~~~~~~~~~~~
make[3]: *** [scripts/Makefile.build:287: kernel/panic.o] Error 1
make[2]: *** [scripts/Makefile.build:554: kernel] Error 2


To reproduce it:
$ make ARCH=arm64 allnoconfig
$ make ARCH=arm64 -j$(nproc)

Thanks,
Lance


On 2025/7/3 10:09, 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 to a new file 'lib/sys_info.c',
> and make it available for other cases which also need to dump system info
> for debugging.
> 
> Also as suggested by Petr Mladek, add 'panic_sys_info=' interface to
> take human readable string like "tasks,mem,locks,timers,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 stability issues
> and was proven helpful.
> 
> Many thanks to Petr Mladek for great suggestions on both the code and
> architectures!
> 
> - Feng
> 
> One to do left is about adding note for obsoleting 'panic_print' cmdline
> as discussed in https://lore.kernel.org/lkml/aFvBuOnD0cAEWJfl@U-2FWC9VHC-2323.local/
> and will be posted later.
> 
> Changelog:
> 
>    Since v2:
>       * Rename to PANIC_CONSOLE_REPLAY (Petr Mladek)
>       * Don't let kernel.h include sys_info.h (Petr Mladek)
>       * Improve documents and coding style (Petr Mladek/Lance Yang)
>       * Add 'panic_console_replay' parameter (Petr Mladek)
>       * Fix compiling problem (0Day bot)
>       * Add reviewed-by tag from Petr for patch 1/5
> 
>    Since 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
>    panic: add 'panic_sys_info' sysctl to take human readable string
>      parameter
>    panic: add 'panic_sys_info=' setup option for kernel cmdline
>    panic: add note that panic_print sysctl interface is deprecated
> 
>   .../admin-guide/kernel-parameters.txt         |  21 ++-
>   Documentation/admin-guide/sysctl/kernel.rst   |  20 ++-
>   include/linux/sys_info.h                      |  27 ++++
>   kernel/panic.c                                |  71 +++++-----
>   lib/Makefile                                  |   2 +-
>   lib/sys_info.c                                | 122 ++++++++++++++++++
>   6 files changed, 221 insertions(+), 42 deletions(-)
>   create mode 100644 include/linux/sys_info.h
>   create mode 100644 lib/sys_info.c
> 

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

[Lance Yang]

On 2025/7/3 11:23, Lance Yang wrote:
> Just hit a build failure with this patch series when building for arm64
> with a minimal configuration:
> 
> kernel/panic.c: In function ‘setup_panic_sys_info’:
> kernel/panic.c:151:23: error: implicit declaration of function 
> ‘sys_info_parse_param’ [-Wimplicit-function-declaration]
> 151 |         panic_print = sys_info_parse_param(buf);
> |                       ^~~~~~~~~~~~~~~~~~~~
> make[3]: *** [scripts/Makefile.build:287: kernel/panic.o] Error 1
> make[2]: *** [scripts/Makefile.build:554: kernel] Error 2
> 
> 
> To reproduce it:
> $ make ARCH=arm64 allnoconfig
> $ make ARCH=arm64 -j$(nproc)

Realized that now: the root cause of the build failure I saw is the
missing "v3" tag in the subject of the patch #03 - sorry!

b4 reported that it couldn't find patch #03 when I tried to apply
this patch series, which is why I was getting the "implicit function
declaration" error ... Obviously, I missed that error before ;(

```
---
   ✓ [PATCH v3 1/5] panic: clean up code for console replay
     ✓ Signed: DKIM/linux.alibaba.com
   ✓ [PATCH v3 2/5] panic: generalize panic_print's function to show sys 
info
     ✓ Signed: DKIM/linux.alibaba.com
   ERROR: missing [3/5]!
   ✓ [PATCH v3 4/5] panic: add 'panic_sys_info=' setup option for kernel 
cmdline
     ✓ Signed: DKIM/linux.alibaba.com
   ✓ [PATCH v3 5/5] panic: add note that panic_print sysctl interface is 
deprecated
     ✓ Signed: DKIM/linux.alibaba.com
---
Total patches: 4
---
WARNING: Thread incomplete!
Applying: panic: clean up code for console replay
Applying: panic: generalize panic_print's function to show sys info
Applying: panic: add 'panic_sys_info=' setup option for kernel cmdline
Applying: panic: add note that panic_print sysctl interface is deprecated
```

Thanks,
Lance

> 
> Thanks,
> Lance
> 
> 
> On 2025/7/3 10:09, 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 to a new file 'lib/sys_info.c',
>> and make it available for other cases which also need to dump system info
>> for debugging.
>>
>> Also as suggested by Petr Mladek, add 'panic_sys_info=' interface to
>> take human readable string like "tasks,mem,locks,timers,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 stability issues
>> and was proven helpful.
>>
>> Many thanks to Petr Mladek for great suggestions on both the code and
>> architectures!
>>
>> - Feng
>>
>> One to do left is about adding note for obsoleting 'panic_print' cmdline
>> as discussed in https://lore.kernel.org/lkml/ 
>> aFvBuOnD0cAEWJfl@U-2FWC9VHC-2323.local/
>> and will be posted later.
>>
>> Changelog:
>>
>>    Since v2:
>>       * Rename to PANIC_CONSOLE_REPLAY (Petr Mladek)
>>       * Don't let kernel.h include sys_info.h (Petr Mladek)
>>       * Improve documents and coding style (Petr Mladek/Lance Yang)
>>       * Add 'panic_console_replay' parameter (Petr Mladek)
>>       * Fix compiling problem (0Day bot)
>>       * Add reviewed-by tag from Petr for patch 1/5
>>
>>    Since 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
>>    panic: add 'panic_sys_info' sysctl to take human readable string
>>      parameter
>>    panic: add 'panic_sys_info=' setup option for kernel cmdline
>>    panic: add note that panic_print sysctl interface is deprecated
>>
>>   .../admin-guide/kernel-parameters.txt         |  21 ++-
>>   Documentation/admin-guide/sysctl/kernel.rst   |  20 ++-
>>   include/linux/sys_info.h                      |  27 ++++
>>   kernel/panic.c                                |  71 +++++-----
>>   lib/Makefile                                  |   2 +-
>>   lib/sys_info.c                                | 122 ++++++++++++++++++
>>   6 files changed, 221 insertions(+), 42 deletions(-)
>>   create mode 100644 include/linux/sys_info.h
>>   create mode 100644 lib/sys_info.c
>>
> 

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

[Feng Tang]

On Thu, Jul 03, 2025 at 12:56:54PM +0800, Lance Yang wrote:
> 
> 
> On 2025/7/3 11:23, Lance Yang wrote:
> > Just hit a build failure with this patch series when building for arm64
> > with a minimal configuration:
> > 
> > kernel/panic.c: In function ‘setup_panic_sys_info’:
> > kernel/panic.c:151:23: error: implicit declaration of function
> > ‘sys_info_parse_param’ [-Wimplicit-function-declaration]
> > 151 |         panic_print = sys_info_parse_param(buf);
> > |                       ^~~~~~~~~~~~~~~~~~~~
> > make[3]: *** [scripts/Makefile.build:287: kernel/panic.o] Error 1
> > make[2]: *** [scripts/Makefile.build:554: kernel] Error 2
> > 
> > 
> > To reproduce it:
> > $ make ARCH=arm64 allnoconfig
> > $ make ARCH=arm64 -j$(nproc)
> 
> Realized that now: the root cause of the build failure I saw is the
> missing "v3" tag in the subject of the patch #03 - sorry!
> 
> b4 reported that it couldn't find patch #03 when I tried to apply
> this patch series, which is why I was getting the "implicit function
> declaration" error ... Obviously, I missed that error before ;(
 
I see, as I just rerun 'allyes' and 'allno' build and they passed. I didn't
know that the patch version tag will cause the patch applying issue.

Thanks for the review and compiling test!

- Feng

> ```
> ---
>   ✓ [PATCH v3 1/5] panic: clean up code for console replay
>     ✓ Signed: DKIM/linux.alibaba.com
>   ✓ [PATCH v3 2/5] panic: generalize panic_print's function to show sys info
>     ✓ Signed: DKIM/linux.alibaba.com
>   ERROR: missing [3/5]!
>   ✓ [PATCH v3 4/5] panic: add 'panic_sys_info=' setup option for kernel
> cmdline
>     ✓ Signed: DKIM/linux.alibaba.com
>   ✓ [PATCH v3 5/5] panic: add note that panic_print sysctl interface is
> deprecated
>     ✓ Signed: DKIM/linux.alibaba.com
> ---
> Total patches: 4
> ---
> WARNING: Thread incomplete!
> Applying: panic: clean up code for console replay
> Applying: panic: generalize panic_print's function to show sys info
> Applying: panic: add 'panic_sys_info=' setup option for kernel cmdline
> Applying: panic: add note that panic_print sysctl interface is deprecated
> ```
> 
> Thanks,
> Lance
> 

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

[Askar Safin]

I just tested bit 5. It doesn't replay all console messages (i. e. everything printed to /dev/console ).
Instead it merely replays kernel messages (i. e. printk/kmsg).
So, please, rename PANIC_CONSOLE_REPLAY back to PANIC_PRINT_ALL_PRINTK_MSG or possibly to PANIC_KMSG_REPLAY.
And update admin-guide/sysctl/kernel.rst

--
Askar Safin

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

[Feng Tang]

Hi Askar Safin,

On Tue, Jul 15, 2025 at 12:09:40AM +0300, Askar Safin wrote:
> I just tested bit 5. It doesn't replay all console messages (i. e. everything printed to /dev/console ).
> Instead it merely replays kernel messages (i. e. printk/kmsg).
> So, please, rename PANIC_CONSOLE_REPLAY back to PANIC_PRINT_ALL_PRINTK_MSG or possibly to PANIC_KMSG_REPLAY.
> And update admin-guide/sysctl/kernel.rst

Thanks for trying the patch! 

Petr could have better understanding on this, as he have been working
on this for many years and maintained printk.

I brought out the name for kernel space debugging, to replay all the
printk message on the tty console I have. 

My understanding is, 'console' have kind of different meaning in kernel
space than just /dev/console. In printk.c, you can see 'console' is
used everywhere,  which are mostly bound to kernel message.like
console_try_replay_all(), console_flush_all(), console_flush_on_panic(),
which are consistent with the new name suggested by Petr.

Thanks,
Feng

> 
> --
> Askar Safin

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

[Askar Safin]

 ---- On Tue, 15 Jul 2025 04:49:12 +0400  Feng Tang <feng.tang@linux.alibaba.com> wrote --- 
 > Thanks for trying the patch! 
I didn't try it. :) I merely run normal mainline or distro kernel in qemu.

 > My understanding is, 'console' have kind of different meaning in kernel
 > space than just /dev/console. In printk.c, you can see 'console' is
 > used everywhere,  which are mostly bound to kernel message.like
 > console_try_replay_all(), console_flush_all(), console_flush_on_panic(),
 > which are consistent with the new name suggested by Petr.
Okay, I agree.

But I still kindly ask you to revert changes to Documentation/admin-guide/kernel-parameters.txt .
Previous documentation is better.
admin-guide is written for admins, not for kernel developers. And they understand "console" as /dev/console .

I run kernel with panic_print=32 in hope that this will flush console (i. e. /dev/console), because this is how I interpreted
your patched documentation. And I got different effect.

--
Askar Safin
https://types.pl/@safinaskar

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

[Feng Tang]

On Tue, Jul 15, 2025 at 05:18:10AM +0400, Askar Safin wrote:
>  ---- On Tue, 15 Jul 2025 04:49:12 +0400  Feng Tang <feng.tang@linux.alibaba.com> wrote --- 
>  > Thanks for trying the patch! 
> I didn't try it. :) I merely run normal mainline or distro kernel in qemu.
> 
>  > My understanding is, 'console' have kind of different meaning in kernel
>  > space than just /dev/console. In printk.c, you can see 'console' is
>  > used everywhere,  which are mostly bound to kernel message.like
>  > console_try_replay_all(), console_flush_all(), console_flush_on_panic(),
>  > which are consistent with the new name suggested by Petr.
> Okay, I agree.
> 
> But I still kindly ask you to revert changes to Documentation/admin-guide/kernel-parameters.txt .
> Previous documentation is better.
> admin-guide is written for admins, not for kernel developers. And they understand "console" as /dev/console .
> 
> I run kernel with panic_print=32 in hope that this will flush console (i. e. /dev/console), because this is how I interpreted
> your patched documentation. And I got different effect.

I see. How about changing the patch to: 

-			bit 5: print all printk messages in buffer
+			bit 5: replay all kernel messages on consoles at the end of panic

Thanks,
Feng

> 
> --
> Askar Safin
> https://types.pl/@safinaskar

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

[Askar Safin]

 ---- On Tue, 15 Jul 2025 05:34:39 +0400  Feng Tang <feng.tang@linux.alibaba.com> wrote --- 
 > I see. How about changing the patch to: 
 > 
 > -            bit 5: print all printk messages in buffer
 > +            bit 5: replay all kernel messages on consoles at the end of panic

Yes, I agree!
--
Askar Safin
https://types.pl/@safinaskar

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

[Feng Tang]

On Tue, Jul 15, 2025 at 06:48:47AM +0400, Askar Safin wrote:
> 
>  ---- On Tue, 15 Jul 2025 05:34:39 +0400  Feng Tang <feng.tang@linux.alibaba.com> wrote --- 
>  > I see. How about changing the patch to: 
>  > 
>  > -            bit 5: print all printk messages in buffer
>  > +            bit 5: replay all kernel messages on consoles at the end of panic
> 
> Yes, I agree!

Hi Andrew,

Could you help to squash below patch to 1/5 patch "panic: clean up code
for console replay" in the nonmmu-unstable branch? Thanks!

- Feng

---
 Documentation/admin-guide/kernel-parameters.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f34de9978a91..a84d3f7f5bbf 100644
--- 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: replay all messages on consoles at the end of panic
+			bit 5: replay all kernel 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,

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

[Petr Mladek]

On Thu 2025-07-03 10:10:01, 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. Actually this function could also be used to help
> debugging other cases like task-hung, soft/hard lockup, etc. where user
> may need the snapshot of system info at that time.
> 
> Extract system info dump function related code from panic.c to separate
> file sys_info.[ch], for wider usage by other kernel parts for debugging.
> 
> Also modify the macro names about singulars/plurals.
> 
> 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

PS: I know that it has already been merged into the mainline
    but I wanted to double check it.

Re: [PATCH 3/5] panic: add 'panic_sys_info' sysctl to take human readable string parameter

[Petr Mladek]

On Thu 2025-07-03 10:10:02, Feng Tang wrote:
> Bitmap definition for 'panic_print' is hard to remember and decode.
> Add 'panic_sys_info='sysctl to take human readable string like
> "tasks,mem,timers,locks,ftrace,..." and translate it into bitmap.
> 
> The detailed mapping is:
> 	SYS_INFO_TASKS		"tasks"
> 	SYS_INFO_MEM		"mem"
> 	SYS_INFO_TIMERS		"timers"
> 	SYS_INFO_LOCKS		"locks"
> 	SYS_INFO_FTRACE		"ftrace"
> 	SYS_INFO_ALL_CPU_BT	"all_bt"
> 	SYS_INFO_BLOCKED_TASKS	"blocked_tasks"
> 
> --- a/lib/sys_info.c
> +++ b/lib/sys_info.c
> +static const char sys_info_avail[] = "tasks,mem,timers,locks,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)
> +{
> +	char names[sizeof(sys_info_avail) + 1];

The "+ 1" looks superfluous.

I guess that it is for the trailing '\0'. But sys_info_avail[] already
includes the trailing '\0' so it should be already counted by the sizeof().

Note that it would be needed with strlen(). But it should not be
needed with sizeof().

> +	struct ctl_table table;
> +	unsigned long *si_bits_global;
> +
> +	si_bits_global = ro_table->data;
> +
> +	if (write) {
> +		unsigned long si_bits;
> +		int ret;
> +
> +		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 */
> +		char *delim = "";
> +		int i, len = 0;
> +

It looks to me that names[] can later be used non-initialized when
*si_bits_global == 0. We should initialized it here, something like:

		names[0] = '\0';

> +		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
> +			if (*si_bits_global & si_names[i].bit) {
> +				len += scnprintf(names + len, sizeof(names) - len,
> +					"%s%s", delim, si_names[i].name);
> +				delim = ",";
> +			}
> +		}
> +
> +		table = *ro_table;
> +		table.data = names;
> +		table.maxlen = sizeof(names);
> +		return proc_dostring(&table, write, buffer, lenp, ppos);
> +	}
> +}
> +#endif

Otherwise, it looks good.

Best Regards,
Petr

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

[Petr Mladek]

On Thu 2025-07-03 10:10:03, Feng Tang wrote:
> 'panic_sys_info=' sysctl interface is already added for runtime setting.
> Add counterpart kernel cmdline option for boottime setting.
> 
> Suggested-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>

Just for record:

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

Best Regards,
Petr

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

[Petr Mladek]

On Thu 2025-07-03 10:10:04, Feng Tang wrote:
> Add a dedicated core parameter 'panic_console_replay' for controlling
> console replay, and add note that 'panic_print' sysctl interface  will
> be obsoleted by 'panic_sys_info' and 'panic_console_replay'.  When it
> happens, the SYS_INFO_PANIC_CONSOLE_REPLAY can be removed as well.
> 
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -77,6 +78,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)
> +{
> +	pr_info_once("Kernel: 'panic_print' sysctl interface will be obsoleted by both 'panic_sys_info' and 'panic_console_replay'\n");
> +	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
> +}

This warning is printed "only" when the value is accessed via the
procfs. It would be great to print it also when it is set
via the command line parameter.

It would require replacing

core_param(panic_print, panic_print, ulong, 0644);

with

core_param_cb(panic_print, &panic_print_ops, &panic_print, 0644);

Best Regards,
Petr

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

[Petr Mladek]

On Tue 2025-07-15 11:27:54, Feng Tang wrote:
> On Tue, Jul 15, 2025 at 06:48:47AM +0400, Askar Safin wrote:
> > 
> >  ---- On Tue, 15 Jul 2025 05:34:39 +0400  Feng Tang <feng.tang@linux.alibaba.com> wrote --- 
> >  > I see. How about changing the patch to: 
> >  > 
> >  > -            bit 5: print all printk messages in buffer
> >  > +            bit 5: replay all kernel messages on consoles at the end of panic
> > 
> > Yes, I agree!
> 
> Hi Andrew,
> 
> Could you help to squash below patch to 1/5 patch "panic: clean up code
> for console replay" in the nonmmu-unstable branch? Thanks!
> 
> - Feng
> 
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index f34de9978a91..a84d3f7f5bbf 100644
> --- 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: replay all messages on consoles at the end of panic
> +			bit 5: replay all kernel 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,

Yes, this looks better.

It sees that this change is missing in the mainline.
Fang, could you please send it as a followup fix, please?

Best Regards,
Petr

Re: [PATCH 3/5] panic: add 'panic_sys_info' sysctl to take human readable string parameter

[Feng Tang]

On Tue, Aug 12, 2025 at 12:23:07PM +0200, Petr Mladek wrote:
> On Thu 2025-07-03 10:10:02, Feng Tang wrote:
> > Bitmap definition for 'panic_print' is hard to remember and decode.
> > Add 'panic_sys_info='sysctl to take human readable string like
> > "tasks,mem,timers,locks,ftrace,..." and translate it into bitmap.
> > 
> > The detailed mapping is:
> > 	SYS_INFO_TASKS		"tasks"
> > 	SYS_INFO_MEM		"mem"
> > 	SYS_INFO_TIMERS		"timers"
> > 	SYS_INFO_LOCKS		"locks"
> > 	SYS_INFO_FTRACE		"ftrace"
> > 	SYS_INFO_ALL_CPU_BT	"all_bt"
> > 	SYS_INFO_BLOCKED_TASKS	"blocked_tasks"
> > 
> > --- a/lib/sys_info.c
> > +++ b/lib/sys_info.c
> > +static const char sys_info_avail[] = "tasks,mem,timers,locks,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)
> > +{
> > +	char names[sizeof(sys_info_avail) + 1];
> 
> The "+ 1" looks superfluous.
> 
> I guess that it is for the trailing '\0'. But sys_info_avail[] already
> includes the trailing '\0' so it should be already counted by the sizeof().
> 
> Note that it would be needed with strlen(). But it should not be
> needed with sizeof().

Yes, you are right. Will remove it.

> > +	struct ctl_table table;
> > +	unsigned long *si_bits_global;
> > +
> > +	si_bits_global = ro_table->data;
> > +
> > +	if (write) {
> > +		unsigned long si_bits;
> > +		int ret;
> > +
> > +		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 */
> > +		char *delim = "";
> > +		int i, len = 0;
> > +
> 
> It looks to me that names[] can later be used non-initialized when
> *si_bits_global == 0. We should initialized it here, something like:
> 
> 		names[0] = '\0';

Good catch! Will add this.

> > +		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
> > +			if (*si_bits_global & si_names[i].bit) {
> > +				len += scnprintf(names + len, sizeof(names) - len,
> > +					"%s%s", delim, si_names[i].name);
> > +				delim = ",";
> > +			}
> > +		}
> > +
> > +		table = *ro_table;
> > +		table.data = names;
> > +		table.maxlen = sizeof(names);
> > +		return proc_dostring(&table, write, buffer, lenp, ppos);
> > +	}
> > +}
> > +#endif
> 
> Otherwise, it looks good.
 
Thanks for reviewing the patchset!

- Feng

> Best Regards,
> Petr

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

[Feng Tang]

On Tue, Aug 12, 2025 at 01:59:32PM +0200, Petr Mladek wrote:
> On Tue 2025-07-15 11:27:54, Feng Tang wrote:
> > On Tue, Jul 15, 2025 at 06:48:47AM +0400, Askar Safin wrote:
> > > 
> > >  ---- On Tue, 15 Jul 2025 05:34:39 +0400  Feng Tang <feng.tang@linux.alibaba.com> wrote --- 
> > >  > I see. How about changing the patch to: 
> > >  > 
> > >  > -            bit 5: print all printk messages in buffer
> > >  > +            bit 5: replay all kernel messages on consoles at the end of panic
> > > 
> > > Yes, I agree!
> > 
> > Hi Andrew,
> > 
> > Could you help to squash below patch to 1/5 patch "panic: clean up code
> > for console replay" in the nonmmu-unstable branch? Thanks!
> > 
> > - Feng
> > 
> > ---
> >  Documentation/admin-guide/kernel-parameters.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index f34de9978a91..a84d3f7f5bbf 100644
> > --- 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: replay all messages on consoles at the end of panic
> > +			bit 5: replay all kernel 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,
> 
> Yes, this looks better.
> 
> It sees that this change is missing in the mainline.
> Fang, could you please send it as a followup fix, please?

Sure. Will do.

Thanks,
Feng

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

[Feng Tang]

On Tue, Aug 12, 2025 at 01:52:58PM +0200, Petr Mladek wrote:
> On Thu 2025-07-03 10:10:04, Feng Tang wrote:
> > Add a dedicated core parameter 'panic_console_replay' for controlling
> > console replay, and add note that 'panic_print' sysctl interface  will
> > be obsoleted by 'panic_sys_info' and 'panic_console_replay'.  When it
> > happens, the SYS_INFO_PANIC_CONSOLE_REPLAY can be removed as well.
> > 
> > --- a/kernel/panic.c
> > +++ b/kernel/panic.c
> > @@ -77,6 +78,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)
> > +{
> > +	pr_info_once("Kernel: 'panic_print' sysctl interface will be obsoleted by both 'panic_sys_info' and 'panic_console_replay'\n");
> > +	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
> > +}
> 
> This warning is printed "only" when the value is accessed via the
> procfs. It would be great to print it also when it is set
> via the command line parameter.

Yes, this is indeed a remaining issue to be solved, as mentioned in
the cover letter.

> 
> It would require replacing
> 
> core_param(panic_print, panic_print, ulong, 0644);
> 
> with
> 
> core_param_cb(panic_print, &panic_print_ops, &panic_print, 0644);

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 adding a new 'kernel_param_cb' can work without
the "panic" prefix, 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

Thanks,
Feng

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

[Petr Mladek]

On Wed 2025-08-13 09:05:30, Feng Tang wrote:
> On Tue, Aug 12, 2025 at 01:52:58PM +0200, Petr Mladek wrote:
> > On Thu 2025-07-03 10:10:04, Feng Tang wrote:
> > > Add a dedicated core parameter 'panic_console_replay' for controlling
> > > console replay, and add note that 'panic_print' sysctl interface  will
> > > be obsoleted by 'panic_sys_info' and 'panic_console_replay'.  When it
> > > happens, the SYS_INFO_PANIC_CONSOLE_REPLAY can be removed as well.
> > > 
> > > --- a/kernel/panic.c
> > > +++ b/kernel/panic.c
> > > @@ -77,6 +78,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)
> > > +{
> > > +	pr_info_once("Kernel: 'panic_print' sysctl interface will be obsoleted by both 'panic_sys_info' and 'panic_console_replay'\n");
> > > +	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
> > > +}
> > 
> > This warning is printed "only" when the value is accessed via the
> > procfs. It would be great to print it also when it is set
> > via the command line parameter.
> 
> Yes, this is indeed a remaining issue to be solved, as mentioned in
> the cover letter.

I see now.

> > It would require replacing
> > 
> > core_param(panic_print, panic_print, ulong, 0644);
> > 
> > with
> > 
> > core_param_cb(panic_print, &panic_print_ops, &panic_print, 0644);
> 
> 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='.

I see. It is pity that it is messed like this.

> While below patch of adding a new 'kernel_param_cb' can work without
> the "panic" prefix, but I'm not sure if it is worth the change:

I think that it is worth adding. IMHO, the parameter will primary be used
from the command line. So, this is an important path how to make people
aware of the obsoleting.

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

I would call it __core_param_cb(). And I move the definition
down to the section where core_param() and core_param_unsafe()
are defined. Also it would deserve a comment explaining
why the "__" prefix is used.

>  /**
>   * postcore_param_cb - general callback for a module/cmdline parameter
>   *                     to be evaluated before postcore initcall level

Best Regards,
Petr