[RFC 0/1] Add sysctl entry for controlling crash_kexec_post_notifiers

[RFC 0/1] Add sysctl entry for controlling crash_kexec_post_notifiers

[Alejandro Jimenez]

I noticed that in contrast to other kernel core parameters (e.g. kernel.panic,
kernel.panic_on_warn, kernel.panic_print) crash_kexec_post_notifiers is not
available as a sysctl tunable. I am aware that because it is a kernel core
parameter, there is already an entry under:

  /sys/module/kernel/parameters/crash_kexec_post_notifiers

and that allows us to read/modify it at runtime. However, I thought it should
also be available via sysctl, since users that want to read/set this value at
runtime might look there first.

I believe there is an ongoing effort to clean up kernel/sysctl.c, but it wasn't
clear to me if this entry (and perhaps the other panic related entries too)
should be placed on kernel/panic.c. I wanted to verify first that this change
would be welcomed before doing additional refactoring work.

I'd appreciate any comments or suggestions.

Thank you,
Alejandro

Alejandro Jimenez (1):
  kernel/sysctl: Add sysctl entry for crash_kexec_post_notifiers

 Documentation/admin-guide/sysctl/kernel.rst | 8 ++++++++
 include/uapi/linux/sysctl.h                 | 1 +
 kernel/sysctl.c                             | 7 +++++++
 3 files changed, 16 insertions(+)

-- 
2.34.1

[RFC 1/1] kernel/sysctl: Add sysctl entry for crash_kexec_post_notifiers

[Alejandro Jimenez]

Introduce a new sysctl:

  /proc/sys/kernel/crash_kexec_post_notifiers

that allows the crash_kexec_post_notifiers tunable to be listed, read, and
modified via sysctl(8) at runtime.

crash_kexec_post_notifiers can now be set via sysctl:

  # sysctl -w kernel.crash_kexec_post_notifiers=1

or using the sysfs entry:

 #  echo 0 > /sys/module/kernel/parameters/crash_kexec_post_notifiers

which is also available for other core kernel parameters like panic,
panic_print, and panic_on_warn.

Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Reviewed-by: Stephen Brennan <stephen.s.brennan@oracle.com>
---
 Documentation/admin-guide/sysctl/kernel.rst | 8 ++++++++
 include/uapi/linux/sysctl.h                 | 1 +
 kernel/sysctl.c                             | 7 +++++++
 3 files changed, 16 insertions(+)

diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index 1144ea3229a3..8e07121e2a58 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -213,6 +213,14 @@ If `core_pattern`_ does not include "%p" (default does not)
 and ``core_uses_pid`` is set, then .PID will be appended to
 the filename.
 
+crash_kexec_post_notifiers
+============
+
+Allow the callbacks in panic notifier list to be called before kdump and dumping
+kmsg.
+
+0 Do not call panic notifier list callbacks before kdump (default).
+1 Call panic notifier list callbacks before kdump.
 
 ctrl-alt-del
 ============
diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
index 6a3b194c50fe..921e3ea01881 100644
--- a/include/uapi/linux/sysctl.h
+++ b/include/uapi/linux/sysctl.h
@@ -154,6 +154,7 @@ enum
 	KERN_PANIC_ON_NMI=76, /* int: whether we will panic on an unrecovered */
 	KERN_PANIC_ON_WARN=77, /* int: call panic() in WARN() functions */
 	KERN_PANIC_PRINT=78, /* ulong: bitmask to print system info on panic */
+	KERN_CRASH_KEXEC_POST_NOTIFIERS=79, /* bool: call panic notifier list before kdump */
 };
 
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 830aaf8ca08e..8e0be72b5fba 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2339,6 +2339,13 @@ static struct ctl_table kern_table[] = {
 		.extra2		= SYSCTL_INT_MAX,
 	},
 #endif
+	{
+		.procname	= "crash_kexec_post_notifiers",
+		.data		= &crash_kexec_post_notifiers,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dobool,
+	},
 	{ }
 };
 
-- 
2.34.1

Re: [RFC 1/1] kernel/sysctl: Add sysctl entry for crash_kexec_post_notifiers

[Luis Chamberlain]

On Fri, Apr 01, 2022 at 04:23:00PM -0400, Alejandro Jimenez wrote:
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 830aaf8ca08e..8e0be72b5fba 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2339,6 +2339,13 @@ static struct ctl_table kern_table[] = {
>  		.extra2		= SYSCTL_INT_MAX,
>  	},
>  #endif
> +	{
> +		.procname	= "crash_kexec_post_notifiers",
> +		.data		= &crash_kexec_post_notifiers,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dobool,
> +	},
>  	{ }

