2022-11-19 08:15:14

by Xu Qiang

[permalink] [raw]
Subject: [PATCH RESEND] panic: Add register_panic_notifier and unregister_panic_notifier

Add two methods to manipulate panic_notifier_list and export them.
Subsequently, panic_notifier_list is changed to static variable.

Signed-off-by: Xu Qiang <[email protected]>
---
include/linux/panic_notifier.h | 3 +++
kernel/panic.c | 12 ++++++++++++
2 files changed, 15 insertions(+)

diff --git a/include/linux/panic_notifier.h b/include/linux/panic_notifier.h
index 41e32483d7a7..9543d498b90b 100644
--- a/include/linux/panic_notifier.h
+++ b/include/linux/panic_notifier.h
@@ -5,6 +5,9 @@
#include <linux/notifier.h>
#include <linux/types.h>

+int register_panic_notifier(struct notifier_block *nb);
+int unregister_panic_notifier(struct notifier_block *nb);
+
extern struct atomic_notifier_head panic_notifier_list;

extern bool crash_kexec_post_notifiers;
diff --git a/kernel/panic.c b/kernel/panic.c
index 75fe389e8814..8f34dbd389cf 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -200,6 +200,18 @@ static void panic_print_sys_info(bool console_flush)
ftrace_dump(DUMP_ALL);
}

+int register_panic_notifier(struct notifier_block *nb)
+{
+ return atomic_notifier_chain_register(&panic_notifier_list, nb);
+}
+EXPORT_SYMBOL(register_panic_notifier);
+
+int unregister_panic_notifier(struct notifier_block *nb)
+{
+ return atomic_notifier_chain_unregister(&panic_notifier_list, nb);
+}
+EXPORT_SYMBOL(unregister_panic_notifier);
+
/**
* panic - halt the system
* @fmt: The text string to print
--
2.17.1



2022-11-20 16:19:09

by Guilherme G. Piccoli

[permalink] [raw]
Subject: Re: [PATCH RESEND] panic: Add register_panic_notifier and unregister_panic_notifier

On 19/11/2022 05:03, Xu Qiang wrote:
> Add two methods to manipulate panic_notifier_list and export them.
> Subsequently, panic_notifier_list is changed to static variable.
>
> Signed-off-by: Xu Qiang <[email protected]>
> ---
> include/linux/panic_notifier.h | 3 +++
> kernel/panic.c | 12 ++++++++++++
> 2 files changed, 15 insertions(+)
>

Hi Xu Qiang, thanks for your patch!

Did you manage to change all users in the kernel? I only received this
email that introduces the helpers, but I don't see them getting used in
code...

Also, did you follow [0]? I stopped a bit this work since the main
reviewers got busy in other stuff (so did I), but I intend to resume
that and submit a new version. With that said, these helpers you're
adding now will eventually require to be all replaced if my work reach a
consensus and gets merged..so, personally I don't think it's a necessary
addition for now [but don't oppose as well =)]

Cheers,


Guilherme


[0]
https://lore.kernel.org/linux-kernel/[email protected]/

> diff --git a/include/linux/panic_notifier.h b/include/linux/panic_notifier.h
> index 41e32483d7a7..9543d498b90b 100644
> --- a/include/linux/panic_notifier.h
> +++ b/include/linux/panic_notifier.h
> @@ -5,6 +5,9 @@
> #include <linux/notifier.h>
> #include <linux/types.h>
>
> +int register_panic_notifier(struct notifier_block *nb);
> +int unregister_panic_notifier(struct notifier_block *nb);
> +
> extern struct atomic_notifier_head panic_notifier_list;
>
> extern bool crash_kexec_post_notifiers;
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 75fe389e8814..8f34dbd389cf 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -200,6 +200,18 @@ static void panic_print_sys_info(bool console_flush)
> ftrace_dump(DUMP_ALL);
> }
>
> +int register_panic_notifier(struct notifier_block *nb)
> +{
> + return atomic_notifier_chain_register(&panic_notifier_list, nb);
> +}
> +EXPORT_SYMBOL(register_panic_notifier);
> +
> +int unregister_panic_notifier(struct notifier_block *nb)
> +{
> + return atomic_notifier_chain_unregister(&panic_notifier_list, nb);
> +}
> +EXPORT_SYMBOL(unregister_panic_notifier);
> +
> /**
> * panic - halt the system
> * @fmt: The text string to print

2022-11-21 23:00:41

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH RESEND] panic: Add register_panic_notifier and unregister_panic_notifier

On Sat, 19 Nov 2022 08:03:05 +0000 Xu Qiang <[email protected]> wrote:

> Add two methods to manipulate panic_notifier_list and export them.
> Subsequently, panic_notifier_list is changed to static variable.

Fair enough, I guess. But...

- It would be better to include a followup patch which converts at
least some of the existing sites, so we know the code is getting
tested.

- Better names would be panic_notifier_register() and
panic_notifier_unregister()

- You forgot to make panic_notifier_list static to panic.c.