[PATCH 0/3] add new notifier function ,take4

[PATCH 0/3] add new notifier function ,take4

[Takenori Nagano]

Hi,

changelog take3 -> take4

- Rebased 2.6.25-mm1
- Add a document
- Add kdump on panic_notifier

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 control files.

And, third patch moves crash_kexec() to panic_notifier. It helps us to do
something before taking a crash dump. It's useful for some RAS tools developer.
If you want to use it, you have to set config option DUMP_ON_PANIC_NOTIFIER to
Y. Default value of DUMP_ON_PANIC_NOTIFIER is N.
If you set DUMP_ON_PANIC_NOTIFIER to N, kdump has no difference before.

------
Example)

# cd /sys/kernel/notifiers/
# ls
panic_notifier_list
# cd panic_notifier_list/
# ls
ipmi_msghandler  ipmi_wdog
# insmod notifier_test.ko
# ls
ipmi_msghandler  ipmi_wdog  notifier_test1  notifier_test2
# cat */priority
200
150
500
1000
Kernel panic - not syncing: Panic by panic_module.
__tunable_atomic_notifier_call_chain enter
notifier_test: notifier_test_panic2() is called.
notifier_test: notifier_test_panic() is called.
msg_handler:panic_event was called.
ipmi_wdog:wdog_panic_handler was called.

.....(reboot)

# cd /sys/kernel/notifiers/panic_notifier_list/
# ls
ipmi_msghandler  ipmi_wdog  notifier_test1  notifier_test2
# cat */priority
200
150
500
1000
# echo 10000 > ipmi_msghandler/priority
# echo 5000 > ipmi_wdog/priority
# echo 3000 > notifier_test1/priority
# echo 1500 > notifier_test2/priority
# cat */priority
10000
5000
3000
1500
Kernel panic - not syncing: Panic by panic_module.
__tunable_atomic_notifier_call_chain enter
msg_handler:panic_event was called.
ipmi_wdog:wdog_panic_handler was called.
notifier_test: notifier_test_panic() is called.
notifier_test: notifier_test_panic2() is called.

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

Re: [PATCH 0/3] add new notifier function ,take4

[Vivek Goyal]

On Wed, Apr 23, 2008 at 08:11:19PM +0900, Takenori Nagano wrote:
> Hi,
> 
> changelog take3 -> take4
> 
> - Rebased 2.6.25-mm1
> - Add a document
> - Add kdump on panic_notifier
> 
> 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 control files.
> 
> And, third patch moves crash_kexec() to panic_notifier. It helps us to do
> something before taking a crash dump. It's useful for some RAS tools developer.

Hi Takenori,

What's the "something" which you want to do after panic and before kdump.
Can you please give a concrete example. We have talked about this quite
a few times but nothing concrete has come out so far, except a generic
statement that it helps "RAS tool developers".

The only thing we could think of was debuggers (kdb and kgdb) and one easy
solution is that debuggers/users can put a break point at panic() instead
of introducing this infrastructure.

Thanks
Vivek

Re: [PATCH 0/3] add new notifier function ,take4

[Eric W. Biederman]

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

> Hi,
>
> changelog take3 -> take4
>
> - Rebased 2.6.25-mm1
> - Add a document
> - Add kdump on panic_notifier
>
> 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 control
> files.
>
> And, third patch moves crash_kexec() to panic_notifier. It helps us to do
> something before taking a crash dump. It's useful for some RAS tools developer.
> If you want to use it, you have to set config option DUMP_ON_PANIC_NOTIFIER to
> Y. Default value of DUMP_ON_PANIC_NOTIFIER is N.
> If you set DUMP_ON_PANIC_NOTIFIER to N, kdump has no difference before.

NAK.

Reasonable alternatives have been suggested.  No rebuttal has been given.
Just buggy patches with insufficient description of what you are doing
and why.

I am tired of seeing the same patch come up again and again without even
all of the easy problems that are pointed out addressed, much less the
design issues considered or addressed.

I see no evidence that we need this mechanism to achieve any of
the goals proposed.

This mechanism drastically reduces the maintainability of the
kexec on panic code as it makes the code path indiscoverable
and thus unreviewable.

This mechanism gives an unnecessary and confusing policy control
to users when we should be able to auto-tune based on the situation.

CONFIGURABILITY IS BAD in this context.

I think this entire approach is a BAD IDEA.  Please go back to
the drawing board.  Please describe the specific problems you
are trying to solve and why the existing mechanisms can not be
made to work and we can work with you.

Eric