2007-08-01 09:27:18

by Takenori Nagano

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

Eric W. Biederman wrote:
> Takenori Nagano <[email protected]> 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,


2007-08-01 10:04:59

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

Takenori Nagano <[email protected]> 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

2007-08-02 08:14:35

by Takenori Nagano

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

Eric W. Biederman wrote:
> Takenori Nagano <[email protected]> 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,

2007-08-02 11:28:58

by Vivek Goyal

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

On Wed, Aug 01, 2007 at 04:00:48AM -0600, Eric W. Biederman wrote:
> Takenori Nagano <[email protected]> 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

2007-08-03 04:06:23

by Keith Owens

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

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 <[email protected]> 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.

2007-08-03 06:26:20

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

On Fri, 03 Aug 2007 14:05:47 +1000 Keith Owens <[email protected]> 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.

2007-08-03 06:34:32

by Keith Owens

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

Andrew Morton (on Thu, 2 Aug 2007 23:25:02 -0700) wrote:
>On Fri, 03 Aug 2007 14:05:47 +1000 Keith Owens <[email protected]> wrote:

Switching to [email protected], 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.

2007-08-03 07:14:46

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

Andrew Morton <[email protected]> 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

2007-08-03 07:38:28

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

On Fri, 03 Aug 2007 16:34:04 +1000 Keith Owens <[email protected]> wrote:

> Andrew Morton (on Thu, 2 Aug 2007 23:25:02 -0700) wrote:
> >On Fri, 03 Aug 2007 14:05:47 +1000 Keith Owens <[email protected]> wrote:
>
> Switching to [email protected], 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.

2007-08-05 11:07:47

by Vivek Goyal

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

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

2007-08-14 08:35:52

by Takenori Nagano

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

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.

2007-08-14 08:37:38

by Bernhard Walle

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

* Takenori Nagano <[email protected]> [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

2007-08-14 08:49:55

by Takenori Nagano

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

Bernhard Walle wrote:
> * Takenori Nagano <[email protected]> [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 $B!H(B-$B!I(B ($B!H(Bexport list of RAS tools
> to user space$B!I(B).

Ah, sorry.

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

2007-08-14 08:53:56

by Bernhard Walle

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

* Takenori Nagano <[email protected]> [2007-08-14 10:48]:
> Bernhard Walle wrote:
> > * Takenori Nagano <[email protected]> [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

2007-08-14 13:25:10

by Vivek Goyal

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

On Tue, Aug 14, 2007 at 10:37:10AM +0200, Bernhard Walle wrote:
> * Takenori Nagano <[email protected]> [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

2007-08-16 09:27:50

by Takenori Nagano

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

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

2007-08-16 09:45:22

by Bernhard Walle

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

* Takenori Nagano <[email protected]> [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

2007-08-17 10:56:43

by Vivek Goyal

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

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

2007-08-21 07:45:54

by Takenori Nagano

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

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

2007-08-21 13:14:23

by Jay Lan

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

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
>

2007-08-21 13:21:36

by Bernhard Walle

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

* Jay Lan <[email protected]> [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

2007-08-23 03:52:23

by Vivek Goyal

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

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

2007-08-23 03:56:20

by Vivek Goyal

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

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

2007-08-23 17:34:46

by Jay Lan

[permalink] [raw]
Subject: Re: [patch] add kdump_after_notifier

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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kexec