On 30/08/2024 14:54, Stephen Brennan wrote:
> [...]
> Could we maybe go further than this, and delete the public declarations
> of crash_kexec_post_notifiers in "include/linux"? (I see two). We could
> replace the users that set it to true with a function that logs the
> change so that it's impossible for new code to set it directly without
> notifying the user. Something like this? Compile tested only for x86.
>
> commit da8691a25d7b0c2f914720bc054dd1d9dbe4b373
> Author: Stephen Brennan <stephen.s.brennan@xxxxxxxxxx>
> Date: Fri Aug 30 10:49:24 2024 -0700
>
> panic: make crash_kexec_post_notifiers private
>
> This requires that any in-kernel user setting it directly must log the
> reason so that users are aware their panic behavior may be different
> from their configuration.
>
> Signed-off-by: Stephen Brennan <stephen.s.brennan@xxxxxxxxxx>
>
Thanks Stephen! I'm totally into that, your approach is very good.
Cheers,
Guilherme
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index a612e7513a4f8..9966f29409599 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -1818,7 +1818,7 @@ int __init setup_fadump(void)
> * lets panic() function take crash friendly path before panic
> * notifiers are invoked.
> */
> - crash_kexec_post_notifiers = true;
> + enable_crash_kexec_post_notifiers("PPC/fadump");
>
> return 1;
> }
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 0ce17766c0e52..6e9f5f8d13cc5 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -256,7 +256,7 @@ static int __init snp_rmptable_init(void)
> * Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic
> * notifier is invoked to do SNP IOMMU shutdown before kdump.
> */
> - crash_kexec_post_notifiers = true;
> + enable_crash_kexec_post_notifiers("AMD/SEV");
>
> return 0;
>
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 9c452bfbd5719..fa3bbb66235de 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -303,8 +303,7 @@ int __init hv_common_init(void)
> if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
> u64 hyperv_crash_ctl;
>
> - crash_kexec_post_notifiers = true;
> - pr_info("Hyper-V: enabling crash_kexec_post_notifiers\n");
> + enable_crash_kexec_post_notifiers("Hyper-V");
>
> /*
> * Panic message recording (sysctl_record_panic_msg)
> diff --git a/include/linux/panic.h b/include/linux/panic.h
> index 54d90b6c5f47b..697184664c6f4 100644
> --- a/include/linux/panic.h
> +++ b/include/linux/panic.h
> @@ -31,8 +31,6 @@ extern int sysctl_panic_on_rcu_stall;
> extern int sysctl_max_rcu_stall_to_panic;
> extern int sysctl_panic_on_stackoverflow;
>
> -extern bool crash_kexec_post_notifiers;
> -
> extern void __stack_chk_fail(void);
> void abort(void);
>
> diff --git a/include/linux/panic_notifier.h b/include/linux/panic_notifier.h
> index 41e32483d7a7b..97c31cf5c2fdb 100644
> --- a/include/linux/panic_notifier.h
> +++ b/include/linux/panic_notifier.h
> @@ -7,6 +7,6 @@
>
> extern struct atomic_notifier_head panic_notifier_list;
>
> -extern bool crash_kexec_post_notifiers;
> +void enable_crash_kexec_post_notifiers(const char *reason);
>
> #endif /* _LINUX_PANIC_NOTIFIERS_H */
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 63cf89393c6eb..634c6b99717c5 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -33,6 +33,9 @@
> /* Per cpu memory for storing cpu states in case of system crash. */
> note_buf_t __percpu *crash_notes;
>
> +/* Defined in kernel/panic.c and needed here, but not intended to be public. */
> +extern bool crash_kexec_post_notifiers;
> +
> #ifdef CONFIG_CRASH_DUMP
>
> int kimage_crash_copy_vmcoreinfo(struct kimage *image)
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 2a0449144f82e..f4ae3abbea7ed 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -137,6 +137,12 @@ static long no_blink(int state)
> return 0;
> }
>
> +void enable_crash_kexec_post_notifiers(const char *reason)
> +{
> + crash_kexec_post_notifiers = true;
> + pr_info("%s: enabling crash_kexec_post_notifiers\n", reason);
> +}
> +
> /* Returns how long it waited in ms */
> long (*panic_blink)(int state);
> EXPORT_SYMBOL(panic_blink);
>
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
