[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4A56E83B.50600@mxs.nes.nec.co.jp>
Date: Fri, 10 Jul 2009 16:05:31 +0900
From: "Ken'ichi Ohmichi" <oomichi@....nes.nec.co.jp>
To: Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>
CC: kexec-ml <kexec@...ts.infradead.org>,
lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kdump: Enable kdump if 2nd-kernel is loaded.
Hi Seto-san,
Thank you for your comment.
Hidetoshi Seto wrote:
>> This patch enables a kdump if 2nd-kernel is loaded.
>> (The patch is based on linux-2.6.31-rc2.)
>>
>> Now, a kdump is enabled if a kernel parameter "oops=panic" is specified and
>> 2nd-kernel is loaded. I think that a kdump should be enabled regardless of
>> "oops=panic" if 2nd-kernel is loaded, because a system administrator loads
>> 2nd-kernel for enabling a kdump.
>
> I think this description is slightly wrong because kdump will be invoked
> from panic, regardless of the panic_on_oops.
>
> Maybe:
> A kdump on oops is enabled if a kernel parameter "oops=panic" ...
> ~~~~~~~
Right, I should add "on oops" to the above description.
>> * Reference
>> The discussion about this patch
>> http://lists.infradead.org/pipermail/kexec/2009-July/003417.html
>
> I'd like to quote your comment:
>
>>> I tried to test a kdump on linux-2.6.31-rc1 *without* a kernel parameter
>>> "oops=panic" by `echo c > /proc/sysrq-trigger`, but a kdump did not work
>>> because a kdump, which is occurred by `echo c > /proc/sysrq-trigger`, has
>>> been changed to a NULL pointer error instead of calling crash_kexec()
>>> since linux-2.6.31-rc1.
>
> So the real problem is that kdump is not triggered by the NULL pointer oops
> if !panic_on_oops, isn't it?
>
> It seems that you should report this trouble of sysrq-c as a regression.
I don't think this problem is a regression of sysrq-c.
This change of sysrq-c looks reasonable to me. Because sysrq-c is used
for the test of kdump, and its code path has been changed to the same
path as a kdump on oops (a real problem, not test).
So we can test a kdump by a real oops, that is good to me.
As a result, I could find this problem a kdump is not enabled without
"oops=panic".
>> Signed-off-by: Ken'ichi Ohmichi <oomichi@....nes.nec.co.jp>
>> Acked-by: Simon Horman <horms@...ge.net.au>
>> ---
>> --- a/kernel/kexec.c 2009-07-08 12:30:26.000000000 +0900
>> +++ b/kernel/kexec.c 2009-07-08 12:38:08.000000000 +0900
>> @@ -57,6 +57,8 @@ struct resource crashk_res = {
>>
>> int kexec_should_crash(struct task_struct *p)
>> {
>> + if (kexec_crash_image)
>> + return 1;
>> if (in_interrupt() || !p->pid || is_global_init(p) || panic_on_oops)
>> return 1;
>> return 0;
>
> I think kexec cannot crash if there is no image, right?
>
> Then:
>
> if (kexec_crash_image)
> return 1;
> return 0;
>
> or
>
> return (kexec_crash_image) ? 1 : 0;
>
> or
>
> since crash_kexec() is nop if !kexec_crash_image,
> replace all:
> if (kexec_should_crash(p))
> crash_kexec(reg);
> at everywhere in kernel to a simple line:
> crash_kexec(reg);
> and remove kexec_should_crash() completely
>
> would be better fix.
Good point, I like the first option because it is safe that a caller of
crash_kexec() checks it. That is not a strong opinion ;-)
Thanks
Ken'ichi Ohmichi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists