Message ID | 1396435346-7624-1-git-send-email-stefan.bader@canonical.com |
---|---|
State | New |
Headers | show |
Oh... I could have added the BugLink BugLink: http://bugs.launchpad.net/bugs/1268906
On Wed, Apr 02, 2014 at 12:42:26PM +0200, Stefan Bader wrote: > Since commit c2daa3bed53a81171cf8c1a36db798e82b91afe8 > Author: Peter Zijlstra <peterz@infradead.org> > sched, x86: Provide a per-cpu preempt_count implementation > > there is a problem on 32bit x86 which causes kvm to run wildly in a > tight loop. The cause is that since the preempt_count change it seems > possible on 32bit to have the thread info flag TIF_NEED_RESCHED set > but the IPI that in theory should update that flag into the per-cpu > counter does not see it. This leaves us in a bad place as this > counts as the process should reschule but is not allowed to. And > kvm unfortunately relies on those things to be in sync. > > There is actually a list of fixes to this in general which was not yet > pushed to stable (since we never found a resolution to this issue). So > i386 may behave unfair in general. This here just allows to run kvm on > i386 at all. > > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > arch/x86/kvm/x86.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 6dc4904..55b1792 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6092,6 +6092,20 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) > } > if (need_resched()) { > srcu_read_unlock(&kvm->srcu, vcpu->srcu_idx); > +#ifdef CONFIG_X86_32 > + /* > + * This is a hack around a current bug on i386 > + * that causes TIF_NEED_RESCHED not being propagated > + * into the per-cpu counter. This causes cond_resched() > + * which is what kvm_resched() calls to exit without > + * actually rescheduling. We continue immediately with > + * the loop but vcpu_enter_guest checks for the process > + * flag and immediately exits and this loops busily > + * runs forever. > + */ > + if (tif_need_resched()) > + set_preempt_need_resched(); > +#endif > kvm_resched(vcpu); > vcpu->srcu_idx = srcu_read_lock(&kvm->srcu); > } This looks much more like what I was expecting. I assume you have tested this, and as I recall it was very reproducible without this fix. Acked-by: Andy Whitcroft <apw@canonical.com> -apw
Applied to Trusty. -apw
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 6dc4904..55b1792 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -6092,6 +6092,20 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) } if (need_resched()) { srcu_read_unlock(&kvm->srcu, vcpu->srcu_idx); +#ifdef CONFIG_X86_32 + /* + * This is a hack around a current bug on i386 + * that causes TIF_NEED_RESCHED not being propagated + * into the per-cpu counter. This causes cond_resched() + * which is what kvm_resched() calls to exit without + * actually rescheduling. We continue immediately with + * the loop but vcpu_enter_guest checks for the process + * flag and immediately exits and this loops busily + * runs forever. + */ + if (tif_need_resched()) + set_preempt_need_resched(); +#endif kvm_resched(vcpu); vcpu->srcu_idx = srcu_read_lock(&kvm->srcu); }
Since commit c2daa3bed53a81171cf8c1a36db798e82b91afe8 Author: Peter Zijlstra <peterz@infradead.org> sched, x86: Provide a per-cpu preempt_count implementation there is a problem on 32bit x86 which causes kvm to run wildly in a tight loop. The cause is that since the preempt_count change it seems possible on 32bit to have the thread info flag TIF_NEED_RESCHED set but the IPI that in theory should update that flag into the per-cpu counter does not see it. This leaves us in a bad place as this counts as the process should reschule but is not allowed to. And kvm unfortunately relies on those things to be in sync. There is actually a list of fixes to this in general which was not yet pushed to stable (since we never found a resolution to this issue). So i386 may behave unfair in general. This here just allows to run kvm on i386 at all. Signed-off-by: Stefan Bader <stefan.bader@canonical.com> --- arch/x86/kvm/x86.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)