[patch] add kdump_after_notifier

[patch] add kdump_after_notifier

[Takenori Nagano]

Hi,

In latest kernel, we can't use panic_notifier_list if kdump is enabled.
panic_notifier_list is very useful function for debug, failover, etc...

So this patch adds a control file /proc/sys/kernel/dump_after_notifier
and resolves a problem users can not use both kdump and panic_notifier_list
at the same time.

kdump_after_notifier = 0
 -> panic()
    -> crash_kexec(NULL)

kdump_after_notifier = 1
 -> panic()
    -> atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
    -> crash_kexec(NULL)


Signed-off-by: Takenori Nagano <t-nagano@ah.jp.nec.com>
Signed-off-by: Kazuto Miyoshi <k-miyoshi@cb.jp.nec.com>
---

diff -uprN linux-2.6.22.orig/include/linux/kexec.h
linux-2.6.22/include/linux/kexec.h
--- linux-2.6.22.orig/include/linux/kexec.h	2007-07-09 08:32:17.000000000 +0900
+++ linux-2.6.22/include/linux/kexec.h	2007-07-19 18:55:04.236000000 +0900
@@ -123,6 +123,7 @@ int kexec_should_crash(struct task_struc
 void crash_save_cpu(struct pt_regs *regs, int cpu);
 extern struct kimage *kexec_image;
 extern struct kimage *kexec_crash_image;
+extern int kdump_after_notifier;

 #ifndef kexec_flush_icache_page
 #define kexec_flush_icache_page(page)
@@ -160,5 +161,6 @@ struct pt_regs;
 struct task_struct;
 static inline void crash_kexec(struct pt_regs *regs) { }
 static inline int kexec_should_crash(struct task_struct *p) { return 0; }
+#define kdump_after_notifier 0
 #endif /* CONFIG_KEXEC */
 #endif /* LINUX_KEXEC_H */
diff -uprN linux-2.6.22.orig/include/linux/sysctl.h
linux-2.6.22/include/linux/sysctl.h
--- linux-2.6.22.orig/include/linux/sysctl.h	2007-07-09 08:32:17.000000000 +0900
+++ linux-2.6.22/include/linux/sysctl.h	2007-07-19 18:55:04.260000000 +0900
@@ -165,6 +165,7 @@ enum
 	KERN_MAX_LOCK_DEPTH=74,
 	KERN_NMI_WATCHDOG=75, /* int: enable/disable nmi watchdog */
 	KERN_PANIC_ON_NMI=76, /* int: whether we will panic on an unrecovered */
+	KERN_KDUMP_AFTER_NOTIFIER=77, /* int: kdump after panic_notifier */
 };


diff -uprN linux-2.6.22.orig/kernel/kexec.c linux-2.6.22/kernel/kexec.c
--- linux-2.6.22.orig/kernel/kexec.c	2007-07-09 08:32:17.000000000 +0900
+++ linux-2.6.22/kernel/kexec.c	2007-07-19 18:55:04.304000000 +0900
@@ -22,6 +22,7 @@
 #include <linux/hardirq.h>
 #include <linux/elf.h>
 #include <linux/elfcore.h>
+#include <linux/sysctl.h>

 #include <asm/page.h>
 #include <asm/uaccess.h>
@@ -31,6 +32,7 @@

 /* Per cpu memory for storing cpu states in case of system crash. */
 note_buf_t* crash_notes;
+int kdump_after_notifier;

 /* Location of the reserved area for the crash kernel */
 struct resource crashk_res = {
@@ -1123,6 +1125,30 @@ void crash_save_cpu(struct pt_regs *regs
 	final_note(buf);
 }

+#ifdef CONFIG_SYSCTL
+static ctl_table kdump_after_notifier_table[] = {
+	{
+		.ctl_name = KERN_KDUMP_AFTER_NOTIFIER,
+		.procname = "kdump_after_notifier",
+		.data = &kdump_after_notifier,
+		.maxlen = sizeof(int),
+		.mode = 0644,
+		.proc_handler = &proc_dointvec,
+	},
+	{ .ctl_name = 0 }
+};
+
+static ctl_table kexec_sys_table[] = {
+	{
+		.ctl_name = CTL_KERN,
+		.procname = "kernel",
+		.mode = 0555,
+		.child = kdump_after_notifier_table,
+	},
+	{ .ctl_name = 0 }
+};
+#endif
+
 static int __init crash_notes_memory_init(void)
 {
 	/* Allocate memory for saving cpu registers. */
@@ -1132,6 +1158,9 @@ static int __init crash_notes_memory_ini
 		" states failed\n");
 		return -ENOMEM;
 	}
+#ifdef CONFIG_SYSCTL
+	register_sysctl_table(kexec_sys_table, 0);
+#endif
 	return 0;
 }
 module_init(crash_notes_memory_init)
