* [PATCH V2 1/5] panic: clean up code for console replay
2025-06-16 1:08 [PATCH V2 0/5] generalize panic_print's dump function to be used by other kernel parts Feng Tang
@ 2025-06-16 1:08 ` Feng Tang
2025-06-20 14:19 ` Petr Mladek
2025-06-16 1:08 ` [PATCH V2 2/5] panic: generalize panic_print's function to show sys info Feng Tang
` (3 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Feng Tang @ 2025-06-16 1:08 UTC (permalink / raw)
To: Andrew Morton, Petr Mladek, Steven Rostedt, Lance Yang,
Jonathan Corbet, linux-kernel
Cc: paulmck, john.ogness, 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)
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V2 1/5] panic: clean up code for console replay
2025-06-16 1:08 ` [PATCH V2 1/5] panic: clean up code for console replay Feng Tang
@ 2025-06-20 14:19 ` Petr Mladek
0 siblings, 0 replies; 21+ messages in thread
From: Petr Mladek @ 2025-06-20 14:19 UTC (permalink / raw)
To: Feng Tang
Cc: Andrew Morton, Steven Rostedt, Lance Yang, Jonathan Corbet,
linux-kernel, paulmck, john.ogness
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V2 2/5] panic: generalize panic_print's function to show sys info
2025-06-16 1:08 [PATCH V2 0/5] generalize panic_print's dump function to be used by other kernel parts Feng Tang
2025-06-16 1:08 ` [PATCH V2 1/5] panic: clean up code for console replay Feng Tang
@ 2025-06-16 1:08 ` Feng Tang
2025-06-20 15:21 ` Petr Mladek
2025-06-16 1:08 ` [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap Feng Tang
` (2 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Feng Tang @ 2025-06-16 1:08 UTC (permalink / raw)
To: Andrew Morton, Petr Mladek, Steven Rostedt, Lance Yang,
Jonathan Corbet, linux-kernel
Cc: paulmck, john.ogness, 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)
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V2 2/5] panic: generalize panic_print's function to show sys info
2025-06-16 1:08 ` [PATCH V2 2/5] panic: generalize panic_print's function to show sys info Feng Tang
@ 2025-06-20 15:21 ` Petr Mladek
2025-06-23 3:07 ` Feng Tang
0 siblings, 1 reply; 21+ messages in thread
From: Petr Mladek @ 2025-06-20 15:21 UTC (permalink / raw)
To: Feng Tang
Cc: Andrew Morton, Steven Rostedt, Lance Yang, Jonathan Corbet,
linux-kernel, paulmck, john.ogness
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 2/5] panic: generalize panic_print's function to show sys info
2025-06-20 15:21 ` Petr Mladek
@ 2025-06-23 3:07 ` Feng Tang
0 siblings, 0 replies; 21+ messages in thread
From: Feng Tang @ 2025-06-23 3:07 UTC (permalink / raw)
To: Petr Mladek
Cc: Andrew Morton, Steven Rostedt, Lance Yang, Jonathan Corbet,
linux-kernel, paulmck, john.ogness
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap
2025-06-16 1:08 [PATCH V2 0/5] generalize panic_print's dump function to be used by other kernel parts Feng Tang
2025-06-16 1:08 ` [PATCH V2 1/5] panic: clean up code for console replay Feng Tang
2025-06-16 1:08 ` [PATCH V2 2/5] panic: generalize panic_print's function to show sys info Feng Tang
@ 2025-06-16 1:08 ` Feng Tang
2025-06-16 4:25 ` kernel test robot
2025-06-23 14:04 ` Petr Mladek
2025-06-16 1:08 ` [PATCH V2 4/5] panic: add 'panic_sys_info=' setup option for sysctl and kernel cmdline Feng Tang
2025-06-16 1:08 ` [PATCH V2 5/5] panic: add note that panic_print interface is deprecated Feng Tang
4 siblings, 2 replies; 21+ messages in thread
From: Feng Tang @ 2025-06-16 1:08 UTC (permalink / raw)
To: Andrew Morton, Petr Mladek, Steven Rostedt, Lance Yang,
Jonathan Corbet, linux-kernel
Cc: paulmck, john.ogness, 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)
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap
2025-06-16 1:08 ` [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap Feng Tang
@ 2025-06-16 4:25 ` kernel test robot
2025-06-16 15:08 ` Feng Tang
2025-06-23 14:04 ` Petr Mladek
1 sibling, 1 reply; 21+ messages in thread
From: kernel test robot @ 2025-06-16 4:25 UTC (permalink / raw)
To: Feng Tang, Andrew Morton, Petr Mladek, Steven Rostedt, Lance Yang,
Jonathan Corbet, linux-kernel
Cc: llvm, oe-kbuild-all, Linux Memory Management List, paulmck,
john.ogness, Feng Tang
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap
2025-06-16 4:25 ` kernel test robot
@ 2025-06-16 15:08 ` Feng Tang
0 siblings, 0 replies; 21+ messages in thread
From: Feng Tang @ 2025-06-16 15:08 UTC (permalink / raw)
To: kernel test robot
Cc: Andrew Morton, Petr Mladek, Steven Rostedt, Lance Yang,
Jonathan Corbet, linux-kernel, llvm, oe-kbuild-all,
Linux Memory Management List, paulmck, john.ogness
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)
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap
2025-06-16 1:08 ` [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap Feng Tang
2025-06-16 4:25 ` kernel test robot
@ 2025-06-23 14:04 ` Petr Mladek
2025-06-24 1:48 ` Feng Tang
1 sibling, 1 reply; 21+ messages in thread
From: Petr Mladek @ 2025-06-23 14:04 UTC (permalink / raw)
To: Feng Tang
Cc: Andrew Morton, Steven Rostedt, Lance Yang, Jonathan Corbet,
linux-kernel, paulmck, john.ogness
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap
2025-06-23 14:04 ` Petr Mladek
@ 2025-06-24 1:48 ` Feng Tang
0 siblings, 0 replies; 21+ messages in thread
From: Feng Tang @ 2025-06-24 1:48 UTC (permalink / raw)
To: Petr Mladek
Cc: Andrew Morton, Steven Rostedt, Lance Yang, Jonathan Corbet,
linux-kernel, paulmck, john.ogness
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V2 4/5] panic: add 'panic_sys_info=' setup option for sysctl and kernel cmdline
2025-06-16 1:08 [PATCH V2 0/5] generalize panic_print's dump function to be used by other kernel parts Feng Tang
` (2 preceding siblings ...)
2025-06-16 1:08 ` [PATCH V2 3/5] sys_info: add help to translate sys_info string to bitmap Feng Tang
@ 2025-06-16 1:08 ` Feng Tang
2025-06-23 15:04 ` Petr Mladek
2025-06-16 1:08 ` [PATCH V2 5/5] panic: add note that panic_print interface is deprecated Feng Tang
4 siblings, 1 reply; 21+ messages in thread
From: Feng Tang @ 2025-06-16 1:08 UTC (permalink / raw)
To: Andrew Morton, Petr Mladek, Steven Rostedt, Lance Yang,
Jonathan Corbet, linux-kernel
Cc: paulmck, john.ogness, 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)
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V2 4/5] panic: add 'panic_sys_info=' setup option for sysctl and kernel cmdline
2025-06-16 1:08 ` [PATCH V2 4/5] panic: add 'panic_sys_info=' setup option for sysctl and kernel cmdline Feng Tang
@ 2025-06-23 15:04 ` Petr Mladek
2025-06-24 1:55 ` Feng Tang
0 siblings, 1 reply; 21+ messages in thread
From: Petr Mladek @ 2025-06-23 15:04 UTC (permalink / raw)
To: Feng Tang
Cc: Andrew Morton, Steven Rostedt, Lance Yang, Jonathan Corbet,
linux-kernel, paulmck, john.ogness
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 4/5] panic: add 'panic_sys_info=' setup option for sysctl and kernel cmdline
2025-06-23 15:04 ` Petr Mladek
@ 2025-06-24 1:55 ` Feng Tang
0 siblings, 0 replies; 21+ messages in thread
From: Feng Tang @ 2025-06-24 1:55 UTC (permalink / raw)
To: Petr Mladek
Cc: Andrew Morton, Steven Rostedt, Lance Yang, Jonathan Corbet,
linux-kernel, paulmck, john.ogness
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V2 5/5] panic: add note that panic_print interface is deprecated
2025-06-16 1:08 [PATCH V2 0/5] generalize panic_print's dump function to be used by other kernel parts Feng Tang
` (3 preceding siblings ...)
2025-06-16 1:08 ` [PATCH V2 4/5] panic: add 'panic_sys_info=' setup option for sysctl and kernel cmdline Feng Tang
@ 2025-06-16 1:08 ` Feng Tang
2025-06-16 1:45 ` Lance Yang
2025-06-23 15:22 ` Petr Mladek
4 siblings, 2 replies; 21+ messages in thread
From: Feng Tang @ 2025-06-16 1:08 UTC (permalink / raw)
To: Andrew Morton, Petr Mladek, Steven Rostedt, Lance Yang,
Jonathan Corbet, linux-kernel
Cc: paulmck, john.ogness, 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)
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH V2 5/5] panic: add note that panic_print interface is deprecated
2025-06-16 1:08 ` [PATCH V2 5/5] panic: add note that panic_print interface is deprecated Feng Tang
@ 2025-06-16 1:45 ` Lance Yang
2025-06-16 2:39 ` Feng Tang
2025-06-23 15:13 ` Petr Mladek
2025-06-23 15:22 ` Petr Mladek
1 sibling, 2 replies; 21+ messages in thread
From: Lance Yang @ 2025-06-16 1:45 UTC (permalink / raw)
To: Feng Tang
Cc: paulmck, john.ogness, Petr Mladek, Andrew Morton, Steven Rostedt,
linux-kernel, Jonathan Corbet
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",
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 5/5] panic: add note that panic_print interface is deprecated
2025-06-16 1:45 ` Lance Yang
@ 2025-06-16 2:39 ` Feng Tang
2025-06-23 15:13 ` Petr Mladek
1 sibling, 0 replies; 21+ messages in thread
From: Feng Tang @ 2025-06-16 2:39 UTC (permalink / raw)
To: Lance Yang
Cc: paulmck, john.ogness, Petr Mladek, Andrew Morton, Steven Rostedt,
linux-kernel, Jonathan Corbet
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 5/5] panic: add note that panic_print interface is deprecated
2025-06-16 1:45 ` Lance Yang
2025-06-16 2:39 ` Feng Tang
@ 2025-06-23 15:13 ` Petr Mladek
1 sibling, 0 replies; 21+ messages in thread
From: Petr Mladek @ 2025-06-23 15:13 UTC (permalink / raw)
To: Lance Yang
Cc: Feng Tang, paulmck, john.ogness, Andrew Morton, Steven Rostedt,
linux-kernel, Jonathan Corbet
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 5/5] panic: add note that panic_print interface is deprecated
2025-06-16 1:08 ` [PATCH V2 5/5] panic: add note that panic_print interface is deprecated Feng Tang
2025-06-16 1:45 ` Lance Yang
@ 2025-06-23 15:22 ` Petr Mladek
2025-06-24 1:58 ` Feng Tang
2025-06-25 9:30 ` Feng Tang
1 sibling, 2 replies; 21+ messages in thread
From: Petr Mladek @ 2025-06-23 15:22 UTC (permalink / raw)
To: Feng Tang
Cc: Andrew Morton, Steven Rostedt, Lance Yang, Jonathan Corbet,
linux-kernel, paulmck, john.ogness
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 5/5] panic: add note that panic_print interface is deprecated
2025-06-23 15:22 ` Petr Mladek
@ 2025-06-24 1:58 ` Feng Tang
2025-06-25 9:30 ` Feng Tang
1 sibling, 0 replies; 21+ messages in thread
From: Feng Tang @ 2025-06-24 1:58 UTC (permalink / raw)
To: Petr Mladek
Cc: Andrew Morton, Steven Rostedt, Lance Yang, Jonathan Corbet,
linux-kernel, paulmck, john.ogness
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 5/5] panic: add note that panic_print interface is deprecated
2025-06-23 15:22 ` Petr Mladek
2025-06-24 1:58 ` Feng Tang
@ 2025-06-25 9:30 ` Feng Tang
1 sibling, 0 replies; 21+ messages in thread
From: Feng Tang @ 2025-06-25 9:30 UTC (permalink / raw)
To: Petr Mladek
Cc: Andrew Morton, Steven Rostedt, Lance Yang, Jonathan Corbet,
linux-kernel, paulmck, john.ogness
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
^ permalink raw reply related [flat|nested] 21+ messages in thread