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

[Petr Mladek <pmladek@suse.com>]

On Tue 2026-10-20 14:01:43, Petr Mladek wrote:
> 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.

Warning: You might skip the first part about my confusion.
	Maybe, I am the only one who is a bit lost this way.

	My motivation is to do not repeat the mess which I saw in
	the printk registration code. People added many features
	there for over two decades. Each change made sense on its own.
	But, for me, it was really hard to understand and distinguish
	what were the wanted and unwanted effects.


I trying to understans why I am confused by the panic notifiers code.
There are several aspects:

1. I was involved in many mail threads where we discussed calling:

	notifiers  before/after  kdump

   but the variable "crash_kexec_post_notifiers" defines calling:

	kdump   before/after  notifiers

   It actually matches the code because __crash_kexec() is called
   twice in vpanic() before and after the notifiers.

   Sigh, it looks like a trivial mental switch. But it is not
   that trivial when we though about splitting the notifiers
   into more lists and discussed where to call them.

   The discussion always was where to call notifiers and not where to
   call kdump.

   And we are also discussing about disabling notifiers. Not about
   disabling kdump.


2. We call the new list "panic_pre_kdump" and this patchset calls
   it in both vpanic() and __crash_kexec() code paths.

   The name "pre_kdump" suggests that it is needed only when kdump
   is called. But we want to call it in panic() even when
   kdump is not called.


3. The new "pre_kdump" notifiers are called before stopping CPUs.
   While the rest is called in the more safe environment
   with stopped CPUs.


4. I am a bit confused by this patch set because it allows
   dumping printk log before kdump. It does not fit with
   my understanding of having "pre_kdump" list only for
   the strictly needed operations.


5. There is the idea of splitting the remainig notifiers into
   _safe and _unsafe lists, see
   https://lore.kernel.org/r/atcjhe4329sYHWvZ@pathway.suse.cz

   It would complicate the design even more. But it might help
   with the endless war whether "crash_kexec_post_notifiers"
   should be "true" or "false" by a platform-specific default.

   I wonder how all this fits together.


Top-level look:
===============

Sigh, one problem is that the notifiers have different purpose:

   1. Notify hypervisor (platform). This looks like a must-to-have.

   2. Platform specific dump of printk messages. This is
      nice-to-have. But note that we also have kmsg_dump API
      for this, see kmsg_dump_desc().

   3. Print extra info. Also this is nice-to-have.

      Some is optional when kdump is created because it might be
      read from there. Some can't be found in crash dump, e.g.
      dumping lowlevel HW state.

      In addition, there is sys_info() info API which allows to
      dump state of various subsystems.

   4. Some notifiers are smp-safe and some just a best effort
      and more safe after disabling other CPUs.

      In addition, I guess that it is better to call crash_kexec()
      without stopping other CPUs because stopping is not reliable
      without NMI. And it would create extra noise in the backtrace.
      And ???


Now, different people have different preferences but mainly:

   + the more safe kdump the better
   + the more safe reboot the better
   + the more info the better


Now, the question is how to satisfy the various requirements
and keep the design sane. We need a compromise but where.

Continue thinking...

Best Regards,
Petr