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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
