On Mon, Jan 31, 2011 at 05:59:39PM -0500, Vivek Goyal wrote:
>
>Anyway, if an image is loaded and we have setup to capture dump also
>why do we need to save kmsg with the help of an helper. I am assuming
>this is more of a debugging aid if we have no other way to capture the
>kernel log buffer. So if somebody has setup kdump to capture the
>vmcore, why to call another handler which tries to save part of the
>vmcore (kmsg) separately.
>
Well, kmsg dumper is not only for panic/oops, it can also save kernel
messages when the system is reboot/halt.
Yeah, it is true that vmcore contains log_buf, but I think the users
of ramoops/mtdoops are mainly those who don't have kdump configured
in their kernels, they are cheaper than kdump, but less powerful.
Thanks.
Américo Wang <[email protected]> writes:
> On Mon, Jan 31, 2011 at 05:59:39PM -0500, Vivek Goyal wrote:
>>
>>Anyway, if an image is loaded and we have setup to capture dump also
>>why do we need to save kmsg with the help of an helper. I am assuming
>>this is more of a debugging aid if we have no other way to capture the
>>kernel log buffer. So if somebody has setup kdump to capture the
>>vmcore, why to call another handler which tries to save part of the
>>vmcore (kmsg) separately.
>>
>
> Well, kmsg dumper is not only for panic/oops, it can also save kernel
> messages when the system is reboot/halt.
>
> Yeah, it is true that vmcore contains log_buf, but I think the users
> of ramoops/mtdoops are mainly those who don't have kdump configured
> in their kernels, they are cheaper than kdump, but less powerful
The issue is the inane call inside crash_kexec.
It requires both a kexec kernel to be loaded and it requires you to be
crashing. Given that when I audited the kmsg_dump handlers they really
weren't safe in a crash dump scenario we should just remove it.
Eric
On Mon, Jan 31, 2011 at 11:33:15PM -0800, Eric W. Biederman wrote:
>Américo Wang <[email protected]> writes:
>
>> On Mon, Jan 31, 2011 at 05:59:39PM -0500, Vivek Goyal wrote:
>>>
>>>Anyway, if an image is loaded and we have setup to capture dump also
>>>why do we need to save kmsg with the help of an helper. I am assuming
>>>this is more of a debugging aid if we have no other way to capture the
>>>kernel log buffer. So if somebody has setup kdump to capture the
>>>vmcore, why to call another handler which tries to save part of the
>>>vmcore (kmsg) separately.
>>>
>>
>> Well, kmsg dumper is not only for panic/oops, it can also save kernel
>> messages when the system is reboot/halt.
>>
>> Yeah, it is true that vmcore contains log_buf, but I think the users
>> of ramoops/mtdoops are mainly those who don't have kdump configured
>> in their kernels, they are cheaper than kdump, but less powerful
>
>The issue is the inane call inside crash_kexec.
>
>It requires both a kexec kernel to be loaded and it requires you to be
>crashing. Given that when I audited the kmsg_dump handlers they really
>weren't safe in a crash dump scenario we should just remove it.
>
Probably, I think we need to get rid of KMSG_DUMP_KEXEC.
On Tue, Feb 01, 2011 at 03:38:53PM +0800, Américo Wang wrote:
>On Mon, Jan 31, 2011 at 11:33:15PM -0800, Eric W. Biederman wrote:
>>The issue is the inane call inside crash_kexec.
>>
>>It requires both a kexec kernel to be loaded and it requires you to be
>>crashing. Given that when I audited the kmsg_dump handlers they really
>>weren't safe in a crash dump scenario we should just remove it.
>>
>
>Probably, I think we need to get rid of KMSG_DUMP_KEXEC.
>
Here we go.
--------->
KMSG_DUMP_KEXEC is useless because we already save kernel messages
inside /proc/vmcore, and it is unsafe to allow modules to do
other stuffs in a crash dump scenario.
Reported-by: Vivek Goyal <[email protected]>
Cc: "Eric W. Biederman" <[email protected]>
Signed-off-by: WANG Cong <[email protected]>
---
diff --git a/drivers/char/ramoops.c b/drivers/char/ramoops.c
index 1a9f5f6..8653ba4 100644
--- a/drivers/char/ramoops.c
+++ b/drivers/char/ramoops.c
@@ -69,8 +69,7 @@ static void ramoops_do_dump(struct kmsg_dumper *dumper,
struct timeval timestamp;
if (reason != KMSG_DUMP_OOPS &&
- reason != KMSG_DUMP_PANIC &&
- reason != KMSG_DUMP_KEXEC)
+ reason != KMSG_DUMP_PANIC)
return;
/* Only dump oopses if dump_oops is set */
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index e3e40f4..56eac4e 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -308,8 +308,7 @@ static void mtdoops_do_dump(struct kmsg_dumper *dumper,
char *dst;
if (reason != KMSG_DUMP_OOPS &&
- reason != KMSG_DUMP_PANIC &&
- reason != KMSG_DUMP_KEXEC)
+ reason != KMSG_DUMP_PANIC)
return;
/* Only dump oopses if dump_oops is set */
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 2a0d7d6..05fa2a9 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -17,7 +17,6 @@
enum kmsg_dump_reason {
KMSG_DUMP_OOPS,
KMSG_DUMP_PANIC,
- KMSG_DUMP_KEXEC,
KMSG_DUMP_RESTART,
KMSG_DUMP_HALT,
KMSG_DUMP_POWEROFF,
diff --git a/kernel/kexec.c b/kernel/kexec.c
index ec19b92..3ac0218 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -32,7 +32,6 @@
#include <linux/console.h>
#include <linux/vmalloc.h>
#include <linux/swap.h>
-#include <linux/kmsg_dump.h>
#include <asm/page.h>
#include <asm/uaccess.h>
@@ -1078,8 +1077,6 @@ void crash_kexec(struct pt_regs *regs)
if (kexec_crash_image) {
struct pt_regs fixed_regs;
- kmsg_dump(KMSG_DUMP_KEXEC);
-
crash_setup_regs(&fixed_regs, regs);
crash_save_vmcoreinfo();
machine_crash_shutdown(&fixed_regs);
On Tue, Feb 01, 2011 at 04:13:13PM +0800, Am?rico Wang wrote:
> On Tue, Feb 01, 2011 at 03:38:53PM +0800, Am?rico Wang wrote:
> >On Mon, Jan 31, 2011 at 11:33:15PM -0800, Eric W. Biederman wrote:
> >>The issue is the inane call inside crash_kexec.
> >>
> >>It requires both a kexec kernel to be loaded and it requires you to be
> >>crashing. Given that when I audited the kmsg_dump handlers they really
> >>weren't safe in a crash dump scenario we should just remove it.
> >>
> >
> >Probably, I think we need to get rid of KMSG_DUMP_KEXEC.
> >
>
> Here we go.
>
> --------->
>
> KMSG_DUMP_KEXEC is useless because we already save kernel messages
> inside /proc/vmcore, and it is unsafe to allow modules to do
> other stuffs in a crash dump scenario.
>
I think this is right thing to do. First of all kmsg_dump() call inside
crash_kexec() does not make any sense. Secondly, if kdump kernel is
loaded, we anyway are going to capture log buffers in vmcore and there
should not be any need to try to save messages twice.
Now one can argue that what if kdump does not capture the dump. I think
then right solution (though painful one) is to try to make kdump as
reliable as possible instead of trying to come up with backup mechanisms
to save logs in case kdump fails.
Acked-by: Vivek Goyal <[email protected]>
Thanks
Vivek
> Reported-by: Vivek Goyal <[email protected]>
> Cc: "Eric W. Biederman" <[email protected]>
> Signed-off-by: WANG Cong <[email protected]>
>
>
> ---
> diff --git a/drivers/char/ramoops.c b/drivers/char/ramoops.c
> index 1a9f5f6..8653ba4 100644
> --- a/drivers/char/ramoops.c
> +++ b/drivers/char/ramoops.c
> @@ -69,8 +69,7 @@ static void ramoops_do_dump(struct kmsg_dumper *dumper,
> struct timeval timestamp;
>
> if (reason != KMSG_DUMP_OOPS &&
> - reason != KMSG_DUMP_PANIC &&
> - reason != KMSG_DUMP_KEXEC)
> + reason != KMSG_DUMP_PANIC)
> return;
>
> /* Only dump oopses if dump_oops is set */
> diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
> index e3e40f4..56eac4e 100644
> --- a/drivers/mtd/mtdoops.c
> +++ b/drivers/mtd/mtdoops.c
> @@ -308,8 +308,7 @@ static void mtdoops_do_dump(struct kmsg_dumper *dumper,
> char *dst;
>
> if (reason != KMSG_DUMP_OOPS &&
> - reason != KMSG_DUMP_PANIC &&
> - reason != KMSG_DUMP_KEXEC)
> + reason != KMSG_DUMP_PANIC)
> return;
>
> /* Only dump oopses if dump_oops is set */
> diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
> index 2a0d7d6..05fa2a9 100644
> --- a/include/linux/kmsg_dump.h
> +++ b/include/linux/kmsg_dump.h
> @@ -17,7 +17,6 @@
> enum kmsg_dump_reason {
> KMSG_DUMP_OOPS,
> KMSG_DUMP_PANIC,
> - KMSG_DUMP_KEXEC,
> KMSG_DUMP_RESTART,
> KMSG_DUMP_HALT,
> KMSG_DUMP_POWEROFF,
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index ec19b92..3ac0218 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -32,7 +32,6 @@
> #include <linux/console.h>
> #include <linux/vmalloc.h>
> #include <linux/swap.h>
> -#include <linux/kmsg_dump.h>
>
> #include <asm/page.h>
> #include <asm/uaccess.h>
> @@ -1078,8 +1077,6 @@ void crash_kexec(struct pt_regs *regs)
> if (kexec_crash_image) {
> struct pt_regs fixed_regs;
>
> - kmsg_dump(KMSG_DUMP_KEXEC);
> -
> crash_setup_regs(&fixed_regs, regs);
> crash_save_vmcoreinfo();
> machine_crash_shutdown(&fixed_regs);
On Tue, Feb 01, 2011 at 10:28:45AM -0500, Vivek Goyal wrote:
> On Tue, Feb 01, 2011 at 04:13:13PM +0800, Am?rico Wang wrote:
> > On Tue, Feb 01, 2011 at 03:38:53PM +0800, Am?rico Wang wrote:
> > >On Mon, Jan 31, 2011 at 11:33:15PM -0800, Eric W. Biederman wrote:
> > >>The issue is the inane call inside crash_kexec.
> > >>
> > >>It requires both a kexec kernel to be loaded and it requires you to be
> > >>crashing. Given that when I audited the kmsg_dump handlers they really
> > >>weren't safe in a crash dump scenario we should just remove it.
> > >>
> > >
> > >Probably, I think we need to get rid of KMSG_DUMP_KEXEC.
> > >
> >
> > Here we go.
> >
> > --------->
> >
> > KMSG_DUMP_KEXEC is useless because we already save kernel messages
> > inside /proc/vmcore, and it is unsafe to allow modules to do
> > other stuffs in a crash dump scenario.
> >
>
> I think this is right thing to do. First of all kmsg_dump() call inside
> crash_kexec() does not make any sense. Secondly, if kdump kernel is
> loaded, we anyway are going to capture log buffers in vmcore and there
> should not be any need to try to save messages twice.
>
> Now one can argue that what if kdump does not capture the dump. I think
> then right solution (though painful one) is to try to make kdump as
> reliable as possible instead of trying to come up with backup mechanisms
> to save logs in case kdump fails.
I'm of the same mind, kmsg_dump is fine by itself, but wedging it in to
duplicate something kdump already does on the grounds that kdump might
fail is Doing It Wrong. If one has concerns about kdump failing, put
effort into solidifying kdump.
Acked-by: Jarod Wilson <[email protected]>
--
Jarod Wilson
[email protected]
> On Tue, Feb 01, 2011 at 03:38:53PM +0800, Américo Wang wrote:
> >On Mon, Jan 31, 2011 at 11:33:15PM -0800, Eric W. Biederman wrote:
> >>The issue is the inane call inside crash_kexec.
> >>
> >>It requires both a kexec kernel to be loaded and it requires you to be
> >>crashing. Given that when I audited the kmsg_dump handlers they really
> >>weren't safe in a crash dump scenario we should just remove it.
> >>
> >
> >Probably, I think we need to get rid of KMSG_DUMP_KEXEC.
> >
>
> Here we go.
>
> --------->
>
> KMSG_DUMP_KEXEC is useless because we already save kernel messages
> inside /proc/vmcore, and it is unsafe to allow modules to do
> other stuffs in a crash dump scenario.
>
> Reported-by: Vivek Goyal <[email protected]>
> Cc: "Eric W. Biederman" <[email protected]>
> Signed-off-by: WANG Cong <[email protected]>
I wrote why this is no good idea by another mail. Please see it.
Anyway you have a right to don't use this feature.
Nack.
On Thu, Feb 03, 2011 at 09:59:04AM +0900, KOSAKI Motohiro wrote:
> > On Tue, Feb 01, 2011 at 03:38:53PM +0800, Am?rico Wang wrote:
> > >On Mon, Jan 31, 2011 at 11:33:15PM -0800, Eric W. Biederman wrote:
> > >>The issue is the inane call inside crash_kexec.
> > >>
> > >>It requires both a kexec kernel to be loaded and it requires you to be
> > >>crashing. Given that when I audited the kmsg_dump handlers they really
> > >>weren't safe in a crash dump scenario we should just remove it.
> > >>
> > >
> > >Probably, I think we need to get rid of KMSG_DUMP_KEXEC.
> > >
> >
> > Here we go.
> >
> > --------->
> >
> > KMSG_DUMP_KEXEC is useless because we already save kernel messages
> > inside /proc/vmcore, and it is unsafe to allow modules to do
> > other stuffs in a crash dump scenario.
> >
> > Reported-by: Vivek Goyal <[email protected]>
> > Cc: "Eric W. Biederman" <[email protected]>
> > Signed-off-by: WANG Cong <[email protected]>
>
> I wrote why this is no good idea by another mail. Please see it.
> Anyway you have a right to don't use this feature.
>
But you have not explained that why do you need to hook into crash_kexec()
and you have also not explained why do you need to send out kdump_msg()
notification if kdump is configured.
Some detailed explanation here would help.
Thanks
Vivek
> Nack.
>
> > I wrote why this is no good idea by another mail. Please see it.
> > Anyway you have a right to don't use this feature.
> >
>
> But you have not explained that why do you need to hook into crash_kexec()
> and you have also not explained why do you need to send out kdump_msg()
> notification if kdump is configured.
>
> Some detailed explanation here would help.
Hi,
I send it you now :)
On Thu, 3 Feb 2011 13:53:01 +0900 (JST)
KOSAKI Motohiro <[email protected]> wrote:
> > > I wrote why this is no good idea by another mail. Please see it.
> > > Anyway you have a right to don't use this feature.
> > >
> >
> > But you have not explained that why do you need to hook into crash_kexec()
> > and you have also not explained why do you need to send out kdump_msg()
> > notification if kdump is configured.
> >
> > Some detailed explanation here would help.
>
> Hi,
>
> I send it you now :)
>
What happened with this? kexec-remove-kmsg_dump_kexec.patch has two acks
and one unexplained nack :(
Andrew Morton <[email protected]> writes:
> On Thu, 3 Feb 2011 13:53:01 +0900 (JST)
> KOSAKI Motohiro <[email protected]> wrote:
>
>> > > I wrote why this is no good idea by another mail. Please see it.
>> > > Anyway you have a right to don't use this feature.
>> > >
>> >
>> > But you have not explained that why do you need to hook into crash_kexec()
>> > and you have also not explained why do you need to send out kdump_msg()
>> > notification if kdump is configured.
>> >
>> > Some detailed explanation here would help.
>>
>> Hi,
>>
>> I send it you now :)
>>
>
> What happened with this? kexec-remove-kmsg_dump_kexec.patch has two acks
> and one unexplained nack :(
As I recall the nack was based on a theoretical use case, and a cleanup
of kmsg_dump to make it more robust which I don't believe has happened,
instead of something real.
My feel is that we should remove kmsg_dump and if a real use case comes
up reconsider it at that time.
I don't think anyone cares too strongly at the moment because the
features are not expected to be used in conjunction with each other, nor
even expected to be compiled into the same kernel. However given that
they are not used to be used in conjunction with each other a call into
kmsg_dump from crash_kexec is really just cluttering the code for no
benefit to anyone.
I do believe kmsg_dump suffers from all of the same problems lkdtm
suffered from long ago which is it only works in a working kernel,
and it is unlikely to tell you anything on system failure.
Eric
(2011/05/27 5:10), Andrew Morton wrote:
> On Thu, 3 Feb 2011 13:53:01 +0900 (JST)
> KOSAKI Motohiro <[email protected]> wrote:
>
>>>> I wrote why this is no good idea by another mail. Please see it.
>>>> Anyway you have a right to don't use this feature.
>>>>
>>>
>>> But you have not explained that why do you need to hook into crash_kexec()
>>> and you have also not explained why do you need to send out kdump_msg()
>>> notification if kdump is configured.
>>>
>>> Some detailed explanation here would help.
>>
>> Hi,
>>
>> I send it you now :)
>>
>
> What happened with this? kexec-remove-kmsg_dump_kexec.patch has two acks
> and one unexplained nack :(
http://groups.google.com/group/linux.kernel/browse_thread/thread/1084f406573d76ac/ee19e34b45f83536?lnk=raot&pli=1
At last mail, Vivek proposed move kms_dump() instead remove. and I asked following question and
I've got no response. I'm still waiting his.
> I'm sorry I've missed this mail long time.
>
>> > ---------------------------------------------------------------------
>> > @@ -74,6 +75,7 @@ NORET_TYPE void panic(const char * fmt, ...)
>> > dump_stack();
>> > #endif
>> > + kmsg_dump(KMSG_DUMP_PANIC);
>> > /*
>> > * If we have crashed and we have a crash kernel loaded let it handle
>> > * everything else.
>> > * Do we want to call this before we try to display a message?
>> > */
>> > crash_kexec(NULL);
>> > ---------------------------------------------------------------------
>> And I think to compensate for that somebody introduced additional
>> kmsg_dump(KEXEC) call inside crash_kexec() and put it under CONFIG
>> option so that one can change the behavior based on config options.
>> I think this makes the logic somewhat twisted and an unnecessary call
>> inside crash_kexec(). So until and unless there is a strong reason we
>> can get rid of KEXEC event and move kmsg_dump call before crash_kexec()
>> for now and see how does it go, IMHO.
>
>
> I think I can agree your proposal. But could you please explain why do
> you think kmsg _before_ kdump and kmsg _in_ kdump are so different?
> I think it is only C level difference. CPU don't care C function and
> anyway the kernel call kmsg_dump() because invoke second kernel even
> if you proposal applied.
> It is only curious. I'm not against your proposal.
> Thanks.
On Sat, May 28, 2011 at 9:43 AM, Eric W. Biederman
<[email protected]> wrote:
> Andrew Morton <[email protected]> writes:
>
>> On Thu, 3 Feb 2011 13:53:01 +0900 (JST)
>> KOSAKI Motohiro <[email protected]> wrote:
>>
>>> > > I wrote why this is no good idea by another mail. Please see it.
>>> > > Anyway you have a right to don't use this feature.
>>> > >
>>> >
>>> > But you have not explained that why do you need to hook into crash_kexec()
>>> > and you have also not explained why do you need to send out kdump_msg()
>>> > notification if kdump is configured.
>>> >
>>> > Some detailed explanation here would help.
>>>
>>> Hi,
>>>
>>> I send it you now :)
>>>
>>
>> What happened with this? kexec-remove-kmsg_dump_kexec.patch has two acks
>> and one unexplained nack :(
>
> As I recall the nack was based on a theoretical use case, and a cleanup
> of kmsg_dump to make it more robust which I don't believe has happened,
> instead of something real.
>
> My feel is that we should remove kmsg_dump and if a real use case comes
> up reconsider it at that time.
>
> I don't think anyone cares too strongly at the moment because the
> features are not expected to be used in conjunction with each other, nor
> even expected to be compiled into the same kernel. However given that
> they are not used to be used in conjunction with each other a call into
> kmsg_dump from crash_kexec is really just cluttering the code for no
> benefit to anyone.
Seiji once proposed NVRAM which uses kmsg_dump, he wants
to have another way to store panic information when kdump doesn't
work which is _not_ rare in the real world.
Hi,
As Americo said, sometimes kdump doesn't work in the real world.
So, I would like to keep discussing implementation of kmsg_dump(KMSG_DUMP_KEXEC)
rather than just removing it.
Seiji
On Tue, May 31, 2011 at 04:58:00PM -0400, Seiji Aguchi wrote:
> Hi,
>
> As Americo said, sometimes kdump doesn't work in the real world.
>
> So, I would like to keep discussing implementation of kmsg_dump(KMSG_DUMP_KEXEC)
> rather than just removing it.
What are you using kmsg_dump() for? Using mtdoops, ramoops or something
else? Is it working reliably for you?
Thanks
Vivek
On Mon, May 30, 2011 at 02:13:33PM +0900, KOSAKI Motohiro wrote:
> (2011/05/27 5:10), Andrew Morton wrote:
> > On Thu, 3 Feb 2011 13:53:01 +0900 (JST)
> > KOSAKI Motohiro <[email protected]> wrote:
> >
> >>>> I wrote why this is no good idea by another mail. Please see it.
> >>>> Anyway you have a right to don't use this feature.
> >>>>
> >>>
> >>> But you have not explained that why do you need to hook into crash_kexec()
> >>> and you have also not explained why do you need to send out kdump_msg()
> >>> notification if kdump is configured.
> >>>
> >>> Some detailed explanation here would help.
> >>
> >> Hi,
> >>
> >> I send it you now :)
> >>
> >
> > What happened with this? kexec-remove-kmsg_dump_kexec.patch has two acks
> > and one unexplained nack :(
>
> http://groups.google.com/group/linux.kernel/browse_thread/thread/1084f406573d76ac/ee19e34b45f83536?lnk=raot&pli=1
>
> At last mail, Vivek proposed move kms_dump() instead remove. and I asked following question and
> I've got no response. I'm still waiting his.
>
>
> > I'm sorry I've missed this mail long time.
> >
> >> > ---------------------------------------------------------------------
> >> > @@ -74,6 +75,7 @@ NORET_TYPE void panic(const char * fmt, ...)
> >> > dump_stack();
> >> > #endif
> >> > + kmsg_dump(KMSG_DUMP_PANIC);
> >> > /*
> >> > * If we have crashed and we have a crash kernel loaded let it handle
> >> > * everything else.
> >> > * Do we want to call this before we try to display a message?
> >> > */
> >> > crash_kexec(NULL);
> >> > ---------------------------------------------------------------------
> >> And I think to compensate for that somebody introduced additional
> >> kmsg_dump(KEXEC) call inside crash_kexec() and put it under CONFIG
> >> option so that one can change the behavior based on config options.
> >> I think this makes the logic somewhat twisted and an unnecessary call
> >> inside crash_kexec(). So until and unless there is a strong reason we
> >> can get rid of KEXEC event and move kmsg_dump call before crash_kexec()
> >> for now and see how does it go, IMHO.
> >
> >
> > I think I can agree your proposal. But could you please explain why do
> > you think kmsg _before_ kdump and kmsg _in_ kdump are so different?
> > I think it is only C level difference. CPU don't care C function and
> > anyway the kernel call kmsg_dump() because invoke second kernel even
> > if you proposal applied.
> > It is only curious. I'm not against your proposal.
> > Thanks.
Few reasons.
- There is no correlation between crash_kexec() and kdump_msg(). What
you are creating is equivalent of panic notifiers and calling those
notifiers before dump happened. So calling it inside of crash_kexec()
does not make much sense from code point of view.
- Why does somebody need to keep track of event KMSG_DUMP_KEXEC?
- There is one kernel CONFIG option introduce which looks completely
superfluous.
My general take on the whole issue.
- In general I think exporting a hook to module so that they can do
anything before crash is a bad idea. Now this can be overloaded to
do things like sending crash notifications in clustered environement
where we recommend doing it in second kernel.
- Even if we really have to do it, there seemed to be two concern
areas.
- Reliability of kdump_msg() generic infrastructure and its
capability in terms of handling races with other cpus and
NMIs.
- Reliability of module which is getting the callback from
kdump_msg().
I think in one of the mails I was discussing that common infrastructure
between kdump and kmsg_dump() can be put in a separate function, like
stopping all cpus etc to avoid races in generic infrastrucutre and
then first we can all kmsg_dump() and then crash_kexec().
But this still does not provide us any protection against modules getting
control after crash and possiblly worsen the situation.
Thanks
Vivek
Hi,
>What are you using kmsg_dump() for? Using mtdoops, ramoops or something
>else? Is it working reliably for you?
I plan to use kmsg_dump() for set_variable service of UEFI.
I proposed a prototype patch this month and will improve it.
(kmsg_dump is used inside pstore.)
https://lkml.org/lkml/2011/5/10/340
Seiji
Seiji Aguchi <[email protected]> writes:
> Hi,
>
>>What are you using kmsg_dump() for? Using mtdoops, ramoops or something
>>else? Is it working reliably for you?
>
> I plan to use kmsg_dump() for set_variable service of UEFI.
> I proposed a prototype patch this month and will improve it.
> (kmsg_dump is used inside pstore.)
>
> https://lkml.org/lkml/2011/5/10/340
Shudder. Firmware calls in the crash path.
If that is the use, we need to remove the kmsg_dump(KMSG_DUMP_KEXEC)
hook from crash_kexec yesterday. It is leading to some really ludicrous
suggestions that are on the way from making kexec on panic unreliable
and useless.
There will always be EFI implementations where that will not work and
there will be no way we can fix those.
There is a long history of people trying to do things in a crashing
kernel, things that simply do not work when the system is in a bad
state. kmsg_dump() when I reviewed the code had significant
implementation problems for being called from interrupt handlers
and the like.
To introduce a different solution for capturing information when a
kernel crashes we need to see numbers that in a large number of
situations that the mechanism you are proposing is more reliable and/or
more maintainable than the current kexec on panic implementation.
The best work I know of on the reliability of the current situation
is "Evaluating Linux Kernel Crash Dumping Mechanisms", by Fernando Luis Vazquez Cao.
http://www.linuxsymposium.org/archives/OLS/Reprints-2006/cao-reprint.pdf
Now it does happen to be a fact that our efi support in linux is
so buggy kexec does not work let alone kexec on panic (if the target
kernel has any efi support). But our efi support being buggy is not
a reason to add more ways to fail when we have a kernel with efi
support. It is an argument to remove our excessive use of EFI
calls.
So let's just remove the ridiculous kmsg_dump(KMSG_DUMP_KEXEC) hook from
crash_kexec and remove any temptation for abuses like wanting to use
kmsg_dump() on anything but a deeply embedded system where there simply
is not enough memory for 2 kernels.
Eric
On Wed, 01 Jun 2011 20:26:20 -0700
[email protected] (Eric W. Biederman) wrote:
> >>What are you using kmsg_dump() for? Using mtdoops, ramoops or something
> >>else? Is it working reliably for you?
> >
> > I plan to use kmsg_dump() for set_variable service of UEFI.
> > I proposed a prototype patch this month and will improve it.
> > (kmsg_dump is used inside pstore.)
> >
> > https://lkml.org/lkml/2011/5/10/340
>
> Shudder. Firmware calls in the crash path.
>
> If that is the use, we need to remove the kmsg_dump(KMSG_DUMP_KEXEC)
> hook from crash_kexec yesterday. It is leading to some really ludicrous
> suggestions that are on the way from making kexec on panic unreliable
> and useless.
>
> There will always be EFI implementations where that will not work and
> there will be no way we can fix those.
>
> There is a long history of people trying to do things in a crashing
> kernel, things that simply do not work when the system is in a bad
> state. kmsg_dump() when I reviewed the code had significant
> implementation problems for being called from interrupt handlers
> and the like.
>
> To introduce a different solution for capturing information when a
> kernel crashes we need to see numbers that in a large number of
> situations that the mechanism you are proposing is more reliable and/or
> more maintainable than the current kexec on panic implementation.
>
> The best work I know of on the reliability of the current situation
> is "Evaluating Linux Kernel Crash Dumping Mechanisms", by Fernando Luis Vazquez Cao.
> http://www.linuxsymposium.org/archives/OLS/Reprints-2006/cao-reprint.pdf
>
>
> Now it does happen to be a fact that our efi support in linux is
> so buggy kexec does not work let alone kexec on panic (if the target
> kernel has any efi support). But our efi support being buggy is not
> a reason to add more ways to fail when we have a kernel with efi
> support. It is an argument to remove our excessive use of EFI
> calls.
>
> So let's just remove the ridiculous kmsg_dump(KMSG_DUMP_KEXEC) hook from
> crash_kexec and remove any temptation for abuses like wanting to use
> kmsg_dump() on anything but a deeply embedded system where there simply
> is not enough memory for 2 kernels.
>
So am I allowed to merge kexec-remove-kmsg_dump_kexec.patch yet?
From: WANG Cong <[email protected]>
KMSG_DUMP_KEXEC is useless because we already save kernel messages inside
/proc/vmcore, and it is unsafe to allow modules to do other stuffs in a
crash dump scenario.
[[email protected]: fix powerpc build]
Signed-off-by: WANG Cong <[email protected]>
Reported-by: Vivek Goyal <[email protected]>
Acked-by: Vivek Goyal <[email protected]>
Acked-by: Jarod Wilson <[email protected]>
Cc: "Eric W. Biederman" <[email protected]>
Cc: KOSAKI Motohiro <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
arch/powerpc/platforms/pseries/nvram.c | 1 -
drivers/char/ramoops.c | 3 +--
drivers/mtd/mtdoops.c | 3 +--
include/linux/kmsg_dump.h | 1 -
kernel/kexec.c | 3 ---
5 files changed, 2 insertions(+), 9 deletions(-)
diff -puN drivers/char/ramoops.c~kexec-remove-kmsg_dump_kexec drivers/char/ramoops.c
--- a/drivers/char/ramoops.c~kexec-remove-kmsg_dump_kexec
+++ a/drivers/char/ramoops.c
@@ -69,8 +69,7 @@ static void ramoops_do_dump(struct kmsg_
struct timeval timestamp;
if (reason != KMSG_DUMP_OOPS &&
- reason != KMSG_DUMP_PANIC &&
- reason != KMSG_DUMP_KEXEC)
+ reason != KMSG_DUMP_PANIC)
return;
/* Only dump oopses if dump_oops is set */
diff -puN drivers/mtd/mtdoops.c~kexec-remove-kmsg_dump_kexec drivers/mtd/mtdoops.c
--- a/drivers/mtd/mtdoops.c~kexec-remove-kmsg_dump_kexec
+++ a/drivers/mtd/mtdoops.c
@@ -308,8 +308,7 @@ static void mtdoops_do_dump(struct kmsg_
char *dst;
if (reason != KMSG_DUMP_OOPS &&
- reason != KMSG_DUMP_PANIC &&
- reason != KMSG_DUMP_KEXEC)
+ reason != KMSG_DUMP_PANIC)
return;
/* Only dump oopses if dump_oops is set */
diff -puN include/linux/kmsg_dump.h~kexec-remove-kmsg_dump_kexec include/linux/kmsg_dump.h
--- a/include/linux/kmsg_dump.h~kexec-remove-kmsg_dump_kexec
+++ a/include/linux/kmsg_dump.h
@@ -18,7 +18,6 @@
enum kmsg_dump_reason {
KMSG_DUMP_OOPS,
KMSG_DUMP_PANIC,
- KMSG_DUMP_KEXEC,
KMSG_DUMP_RESTART,
KMSG_DUMP_HALT,
KMSG_DUMP_POWEROFF,
diff -puN kernel/kexec.c~kexec-remove-kmsg_dump_kexec kernel/kexec.c
--- a/kernel/kexec.c~kexec-remove-kmsg_dump_kexec
+++ a/kernel/kexec.c
@@ -32,7 +32,6 @@
#include <linux/console.h>
#include <linux/vmalloc.h>
#include <linux/swap.h>
-#include <linux/kmsg_dump.h>
#include <linux/syscore_ops.h>
#include <asm/page.h>
@@ -1079,8 +1078,6 @@ void crash_kexec(struct pt_regs *regs)
if (kexec_crash_image) {
struct pt_regs fixed_regs;
- kmsg_dump(KMSG_DUMP_KEXEC);
-
crash_setup_regs(&fixed_regs, regs);
crash_save_vmcoreinfo();
machine_crash_shutdown(&fixed_regs);
diff -puN arch/powerpc/platforms/pseries/nvram.c~kexec-remove-kmsg_dump_kexec arch/powerpc/platforms/pseries/nvram.c
--- a/arch/powerpc/platforms/pseries/nvram.c~kexec-remove-kmsg_dump_kexec
+++ a/arch/powerpc/platforms/pseries/nvram.c
@@ -490,7 +490,6 @@ static void oops_to_nvram(struct kmsg_du
/* These are almost always orderly shutdowns. */
return;
case KMSG_DUMP_OOPS:
- case KMSG_DUMP_KEXEC:
break;
case KMSG_DUMP_PANIC:
panicking = true;
_
Hi
Sorry for the delay. I had got stuck LinuxCon Japan and piled up plenty
paper works.
>>> I think I can agree your proposal. But could you please explain why do
>>> you think kmsg _before_ kdump and kmsg _in_ kdump are so different?
>>> I think it is only C level difference. CPU don't care C function and
>>> anyway the kernel call kmsg_dump() because invoke second kernel even
>>> if you proposal applied.
>>> It is only curious. I'm not against your proposal.
>>> Thanks.
>
> Few reasons.
>
> - There is no correlation between crash_kexec() and kdump_msg(). What
> you are creating is equivalent of panic notifiers and calling those
> notifiers before dump happened. So calling it inside of crash_kexec()
> does not make much sense from code point of view.
Thank you for the replay. I got you _think_ no makes sense, but I haven't
explain what you talk about the code of "code point of view".
If you read the code, you can understand kdump_msg() and panic_notifiers
are not same point.
> - Why does somebody need to keep track of event KMSG_DUMP_KEXEC?
I believe I answered already at last thread.
http://groups.google.com/group/linux.kernel/browse_thread/thread/1084f406573d76ac/daa1a08804385089?q=kexec%3A+remove+KMSG_DUMP_KEXEC&lnk=ol&
> - There is one kernel CONFIG option introduce which looks completely
> superfluous.
What you mean "superfluous"? We already have billion kernel CONFIG.
Is it also bad?
> My general take on the whole issue.
>
> - In general I think exporting a hook to module so that they can do
> anything before crash is a bad idea. Now this can be overloaded to
> do things like sending crash notifications in clustered environement
> where we recommend doing it in second kernel.
??
It's not my issue and I haven't talked about such thing. I guess you
confuse I and Aguch-san or someone else.
>
> - Even if we really have to do it, there seemed to be two concern
> areas.
>
> - Reliability of kdump_msg() generic infrastructure and its
> capability in terms of handling races with other cpus and
> NMIs.
>
> - Reliability of module which is getting the callback from
> kdump_msg().
Indeed. I thought Aguch-san said he promised he work on improve them.
However it doesn't happen yet. Okay, I
> I think in one of the mails I was discussing that common infrastructure
> between kdump and kmsg_dump() can be put in a separate function, like
> stopping all cpus etc to avoid races in generic infrastrucutre and
> then first we can all kmsg_dump() and then crash_kexec().
Nice idea! Yes. I didn't think enterprise folks start to use this feature
and it now happen.
If nobody are working on this, I'll do it.
> But this still does not provide us any protection against modules getting
> control after crash and possiblly worsen the situation.
I think this doesn't big matter. If module author hope to get hook, they
can use kprobe in nowadays. I don't think we can make perfect kprobe protection.
I think I wrote this at last thread too.
Probably most reliability stupid module detect way is, watching lkml and revewing
kmsg_dump() user conteniously. However, if you strongly worry about this issue,
I can agree we make tiny foolish module protection. (but I don't have concrete
idea yet)
Hi
> Seiji Aguchi <[email protected]> writes:
>
>> Hi,
>>
>>> What are you using kmsg_dump() for? Using mtdoops, ramoops or something
>>> else? Is it working reliably for you?
>>
>> I plan to use kmsg_dump() for set_variable service of UEFI.
>> I proposed a prototype patch this month and will improve it.
>> (kmsg_dump is used inside pstore.)
>>
>> https://lkml.org/lkml/2011/5/10/340
>
> Shudder. Firmware calls in the crash path.
>
> If that is the use, we need to remove the kmsg_dump(KMSG_DUMP_KEXEC)
> hook from crash_kexec yesterday. It is leading to some really ludicrous
> suggestions that are on the way from making kexec on panic unreliable
> and useless.
Do you have concrete example?
If you only talked about theoretical issue, probably making boot parameter
is enough and reasonable way.
> There will always be EFI implementations where that will not work and
> there will be no way we can fix those.
>
> There is a long history of people trying to do things in a crashing
> kernel, things that simply do not work when the system is in a bad
> state. kmsg_dump() when I reviewed the code had significant
> implementation problems for being called from interrupt handlers
> and the like.
>
> To introduce a different solution for capturing information when a
> kernel crashes we need to see numbers that in a large number of
> situations that the mechanism you are proposing is more reliable and/or
> more maintainable than the current kexec on panic implementation.
>
> The best work I know of on the reliability of the current situation
> is "Evaluating Linux Kernel Crash Dumping Mechanisms", by Fernando Luis Vazquez Cao.
> http://www.linuxsymposium.org/archives/OLS/Reprints-2006/cao-reprint.pdf
Every reliability improvement idea is welcome! This also improve embedded too.
> Now it does happen to be a fact that our efi support in linux is
> so buggy kexec does not work let alone kexec on panic (if the target
> kernel has any efi support). But our efi support being buggy is not
> a reason to add more ways to fail when we have a kernel with efi
> support. It is an argument to remove our excessive use of EFI
> calls.
Which part is buggy? As far as I know, IBM, HP and Fujitsu have EFI supported
server product and it works.
So, if you are suffered from buggy efi, I think the best way is to make config
option and you disable it and Aguch-san enable it.
> So let's just remove the ridiculous kmsg_dump(KMSG_DUMP_KEXEC) hook from
> crash_kexec and remove any temptation for abuses like wanting to use
> kmsg_dump() on anything but a deeply embedded system where there simply
> is not enough memory for 2 kernels.
This sentence has two misunderstands. 1) modern embedded (e.g. Android) has
lots memory rather than 10 years past PC 2) they often don't need full feature
second kernel. they often don't need full crash dump.
Thanks.
On Thu, Jun 09, 2011 at 08:00:08PM +0900, KOSAKI Motohiro wrote:
> Hi
>
> Sorry for the delay. I had got stuck LinuxCon Japan and piled up plenty
> paper works.
>
> >>> I think I can agree your proposal. But could you please explain why do
> >>> you think kmsg _before_ kdump and kmsg _in_ kdump are so different?
> >>> I think it is only C level difference. CPU don't care C function and
> >>> anyway the kernel call kmsg_dump() because invoke second kernel even
> >>> if you proposal applied.
> >>> It is only curious. I'm not against your proposal.
> >>> Thanks.
> >
> > Few reasons.
> >
> > - There is no correlation between crash_kexec() and kdump_msg(). What
> > you are creating is equivalent of panic notifiers and calling those
> > notifiers before dump happened. So calling it inside of crash_kexec()
> > does not make much sense from code point of view.
>
> Thank you for the replay. I got you _think_ no makes sense, but I haven't
> explain what you talk about the code of "code point of view".
> If you read the code, you can understand kdump_msg() and panic_notifiers
> are not same point.
>
>
> > - Why does somebody need to keep track of event KMSG_DUMP_KEXEC?
>
> I believe I answered already at last thread.
>
> http://groups.google.com/group/linux.kernel/browse_thread/thread/1084f406573d76ac/daa1a08804385089?q=kexec%3A+remove+KMSG_DUMP_KEXEC&lnk=ol&
>
Frankly speaking I never understood it. The only thing I got is that
you are saying embedded devices want to do something upon a KEXEC and
I do not understand what's that action embedded devices want to take
upon an KEXEC.
And it is not just KEXEC, I am also curious about hooks in places
like emergency_restart(). Who needs to know about it and is it
safe to do.
So if would be great if you could explain it in detail again.
>
> > - There is one kernel CONFIG option introduce which looks completely
> > superfluous.
>
> What you mean "superfluous"? We already have billion kernel CONFIG.
> Is it also bad?
We have lots of config options but every config option goes through
some thought process and it is taken in if it makes sense. In this
case this additional config option did not make any sense.
>
> > My general take on the whole issue.
> >
> > - In general I think exporting a hook to module so that they can do
> > anything before crash is a bad idea. Now this can be overloaded to
> > do things like sending crash notifications in clustered environement
> > where we recommend doing it in second kernel.
>
> ??
> It's not my issue and I haven't talked about such thing. I guess you
> confuse I and Aguch-san or someone else.
Once you export the hook to a module, anybody can do that. In the
past people have asked for it reapeatedly. So I am just giving you
one example that what will people start doing once the hook is
there.
>
> >
> > - Even if we really have to do it, there seemed to be two concern
> > areas.
> >
> > - Reliability of kdump_msg() generic infrastructure and its
> > capability in terms of handling races with other cpus and
> > NMIs.
> >
> > - Reliability of module which is getting the callback from
> > kdump_msg().
>
> Indeed. I thought Aguch-san said he promised he work on improve them.
> However it doesn't happen yet. Okay, I
>
>
> > I think in one of the mails I was discussing that common infrastructure
> > between kdump and kmsg_dump() can be put in a separate function, like
> > stopping all cpus etc to avoid races in generic infrastrucutre and
> > then first we can all kmsg_dump() and then crash_kexec().
>
> Nice idea! Yes. I didn't think enterprise folks start to use this feature
> and it now happen.
> If nobody are working on this, I'll do it.
It would be great if you could work on it and make sure kdump_msg()
and crash_kexec() could share common infrastructure which takes care
of common actions like stopping cpus, taking care of NMIs and invocation
of panic() on mutliple cpus etc.
>
>
> > But this still does not provide us any protection against modules getting
> > control after crash and possiblly worsen the situation.
>
> I think this doesn't big matter. If module author hope to get hook, they
> can use kprobe in nowadays. I don't think we can make perfect kprobe protection.
> I think I wrote this at last thread too.
>
> Probably most reliability stupid module detect way is, watching lkml and revewing
> kmsg_dump() user conteniously. However, if you strongly worry about this issue,
> I can agree we make tiny foolish module protection. (but I don't have concrete
> idea yet)
I do worry about modules. Once the system has paniced, I personally don't
think that anbody and everybody should be able to look at that event and
take whatever actions they want to.
Having said that personally I like the idea of being able to save
backtrace on some non volatile storage and access it over next boot
through pstore interface.
So if care is taken to make kmsg_dump() generic infrastructure fool proof
probably it is good start and then we can look into module thing later.
Thanks
Vivek