diff mbox

UBUNTU: SAUCE: kvm: Force preempt folding in kvm on i386

Message ID 1396435346-7624-1-git-send-email-stefan.bader@canonical.com
State New
Headers show

Commit Message

Stefan Bader April 2, 2014, 10:42 a.m. UTC
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(+)

Comments

Stefan Bader April 2, 2014, 11:39 a.m. UTC | #1
Oh... I could have added the BugLink

BugLink: http://bugs.launchpad.net/bugs/1268906
Andy Whitcroft April 2, 2014, 12:09 p.m. UTC | #2
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
Andy Whitcroft April 2, 2014, 12:15 p.m. UTC | #3
Applied to Trusty.

-apw
diff mbox

Patch

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);
 		}