Re: [PATCH v2 1/6] panic: Add a notifier chain for pre-kdump callbacks

[Petr Mladek <pmladek@suse.com>]

On Mon 2026-10-19 19:05:21, Guilherme G. Piccoli wrote:
> On 28/09/2026 14:55, Zack Rusin wrote:
> > Selected crash callbacks should not require running every panic notifier
> > before kdump. Add a separate atomic chain with an at-most-once dispatch
> > helper, and invoke it before panic reaches the kmsg dumpers.
> > 
> > The helper permits another call from the crash path without repeating
> > callbacks or recursing into them. Document that callbacks may run in NMI
> > context, with other CPUs running and the kexec lock held.
> > 
> > Link: https://lore.kernel.org/r/aquttpMVKX8e6zGB@pathway.suse.cz
> > Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
> > ---
> > New in v2: separate pre-kdump chain, with a set-once dispatch guard.
> > 
> >  include/linux/panic_notifier.h |  3 +++
> >  kernel/panic.c                 | 26 ++++++++++++++++++++++++++
> >  2 files changed, 29 insertions(+)
> > 
> > diff --git a/include/linux/panic_notifier.h b/include/linux/panic_notifier.h
> > index 41e32483d7a7..3ad390764a41 100644
> > --- a/include/linux/panic_notifier.h
> > +++ b/include/linux/panic_notifier.h
> > @@ -6,6 +6,9 @@
> >  #include <linux/types.h>
> >  
> >  extern struct atomic_notifier_head panic_notifier_list;
> > +extern struct atomic_notifier_head panic_pre_kdump_list;
> > +
> > +void panic_notify_pre_kdump(char *msg);
> >  
> >  extern bool crash_kexec_post_notifiers;
> >  
> > diff --git a/kernel/panic.c b/kernel/panic.c
> > index 213725b612aa..d78e90abb192 100644
> > --- a/kernel/panic.c
> > +++ b/kernel/panic.c
> > @@ -82,6 +82,10 @@ ATOMIC_NOTIFIER_HEAD(panic_notifier_list);
> >  
> >  EXPORT_SYMBOL(panic_notifier_list);
> >  
> > +struct atomic_notifier_head panic_pre_kdump_list =
> > +	ATOMIC_NOTIFIER_INIT(panic_pre_kdump_list);
> > +EXPORT_SYMBOL_GPL(panic_pre_kdump_list);
> > +
> >  static void panic_print_deprecated(void)
> >  {
> >  	pr_info_once("Kernel: The 'panic_print' parameter is now deprecated. Please use 'panic_sys_info' and 'panic_console_replay' instead.\n");
> > @@ -567,6 +571,27 @@ static void panic_other_cpus_shutdown(bool crash_kexec)
> >  		crash_smp_send_stop();
> >  }
> >  
> > +/**
> > + * panic_notify_pre_kdump - invoke selected callbacks before kdump
> > + * @msg: Panic message, or NULL for direct crash-kexec entry
> > + *
> > + * Dispatch at most once per boot, including recursive entry. Callbacks run
> > + * with interrupts disabled, possibly in NMI context, before other CPUs have
> > + * necessarily stopped. The kexec lock may be held. They must not sleep,
> > + * allocate, wait for other CPUs, or call into kexec management operations.
> > + * The message is read-only and may only be used during the callback.
> > + */
> > +void panic_notify_pre_kdump(char *msg)
> > +{
> > +	static atomic_t started = ATOMIC_INIT(0);
> > +	unsigned long flags;
> > +
> > +	local_irq_save(flags);
> > +	if (!atomic_xchg(&started, 1))

It is far from clear why this is needed. It would deserve a comment in
the code and explanation in the commit message.

Well, I am not even sure whether this synchronization is enough.
If __crash_exec() and vpanic() are running in parallel on two CPUS
then one path will skip the notifiers but it will not wait until
they are finished.

But I think that the problem is the explicitly called __crash_exec()
is not synchronized against vpanic() in the first place. IMHO,
it is an existing problem which probably should get fixed.

> > +		atomic_notifier_call_chain(&panic_pre_kdump_list, 0, msg);
> > +	local_irq_restore(flags);
> > +}
> > +
> >  /**
> >   * vpanic - halt the system
> >   * @fmt: The text string to print
> > @@ -682,6 +707,7 @@ void vpanic(const char *fmt, va_list args)
> >  
> >  	sys_info(panic_print);
> >  
> > +	panic_notify_pre_kdump(buf);
> >  	kmsg_dump_desc(KMSG_DUMP_PANIC, buf);
> >  
> >  	/*
> 
> Hi Zack, thanks for trying to improve this conundrum called panic
> notifiers heh
> 
> I confess at first sight I got a bit confused, and only after reading
> patch 1 and 2 twice, things made sense to me: you're hooking
> __crash_kexec() directly!
> 
> So, in my head, 2 things that would make sense upfront: merge patches #1
> and #2, and comment a bit about it in the code.

+1

But honestly, the semantic and ordering of all the variables and
functions is very complicated. It is very hard to follow and
understand all the rules.

I am going to think about it. But I'll first comment the 2nd
and 3rd patch.

Best Regards,
Petr