2011-03-23 18:40:51

by Takao Indoh

[permalink] [raw]
Subject: [PATCH][KDUMP] Ignore spurious IPI

Hi all,

I found a problem that kdump(2nd kernel) sometimes hangs up. It seems
that system panic occurs as follows.

(1)
2nd kernel boot up

(2)
A pending IPI from 1st kernel comes after unmasking interrupts at the
following point.

asmlinkage void __init start_kernel(void)
{
(snip)
time_init();
profile_init();
if (!irqs_disabled())
printk(KERN_CRIT "start_kernel(): bug: interrupts were "
"enabled early\n");
early_boot_irqs_disabled = false;
local_irq_enable(); <=======================================HERE

(3)
Kernel tries to handle the interrupt, but some data structures are not
initialized yet at this point. As a result, in the
generic_smp_call_function_single_interrupt(), NULL pointer dereference
occurs when list_replace_init() tries to access &q->list.next.

I took a look at local_apic_timer_interrupt() and found a few lines to
handle such a pending LAPIC interrupt(in this case, timer interrupt).
Therefore I made a patch to ignore spurious IPI in the same manner. I
confirmed this problem does not occur with this patch.

Any comments?

Signed-off-by: Takao Indoh <[email protected]>
---
kernel/smp.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/kernel/smp.c b/kernel/smp.c
index 9910744..f2f561b 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -260,6 +260,12 @@ void generic_smp_call_function_single_interrupt(void)
*/
WARN_ON_ONCE(!cpu_online(smp_processor_id()));

+ if (unlikely(!q->list.next)) {
+ /* Pending interrupt from previous kernel(e.g. kdump), just ignore */
+ pr_warning("Spurious IPI on cpu %d\n", smp_processor_id());
+ return;
+ }
+
raw_spin_lock(&q->lock);
list_replace_init(&q->list, &list);
raw_spin_unlock(&q->lock);


2011-03-24 14:20:44

by Milton Miller

[permalink] [raw]
Subject: Re: [KDUMP] Ignore spurious IPI

On Wed, 23 Mar 2011 about 18:40:12 -0000, Takao Indoh wrote:
> Hi all,
>
> I found a problem that kdump(2nd kernel) sometimes hangs up. It seems
> that system panic occurs as follows.
..
> (2)
> A pending IPI from 1st kernel comes after unmasking interrupts at the
> following point.
>
> asmlinkage void __init start_kernel(void)
> {
> (snip)
> time_init();
> profile_init();
> if (!irqs_disabled())
> printk(KERN_CRIT "start_kernel(): bug: interrupts were "
> "enabled early\n");
> early_boot_irqs_disabled = false;
> local_irq_enable(); <=======================================HERE
>
> (3)
> Kernel tries to handle the interrupt, but some data structures are not
> initialized yet at this point. As a result, in the
> generic_smp_call_function_single_interrupt(), NULL pointer dereference
> occurs when list_replace_init() tries to access &q->list.next.
>
[tried to match lapic timer interrupt]
> Any comments?

So this occurs because unlike device interrupts, this vector has the action
defined statically and no per-interrupt disable on your architecture?


If so, just initialize the data structure earlier -- change
init_call_single_data from early_initcall to an explict call after the
per-cpu areas are initialized.

milton

2011-03-24 21:26:18

by Takao Indoh

[permalink] [raw]
Subject: Re: [KDUMP] Ignore spurious IPI

On Thu, 24 Mar 2011 08:20:32 -0600, Milton Miller wrote:

>On Wed, 23 Mar 2011 about 18:40:12 -0000, Takao Indoh wrote:
>> Hi all,
>>
>> I found a problem that kdump(2nd kernel) sometimes hangs up. It seems
>> that system panic occurs as follows.
>..
>> (2)
>> A pending IPI from 1st kernel comes after unmasking interrupts at the
>> following point.
>>
>> asmlinkage void __init start_kernel(void)
>> {
>> (snip)
>> time_init();
>> profile_init();
>> if (!irqs_disabled())
>> printk(KERN_CRIT "start_kernel(): bug: interrupts were "
>> "enabled early\n");
>> early_boot_irqs_disabled = false;
>> local_irq_enable(); <=======================================HERE
>>
>> (3)
>> Kernel tries to handle the interrupt, but some data structures are not
>> initialized yet at this point. As a result, in the
>> generic_smp_call_function_single_interrupt(), NULL pointer dereference
>> occurs when list_replace_init() tries to access &q->list.next.
>>
>[tried to match lapic timer interrupt]
>> Any comments?
>
>So this occurs because unlike device interrupts, this vector has the action
>defined statically and no per-interrupt disable on your architecture?

I think there is not per-interrupt disable for IPI.

>If so, just initialize the data structure earlier -- change
>init_call_single_data from early_initcall to an explict call after the
>per-cpu areas are initialized.

That makes sense. I'll do this, thanks.

Thanks,
Takao Indoh


>
>milton