diff -uprN linux-2.6.22.orig/kernel/panic.c linux-2.6.22/kernel/panic.c
--- linux-2.6.22.orig/kernel/panic.c	2007-07-09 08:32:17.000000000 +0900
+++ linux-2.6.22/kernel/panic.c	2007-07-19 18:55:04.340000000 +0900
@@ -85,7 +85,8 @@ NORET_TYPE void panic(const char * fmt,
 	 * everything else.
 	 * Do we want to call this before we try to display a message?
 	 */
-	crash_kexec(NULL);
+	if (!kdump_after_notifier)
+		crash_kexec(NULL);

 #ifdef CONFIG_SMP
 	/*
@@ -98,6 +99,8 @@ NORET_TYPE void panic(const char * fmt,

 	atomic_notifier_call_chain(&panic_notifier_list, 0, buf);

+	crash_kexec(NULL);
+
 	if (!panic_blink)
 		panic_blink = no_blink;

Re: [patch] add kdump_after_notifier

[Bernhard Walle]

* Takenori Nagano <t-nagano@ah.jp.nec.com> [2007-07-19 14:15]:
> 
> In latest kernel, we can't use panic_notifier_list if kdump is enabled.
> panic_notifier_list is very useful function for debug, failover, etc...
> 
> So this patch adds a control file /proc/sys/kernel/dump_after_notifier
> and resolves a problem users can not use both kdump and panic_notifier_list
> at the same time.
> 
> kdump_after_notifier = 0
>  -> panic()
>     -> crash_kexec(NULL)
> 
> kdump_after_notifier = 1
>  -> panic()
>     -> atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
>     -> crash_kexec(NULL)

What's problematic about this patch? I also would like to see that
feature.


Thanks,
   Bernhard

Re: [patch] add kdump_after_notifier

[Vivek Goyal]

On Thu, Jul 26, 2007 at 04:07:02PM +0200, Bernhard Walle wrote:
> * Takenori Nagano <t-nagano@ah.jp.nec.com> [2007-07-19 14:15]:
> > 
> > In latest kernel, we can't use panic_notifier_list if kdump is enabled.
> > panic_notifier_list is very useful function for debug, failover, etc...
> > 
> > So this patch adds a control file /proc/sys/kernel/dump_after_notifier
> > and resolves a problem users can not use both kdump and panic_notifier_list
> > at the same time.
> > 
> > kdump_after_notifier = 0
> >  -> panic()
> >     -> crash_kexec(NULL)
> > 
> > kdump_after_notifier = 1
> >  -> panic()
> >     -> atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
> >     -> crash_kexec(NULL)
> 
> What's problematic about this patch? I also would like to see that
> feature.

I would like to see the code which will get executed after panic and 
before crash_kexec(). This potentially makes crash dump feature unreliable
in the sense one can now register on panic_notifier_list and try to
do whole lot of things and might get stuck there. After the system
has crashed, one is not supposed to do a whole lot.

Thanks
Vivek

Re: [patch] add kdump_after_notifier

[Bernhard Walle]

* Vivek Goyal <vgoyal@in.ibm.com> [2007-07-26 17:32]:
> On Thu, Jul 26, 2007 at 04:07:02PM +0200, Bernhard Walle wrote:
> > * Takenori Nagano <t-nagano@ah.jp.nec.com> [2007-07-19 14:15]:
> > > 
> > > In latest kernel, we can't use panic_notifier_list if kdump is enabled.
> > > panic_notifier_list is very useful function for debug, failover, etc...
> > > 
> > > So this patch adds a control file /proc/sys/kernel/dump_after_notifier
> > > and resolves a problem users can not use both kdump and panic_notifier_list
> > > at the same time.
> > > 
> > > kdump_after_notifier = 0
> > >  -> panic()
> > >     -> crash_kexec(NULL)
> > > 
> > > kdump_after_notifier = 1
> > >  -> panic()
> > >     -> atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
> > >     -> crash_kexec(NULL)
> > 
> > What's problematic about this patch? I also would like to see that
> > feature.
> 
> I would like to see the code which will get executed after panic and 
> before crash_kexec(). This potentially makes crash dump feature unreliable
> in the sense one can now register on panic_notifier_list and try to
> do whole lot of things and might get stuck there. After the system
> has crashed, one is not supposed to do a whole lot.

Of course, but that's why the patch doesn't change this by default but
gives the user the choice.


Thanks,
   Bernhard

Re: [patch] add kdump_after_notifier

[Vivek Goyal]

On Thu, Jul 26, 2007 at 05:34:40PM +0200, Bernhard Walle wrote:
> * Vivek Goyal <vgoyal@in.ibm.com> [2007-07-26 17:32]:
> > On Thu, Jul 26, 2007 at 04:07:02PM +0200, Bernhard Walle wrote:
> > > * Takenori Nagano <t-nagano@ah.jp.nec.com> [2007-07-19 14:15]:
> > > > 
> > > > In latest kernel, we can't use panic_notifier_list if kdump is enabled.
> > > > panic_notifier_list is very useful function for debug, failover, etc...
> > > > 
> > > > So this patch adds a control file /proc/sys/kernel/dump_after_notifier
> > > > and resolves a problem users can not use both kdump and panic_notifier_list
> > > > at the same time.
> > > > 
> > > > kdump_after_notifier = 0
> > > >  -> panic()
> > > >     -> crash_kexec(NULL)
> > > > 
> > > > kdump_after_notifier = 1
> > > >  -> panic()
> > > >     -> atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
> > > >     -> crash_kexec(NULL)
> > > 
> > > What's problematic about this patch? I also would like to see that
> > > feature.
> > 
> > I would like to see the code which will get executed after panic and 
> > before crash_kexec(). This potentially makes crash dump feature unreliable
> > in the sense one can now register on panic_notifier_list and try to
> > do whole lot of things and might get stuck there. After the system
> > has crashed, one is not supposed to do a whole lot.
> 
> Of course, but that's why the patch doesn't change this by default but
> gives the user the choice.
> 

I am skeptical that how many users will really know that whether to set
this option as 1 or 0. Telling them setting it zero is  more reliable as
compared to 1 is kind of vague. What value will distro set it to by default?

Can we be more specific in terms of functionality and code that exactly
what we are trying to do after panic?

Thanks
Vivek

> 
> Thanks,
>    Bernhard

Re: [patch] add kdump_after_notifier

[Bernhard Walle]

* Vivek Goyal <vgoyal@in.ibm.com> [2007-07-26 17:44]:
> > 
> > Of course, but that's why the patch doesn't change this by default but
> > gives the user the choice.
> > 
> 
> What value will distro set it to by default?

0.

> Can we be more specific in terms of functionality and code that exactly
> what we are trying to do after panic?

Well, KDB, but now everybody answers with “not mainline -- doesn't
count”.


Thanks,
   Bernhard

Re: [patch] add kdump_after_notifier

[Vivek Goyal]

On Thu, Jul 26, 2007 at 05:47:18PM +0200, Bernhard Walle wrote:
> * Vivek Goyal <vgoyal@in.ibm.com> [2007-07-26 17:44]:
> > > 
> > > Of course, but that's why the patch doesn't change this by default but
> > > gives the user the choice.
> > > 
> > 
> > What value will distro set it to by default?
> 
> 0.
> 
> > Can we be more specific in terms of functionality and code that exactly
> > what we are trying to do after panic?
> 
> Well, KDB, but now everybody answers with “not mainline -- doesn't
> count”.
> 

That's true. Its not mainline. We had similar discussion in the past
also. I think we should allow only audited code to be run after panic().
Leaving it open to modules or unaudited code makes this solution
something like LKCD where whole lot of code used to run after the crash,
hence was unreliable.

If KDB goes mainline, then I think it is not a bad idea to call debugger
first (if it is enabled) and then one can trigger crash dump from inside
the debugger.

Thanks
Vivek

Re: [patch] add kdump_after_notifier

[Bernhard Walle]

* Vivek Goyal <vgoyal@in.ibm.com> [2007-07-26 17:54]:
> 
> That's true. Its not mainline. We had similar discussion in the past
> also. I think we should allow only audited code to be run after panic().
> Leaving it open to modules or unaudited code makes this solution
> something like LKCD where whole lot of code used to run after the crash,
> hence was unreliable.
> 
> If KDB goes mainline, then I think it is not a bad idea to call debugger
> first (if it is enabled) and then one can trigger crash dump from inside
> the debugger.

Well, after thinking again about this, I think the best solution would
to call kdump from KDB with a command as it was done by LKCD. The new
sysctl is too specific ...


Thanks,
   Bernhard

Re: [patch] add kdump_after_notifier

[Bernhard Walle]

* Bernhard Walle <bwalle@suse.de> [2007-07-26 18:14]:
> * Vivek Goyal <vgoyal@in.ibm.com> [2007-07-26 17:54]:
> > 
> > That's true. Its not mainline. We had similar discussion in the past
> > also. I think we should allow only audited code to be run after panic().
> > Leaving it open to modules or unaudited code makes this solution
> > something like LKCD where whole lot of code used to run after the crash,
> > hence was unreliable.
> > 
> > If KDB goes mainline, then I think it is not a bad idea to call debugger
> > first (if it is enabled) and then one can trigger crash dump from inside
> > the debugger.
> 
> Well, after thinking again about this, I think the best solution would
> to call kdump from KDB with a command as it was done by LKCD.

Ok, but that doesn't solve the problem (it would solve the opposite
problem).

My question is: Why cannot kdump use the notifier call chain? Wouldn't
this simplify things in general, apart from KDB. Not talking about the
priority of kdump now ...


Thanks,
   Bernhard

Re: [patch] add kdump_after_notifier

[Takenori Nagano]

Hi Vivek,

Vivek Goyal wrote:
> On Thu, Jul 26, 2007 at 05:47:18PM +0200, Bernhard Walle wrote:
>> * Vivek Goyal <vgoyal@in.ibm.com> [2007-07-26 17:44]:
>>>> Of course, but that's why the patch doesn't change this by default but
>>>> gives the user the choice.
>>>>
>>> What value will distro set it to by default?
>> 0.
>>
>>> Can we be more specific in terms of functionality and code that exactly
>>> what we are trying to do after panic?
>> Well, KDB, but now everybody answers with “not mainline -- doesn't
>> count”.
>>
> 
> That's true. Its not mainline. We had similar discussion in the past
> also. I think we should allow only audited code to be run after panic().
> Leaving it open to modules or unaudited code makes this solution
> something like LKCD where whole lot of code used to run after the crash,
> hence was unreliable.

It is *not* KDB specific problem. Please grep in mainline kernel. You can find
some function using panic_notifier_list. (IPMI, softdog, heartbeat, etc...)
My patch gives a chance to use kdump for panic_notifier user. It is good for
kdump too, because kdump user goes to increase. :-)

Bernhard's idea (kdump uses panic_notifier) is very good for me. But it isn't
good for kdump user, because they want to take a dump ASAP when panicked.

Vivek, please think about this problem again.

If there is a developer who has the opinion on this problem, please give us your
opinion.

Thanks.

> If KDB goes mainline, then I think it is not a bad idea to call debugger
> first (if it is enabled) and then one can trigger crash dump from inside
> the debugger.
> Thanks
> Vivek
> 
> 

Re: [patch] add kdump_after_notifier

[Vivek Goyal]

On Fri, Jul 27, 2007 at 08:28:48AM +0900, Takenori Nagano wrote:
> Hi Vivek,
> 
> Vivek Goyal wrote:
> > On Thu, Jul 26, 2007 at 05:47:18PM +0200, Bernhard Walle wrote:
> >> * Vivek Goyal <vgoyal@in.ibm.com> [2007-07-26 17:44]:
> >>>> Of course, but that's why the patch doesn't change this by default but
> >>>> gives the user the choice.
> >>>>
> >>> What value will distro set it to by default?
> >> 0.
> >>
> >>> Can we be more specific in terms of functionality and code that exactly
> >>> what we are trying to do after panic?
> >> Well, KDB, but now everybody answers with “not mainline -- doesn't
> >> count”.
> >>
> > 
> > That's true. Its not mainline. We had similar discussion in the past
> > also. I think we should allow only audited code to be run after panic().
> > Leaving it open to modules or unaudited code makes this solution
> > something like LKCD where whole lot of code used to run after the crash,
> > hence was unreliable.
> 
> It is *not* KDB specific problem. Please grep in mainline kernel. You can find
> some function using panic_notifier_list. (IPMI, softdog, heartbeat, etc...)
> My patch gives a chance to use kdump for panic_notifier user. It is good for
> kdump too, because kdump user goes to increase. :-)

I grepped for couple of items. Heartbeat functionality of for stopping
responding to service processsor in case of panic so that service processor
knows that system has crashed and it needs to reboot the machine. But if
somebody has configured and enabled kdump then we don't want service 
processor to stop responding to heartbeat otherwise service processor
will reboot the machine and we will not be able to capture the dump.

In case of detecting softlockup, panic notifier is used so that in case
of panic we don't want to flag other threads are not being scheduled and
it is a softlockup. In case of kdump this condition is not valid. Immediately
after kdump we will boot to next OS and previous kernel's context is wiped
off.

Can you please be specific what exactly is the problem you are facing? In
what situation is this call creating the problem?

> 
> Bernhard's idea (kdump uses panic_notifier) is very good for me. But it isn't
> good for kdump user, because they want to take a dump ASAP when panicked.
> 

This one is better than registering kdump as one of the users of a
panic_notifier() list. 

I think if there are any crash specific actions, they should be taken care
in next kernel while it is booting.

If something is really very time critical, and has to be done immediately
after panic (I am not sure how can one ensure that given the fact any number
of users can register on panic_notifier_list and you are not sure about your
order in the list and when one will get the control), then probably that
piece of code should be in kernel and called before crash_kexec().

What is that specific piece of action which you can't do in second kernel?

Eric, do you have any thoughts on this. I think these guys are referring
to failover problem where immediately after panic() they want to send
message to other node.

Thanks
Vivek

Re: [patch] add kdump_after_notifier

[Eric W. Biederman]

Vivek Goyal <vgoyal@in.ibm.com> writes:

>> Bernhard's idea (kdump uses panic_notifier) is very good for me. But it isn't
>> good for kdump user, because they want to take a dump ASAP when panicked.
>> 
>
> This one is better than registering kdump as one of the users of a
> panic_notifier() list. 
>
> I think if there are any crash specific actions, they should be taken care
> in next kernel while it is booting.
>
> If something is really very time critical, and has to be done immediately
> after panic (I am not sure how can one ensure that given the fact any number
> of users can register on panic_notifier_list and you are not sure about your
> order in the list and when one will get the control), then probably that
> piece of code should be in kernel and called before crash_kexec().
>
> What is that specific piece of action which you can't do in second kernel?
>
> Eric, do you have any thoughts on this. I think these guys are referring
> to failover problem where immediately after panic() they want to send
> message to other node.

My thoughts are roughly the same as they were last time this was suggested.
I think adding a notifier to the kexec on panic path is a bad idea.
This functionality  sounds wrong, because it makes it hard to ensure
reliability of the kexec on panic code path.  We are still doing to
much on it as it stands. The working assumption on that code path
needs to be the kernel is broken. Anything else is just asking for
trouble.

Currently we do have a hook in place for code to be called. It is called
the purgatory section of /sbin/kexec.  And it's user space so you can
do whatever you want there.  Or you can wait until the second kernel
gets more fully booted.

If we really need to do something in the kernel we can patch the kernel
to make a function call from crash_kexec.  We don't need any notifiers
to do this.

A further problem with notifiers is they mess up the state we would
like to debug.  Which again makes them a problem.


So at least until a specific case is made for a specific piece of code
to get in I am totally opposed to the idea.

Eric

Re: [patch] add kdump_after_notifier

[Takenori Nagano]

Eric W. Biederman wrote:
> Vivek Goyal <vgoyal@in.ibm.com> writes:
> 
>>> Bernhard's idea (kdump uses panic_notifier) is very good for me. But it isn't
>>> good for kdump user, because they want to take a dump ASAP when panicked.
>>>
>> This one is better than registering kdump as one of the users of a
>> panic_notifier() list. 
>>
>> I think if there are any crash specific actions, they should be taken care
>> in next kernel while it is booting.
>>
>> If something is really very time critical, and has to be done immediately
>> after panic (I am not sure how can one ensure that given the fact any number
>> of users can register on panic_notifier_list and you are not sure about your
>> order in the list and when one will get the control), then probably that
>> piece of code should be in kernel and called before crash_kexec().
>>
>> What is that specific piece of action which you can't do in second kernel?
>>
>> Eric, do you have any thoughts on this. I think these guys are referring
>> to failover problem where immediately after panic() they want to send
>> message to other node.
> 
> My thoughts are roughly the same as they were last time this was suggested.
> I think adding a notifier to the kexec on panic path is a bad idea.
> This functionality  sounds wrong, because it makes it hard to ensure
> reliability of the kexec on panic code path.  We are still doing to
> much on it as it stands. The working assumption on that code path
> needs to be the kernel is broken. Anything else is just asking for
> trouble.
> 
> Currently we do have a hook in place for code to be called. It is called
> the purgatory section of /sbin/kexec.  And it's user space so you can
> do whatever you want there.  Or you can wait until the second kernel
> gets more fully booted.
> 
> If we really need to do something in the kernel we can patch the kernel
> to make a function call from crash_kexec.  We don't need any notifiers
> to do this.
> 
> A further problem with notifiers is they mess up the state we would
> like to debug.  Which again makes them a problem.
> 
> 
> So at least until a specific case is made for a specific piece of code
> to get in I am totally opposed to the idea.

Hi all,

IMHO, most users don't use kdump, kdump users are only kernel developers and
enterprise users. I think enterprise users want the notifier function, because
they use some driver and software (hardware monitering driver, clustering
software, heartbeat driver, etc...) to raise their system availability.

Some popular distributers added the dump function to their own kernel. We can
use panic_notifier on LKCD (http://lkcd.sourceforge.net/), and diskdump
(http://sourceforge.net/projects/lkdump) provides own notifier function
disk_dump_notifier.

Now, kdump was merged mainline kernel. Then some distributers chose kdump.
I think kdump is greater than other dump function, but kdump has no notifier
function. This is a large problem for enterprise users.

Solutions
 1: my patch
 2: Bernhard's idea
 3: add kdump_notifier_list

I think my patch is better than other solutions, because it has only very few
impact. Vivek, Eric, how do you think?

Thanks.

Re: [patch] add kdump_after_notifier

[Eric W. Biederman]

Takenori Nagano <t-nagano@ah.jp.nec.com> writes:
>
> Hi all,
>
> IMHO, most users don't use kdump, kdump users are only kernel developers and
> enterprise users. 

Not at all.  So far the only kdump related bug report I have seen has
been from fedora Core.

> think enterprise users want the notifier function, because
> they use some driver and software (hardware monitering driver, clustering
> software, heartbeat driver, etc...) to raise their system availability.

Which users want this?  Specifics are needed here not hand waving.
In particular why can't the use the existing hooks that are already
in place.

> Some popular distributers added the dump function to their own kernel. We can
> use panic_notifier on LKCD (http://lkcd.sourceforge.net/), and diskdump
> (http://sourceforge.net/projects/lkdump) provides own notifier function
> disk_dump_notifier.
>
> Now, kdump was merged mainline kernel. Then some distributers chose kdump.
> I think kdump is greater than other dump function, but kdump has no notifier
> function. This is a large problem for enterprise users.

Why?  If this is a large problem we should have people that are willing
to have patches with users of this notifier.

> Solutions
>  1: my patch
>  2: Bernhard's idea
>  3: add kdump_notifier_list

I think you are solving a non-problem.  And the more I get hand waving
the more I think this. 

> I think my patch is better than other solutions, because it has only very few
> impact. Vivek, Eric, how do you think?

No.  The problem with your patch is that it doesn't have a code
impact.  We need to see who is using this and why.

Because you are trying to hide what is going on your code has
a tremendous maintenance and review burden.  I think any hook has a
tremendous maintenance and review burden.  Especially since the people
who want this absolutely refuse to publish their code.

If it is some proprietary solution that needs this and can not
withstand a code review it is absolutely the wrong thing to have
on this path.

The answer is no, and it isn't even worth talking about
until the code for some real users shows up.

Adding a notifier violates a fundamental assumption of
the code path.  The assumption is that the entire kernel
is broken, and you want me to follow a broken pointer
to broken code?

We already have so much code on that code path it is almost
impossible to test and review thoroughly and you want to
add more crap?

My apologies about my tone but I'm very annoyed at the direction of
all of this conversation, please don't try and avoid showing the
users.  Please let's make it upfront.

Eric

Re: [patch] add kdump_after_notifier

[Takenori Nagano]

Eric W. Biederman wrote:
> Takenori Nagano <t-nagano@ah.jp.nec.com> writes:
>> Hi all,
>>
>> IMHO, most users don't use kdump, kdump users are only kernel developers and
>> enterprise users. 
> 
> Not at all.  So far the only kdump related bug report I have seen has
> been from fedora Core.

Sorry, I thought general users push reset button when the machine is
panicked. :-(

> No.  The problem with your patch is that it doesn't have a code
> impact.  We need to see who is using this and why.

My motivation is very simple. I want to use both kdb and kdump, but I think it
is too weak to satisfy kexec guys. Then I brought up the example enterprise
software. But it isn't a lie. I know some drivers which use panic_notifier.
IMHO, they use only major distribution, and they has the workaround or they
don't notice this problem yet. I think they will be in trouble if all
distributions choose only kdump.

BTW, I use kdb and lkcd now, but I want to use kdb and kdump. I sent a patch to
kdb community but it was rejected. kdb maintainer Keith Owens said,

> Both KDB and crash_kexec should be using the panic_notifier_chain, with
> KDB having a higher priority than crash_exec.  The whole point of
> notifier chains is to handle cases like this, so we should not be
> adding more code to the panic routine.
> 
> The real problem here is the way that the crash_exec code is hard coded
> into various places instead of using notifier chains.  The same issue
> exists in arch/ia64/kernel/mca.c because of bad coding practices from
> kexec.

Then I gave up to merge my patch to kdb, and I tried to send another patch to
kexec community. I can understand his opinion, but it is very difficult to
modify that kdump is called from panic_notifier. Because it has a reason why
kdump don't use panic_notifier. So, I made this patch.

Please do something about this problem.

Thanks,

Re: [patch] add kdump_after_notifier

[Eric W. Biederman]

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

>> No.  The problem with your patch is that it doesn't have a code
>> impact.  We need to see who is using this and why.
>
> My motivation is very simple. I want to use both kdb and kdump, but I think it
> is too weak to satisfy kexec guys. Then I brought up the example enterprise
> software. But it isn't a lie. I know some drivers which use panic_notifier.
> IMHO, they use only major distribution, and they has the workaround or they
> don't notice this problem yet. I think they will be in trouble if all
> distributions choose only kdump.

Possibly.

> BTW, I use kdb and lkcd now, but I want to use kdb and kdump. I sent a patch to
> kdb community but it was rejected. kdb maintainer Keith Owens said,

>> Both KDB and crash_kexec should be using the panic_notifier_chain, with
>> KDB having a higher priority than crash_exec.  The whole point of
>> notifier chains is to handle cases like this, so we should not be
>> adding more code to the panic routine.
>>
>> The real problem here is the way that the crash_exec code is hard coded
>> into various places instead of using notifier chains.  The same issue
>> exists in arch/ia64/kernel/mca.c because of bad coding practices from
>> kexec.

I respectfully disagree with his opinion, as using notifier chains
assumes more of the kernel works.  Although following it's argument
to it's logical conclusion we should call crash_kexec as the very
first thing inside of panic.  Given how much state something like
bust_spinlocks messes up that might not be a bad idea.

It does make adding an alternative debug mechanism in there difficult.
Does anyone know if this also affects kgdb?

> Then I gave up to merge my patch to kdb, and I tried to send another patch to
> kexec community. I can understand his opinion, but it is very difficult to
> modify that kdump is called from panic_notifier. Because it has a reason why
> kdump don't use panic_notifier. So, I made this patch.
>
> Please do something about this problem.

Hmm.  Tricky.  These appear to be two code bases with a completely different
philosophy on what errors are being avoided.

The kexec on panic assumption is that the kernel is broken and we better not
touch it something horrible has gone wrong.  And this is the reason why
kexec on panic is replacing lkcd.  Because the strong assumption results
in more errors getting captured with less likely hood of messing up your
system.

The kdb assumption appears to be that the kernel is mostly ok, and that there
are just some specific thing that is wrong.

The easiest way I can think to resolve this is for kdb to simply set
a break point at the entry point of panic() when it initializes.  Then
it wouldn't even need to be on the panic_list.  That approach would probably
even give better debug information because you would not have the effects
of bust_spinlocks to undo.

Is there some reason why kdb doesn't want to hook panic with a some
kind of break point?

Eric

Re: [patch] add kdump_after_notifier

[Takenori Nagano]

Eric W. Biederman wrote:
> Takenori Nagano <t-nagano@ah.jp.nec.com> writes:
>> Then I gave up to merge my patch to kdb, and I tried to send another patch to
>> kexec community. I can understand his opinion, but it is very difficult to
>> modify that kdump is called from panic_notifier. Because it has a reason why
>> kdump don't use panic_notifier. So, I made this patch.
>>
>> Please do something about this problem.
> 
> Hmm.  Tricky.  These appear to be two code bases with a completely different
> philosophy on what errors are being avoided.
> 
> The kexec on panic assumption is that the kernel is broken and we better not
> touch it something horrible has gone wrong.  And this is the reason why
> kexec on panic is replacing lkcd.  Because the strong assumption results
> in more errors getting captured with less likely hood of messing up your
> system.
> 
> The kdb assumption appears to be that the kernel is mostly ok, and that there
> are just some specific thing that is wrong.

Yes, kdump and kdb have a completely different philosophy. But it's natural,
because their duties are different.

I think kdb is a supplementary debug means. kdump is not perfect, because
hardware sometimes breaks down. The probability that hardware (HDD, HBA, memory,
etc...) breaks down is very low, but it is not zero. If kdump fails taking a
dump, kdb data (process status, backtrace, log buffer, etc...) is very useful to
analyze the panic reason. kdb data is very poor in comparison with kdump, but
better than nothing.

So I request a favor of you again, please do something about this problem.

> The easiest way I can think to resolve this is for kdb to simply set
> a break point at the entry point of panic() when it initializes.  Then
> it wouldn't even need to be on the panic_list.  That approach would probably
> even give better debug information because you would not have the effects
> of bust_spinlocks to undo.
> 
> Is there some reason why kdb doesn't want to hook panic with a some
> kind of break point?

I think there is no technical reason. But panic code will be dirty if every
kernel developers adds their own hook. I think this is a reason why kdb uses
panic_notifier.

Thanks,

Re: [patch] add kdump_after_notifier

[Vivek Goyal]

On Wed, Aug 01, 2007 at 04:00:48AM -0600, Eric W. Biederman wrote:
> Takenori Nagano <t-nagano@ah.jp.nec.com> writes:
> 
> >> No.  The problem with your patch is that it doesn't have a code
> >> impact.  We need to see who is using this and why.
> >
> > My motivation is very simple. I want to use both kdb and kdump, but I think it
> > is too weak to satisfy kexec guys. Then I brought up the example enterprise
> > software. But it isn't a lie. I know some drivers which use panic_notifier.
> > IMHO, they use only major distribution, and they has the workaround or they
> > don't notice this problem yet. I think they will be in trouble if all
> > distributions choose only kdump.
> 
> Possibly.
> 
> > BTW, I use kdb and lkcd now, but I want to use kdb and kdump. I sent a patch to
> > kdb community but it was rejected. kdb maintainer Keith Owens said,
> 
> >> Both KDB and crash_kexec should be using the panic_notifier_chain, with
> >> KDB having a higher priority than crash_exec.  The whole point of
> >> notifier chains is to handle cases like this, so we should not be
> >> adding more code to the panic routine.
> >>
> >> The real problem here is the way that the crash_exec code is hard coded
> >> into various places instead of using notifier chains.  The same issue
> >> exists in arch/ia64/kernel/mca.c because of bad coding practices from
> >> kexec.
> 
> I respectfully disagree with his opinion, as using notifier chains
> assumes more of the kernel works.  Although following it's argument
> to it's logical conclusion we should call crash_kexec as the very
> first thing inside of panic.  Given how much state something like
> bust_spinlocks messes up that might not be a bad idea.
> 
> It does make adding an alternative debug mechanism in there difficult.
> Does anyone know if this also affects kgdb?
> 
> > Then I gave up to merge my patch to kdb, and I tried to send another patch to
> > kexec community. I can understand his opinion, but it is very difficult to
> > modify that kdump is called from panic_notifier. Because it has a reason why
> > kdump don't use panic_notifier. So, I made this patch.
> >
> > Please do something about this problem.
> 
> Hmm.  Tricky.  These appear to be two code bases with a completely different
> philosophy on what errors are being avoided.
> 
> The kexec on panic assumption is that the kernel is broken and we better not
> touch it something horrible has gone wrong.  And this is the reason why
> kexec on panic is replacing lkcd.  Because the strong assumption results
> in more errors getting captured with less likely hood of messing up your
> system.
> 
> The kdb assumption appears to be that the kernel is mostly ok, and that there
> are just some specific thing that is wrong.
> 

Thinking more about it. So basically there are two kind of users. One who
believe that despite the kernel has crashed  something meaningful can
be done. In fact kernel also thinks so. That's why we have created
panic_notifier_list and even exported it to modules and now we have some
users. These users most of the time do non-disruptive activities and
can co-exist.

OTOH, we have kexec on panic, which thinks that once kernel is crashed
nothing meaningful can be done and it is disruptive and can't co-exist
with other users.

Some thoughts on possible solutions for this problem.

- Stop exporting panic_notifier_list list to modules. Audit the in kernel
  users of panic_notifier_list. Let crash_kexec() run once all other users
  of panic_notifier_list have been executed. This has fall side of breaking
  down external modules using panic_notifier_list and at the same time
  there is no gurantee that audited code will not run into the issues.

- Continue with existing policy. If kdump is configured, panic_notifier_list
  notifications will not be invoked. Any post panic action should be executed
  in second kernel. There might be 1-2 odd cases like in kernel debugger
  which still needs to be invoked in first kernel. These users should
  explicitly put hooks in panic() routine and refrain from using
  panic_notifier list.

  One thing to keep in mind, doing things in second kernel might not be easy
  as we have lost all the config data of the first kernel. For example,
  if one wants to send a kernel crash event over network to a system
  management software, he might have to pack in lot of software in 
  second kernel's initrd.

- Let the user decide if he wants to run panic_notifier_list after the
  crash or not with the help of a /proc option as suggested by the
  Takenori's patch. Fall side is, on what basis an enterprise user will
  take a decision whether he wants to run the notifiers or not. My gut
  feeling is that distro will end up setting this parameter as 1 by default,
  which would mean first run panic notifiers and then run crash_kexec().

- Make crash_kexec() a user of panic_notifier_list and let it run after all
  the callback handlers have run. This will invariably reduce the reliability
  of kdump.  

Personally I believe that second solution should bring us best of both
the worlds. Making sure post panic actions can be done more reliably at
the same time making sure reliability of kdump is not compromised.

Keith, do you see a value in second solution and would there be any
reason why kdb hook can not be explicitly placed in panic(). There will
not be many users like kdb. Rest of the users should end up performing
post panic actions in second kernel. 

Solutoin 3, can prove to be a stop gap solution but I think this will
make situation confusing for customers at the same time everybody will
try to take short route of performing post panic operations in first kernel.

Thanks
Vivek

Re: [patch] add kdump_after_notifier

[Keith Owens]

Vivek Goyal (on Thu, 2 Aug 2007 16:58:52 +0530) wrote:
>On Wed, Aug 01, 2007 at 04:00:48AM -0600, Eric W. Biederman wrote:
>> Takenori Nagano <t-nagano@ah.jp.nec.com> writes:
>> 
>> >> No.  The problem with your patch is that it doesn't have a code
>> >> impact.  We need to see who is using this and why.
>> >
>> > My motivation is very simple. I want to use both kdb and kdump, but I think it
>> > is too weak to satisfy kexec guys. Then I brought up the example enterprise
>> > software. But it isn't a lie. I know some drivers which use panic_notifier.
>> > IMHO, they use only major distribution, and they has the workaround or they
>> > don't notice this problem yet. I think they will be in trouble if all
>> > distributions choose only kdump.
>> 
>> Possibly.
>> 
>> > BTW, I use kdb and lkcd now, but I want to use kdb and kdump. I sent a patch to
>> > kdb community but it was rejected. kdb maintainer Keith Owens said,
>> 
>> >> Both KDB and crash_kexec should be using the panic_notifier_chain, with
>> >> KDB having a higher priority than crash_exec.  The whole point of
>> >> notifier chains is to handle cases like this, so we should not be
>> >> adding more code to the panic routine.
>> >>
>> >> The real problem here is the way that the crash_exec code is hard coded
>> >> into various places instead of using notifier chains.  The same issue
>> >> exists in arch/ia64/kernel/mca.c because of bad coding practices from
>> >> kexec.
>> 
>> I respectfully disagree with his opinion, as using notifier chains
>> assumes more of the kernel works.  Although following it's argument
>> to it's logical conclusion we should call crash_kexec as the very
>> first thing inside of panic.  Given how much state something like
>> bust_spinlocks messes up that might not be a bad idea.
>> 
>> It does make adding an alternative debug mechanism in there difficult.
>> Does anyone know if this also affects kgdb?
>> 
>> > Then I gave up to merge my patch to kdb, and I tried to send another patch to
>> > kexec community. I can understand his opinion, but it is very difficult to
>> > modify that kdump is called from panic_notifier. Because it has a reason why
>> > kdump don't use panic_notifier. So, I made this patch.
>> >
>> > Please do something about this problem.
>> 
>> Hmm.  Tricky.  These appear to be two code bases with a completely different
>> philosophy on what errors are being avoided.
>> 
>> The kexec on panic assumption is that the kernel is broken and we better not
>> touch it something horrible has gone wrong.  And this is the reason why
>> kexec on panic is replacing lkcd.  Because the strong assumption results
>> in more errors getting captured with less likely hood of messing up your
>> system.
>> 
>> The kdb assumption appears to be that the kernel is mostly ok, and that there
>> are just some specific thing that is wrong.
>> 
>
>Thinking more about it. So basically there are two kind of users. One who
>believe that despite the kernel has crashed  something meaningful can
>be done. In fact kernel also thinks so. That's why we have created
>panic_notifier_list and even exported it to modules and now we have some
>users. These users most of the time do non-disruptive activities and
>can co-exist.
>
>OTOH, we have kexec on panic, which thinks that once kernel is crashed
>nothing meaningful can be done and it is disruptive and can't co-exist
>with other users.
>
>Some thoughts on possible solutions for this problem.
>
>- Stop exporting panic_notifier_list list to modules. Audit the in kernel
>  users of panic_notifier_list. Let crash_kexec() run once all other users
>  of panic_notifier_list have been executed. This has fall side of breaking
>  down external modules using panic_notifier_list and at the same time
>  there is no gurantee that audited code will not run into the issues.
>
>- Continue with existing policy. If kdump is configured, panic_notifier_list
>  notifications will not be invoked. Any post panic action should be executed
>  in second kernel. There might be 1-2 odd cases like in kernel debugger
>  which still needs to be invoked in first kernel. These users should
>  explicitly put hooks in panic() routine and refrain from using
>  panic_notifier list.
>
>  One thing to keep in mind, doing things in second kernel might not be easy
>  as we have lost all the config data of the first kernel. For example,
>  if one wants to send a kernel crash event over network to a system
>  management software, he might have to pack in lot of software in 
>  second kernel's initrd.
>
>- Let the user decide if he wants to run panic_notifier_list after the
>  crash or not with the help of a /proc option as suggested by the
>  Takenori's patch. Fall side is, on what basis an enterprise user will
>  take a decision whether he wants to run the notifiers or not. My gut
>  feeling is that distro will end up setting this parameter as 1 by default,
>  which would mean first run panic notifiers and then run crash_kexec().
>
>- Make crash_kexec() a user of panic_notifier_list and let it run after all
>  the callback handlers have run. This will invariably reduce the reliability
>  of kdump.  
>
>Personally I believe that second solution should bring us best of both
>the worlds. Making sure post panic actions can be done more reliably at
>the same time making sure reliability of kdump is not compromised.
>
>Keith, do you see a value in second solution and would there be any
>reason why kdb hook can not be explicitly placed in panic(). There will
>not be many users like kdb. Rest of the users should end up performing
>post panic actions in second kernel. 
>
>Solutoin 3, can prove to be a stop gap solution but I think this will
>make situation confusing for customers at the same time everybody will
>try to take short route of performing post panic operations in first kernel.
>
>Thanks
>Vivek

Do not concentrate on kdb alone.  The problem above applies to all the
RAS tools, not just kdb.

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 proposed and wrote most of this common interface against 2.6.19-rc5.
See http://marc.info/?l=linux-arch&w=2&r=1&s=crash_stop&q=b, look for
crash_stop.  The crash_stop interface stops all the cpus, saves the
system state in a common format then runs an ordered list of RAS tools.

The order that the RAS tools are run depends on the priority value that
each tool passes to register_die_notifier.  Currently each RAS tool
hard codes its priority but it is trivial to change the tools to make
that priority a parameter, passing the policy decision back to the
user, not the kernel.

Despite having written the code and put it up for comments, the only
feedback I got was from Vivek saying "So I think crash dump will be a
little special case".  kdump is a special case whose priority is hard
wired into the kernel, so of course people are going to argue about the
coexistence of kdump with the other RAS tools.  Unless the kdump
developers agree to some flexibility, this thread will not be resolved
to anybody's satisfaction.  Use a common interface with no special
cases and let the user decide which tools to run and in which order.

The main objection raised against crash_stop is that it will not work
if the kernel stack has overflowed.  That problem is also solvable, I
raised an RFC inside SGI that would detect stack overflow and still let
the cpu continue.  Again, no interest.  I will copy that proposal to
the list as a separate thread.

I have pretty well given up on RAS code in the Linux kernel.  Everybody
has different ideas, there is no overall plan and little interest from
Linus in getting RAS tools into the kernel.  We are just thrashing.

Re: [patch] add kdump_after_notifier

[Andrew Morton]

On Fri, 03 Aug 2007 14:05:47 +1000 Keith Owens <kaos@sgi.com> wrote:

> I have pretty well given up on RAS code in the Linux kernel.  Everybody
> has different ideas, there is no overall plan and little interest from
> Linus in getting RAS tools into the kernel.  We are just thrashing.

Lots of different groups, little commonality in their desired funtionality,
little interest in sharing infrastructure or concepts.  Sometimes people
need a bit of motivational help.

In this case that motivation would come from the understanding that all the
RAS tools would be *required* to use such infrastructure if it was merged. 
Going off and open-coding your own stuff would henceforth not be acceptable.
If it turns out that it really was unsuitable for a particular group's RAS
feature, and we merged it anyway, well, that mismatch is that group's
fault.

It was a sizeable mistake to send those patches to a few obscure mailing
lists - this is the first I've heard of it, for example.

So.  Please, send it all again, copy the correct lists and people, make sure
that at least one client of the infrastructure is wired up and working (ideally,
all such in-kernel clients should be wired up) and let's take a look at it.

Much of the onus is upon the various RAS tool developers to demonstrate why it
is unsuitable for their use and, hopefully, to explain how it can be fixed for
them.

Re: [patch] add kdump_after_notifier

[Keith Owens]

Andrew Morton (on Thu, 2 Aug 2007 23:25:02 -0700) wrote:
>On Fri, 03 Aug 2007 14:05:47 +1000 Keith Owens <kaos@sgi.com> wrote:

Switching to kaos@ocs.com.au, I just resigned from SGI.

>> I have pretty well given up on RAS code in the Linux kernel.  Everybody
>> has different ideas, there is no overall plan and little interest from
>> Linus in getting RAS tools into the kernel.  We are just thrashing.
>
>Lots of different groups, little commonality in their desired funtionality,
>little interest in sharing infrastructure or concepts.  Sometimes people
>need a bit of motivational help.
>
>In this case that motivation would come from the understanding that all the
>RAS tools would be *required* to use such infrastructure if it was merged. 
>Going off and open-coding your own stuff would henceforth not be acceptable.
>If it turns out that it really was unsuitable for a particular group's RAS
>feature, and we merged it anyway, well, that mismatch is that group's
>fault.
>
>It was a sizeable mistake to send those patches to a few obscure mailing
>lists - this is the first I've heard of it, for example.

linux-arch is obscure??  Where else do you send patches that affect
multiple architectures?

>So.  Please, send it all again, copy the correct lists and people, make sure
>that at least one client of the infrastructure is wired up and working (ideally,
>all such in-kernel clients should be wired up) and let's take a look at it.

Already tried that.  The only RAS tool that is currently in the kernel is
kexec/kdump and they insist on doing things their own way.  That makes
it impossible to put a common RAS structure in place, because kexec
will not use it.

Sorry to keep beating on this drum, but kexec insist that their code
must have priority and that they do not trust the rest of the kernel.
Until that changes, there is no point is discussing how to make kexec
coexist with other RAS tools.  If kexec change their mind then we can
look at using a common RAS interface, otherwise it is a waste of time
and I have better things to do with my life.

Re: [patch] add kdump_after_notifier

[Andrew Morton]

On Fri, 03 Aug 2007 16:34:04 +1000 Keith Owens <kaos@ocs.com.au> wrote:

> Andrew Morton (on Thu, 2 Aug 2007 23:25:02 -0700) wrote:
> >On Fri, 03 Aug 2007 14:05:47 +1000 Keith Owens <kaos@sgi.com> wrote:
> 
> Switching to kaos@ocs.com.au, I just resigned from SGI.
> 
> >> I have pretty well given up on RAS code in the Linux kernel.  Everybody
> >> has different ideas, there is no overall plan and little interest from
> >> Linus in getting RAS tools into the kernel.  We are just thrashing.
> >
> >Lots of different groups, little commonality in their desired funtionality,
> >little interest in sharing infrastructure or concepts.  Sometimes people
> >need a bit of motivational help.
> >
> >In this case that motivation would come from the understanding that all the
> >RAS tools would be *required* to use such infrastructure if it was merged. 
> >Going off and open-coding your own stuff would henceforth not be acceptable.
> >If it turns out that it really was unsuitable for a particular group's RAS
> >feature, and we merged it anyway, well, that mismatch is that group's
> >fault.
> >
> >It was a sizeable mistake to send those patches to a few obscure mailing
> >lists - this is the first I've heard of it, for example.
> 
> linux-arch is obscure?? 

Exceedingly.  It's a way of contacting arch maintainers, that's all.  It
isn't really a place to discuss new infrastructural concepts which affect
multiple features.

> Where else do you send patches that affect
> multiple architectures?

This should have gone to linux-kernel.

> >So.  Please, send it all again, copy the correct lists and people, make sure
> >that at least one client of the infrastructure is wired up and working (ideally,
> >all such in-kernel clients should be wired up) and let's take a look at it.
> 
> Already tried that.  The only RAS tool that is currently in the kernel is
> kexec/kdump and they insist on doing things their own way.  That makes
> it impossible to put a common RAS structure in place, because kexec
> will not use it.

eh, write the patch for them, let's look at how much impact it is likely to
have.

> Sorry to keep beating on this drum, but kexec insist that their code
> must have priority and that they do not trust the rest of the kernel.
> Until that changes, there is no point is discussing how to make kexec
> coexist with other RAS tools.  If kexec change their mind then we can
> look at using a common RAS interface, otherwise it is a waste of time
> and I have better things to do with my life.

I saw one email from Vivek expressing on-general-principle concerns.  It
was hardly thorough or irreconcilable-looking.

Let's drag this thing into the daylight and poke at it a bit.

Re: [patch] add kdump_after_notifier

[Eric W. Biederman]

Andrew Morton <akpm@linux-foundation.org> writes:
>
> Much of the onus is upon the various RAS tool developers to demonstrate why it
> is unsuitable for their use and, hopefully, to explain how it can be fixed for
> them.

My current take on the situation.

There are 4 different cases we care about.
- Trivial in kernel message failure reports. (Oops, backtraces and the like)
- Crash dumps.
- Debuggers.
- kernel Probes.

The in kernel failure messages seem to be doing a good job and are
reasonably simple to maintain.

For crash dumping we have sufficient infrastructure in the kernel now in
the kexec on panic work, and it is simpler and more reliable then the
previous attempts.  Although those kernel code paths could be made
simpler yet and probably should be.

Only when it comes to debuggers does it seem we don't have something
we can generally settle on and agree on.

All I know is that any set of code that wants to be common
infrastructure that makes the assumption that the kernel is mostly
not broken is not interesting for use when things are fully automated.
Because it fails to work in real world failure cases.  Those things
only work in the artificial testing environments of developers.

Right now I have seen so little to seriously address these real
world concerns in suggests or patches for some kind of infrastructure
that I'm tired of discussing it.  I admit I haven't seen or heard of those
patches either but even their description sounds non-interesting.

Eric

Re: [patch] add kdump_after_notifier

[Vivek Goyal]

On Fri, Aug 03, 2007 at 02:05:47PM +1000, Keith Owens wrote:

[..]
> >Some thoughts on possible solutions for this problem.
> >
> >- Stop exporting panic_notifier_list list to modules. Audit the in kernel
> >  users of panic_notifier_list. Let crash_kexec() run once all other users
> >  of panic_notifier_list have been executed. This has fall side of breaking
> >  down external modules using panic_notifier_list and at the same time
> >  there is no gurantee that audited code will not run into the issues.
> >
> >- Continue with existing policy. If kdump is configured, panic_notifier_list
> >  notifications will not be invoked. Any post panic action should be executed
> >  in second kernel. There might be 1-2 odd cases like in kernel debugger
> >  which still needs to be invoked in first kernel. These users should
> >  explicitly put hooks in panic() routine and refrain from using
> >  panic_notifier list.
> >
> >  One thing to keep in mind, doing things in second kernel might not be easy
> >  as we have lost all the config data of the first kernel. For example,
> >  if one wants to send a kernel crash event over network to a system
> >  management software, he might have to pack in lot of software in 
> >  second kernel's initrd.
> >
> >- Let the user decide if he wants to run panic_notifier_list after the
> >  crash or not with the help of a /proc option as suggested by the
> >  Takenori's patch. Fall side is, on what basis an enterprise user will
> >  take a decision whether he wants to run the notifiers or not. My gut
> >  feeling is that distro will end up setting this parameter as 1 by default,
> >  which would mean first run panic notifiers and then run crash_kexec().
> >
> >- Make crash_kexec() a user of panic_notifier_list and let it run after all
> >  the callback handlers have run. This will invariably reduce the reliability
> >  of kdump.  
> >
> >Personally I believe that second solution should bring us best of both
> >the worlds. Making sure post panic actions can be done more reliably at
> >the same time making sure reliability of kdump is not compromised.
> >
> >Keith, do you see a value in second solution and would there be any
> >reason why kdb hook can not be explicitly placed in panic(). There will
> >not be many users like kdb. Rest of the users should end up performing
> >post panic actions in second kernel. 
> >
> >Solutoin 3, can prove to be a stop gap solution but I think this will
> >make situation confusing for customers at the same time everybody will
> >try to take short route of performing post panic operations in first kernel.
> >
> >Thanks
> >Vivek
> 
> Do not concentrate on kdb alone.  The problem above applies to all the
> RAS tools, not just kdb.
> 
> 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.
> 

Hi Keith,

Few thoughts. So there are two things there.

- Create a common infrastructure which can be used by various RAS
  tools and common functionality is not duplicated. For ex. functionality
  for stopping cpus, saving register states etc.

- Create a infrastructure so that user can enforce the policy regarding
  in what order various RAS tools should run.

Last time patches did more of first thing. It had put lots of code
and the only user of that code was kexec/kdump. Sometime motivation level
is low regarding why to put so much of infrastructure code in if there
are no users and it can potentially bring down the reliability of kdump.

> 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 proposed and wrote most of this common interface against 2.6.19-rc5.
> See http://marc.info/?l=linux-arch&w=2&r=1&s=crash_stop&q=b, look for
> crash_stop.  The crash_stop interface stops all the cpus, saves the
> system state in a common format then runs an ordered list of RAS tools.
> 

Agreed. It would be great if during next posting one can also post the
patches for exporting the policy decision to user space.

Having said that, who do you think are potential RAS tools among
which a user has to make a choice while determining the order. I
can think of only two. Crash dump and debugger. 

> The order that the RAS tools are run depends on the priority value that
> each tool passes to register_die_notifier.  Currently each RAS tool
> hard codes its priority but it is trivial to change the tools to make
> that priority a parameter, passing the policy decision back to the
> user, not the kernel.
> 

I have got some concerns here and that's what precisely the core
point where this mail thread started. What does one do with die
notifier and panic notifer list? Should RAS tools like debugger
and crash dumpers register on this list and wait for their turn
to execute?

There are many other in kernel users who register on die or panic
notifier list. Like heartbeat, IPMI,..... Given the fact that
these symbols are exported, there will be proprietary users also
for whom we never get a chance to see their code.

Can one really trust all these users and let them run before
any RAS tool gets the control? After the kernel crash, there
is no gurantee regarding how many of these actions can be
successfully completed. Probably in case of in kernel debuggers
one can afford to do that but I think in case of kdump one
should not as all these actions can be reliably performed
in second kernel.

To sum up, couple of options come to mind.

- Register all the RAS tools on die notifier and panic
  notifier lists with fairly high priority. Export list
  of RAS tools to user space and allow users to decide the
  order of execution and priority of RAS tools.

- Create a separate RAS tool notifier list (ras_tool_notifer_list).
  All the RAS tools register on this list. This list gets priority
  over die or panic notifier list. User decides the oder of execution
  of RAS tools. 

  Here assumption is that above list will not be exported to modules.
  All the RAS tools will be in kernel and they always get a priority
  to inspect an event.

What do others think?

Thanks
Vivek

Re: [patch] add kdump_after_notifier

[Takenori Nagano]

Vivek Goyal wrote:
> On Fri, Aug 03, 2007 at 02:05:47PM +1000, Keith Owens wrote:
> 
> 
> To sum up, couple of options come to mind.
> 
> - Register all the RAS tools on die notifier and panic
>   notifier lists with fairly high priority. Export list
>   of RAS tools to user space and allow users to decide the
>   order of execution and priority of RAS tools.
> 
> - Create a separate RAS tool notifier list (ras_tool_notifer_list).
>   All the RAS tools register on this list. This list gets priority
>   over die or panic notifier list. User decides the oder of execution
>   of RAS tools. 
> 
>   Here assumption is that above list will not be exported to modules.
>   All the RAS tools will be in kernel and they always get a priority
>   to inspect an event.
> 
> What do others think?

Very good idea. But there is a problem how to give default priority to RAS tools.

How about priority changeable notifier_list? User can change list order
dynamically if they want. Of course, we have to give highest priority to kdump
by default. It is very useful for users who want to use some RAS tools.

Re: [patch] add kdump_after_notifier

[Bernhard Walle]

* Takenori Nagano <t-nagano@ah.jp.nec.com> [2007-08-14 10:34]:
> Vivek Goyal wrote:
> > On Fri, Aug 03, 2007 at 02:05:47PM +1000, Keith Owens wrote:
> > 
> > 
> > To sum up, couple of options come to mind.
> > 
> > - Register all the RAS tools on die notifier and panic
> >   notifier lists with fairly high priority. Export list
> >   of RAS tools to user space and allow users to decide the
> >   order of execution and priority of RAS tools.
> > 
> > - Create a separate RAS tool notifier list (ras_tool_notifer_list).
> >   All the RAS tools register on this list. This list gets priority
> >   over die or panic notifier list. User decides the oder of execution
> >   of RAS tools. 
> > 
> >   Here assumption is that above list will not be exported to modules.
> >   All the RAS tools will be in kernel and they always get a priority
> >   to inspect an event.
> > 
> > What do others think?
> 
> Very good idea. But there is a problem how to give default priority to RAS tools.
> 
> How about priority changeable notifier_list? User can change list order
> dynamically if they want. Of course, we have to give highest priority to kdump
> by default. It is very useful for users who want to use some RAS tools.

I think that was the idea of the first “-” (“export list of RAS tools
to user space”).


Thanks,
   Bernhard

Re: [patch] add kdump_after_notifier

[Takenori Nagano]

Bernhard Walle wrote:
> * Takenori Nagano <t-nagano@ah.jp.nec.com> [2007-08-14 10:34]:
>> Vivek Goyal wrote:
>>> On Fri, Aug 03, 2007 at 02:05:47PM +1000, Keith Owens wrote:
>>>
>>>
>>> To sum up, couple of options come to mind.
>>>
>>> - Register all the RAS tools on die notifier and panic
>>>   notifier lists with fairly high priority. Export list
>>>   of RAS tools to user space and allow users to decide the
>>>   order of execution and priority of RAS tools.
>>>
>>> - Create a separate RAS tool notifier list (ras_tool_notifer_list).
>>>   All the RAS tools register on this list. This list gets priority
>>>   over die or panic notifier list. User decides the oder of execution
>>>   of RAS tools. 
>>>
>>>   Here assumption is that above list will not be exported to modules.
>>>   All the RAS tools will be in kernel and they always get a priority
>>>   to inspect an event.
>>>
>>> What do others think?
>> Very good idea. But there is a problem how to give default priority to RAS tools.
>>
>> How about priority changeable notifier_list? User can change list order
>> dynamically if they want. Of course, we have to give highest priority to kdump
>> by default. It is very useful for users who want to use some RAS tools.
> 
> I think that was the idea of the first “-” (“export list of RAS tools
> to user space”).

Ah, sorry.

I think first idea is very good. How export the list? (sysfs? procfs?)

Re: [patch] add kdump_after_notifier

[Bernhard Walle]

* Takenori Nagano <t-nagano@ah.jp.nec.com> [2007-08-14 10:48]:
> Bernhard Walle wrote:
> > * Takenori Nagano <t-nagano@ah.jp.nec.com> [2007-08-14 10:34]:
> >> Vivek Goyal wrote:
> >>> On Fri, Aug 03, 2007 at 02:05:47PM +1000, Keith Owens wrote:
> >>>
> >>>
> >>> To sum up, couple of options come to mind.
> >>>
> >>> - Register all the RAS tools on die notifier and panic
> >>>   notifier lists with fairly high priority. Export list
> >>>   of RAS tools to user space and allow users to decide the
> >>>   order of execution and priority of RAS tools.
> >>>
> >>> - Create a separate RAS tool notifier list (ras_tool_notifer_list).
> >>>   All the RAS tools register on this list. This list gets priority
> >>>   over die or panic notifier list. User decides the oder of execution
> >>>   of RAS tools. 
> >>>
> >>>   Here assumption is that above list will not be exported to modules.
> >>>   All the RAS tools will be in kernel and they always get a priority
> >>>   to inspect an event.
> >>>
> >>> What do others think?
> >> Very good idea. But there is a problem how to give default priority to RAS tools.
> >>
> >> How about priority changeable notifier_list? User can change list order
> >> dynamically if they want. Of course, we have to give highest priority to kdump
> >> by default. It is very useful for users who want to use some RAS tools.
> > 
> > I think that was the idea of the first “-” (“export list of RAS tools
> > to user space”).
> 
> Ah, sorry.
> 
> I think first idea is very good. How export the list? (sysfs? procfs?)

I think sysfs would be a good solution, e.g. assigning each RAS tool a
priority from 0 to 100 or something like this.


Thanks,
   Bernhard

Re: [patch] add kdump_after_notifier

[Vivek Goyal]

On Tue, Aug 14, 2007 at 10:37:10AM +0200, Bernhard Walle wrote:
> * Takenori Nagano <t-nagano@ah.jp.nec.com> [2007-08-14 10:34]:
> > Vivek Goyal wrote:
> > > On Fri, Aug 03, 2007 at 02:05:47PM +1000, Keith Owens wrote:
> > > 
> > > 
> > > To sum up, couple of options come to mind.
> > > 
> > > - Register all the RAS tools on die notifier and panic
> > >   notifier lists with fairly high priority. Export list
> > >   of RAS tools to user space and allow users to decide the
> > >   order of execution and priority of RAS tools.
> > > 
> > > - Create a separate RAS tool notifier list (ras_tool_notifer_list).
> > >   All the RAS tools register on this list. This list gets priority
> > >   over die or panic notifier list. User decides the oder of execution
> > >   of RAS tools. 
> > > 
> > >   Here assumption is that above list will not be exported to modules.
> > >   All the RAS tools will be in kernel and they always get a priority
> > >   to inspect an event.
> > > 
> > > What do others think?
> > 
> > Very good idea. But there is a problem how to give default priority to RAS tools.
> > 
> > How about priority changeable notifier_list? User can change list order
> > dynamically if they want. Of course, we have to give highest priority to kdump
> > by default. It is very useful for users who want to use some RAS tools.
> 
> I think that was the idea of the first “-” (“export list of RAS tools
> to user space”).
> 

So for the time being I think we can put RAS tools on die notifier list
and if it runs into issues we can always think of creating a separate list.

Few things come to mind.

- Why there is a separate panic_notifier_list? Can't it be merged with
  die_chain? die_val already got one of the event type as PANIC. If there
  are no specific reasons then we should merge the two lists. Registering
  RAS tools on a single list is easier.
- Modify Kdump to register on die_chain list. 
- Modify Kdb to register on die_chain list.
- Export all the registered members of die_chain through sysfs along with
  their priorities. Priorities should be modifiable. Most likely one 
  shall have to introduce additional field in struct notifier_block. This
  field will be a string as an identifier of the user registerd. e.g
  "Kdump", "Kdb" etc.

Now user will be able to view all the die_chain users through sysfs and
be able to modify the order in which these should run by modifying their
priority. Hence all the RAS tools can co-exist.

Any thoughts?

Thanks
Vivek




> 
> Thanks,
>    Bernhard

Re: [patch] add kdump_after_notifier

[Takenori Nagano]

Vivek Goyal wrote:
 > So for the time being I think we can put RAS tools on die notifier list
> and if it runs into issues we can always think of creating a separate list.
> 
> Few things come to mind.
> 
> - Why there is a separate panic_notifier_list? Can't it be merged with
>   die_chain? die_val already got one of the event type as PANIC. If there
>   are no specific reasons then we should merge the two lists. Registering
>   RAS tools on a single list is easier.

I think it is difficult, because die_chain is defined by each architecture.

> - Modify Kdump to register on die_chain list. 
> - Modify Kdb to register on die_chain list.
> - Export all the registered members of die_chain through sysfs along with
>   their priorities. Priorities should be modifiable. Most likely one 
>   shall have to introduce additional field in struct notifier_block. This
>   field will be a string as an identifier of the user registerd. e.g
>   "Kdump", "Kdb" etc.
> 
> Now user will be able to view all the die_chain users through sysfs and
> be able to modify the order in which these should run by modifying their
> priority. Hence all the RAS tools can co-exist.

This is my image of your proposal.

- Print current order

# cat /sys/class/misc/debug/panic_notifier_list
priority   name
1          IPMI
2          watchdog
3          Kdb
4          Kdump

- change list order

# echo <before> <after> > /sys/class/debug/panic_notifier_list

- example

# cat /sys/class/misc/debug/panic_notifier_list
priority   name
1          IPMI
2          watchdog
3          Kdb
4          Kdump

# echo 4 1 > /sys/class/debug/panic_notifier_list
priority   name
1          Kdump
2          IPMI
3          watchdog
4          Kdb


Is my image almost same as your thought?

Thanks

Re: [patch] add kdump_after_notifier

[Bernhard Walle]

* Takenori Nagano <t-nagano@ah.jp.nec.com> [2007-08-16 11:26]:
> Vivek Goyal wrote:
> > - Modify Kdump to register on die_chain list. 
> > - Modify Kdb to register on die_chain list.
> > - Export all the registered members of die_chain through sysfs along with
> >   their priorities. Priorities should be modifiable. Most likely one 
> >   shall have to introduce additional field in struct notifier_block. This
> >   field will be a string as an identifier of the user registerd. e.g
> >   "Kdump", "Kdb" etc.
> > 
> > Now user will be able to view all the die_chain users through sysfs and
> > be able to modify the order in which these should run by modifying their
> > priority. Hence all the RAS tools can co-exist.
> 
> This is my image of your proposal.
> 
> - Print current order
> 
> # cat /sys/class/misc/debug/panic_notifier_list
> priority   name
> 1          IPMI
> 2          watchdog
> 3          Kdb
> 4          Kdump

I think something like

/sys/class/misc/debug/panic_notifier/ipmi/priority
/sys/class/misc/debug/panic_notifier/watchdog/priority
/sys/class/misc/debug/panic_notifier/kdb/priority
/sys/class/misc/debug/panic_notifier/kdump/priority

would be better. It's more self-describing if you want to change the
order.



Thanks,
   Bernhard

Re: [patch] add kdump_after_notifier

[Vivek Goyal]

On Thu, Aug 16, 2007 at 06:26:35PM +0900, Takenori Nagano wrote:
> Vivek Goyal wrote:
>  > So for the time being I think we can put RAS tools on die notifier list
> > and if it runs into issues we can always think of creating a separate list.
> > 
> > Few things come to mind.
> > 
> > - Why there is a separate panic_notifier_list? Can't it be merged with
> >   die_chain? die_val already got one of the event type as PANIC. If there
> >   are no specific reasons then we should merge the two lists. Registering
> >   RAS tools on a single list is easier.
> 
> I think it is difficult, because die_chain is defined by each architecture.
> 

I think die_chain is arch independent definition (kernel/die_notifier.c)? 
But anyway, to begin with it can be done only for panic_notifier.

> > - Modify Kdump to register on die_chain list. 
> > - Modify Kdb to register on die_chain list.
> > - Export all the registered members of die_chain through sysfs along with
> >   their priorities. Priorities should be modifiable. Most likely one 
> >   shall have to introduce additional field in struct notifier_block. This
> >   field will be a string as an identifier of the user registerd. e.g
> >   "Kdump", "Kdb" etc.
> > 
> > Now user will be able to view all the die_chain users through sysfs and
> > be able to modify the order in which these should run by modifying their
> > priority. Hence all the RAS tools can co-exist.
> 
> This is my image of your proposal.
> 
> - Print current order
> 
> # cat /sys/class/misc/debug/panic_notifier_list
> priority   name
> 1          IPMI
> 2          watchdog
> 3          Kdb
> 4          Kdump
> 

I think Bernhard's suggestion looks better here. I noticed that 
/sys/kernel/debug is already present. So how about following.

/sys/kernel/debug/kdump/priority
/sys/kernel/debug/kdb/priority
/sys/kernel/debug/IPMI/priority

I think at some point of time we shall have to create another file say
description.

/sys/kernel/debug/IPMI/description

Which can tell what does this tool do? Other a user might not have any
clue how to prioritize various things.

Thanks
Vivek

Re: [patch] add kdump_after_notifier

[Takenori Nagano]

Vivek Goyal wrote:
> On Thu, Aug 16, 2007 at 06:26:35PM +0900, Takenori Nagano wrote:
>> Vivek Goyal wrote:
>>  > So for the time being I think we can put RAS tools on die notifier list
>>> and if it runs into issues we can always think of creating a separate list.
>>>
>>> Few things come to mind.
>>>
>>> - Why there is a separate panic_notifier_list? Can't it be merged with
>>>   die_chain? die_val already got one of the event type as PANIC. If there
>>>   are no specific reasons then we should merge the two lists. Registering
>>>   RAS tools on a single list is easier.
>> I think it is difficult, because die_chain is defined by each architecture.
>>
> 
> I think die_chain is arch independent definition (kernel/die_notifier.c)? 
> But anyway, to begin with it can be done only for panic_notifier.

I think die_val (notify_die() argument) values are arch independent.
They are defined in include/asm-<arch>/kdebug.h.

Your idea is good, but I think it has very large impact. It is very hard to fix
them at the same time. So, how about putting off merging two lists?

 > I think Bernhard's suggestion looks better here. I noticed that
> /sys/kernel/debug is already present. So how about following.
> 
> /sys/kernel/debug/kdump/priority
> /sys/kernel/debug/kdb/priority
> /sys/kernel/debug/IPMI/priority

Good.

> 
> I think at some point of time we shall have to create another file say
> description.
> 
> /sys/kernel/debug/IPMI/description
> 
> Which can tell what does this tool do? Other a user might not have any
> clue how to prioritize various things.

Good idea. :-)

Thanks

Re: [patch] add kdump_after_notifier

[Vivek Goyal]

On Tue, Aug 21, 2007 at 04:45:02PM +0900, Takenori Nagano wrote:
> Vivek Goyal wrote:
> > On Thu, Aug 16, 2007 at 06:26:35PM +0900, Takenori Nagano wrote:
> >> Vivek Goyal wrote:
> >>  > So for the time being I think we can put RAS tools on die notifier list
> >>> and if it runs into issues we can always think of creating a separate list.
> >>>
> >>> Few things come to mind.
> >>>
> >>> - Why there is a separate panic_notifier_list? Can't it be merged with
> >>>   die_chain? die_val already got one of the event type as PANIC. If there
> >>>   are no specific reasons then we should merge the two lists. Registering
> >>>   RAS tools on a single list is easier.
> >> I think it is difficult, because die_chain is defined by each architecture.
> >>
> > 
> > I think die_chain is arch independent definition (kernel/die_notifier.c)? 
> > But anyway, to begin with it can be done only for panic_notifier.
> 
> I think die_val (notify_die() argument) values are arch independent.
> They are defined in include/asm-<arch>/kdebug.h.
> 
> Your idea is good, but I think it has very large impact. It is very hard to fix
> them at the same time. So, how about putting off merging two lists?
> 

I think that's fine. For the time being we can just export panic_notifier
list. die_chain is not a problem as of today as die notifiers get invoked
first and then if panic_on_oops is set, kdump is called. So all the RAS
tools registered on die_chain will get executed first.

Thanks
Vivek

Re: [patch] add kdump_after_notifier

[Jay Lan]

Vivek Goyal wrote:
> On Thu, Aug 16, 2007 at 06:26:35PM +0900, Takenori Nagano wrote:
>> Vivek Goyal wrote:
>>  > So for the time being I think we can put RAS tools on die notifier list
>>> and if it runs into issues we can always think of creating a separate list.
>>>
>>> Few things come to mind.
>>>
>>> - Why there is a separate panic_notifier_list? Can't it be merged with
>>>   die_chain? die_val already got one of the event type as PANIC. If there
>>>   are no specific reasons then we should merge the two lists. Registering
>>>   RAS tools on a single list is easier.
>> I think it is difficult, because die_chain is defined by each architecture.
>>
> 
> I think die_chain is arch independent definition (kernel/die_notifier.c)? 
> But anyway, to begin with it can be done only for panic_notifier.
> 
>>> - Modify Kdump to register on die_chain list. 
>>> - Modify Kdb to register on die_chain list.
>>> - Export all the registered members of die_chain through sysfs along with
>>>   their priorities. Priorities should be modifiable. Most likely one 
>>>   shall have to introduce additional field in struct notifier_block. This
>>>   field will be a string as an identifier of the user registerd. e.g
>>>   "Kdump", "Kdb" etc.
>>>
>>> Now user will be able to view all the die_chain users through sysfs and
>>> be able to modify the order in which these should run by modifying their
>>> priority. Hence all the RAS tools can co-exist.
>> This is my image of your proposal.
>>
>> - Print current order
>>
>> # cat /sys/class/misc/debug/panic_notifier_list
>> priority   name
>> 1          IPMI
>> 2          watchdog
>> 3          Kdb
>> 4          Kdump
>>
> 
> I think Bernhard's suggestion looks better here. I noticed that 
> /sys/kernel/debug is already present. So how about following.
> 
> /sys/kernel/debug/kdump/priority
> /sys/kernel/debug/kdb/priority
> /sys/kernel/debug/IPMI/priority

Why separate priority files is better than a central file?
At least i think you get a grand picture of priority being
defined for all parties with a central file?

What do we decide priority if more than one component has
the same priority value?

Thanks,
 - jay

> 
> I think at some point of time we shall have to create another file say
> description.
> 
> /sys/kernel/debug/IPMI/description
> 
> Which can tell what does this tool do? Other a user might not have any
> clue how to prioritize various things.
> 
> Thanks
> Vivek
> 

Re: [patch] add kdump_after_notifier

[Bernhard Walle]

* Jay Lan <jlan@sgi.com> [2007-08-21 15:18]:
> Vivek Goyal wrote:
> > On Thu, Aug 16, 2007 at 06:26:35PM +0900, Takenori Nagano wrote:
> >> Vivek Goyal wrote:
> >>  > So for the time being I think we can put RAS tools on die notifier list
> >>> and if it runs into issues we can always think of creating a separate list.
> >>>
> >>> Few things come to mind.
> >>>
> >>> - Why there is a separate panic_notifier_list? Can't it be merged with
> >>>   die_chain? die_val already got one of the event type as PANIC. If there
> >>>   are no specific reasons then we should merge the two lists. Registering
> >>>   RAS tools on a single list is easier.
> >> I think it is difficult, because die_chain is defined by each architecture.
> >>
> > 
> > I think die_chain is arch independent definition (kernel/die_notifier.c)? 
> > But anyway, to begin with it can be done only for panic_notifier.
> > 
> >>> - Modify Kdump to register on die_chain list. 
> >>> - Modify Kdb to register on die_chain list.
> >>> - Export all the registered members of die_chain through sysfs along with
> >>>   their priorities. Priorities should be modifiable. Most likely one 
> >>>   shall have to introduce additional field in struct notifier_block. This
> >>>   field will be a string as an identifier of the user registerd. e.g
> >>>   "Kdump", "Kdb" etc.
> >>>
> >>> Now user will be able to view all the die_chain users through sysfs and
> >>> be able to modify the order in which these should run by modifying their
> >>> priority. Hence all the RAS tools can co-exist.
> >> This is my image of your proposal.
> >>
> >> - Print current order
> >>
> >> # cat /sys/class/misc/debug/panic_notifier_list
> >> priority   name
> >> 1          IPMI
> >> 2          watchdog
> >> 3          Kdb
> >> 4          Kdump
> >>
> > 
> > I think Bernhard's suggestion looks better here. I noticed that 
> > /sys/kernel/debug is already present. So how about following.
> > 
> > /sys/kernel/debug/kdump/priority
> > /sys/kernel/debug/kdb/priority
> > /sys/kernel/debug/IPMI/priority
> 
> Why separate priority files is better than a central file?
> At least i think you get a grand picture of priority being
> defined for all parties with a central file?

Well, it's more intuitive to set the priority in that case. You don't
have to know a special syntax. However, it may be a good idea to
implement a second read-only file that lists the sorted priorities in
that order the kernel executues the handlers the handlers.

> What do we decide priority if more than one component has
> the same priority value?

You can check this and return EINVAL in that case.


Thanks,
   Bernhard

Re: [patch] add kdump_after_notifier

[Vivek Goyal]

On Tue, Aug 21, 2007 at 06:18:31AM -0700, Jay Lan wrote:
[..]
> >>> Now user will be able to view all the die_chain users through sysfs and
> >>> be able to modify the order in which these should run by modifying their
> >>> priority. Hence all the RAS tools can co-exist.
> >> This is my image of your proposal.
> >>
> >> - Print current order
> >>
> >> # cat /sys/class/misc/debug/panic_notifier_list
> >> priority   name
> >> 1          IPMI
> >> 2          watchdog
> >> 3          Kdb
> >> 4          Kdump
> >>
> > 
> > I think Bernhard's suggestion looks better here. I noticed that 
> > /sys/kernel/debug is already present. So how about following.
> > 
> > /sys/kernel/debug/kdump/priority
> > /sys/kernel/debug/kdb/priority
> > /sys/kernel/debug/IPMI/priority
> 
> Why separate priority files is better than a central file?
> At least i think you get a grand picture of priority being
> defined for all parties with a central file?
> 

I thought of couple of reasons.
- A very different syntax to modify the priority.
- Separate directories allow easy future extensions in terms of more
  files. For example, putting a small "description" file in each dir
  where each registered user can specify what does it do.

But I agree that a single file is good for consolidated view. As bernhard
suggested, may be we should also implement a read only file where one
will get a consolidated view.

> What do we decide priority if more than one component has
> the same priority value?
> 

I think first come first serve would be appropriate in this case instead of
returning -EINVAL.

Thanks
Vivek

Re: [patch] add kdump_after_notifier

[Jay Lan]

Vivek Goyal wrote:
> On Tue, Aug 21, 2007 at 06:18:31AM -0700, Jay Lan wrote:
> [..]
>>>>> Now user will be able to view all the die_chain users through sysfs and
>>>>> be able to modify the order in which these should run by modifying their
>>>>> priority. Hence all the RAS tools can co-exist.
>>>> This is my image of your proposal.
>>>>
>>>> - Print current order
>>>>
>>>> # cat /sys/class/misc/debug/panic_notifier_list
>>>> priority   name
>>>> 1          IPMI
>>>> 2          watchdog
>>>> 3          Kdb
>>>> 4          Kdump
>>>>
>>> I think Bernhard's suggestion looks better here. I noticed that 
>>> /sys/kernel/debug is already present. So how about following.
>>>
>>> /sys/kernel/debug/kdump/priority
>>> /sys/kernel/debug/kdb/priority
>>> /sys/kernel/debug/IPMI/priority
>> Why separate priority files is better than a central file?
>> At least i think you get a grand picture of priority being
>> defined for all parties with a central file?
>>
> 
> I thought of couple of reasons.
> - A very different syntax to modify the priority.
> - Separate directories allow easy future extensions in terms of more
>   files. For example, putting a small "description" file in each dir
>   where each registered user can specify what does it do.

The first can be easily resolved by providing a comment section in the
file with real examples. Users can simply uncomment a line to activate.
But future expansion is certainly is a good reason for this layout.

> 
> But I agree that a single file is good for consolidated view. As bernhard
> suggested, may be we should also implement a read only file where one
> will get a consolidated view.

Yep, this will help!

> 
>> What do we decide priority if more than one component has
>> the same priority value?
>>
> 
> I think first come first serve would be appropriate in this case instead of
> returning -EINVAL.

How does the kernel process the configuration files? By alphabetic order
of the filename? Either way, i think a clear failure/warning dmesg is
very important.

Thanks,
 - jay

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