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

[Petr Mladek <pmladek@suse.com>]

On Tue 2026-10-20 16:30:53, Petr Mladek wrote:
> 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.
> > > > 
> > > > --- a/kernel/panic.c
> > > > +++ b/kernel/panic.c
> > > > @@ -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 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.

With the above in mind, I think about two approaches. I have the
following constrains in mind:

   + keep it simple (2-3 lists max)

   + If people configure kdump then they want to use it.
     Post crash_kexec() paths are just a fallback.

   + If people configure panic_sys_info + crash dump then they
     want it even in crash dump.

   + If people configure any kmsg dump and configure kdump
     then they want it before crash_kexec().

   + If a platform wants to do something then it wants to do
     it before __crash_kexec().

   + If something cause problems then fix it. But allow to disable
     it for debugging purposes.

One very important aspect is when other cpus should get disabled.
I guess that most people would want to call __crash_kexec() before
disabling other CPU. Even though, I am not completely sure why.
But some notifiers might be hard to do smp_safe.

The disabling of other CPUs seems to be the most critical operation
which would split notifiers into two groups.

Approach A: Two list: smp_safe vs. smp_unsafe
=============================================

We could create two list:

   + panic_notifiers_smp_safe_list
   + panic_notifiers_smp_unsafe_list

Where the safe list would be called before __crash_kexec().
And the other after CPUs get disabled.

In the long term, I would get rid of the controversial
"crash_kexec_post_notifiers" options. Instead, I would add:

    + disable_panic_notifiers_smp_safe
    + disable_panic_notifiers_smp_unsafe
    + disable_cpus_before_kdump

Then we might call:

	panic_notify_smp_safe();

	if (disable_cpus_before_kdump) {
		crash_smp_send_stop();
		panic_notify_smp_unsafe();
	}

	__crash_kexec();

	if (!disable_cpus_before_kdump) {
		crash_smp_send_stop();
		panic_notify_smp_unsafe();
	}

Where the panic_notify* functions would get implemented as:

	void panic_notify_smp_safe(void)
	{
		if (disable_panic_notifiers_smp_safe)
			return;

		...
	}


Approach B: Three lists: platform, smp_safe, and smp_unsafe()
=============================================================

It is similar to approach A. We would just add an extra:

     + panic_notifiers_platform_list
     + disable_panic_notifiers_platform_list

This list would be used for smp-safe notifiers which are
required be some platform. It would be the notifiers which
are crucial. Which are currently the reason to set
"_crash_kexec_post_notifiers".

It might be called also when __crash_kexec() is called
outside panic().

It would allow to separate the most crucial stuff from
the rest. It might help when it might be enough to
disable the less crucial stuff for debugging kdump
issues.


Conclusion:
===========

Of course, there are many details which might need to be sorted,
for example:

  + how to put this together with the existing list
    until all notifiers are migrated.

  + which list should be called when (__)crash_kexec() is
    called explicitly outside vpanic().

This patchset, adding vmware_log_notifier, might implement just
the panic_notifiers_platform_list. The rest might be
added by another volunteer.

How does this sounds, please?

Best Regards,
Petr