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

[Baoquan He]

On 10/20/26 at 03:23pm, Zack Rusin wrote:
> Hi, Petr.
> 
> First of all I really appreciate you going over this. Thanks!
> 
> On Tue, Oct 20, 2026 at 12:37 PM Petr Mladek <pmladek@xxxxxxxx> wrote:
> > 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().
> [...]
> > 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?
> 
> That sounds good to me. For v3 I'd do just the platform list:
> 
> - One call in vpanic(), right after kgdb_panic() and before the first
> __crash_kexec(), whatever crash_kexec_post_notifiers is set to. The
> list runs before kdump and before the other CPUs are stopped.
> - One call in __crash_kexec(), after it has taken the kexec lock,
> found a crash image and saved the registers, for paths that enter
> kdump without panic(), like oops_end() or Hyper-V's hv_crash_handle().
> Once the early panic call has run the list, the guard makes this one a
> no-op. That removes the dependence on crash_kexec_post_notifiers you
> pointed out before.
> - A once-only guard shared by both, so the list also runs only once
> when a callback panics.
> - disable_panic_notifiers_platform_list as a boot and runtime switch,
> checked in the same helper, replacing panic_pre_kdump_postpone.
> - Kernel-doc with your definition: smp-safe callbacks a platform
> needs, which would otherwise be a reason to set
> crash_kexec_post_notifiers.
> 
> Roughly:
>         kgdb_panic(buf);
> 
>         panic_notify_platform();
> 
>         if (!_crash_kexec_post_notifiers)
>                 __crash_kexec(NULL);
> 
> and in __crash_kexec():
> 
>         crash_setup_regs(&fixed_regs, regs);
>         /* Direct entry outside panic() */
>         panic_notify_platform();
>         crash_save_vmcoreinfo();
> 
> I'd keep the second call in __crash_kexec() rather than crash_kexec(),
> because afaict crash_kexec() can return without a crash image, e.g.
> from the BPF kfunc, or from oops_end() before it panics. Calling the
> list there would use up the guard even when no kdump follows.
> 
> crash_kexec_post_notifiers, the existing list and the kmsg dumpers
> stay as they are, and moving other platforms over can come later.
> 
> The VMware log copy would be the only user, and it's still best-effort
> diagnostics: kdump and the reboot don't depend on it. It copies the
> log tail into an 8 KiB buffer. On panic() that happens after the panic
> message and any local backtrace; on direct entry from an oops, after
> the exception output. With the list enabled throughout, the copy
> precedes the later panic notifiers, sys_info() and the all-CPU
> backtrace step. The guest-crash report stays a late kmsg dumper that's
> skipped while a crash kernel is loaded, because the host can power off
> the VM inside that call.
> 
> If that matches what you had in mind, I'll do it that way in v3.
> 
> Guilherme, Stephen, Baoquan, does that work for you too?

Hi Zack,

Sorry I didn't check this series before, just trying to catch up.

I like the Approach B direction: one panic_notifiers_platform_list for
platform callbacks that must run before kdump. While I have one concern
about the long term.

Today the list has only the VMware log sender. Later, more handlers may join
(for example pvpanic, Hyper-V, Xen, gsmi, SNP). When the list grows, someone
will want to run only their handler, or to skip some handlers. The easy
answer is "add a new list for my handler". I think this will be a problem too
in the future. Every new list adds a new global ordering rule and a new disable
switch. Petr already said we should keep the number of lists small.

Currently, atomic_notifier_call_chain() runs all registered callbacks. There
is no per-handler enable/disable and no filter. NOTIFY_STOP_MASK can stop the
chain, but we must not use it here, because one handler could hide all the
later handlers. The list-level disable_panic_notifiers_platform_list can
only disable the whole list.

I have some rough though, just for reference:

1. Keep one panic_notifiers_platform_list.

2. Give each handler a small wrapper with a name and an enabled flag:

   struct panic_pre_kdump_notifier {
           struct notifier_block nb;
           const char *name;
           bool enabled;
   };

3. In the dispatch, skip a handler when enabled is false. Print the name
   before calling it. This also helps when a handler hangs and we want to
   know which one it was.

4. Add a boot parameter to disable handlers by name, for example:

           panic_pre_kdump.disable=vmware_log,hyperv_crash

   Parse it early. At panic time we only read the flag. We do not parse
   strings in the panic path.

5. Keep the list-level disable_panic_notifiers_platform_list. It is still
   useful to disable the whole list for debugging.

6. If the list is still too broad later, we can add a reason/class argument
   (for example PANIC, OOPS_DIRECT). A handler can then decide if it cares.
   This is better than adding a new list.

So with them:

- We keep one place to register, one ordering rule, and one place to look
  when something goes wrong.
- "Run only my handler" becomes a debug switch (disable the others), not a
  new global list. Or vice versa.
- We do not change the general struct notifier_block.
- The panic path only reads a bool, so it is safe and cheap.

just some rough thoughts, maybe not well polished, just for reference.
Thinking of these becasue I do meet requirement in real use case where
they really want to run their own one specific panic handler before
kdump while not easy to make it.

Thanks,
Baoquan