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

[Thomas Gleixner <tglx@linutronix.de>]

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