Please don't add any new sysctls to this file anymore. See
sysctl-testing which trims its uses. Plenty of examples for you
to see what to do, hopefully [0].

[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=sysctl-testing

  Luis

Re: [RFC 0/1] Add sysctl entry for controlling crash_kexec_post_notifiers

[Guilherme G. Piccoli]

On 01/04/2022 17:22, Alejandro Jimenez wrote:
> I noticed that in contrast to other kernel core parameters (e.g. kernel.panic,
> kernel.panic_on_warn, kernel.panic_print) crash_kexec_post_notifiers is not
> available as a sysctl tunable. I am aware that because it is a kernel core
> parameter, there is already an entry under:
> 
>   /sys/module/kernel/parameters/crash_kexec_post_notifiers
> 
> and that allows us to read/modify it at runtime. However, I thought it should
> also be available via sysctl, since users that want to read/set this value at
> runtime might look there first.
> 
> I believe there is an ongoing effort to clean up kernel/sysctl.c, but it wasn't
> clear to me if this entry (and perhaps the other panic related entries too)
> should be placed on kernel/panic.c. I wanted to verify first that this change
> would be welcomed before doing additional refactoring work.
> 
> I'd appreciate any comments or suggestions.
> 
> Thank you,
> Alejandro

Hi Alejandro, thanks for you patch. I have a "selfish" concern though,
I'll expose it here.

I'm working a panic refactor, in order to split the panic notifiers in
more lists - good summary of this discussion at [0].
I'm in the half of the patches, hopefully next 2 weeks I have something
ready to submit (I'll be out next week).

As part of this effort, I plan to have a more fine-grained control of
this parameter, and it's going to be a sysctl, but not
"crash_kexec_post_notifiers" - this one should be kept I guess due to
retro-compatibility, but it'd be a layer on top oh the new one.
With that said, unless you have urgent needs for this patch to be
reviewed/merged , I'd like to ask you to wait the series and I can loop
you there, so you may review/comment and see if it fits your use case.

Thanks,


Guilherme


[0] https://lore.kernel.org/lkml/YfPxvzSzDLjO5ldp@alley/

Re: [RFC 0/1] Add sysctl entry for controlling crash_kexec_post_notifiers

[Alejandro Jimenez]

Hi Guilherme,

On 4/2/2022 10:01 AM, Guilherme G. Piccoli wrote:
> On 01/04/2022 17:22, Alejandro Jimenez wrote:
>> I noticed that in contrast to other kernel core parameters (e.g. kernel.panic,
>> kernel.panic_on_warn, kernel.panic_print) crash_kexec_post_notifiers is not
>> available as a sysctl tunable. I am aware that because it is a kernel core
>> parameter, there is already an entry under:
>>
>>    /sys/module/kernel/parameters/crash_kexec_post_notifiers
>>
>> and that allows us to read/modify it at runtime. However, I thought it should
>> also be available via sysctl, since users that want to read/set this value at
>> runtime might look there first.
>>
>> I believe there is an ongoing effort to clean up kernel/sysctl.c, but it wasn't
>> clear to me if this entry (and perhaps the other panic related entries too)
>> should be placed on kernel/panic.c. I wanted to verify first that this change
>> would be welcomed before doing additional refactoring work.
>>
>> I'd appreciate any comments or suggestions.
>>
>> Thank you,
>> Alejandro
> Hi Alejandro, thanks for you patch. I have a "selfish" concern though,
> I'll expose it here.
>
> I'm working a panic refactor, in order to split the panic notifiers in
> more lists - good summary of this discussion at [0].
> I'm in the half of the patches, hopefully next 2 weeks I have something
> ready to submit (I'll be out next week).
>
> As part of this effort, I plan to have a more fine-grained control of
> this parameter, and it's going to be a sysctl, but not
> "crash_kexec_post_notifiers" - this one should be kept I guess due to
> retro-compatibility, but it'd be a layer on top oh the new one.
It would be great to provide finer control and isolate the riskier 
modifiers.

I am using crash_kexec_post_notifiers to control behavior of pvpanic and 
pstore and there is not an urgent need for my change, so I don't mind 
waiting for the new interface to evolve. Please copy me if possible on 
future submissions.

Thank you,
Alejandro

> With that said, unless you have urgent needs for this patch to be
> reviewed/merged , I'd like to ask you to wait the series and I can loop
> you there, so you may review/comment and see if it fits your use case.
>
> Thanks,
>
>
> Guilherme
>
>
> [0] https://lore.kernel.org/lkml/YfPxvzSzDLjO5ldp@alley/