Re: [PATCH 0/2] add new notifier function ,take2

Vivek Goyal
2007-10-18T08:06:45Z

On Thu, Oct 18, 2007 at 12:06:51AM -0700, Andrew Morton wrote:
> On Thu, 18 Oct 2007 15:45:08 +0900 Takenori Nagano <t-nagano@ah.jp.nec.com> wrote:
>
> > Hi,
> >
> > A big thanks to everybody who read and replied to first version. I have tried to
> > incorporate reviewer's comments and suggestions.
> >
> > changelog take1 -> take2
> >
> > - Rebased 2.6.23
> > - comment updated
> > - renamed the notifiner name "tunable_notifier" to "tunable_atomic_notifier"
> > - fixed typo
> >
> >
> > These patches add new notifier function and implement it to panic_notifier_list.
> > We used the hardcoded notifier chain so far, but it was not flexible. New
> > notifier is very flexible, because user can change a list of order by debugfs.
> >
> > Thanks,
> >
> > Example)
> >
> > # cd /sys/kernel/debug/
> > # ls
> > kprobes pktcdvd
> > # insmod ipmi_msghandler.ko
> > # ls
> > kprobes panic_notifier_list pktcdvd
> > # cd panic_notifier_list/
> > # ls
> > ipmi_msghandler
> > # insmod ipmi_watchdog.ko
> > # ls
> > ipmi_msghandler ipmi_wdog
> > # cat ipmi_msghandler/priority
> > 200
> > # cat ipmi_wdog/priority
> > 150
> > #
> > Kernel panic - not syncing: panic
> > ipmi_msghandler : notifier calls panic_event().
> > ipmi_watchdog : notifier calls wdog_panic_handler().
> >
> > .....(reboot)
> >
> > # cat ipmi_msghandler/priority
> > 200
> > # cat ipmi_wdog/priority
> > 150
> > # echo 300 > ipmi_wdog/priority
> > #
> > Kernel panic - not syncing: panic
> > ipmi_watchdog : notifier calls wdog_panic_handler().
> > ipmi_msghandler : notifier calls panic_event().
>
> I can sort-of see what this is doing. Runtime-definable management of
> which notifier functions will be called on a panic? Or maybe I
> misunderstood.
>
> But even if I did understand, I don't understand why Linux needs this
> feature - what are the use cases, what is the value to our users?
>
> Can you please flesh that information out a bit more?
>

Hi Andrew,

Takenori wants to make kdb and kdump co-exist. Currently after panic()
panic_notifier_list is not executed if kdump is configured. Before list
is executed, system will boot into second kernel to capture the dump. Hence
if even if kdb was registered on panic_notifier_list, it will never get
a chance to run.

One of the ideas was to put all the RAS tools on panic_notifier_list, export
the list to user space and let user determine the order in which these
should be called. That's why this patchset is there.

This does bring in some reliability concerns for kdump as now there is
additional dependency on notifier infrastructure after panic.

Eric, had suggested that kdb can put a break point on panic() to handle
the co-existence issue and no need to bring in additional dependencies in
panic path.

I agree with the idea of kdb putting a break point in panic. If there are
more tools down the line who run into co-existence issue with kdump, then
we can re-visit this idea.

Thanks
Vivek

> The patches are somewhat wordwrapped - please check your email client
> configuration, thanks.