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

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

[Takenori Nagano]

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().

--
Takenori Nagano <t-nagano@ah.jp.nec.com>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Andrew Morton]

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?

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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Vivek Goyal]

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.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Takenori Nagano]

Vivek Goyal wrote:
> 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:
>> 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.

Yes, it is true. But I don't mind only kdb and kdump co-exist.

Keith Owen said,

> > My stance is that _all_ the RAS tools (kdb, kgdb, nlkd, netdump, lkcd,
> > crash, kdump etc.) should be using a common interface that safely puts
> > the entire system in a stopped state and saves the state of each cpu.
> > Then each tool can do what it likes, instead of every RAS tool doing
> > its own thing and they all conflict with each other, which is why this
> > thread started.
> > 
> > It is not the kernel's job to decide which RAS tool runs first, second
> > etc., it is the user's decision to set that policy. Different sites
> > will want different orders, some will say "go straight to kdump", other
> > sites will want to invoke a debugger first. Sites must be able to
> > define that policy, but we hard code the policy into the kernel. 

I agreed with him and I made new notifier function that users can change
the order. Priority value in notifier blocks are hardcoded. If users want to
change list order, they have to rebuild kernel. I think it is very unhappy.

This is our discussion.
http://www.gossamer-threads.com/lists/linux/kernel/797220?do=post_view_threaded#797220

In addition, we can use new notifier function to reboot_notifier, die_chain
if users want to use.

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

Sorry, I'll resend.

Thanks,
  Takenori
 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Nick Piggin]

On Thursday 18 October 2007 18:52, Takenori Nagano wrote:
> Vivek Goyal wrote:

> > > My stance is that _all_ the RAS tools (kdb, kgdb, nlkd, netdump, lkcd,
> > > crash, kdump etc.) should be using a common interface that safely puts
> > > the entire system in a stopped state and saves the state of each cpu.
> > > Then each tool can do what it likes, instead of every RAS tool doing
> > > its own thing and they all conflict with each other, which is why this
> > > thread started.
> > >
> > > It is not the kernel's job to decide which RAS tool runs first, second
> > > etc., it is the user's decision to set that policy. Different sites
> > > will want different orders, some will say "go straight to kdump", other
> > > sites will want to invoke a debugger first. Sites must be able to
> > > define that policy, but we hard code the policy into the kernel.
>
> I agreed with him and I made new notifier function that users can change
> the order. Priority value in notifier blocks are hardcoded. If users want
> to change list order, they have to rebuild kernel. I think it is very
> unhappy.

Is it possible to use a single bit of common code and a single
notifier for these things? Or is it too difficult?

One thing I'd suggest is not to use debugfs, if it is going to
be a useful end-user feature.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Takenori Nagano]

Nick Piggin wrote:
>>> Is it possible to use a single bit of common code and a single
>>> notifier for these things? Or is it too difficult?
>> >
>> > I'm sorry, I can't understand your image well. I'd like to know details of
>> > your image.
> 
> Rather than have each of "RAS tools" have their own notifier, and have
> the user specify the priority of the notifiers, introduce some layer
> which _knows_ that, for example, only one of these subsystems will be
> called (it could arbitrate, perhaps distinguish between destructive and
> non-destructive ones). It would need only a single notifier, but would
> then have a specific way of calling into the ras modules.
> 
> Does this make sense? I guess it is a lot more work to do, so maybe your
> solution is the best one for now.

Hi Nick,

Thank you for your explanation. I understand. :-)

This is crash_stop (the common infrastructure for debug tools) by Keith Owens.
http://www.mail-archive.com/linux-arch@vger.kernel.org/msg01929.html

Is it same as your idea? I think it is very nice solution for debug tools
conflict problem.

By the way, on old notify_chain, if admin wants to change the list order, admin
have to recompile the kernel. My patches add new *generic* notify_chain which
admin can modify the list order. My patches are not only for RAS tools problem.

I'm happy if both patches are merged into mainline. :-)

Thanks,
  Takenori

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec