WARNING: kernel/smp.c:292 smp_call_function_single [Was: mmotm 2009-11-24-16-47 uploaded]

WARNING: kernel/smp.c:292 smp_call_function_single [Was: mmotm 2009-11-24-16-47 uploaded]

[Jiri Slaby]

On 11/25/2009 01:47 AM, akpm@linux-foundation.org wrote:
> The mm-of-the-moment snapshot 2009-11-24-16-47 has been uploaded to

Hi, when executing qemu-kvm I often get following warning and a hard lockup.

WARNING: at kernel/smp.c:292 smp_call_function_single+0xbd/0x140()
Hardware name: To Be Filled By O.E.M.
Modules linked in: kvm_intel kvm fuse ath5k ath
Pid: 3265, comm: qemu-kvm Not tainted 2.6.32-rc8-mm1_64 #912
Call Trace:
 [<ffffffff81039678>] warn_slowpath_common+0x78/0xb0
 [<ffffffffa007fd50>] ? __vcpu_clear+0x0/0xd0 [kvm_intel]
 [<ffffffff810396bf>] warn_slowpath_null+0xf/0x20
 [<ffffffff8106410d>] smp_call_function_single+0xbd/0x140
 [<ffffffffa0080af6>] vmx_vcpu_load+0x46/0x170 [kvm_intel]
 [<ffffffffa004dd94>] kvm_arch_vcpu_load+0x24/0x60 [kvm]
 [<ffffffffa0047a8d>] kvm_sched_in+0xd/0x10 [kvm]
 [<ffffffff8102de37>] finish_task_switch+0x67/0xc0
 [<ffffffff814699f8>] schedule+0x2f8/0x9c0
 [<ffffffffa0063538>] ? kvm_apic_has_interrupt+0x48/0x90 [kvm]
 [<ffffffffa0062a58>] ? kvm_cpu_has_interrupt+0x58/0x70 [kvm]
 [<ffffffffa0047c9d>] kvm_vcpu_block+0x6d/0xb0 [kvm]
 [<ffffffff81050f60>] ? autoremove_wake_function+0x0/0x40
 [<ffffffffa0055a5a>] kvm_arch_vcpu_ioctl_run+0x3fa/0xb80 [kvm]
 [<ffffffffa0049955>] kvm_vcpu_ioctl+0x435/0x590 [kvm]
 [<ffffffff8102f4ce>] ? enqueue_entity+0x6e/0xe0
 [<ffffffff8102f5eb>] ? enqueue_task_fair+0x3b/0x80
 [<ffffffff8102f6c3>] ? task_new_fair+0x93/0x120
 [<ffffffff810cd848>] vfs_ioctl+0x38/0xd0
 [<ffffffff810cdd8a>] do_vfs_ioctl+0x8a/0x5a0
 [<ffffffff81062926>] ? sys_futex+0xc6/0x170
 [<ffffffff810ce2ea>] sys_ioctl+0x4a/0x80
 [<ffffffff81002eeb>] system_call_fastpath+0x16/0x1b
---[ end trace ced05997e63d4d13 ]---


It is a regression against 2009-11-13-19-59.

Any ideas?

thanks,
-- 
js

Re: WARNING: kernel/smp.c:292 smp_call_function_single [Was: mmotm 2009-11-24-16-47 uploaded]

[Peter Zijlstra]

On Fri, 2009-11-27 at 16:03 +0100, Jiri Slaby wrote:
> On 11/25/2009 01:47 AM, akpm@linux-foundation.org wrote:
> > The mm-of-the-moment snapshot 2009-11-24-16-47 has been uploaded to
> 
> Hi, when executing qemu-kvm I often get following warning and a hard lockup.
> 
> WARNING: at kernel/smp.c:292 smp_call_function_single+0xbd/0x140()
> Hardware name: To Be Filled By O.E.M.
> Modules linked in: kvm_intel kvm fuse ath5k ath
> Pid: 3265, comm: qemu-kvm Not tainted 2.6.32-rc8-mm1_64 #912
> Call Trace:
>  [<ffffffff81039678>] warn_slowpath_common+0x78/0xb0
>  [<ffffffffa007fd50>] ? __vcpu_clear+0x0/0xd0 [kvm_intel]
>  [<ffffffff810396bf>] warn_slowpath_null+0xf/0x20
>  [<ffffffff8106410d>] smp_call_function_single+0xbd/0x140
>  [<ffffffffa0080af6>] vmx_vcpu_load+0x46/0x170 [kvm_intel]
>  [<ffffffffa004dd94>] kvm_arch_vcpu_load+0x24/0x60 [kvm]
>  [<ffffffffa0047a8d>] kvm_sched_in+0xd/0x10 [kvm]
>  [<ffffffff8102de37>] finish_task_switch+0x67/0xc0
>  [<ffffffff814699f8>] schedule+0x2f8/0x9c0

> 
> It is a regression against 2009-11-13-19-59.
> 
> Any ideas?

Looks like kvm is trying to send an IPI from the preempt notifiers,
which are called with IRQs disabled, not a sane thing to do.

If they really want that, they'll have to use a pre-allocated struct
call_single_data and use __smp_call_function_single.

Re: WARNING: kernel/smp.c:292 smp_call_function_single [Was: mmotm 2009-11-24-16-47 uploaded]

[Thomas Gleixner]

On Fri, 27 Nov 2009, Peter Zijlstra wrote:

> On Fri, 2009-11-27 at 16:03 +0100, Jiri Slaby wrote:
> > On 11/25/2009 01:47 AM, akpm@linux-foundation.org wrote:
> > > The mm-of-the-moment snapshot 2009-11-24-16-47 has been uploaded to
> > 
> > Hi, when executing qemu-kvm I often get following warning and a hard lockup.
> > 
> > WARNING: at kernel/smp.c:292 smp_call_function_single+0xbd/0x140()
> > Hardware name: To Be Filled By O.E.M.
> > Modules linked in: kvm_intel kvm fuse ath5k ath
> > Pid: 3265, comm: qemu-kvm Not tainted 2.6.32-rc8-mm1_64 #912
> > Call Trace:
> >  [<ffffffff81039678>] warn_slowpath_common+0x78/0xb0
> >  [<ffffffffa007fd50>] ? __vcpu_clear+0x0/0xd0 [kvm_intel]
> >  [<ffffffff810396bf>] warn_slowpath_null+0xf/0x20
> >  [<ffffffff8106410d>] smp_call_function_single+0xbd/0x140
> >  [<ffffffffa0080af6>] vmx_vcpu_load+0x46/0x170 [kvm_intel]
> >  [<ffffffffa004dd94>] kvm_arch_vcpu_load+0x24/0x60 [kvm]
> >  [<ffffffffa0047a8d>] kvm_sched_in+0xd/0x10 [kvm]
> >  [<ffffffff8102de37>] finish_task_switch+0x67/0xc0
> >  [<ffffffff814699f8>] schedule+0x2f8/0x9c0
> 
> > 
> > It is a regression against 2009-11-13-19-59.
> > 
> > Any ideas?
> 
> Looks like kvm is trying to send an IPI from the preempt notifiers,
> which are called with IRQs disabled, not a sane thing to do.
> 
> If they really want that, they'll have to use a pre-allocated struct
> call_single_data and use __smp_call_function_single.

Hmm, commit 498657a moved the fire_sched_in_preempt_notifiers() call
into the irqs disabled section recently.

    sched, kvm: Fix race condition involving sched_in_preempt_notifers
    
    In finish_task_switch(), fire_sched_in_preempt_notifiers() is
    called after finish_lock_switch().
    
    However, depending on architecture, preemption can be enabled after
    finish_lock_switch() which breaks the semantics of preempt
    notifiers.
    
    So move it before finish_arch_switch(). This also makes the in-
    notifiers symmetric to out- notifiers in terms of locking - now
    both are called under rq lock.

It's not a surprise that this breaks the existing code which does the
smp function call.

Thanks,

	tglx

Re: WARNING: kernel/smp.c:292 smp_call_function_single [Was: mmotm 2009-11-24-16-47 uploaded]

[Thomas Gleixner]

On Fri, 27 Nov 2009, Thomas Gleixner wrote:
> On Fri, 27 Nov 2009, Peter Zijlstra wrote:
> 
> > On Fri, 2009-11-27 at 16:03 +0100, Jiri Slaby wrote:
> > > On 11/25/2009 01:47 AM, akpm@linux-foundation.org wrote:
> > > > The mm-of-the-moment snapshot 2009-11-24-16-47 has been uploaded to
> > > 
> > > Hi, when executing qemu-kvm I often get following warning and a hard lockup.
> > > 
> > > WARNING: at kernel/smp.c:292 smp_call_function_single+0xbd/0x140()
> > > Hardware name: To Be Filled By O.E.M.
> > > Modules linked in: kvm_intel kvm fuse ath5k ath
> > > Pid: 3265, comm: qemu-kvm Not tainted 2.6.32-rc8-mm1_64 #912
> > > Call Trace:
> > >  [<ffffffff81039678>] warn_slowpath_common+0x78/0xb0
> > >  [<ffffffffa007fd50>] ? __vcpu_clear+0x0/0xd0 [kvm_intel]
> > >  [<ffffffff810396bf>] warn_slowpath_null+0xf/0x20
> > >  [<ffffffff8106410d>] smp_call_function_single+0xbd/0x140
> > >  [<ffffffffa0080af6>] vmx_vcpu_load+0x46/0x170 [kvm_intel]
> > >  [<ffffffffa004dd94>] kvm_arch_vcpu_load+0x24/0x60 [kvm]
> > >  [<ffffffffa0047a8d>] kvm_sched_in+0xd/0x10 [kvm]
> > >  [<ffffffff8102de37>] finish_task_switch+0x67/0xc0
> > >  [<ffffffff814699f8>] schedule+0x2f8/0x9c0
> > 
> > > 
> > > It is a regression against 2009-11-13-19-59.
> > > 
> > > Any ideas?
> > 
> > Looks like kvm is trying to send an IPI from the preempt notifiers,
> > which are called with IRQs disabled, not a sane thing to do.
> > 
> > If they really want that, they'll have to use a pre-allocated struct
> > call_single_data and use __smp_call_function_single.
> 
> Hmm, commit 498657a moved the fire_sched_in_preempt_notifiers() call
> into the irqs disabled section recently.
> 
>     sched, kvm: Fix race condition involving sched_in_preempt_notifers
>     
>     In finish_task_switch(), fire_sched_in_preempt_notifiers() is
>     called after finish_lock_switch().
>     
>     However, depending on architecture, preemption can be enabled after
>     finish_lock_switch() which breaks the semantics of preempt
>     notifiers.

This is patently wrong btw.

schedule()
{

need_resched:
        preempt_disable();
....
	task_switch();
....

        preempt_enable_no_resched();
	if (need_resched())
                goto need_resched;
}

>     
>     So move it before finish_arch_switch(). This also makes the in-
>     notifiers symmetric to out- notifiers in terms of locking - now
>     both are called under rq lock.
> 
> It's not a surprise that this breaks the existing code which does the
> smp function call.

Thanks,

	tglx

Re: WARNING: kernel/smp.c:292 smp_call_function_single [Was: mmotm 2009-11-24-16-47 uploaded]

[Avi Kivity]

On 11/27/2009 06:37 PM, Thomas Gleixner wrote:
> On Fri, 27 Nov 2009, Peter Zijlstra wrote:
>
>    
>> On Fri, 2009-11-27 at 16:03 +0100, Jiri Slaby wrote:
>>      
>>> On 11/25/2009 01:47 AM, akpm@linux-foundation.org wrote:
>>>        
>>>> The mm-of-the-moment snapshot 2009-11-24-16-47 has been uploaded to
>>>>          
>>> Hi, when executing qemu-kvm I often get following warning and a hard lockup.
>>>
>>> WARNING: at kernel/smp.c:292 smp_call_function_single+0xbd/0x140()
>>> Hardware name: To Be Filled By O.E.M.
>>> Modules linked in: kvm_intel kvm fuse ath5k ath
>>> Pid: 3265, comm: qemu-kvm Not tainted 2.6.32-rc8-mm1_64 #912
>>> Call Trace:
>>>   [<ffffffff81039678>] warn_slowpath_common+0x78/0xb0
>>>   [<ffffffffa007fd50>] ? __vcpu_clear+0x0/0xd0 [kvm_intel]
>>>   [<ffffffff810396bf>] warn_slowpath_null+0xf/0x20
>>>   [<ffffffff8106410d>] smp_call_function_single+0xbd/0x140
>>>   [<ffffffffa0080af6>] vmx_vcpu_load+0x46/0x170 [kvm_intel]
>>>   [<ffffffffa004dd94>] kvm_arch_vcpu_load+0x24/0x60 [kvm]
>>>   [<ffffffffa0047a8d>] kvm_sched_in+0xd/0x10 [kvm]
>>>   [<ffffffff8102de37>] finish_task_switch+0x67/0xc0
>>>   [<ffffffff814699f8>] schedule+0x2f8/0x9c0
>>>        
>>      
>>> It is a regression against 2009-11-13-19-59.
>>>
>>> Any ideas?
>>>        
>> Looks like kvm is trying to send an IPI from the preempt notifiers,
>> which are called with IRQs disabled, not a sane thing to do.
>>
>> If they really want that, they'll have to use a pre-allocated struct
>> call_single_data and use __smp_call_function_single.
>>      
> Hmm, commit 498657a moved the fire_sched_in_preempt_notifiers() call
> into the irqs disabled section recently.
>
>      sched, kvm: Fix race condition involving sched_in_preempt_notifers
>
>      In finish_task_switch(), fire_sched_in_preempt_notifiers() is
>      called after finish_lock_switch().
>
>      However, depending on architecture, preemption can be enabled after
>      finish_lock_switch() which breaks the semantics of preempt
>      notifiers.
>
>      So move it before finish_arch_switch(). This also makes the in-
>      notifiers symmetric to out- notifiers in terms of locking - now
>      both are called under rq lock.
>
> It's not a surprise that this breaks the existing code which does the
> smp function call.
>    


Yes, kvm expects preempt notifiers to be run with irqs enabled.  Copying 
patch author.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: WARNING: kernel/smp.c:292 smp_call_function_single [Was: mmotm 2009-11-24-16-47 uploaded]

[Tejun Heo]

Hello,

On 11/28/2009 09:12 PM, Avi Kivity wrote:
>> Hmm, commit 498657a moved the fire_sched_in_preempt_notifiers() call
>> into the irqs disabled section recently.
>>
>>      sched, kvm: Fix race condition involving sched_in_preempt_notifers
>>
>>      In finish_task_switch(), fire_sched_in_preempt_notifiers() is
>>      called after finish_lock_switch().
>>
>>      However, depending on architecture, preemption can be enabled after
>>      finish_lock_switch() which breaks the semantics of preempt
>>      notifiers.
>>
>>      So move it before finish_arch_switch(). This also makes the in-
>>      notifiers symmetric to out- notifiers in terms of locking - now
>>      both are called under rq lock.
>>
>> It's not a surprise that this breaks the existing code which does the
>> smp function call.
> 
> Yes, kvm expects preempt notifiers to be run with irqs enabled.  Copying
> patch author.

Hmmm... then, it's broken both ways.  The previous code may get
preempted after scheduling but before the notifier is run (which
breaks the semantics of the callback horribly), the current code
doesn't satisfy kvm's requirement.  Another thing is that in the
previous implementation the context is different between the 'in' and
'out' callbacks, which is subtle and nasty.  Can kvm be converted to
not do smp calls directly?

For the time being, maybe it's best to back out the fix given that the
only architecture which may be affected by the original bug is ia64
which is the only one with both kvm and the unlocked context switch.

Thanks.

-- 
tejun

Re: WARNING: kernel/smp.c:292 smp_call_function_single [Was: mmotm 2009-11-24-16-47 uploaded]

[Avi Kivity]

On 11/30/2009 10:58 AM, Tejun Heo wrote:
> Hello,
>
> On 11/28/2009 09:12 PM, Avi Kivity wrote:
>    
>>> Hmm, commit 498657a moved the fire_sched_in_preempt_notifiers() call
>>> into the irqs disabled section recently.
>>>
>>>       sched, kvm: Fix race condition involving sched_in_preempt_notifers
>>>
>>>       In finish_task_switch(), fire_sched_in_preempt_notifiers() is
>>>       called after finish_lock_switch().
>>>
>>>       However, depending on architecture, preemption can be enabled after
>>>       finish_lock_switch() which breaks the semantics of preempt
>>>       notifiers.
>>>
>>>       So move it before finish_arch_switch(). This also makes the in-
>>>       notifiers symmetric to out- notifiers in terms of locking - now
>>>       both are called under rq lock.
>>>
>>> It's not a surprise that this breaks the existing code which does the
>>> smp function call.
>>>        
>> Yes, kvm expects preempt notifiers to be run with irqs enabled.  Copying
>> patch author.
>>      
> Hmmm... then, it's broken both ways.  The previous code may get
> preempted after scheduling but before the notifier is run (which
> breaks the semantics of the callback horribly), the current code
> doesn't satisfy kvm's requirement.  Another thing is that in the
> previous implementation the context is different between the 'in' and
> 'out' callbacks, which is subtle and nasty.  Can kvm be converted to
> not do smp calls directly?
>    

No.  kvm uses preempt notifiers to manage extended processor registers 
(much like the fpu).  If we're scheduled into cpu A but state is 
currently live on cpu B, we need to go ahead and pull it in.

Technically, we can delay the IPI to happen after the sched in notifier; 
we can set some illegal state in cpu A and handle the exception by 
sending the IPI and fixing up the state.  But that would be slower, and 
not help the problem at all since some accesses happen with interrupts 
disabled.

Since this is essentially the same problem as the fpu, maybe we can 
solve it the same way.  How does the fpu migrate its state across 
processors?  One way would be to save the state when the task is 
selected for migration.

> For the time being, maybe it's best to back out the fix given that the
> only architecture which may be affected by the original bug is ia64
> which is the only one with both kvm and the unlocked context switch.
>    

Agreed.

-- 
error compiling committee.c: too many arguments to function

Re: WARNING: kernel/smp.c:292 smp_call_function_single [Was: mmotm 2009-11-24-16-47 uploaded]

[Thomas Gleixner]

On Mon, 30 Nov 2009, Tejun Heo wrote:
> Hello,
> 
> On 11/28/2009 09:12 PM, Avi Kivity wrote:
> >> Hmm, commit 498657a moved the fire_sched_in_preempt_notifiers() call
> >> into the irqs disabled section recently.
> >>
> >>      sched, kvm: Fix race condition involving sched_in_preempt_notifers
> >>
> >>      In finish_task_switch(), fire_sched_in_preempt_notifiers() is
> >>      called after finish_lock_switch().
> >>
> >>      However, depending on architecture, preemption can be enabled after
> >>      finish_lock_switch() which breaks the semantics of preempt
> >>      notifiers.
> >>
> >>      So move it before finish_arch_switch(). This also makes the in-
> >>      notifiers symmetric to out- notifiers in terms of locking - now
> >>      both are called under rq lock.
> >>
> >> It's not a surprise that this breaks the existing code which does the
> >> smp function call.
> > 
> > Yes, kvm expects preempt notifiers to be run with irqs enabled.  Copying
> > patch author.
> 
> Hmmm... then, it's broken both ways.  The previous code may get
> preempted after scheduling but before the notifier is run (which
> breaks the semantics of the callback horribly), the current code

No, it _CANNOT_ be preempted at that point:

schedule()
{
	preempt_disable();

	switch_to();

	preempt_enable();
}

> doesn't satisfy kvm's requirement.  Another thing is that in the
> previous implementation the context is different between the 'in' and
> 'out' callbacks, which is subtle and nasty.  Can kvm be converted to
> not do smp calls directly?
> 
> For the time being, maybe it's best to back out the fix given that the
> only architecture which may be affected by the original bug is ia64
> which is the only one with both kvm and the unlocked context switch.

Do you have a pointer to the original bug report ?

Thanks,

	tglx

Re: WARNING: kernel/smp.c:292 smp_call_function_single [Was: mmotm 2009-11-24-16-47 uploaded]

[Tejun Heo]

Hello,

On 11/30/2009 07:02 PM, Thomas Gleixner wrote:
> No, it _CANNOT_ be preempted at that point:
> 
> schedule()
> {
> 	preempt_disable();
> 
> 	switch_to();
> 
> 	preempt_enable();
> }

Yes, you're right.

>> For the time being, maybe it's best to back out the fix given that the
>> only architecture which may be affected by the original bug is ia64
>> which is the only one with both kvm and the unlocked context switch.
> 
> Do you have a pointer to the original bug report ?

Nope, I was referring to the imaginary race condition, so there's no
bug to worry about.  The only problem is the asymmetry between in and
out callbacks.  Then again, it's not really possible to match them on
unlocked ctxsw archs anyway, so I guess the only thing to do is to
document the context difference between in and out.

Sorry about the fuss.  I'll send out patch to revert it and document
the difference.

Thanks.

-- 
tejun

[PATCH tip/sched/urgent] sched: revert 498657a478c60be092208422fefa9c7b248729c2

[Tejun Heo]

498657a478c60be092208422fefa9c7b248729c2 incorrectly assumed that
preempt wasn't disabled around context_switch() and thus was fixing
imaginary problem.  It also broke kvm because it depended on
->sched_in() to be called with irq enabled so that it can do smp calls
from there.

Revert the incorrect commit and add comment describing different
contexts under with the two callbacks are invoked.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Avi Kivity <avi@redhat.com>
---
Again, my apologies for the unnecessary fuss.  I for some reason was
thinking schedule_tail() is always called after context_switch() and
the ifdefed preemption enable there led me to think that at that point
preemption was enabled once lock switch is over.

Thank you.

 include/linux/preempt.h |    4 ++++
 kernel/sched.c          |    2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/preempt.h b/include/linux/preempt.h
index 72b1a10..736892c 100644
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -105,6 +105,10 @@ struct preempt_notifier;
  * @sched_out: we've just been preempted
  *    notifier: struct preempt_notifier for the task being preempted
  *    next: the task that's kicking us out
+ *
+ * Please note that sched_in and out are called under different
+ * contexts.  sched_in is called with rq lock held and irq disabled
+ * while sched_out is called without rq lock and irq enabled.
  */
 struct preempt_ops {
 	void (*sched_in)(struct preempt_notifier *notifier, int cpu);
diff --git a/kernel/sched.c b/kernel/sched.c
index 3c91f11..e36c868 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -2758,9 +2758,9 @@ static void finish_task_switch(struct rq *rq, struct task_struct *prev)
 	prev_state = prev->state;
 	finish_arch_switch(prev);
 	perf_event_task_sched_in(current, cpu_of(rq));
-	fire_sched_in_preempt_notifiers(current);
 	finish_lock_switch(rq, prev);
 
+	fire_sched_in_preempt_notifiers(current);
 	if (mm)
 		mmdrop(mm);
 	if (unlikely(prev_state == TASK_DEAD)) {

Re: [PATCH tip/sched/urgent] sched: revert 498657a478c60be092208422fefa9c7b248729c2

[Avi Kivity]

On 11/30/2009 01:02 PM, Tejun Heo wrote:
> 498657a478c60be092208422fefa9c7b248729c2 incorrectly assumed that
> preempt wasn't disabled around context_switch() and thus was fixing
> imaginary problem.  It also broke kvm because it depended on
> ->sched_in() to be called with irq enabled so that it can do smp calls
> from there.
>
> Revert the incorrect commit and add comment describing different
> contexts under with the two callbacks are invoked.
>
>    *    notifier: struct preempt_notifier for the task being preempted
>    *    next: the task that's kicking us out
> + *
> + * Please note that sched_in and out are called under different
> + * contexts.  sched_in is called with rq lock held and irq disabled
> + * while sched_out is called without rq lock and irq enabled.
>    */
>    

Is this transposed?  sched_in must be called with interrupts enabled for 
the kvm IPI to work.

-- 
error compiling committee.c: too many arguments to function