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

Feng Tang posted 5 patches 3 months ago
[PATCH v3 5/5] panic: add note that panic_print sysctl interface is deprecated
Posted by Feng Tang 3 months ago
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 v3 5/5] panic: add note that panic_print sysctl interface is deprecated
Posted by Petr Mladek 1 month, 3 weeks ago
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 5/5] panic: add note that panic_print sysctl interface is deprecated
Posted by Feng Tang 1 month, 3 weeks ago
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
Posted by Petr Mladek 1 month, 3 weeks ago
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