[Trusty] UBUNTU: SAUCE: Fix kvm hangs on i386

[Stefan Bader]

On 02.04.2014 11:31, Andy Whitcroft wrote:
> On Wed, Apr 02, 2014 at 10:59:48AM +0200, Stefan Bader wrote:
>> Unfortunately completely failed to proceed with this one (me and
>> upstream). This hack is curing at least the big issue with kvm.
>> As noted in the patch, there is a series of fixes after 3.13 which
>> I had been picking up while looking into this. These would be stable
>> material (minus maybe the idle changes though they might be useful for
>> other reasons and I did not want to make guesses about the things that
>> patch #3 changes there).
>>
>> 1. x86, acpi, idle: Restructure the mwait idle routines
>> 2. x86, idle: Use static_cpu_has() for CLFLUSH workaround, add barriers
>> 3. sched/preempt: Fix up missed PREEMPT_NEED_RESCHED folding
>> 4. sched/preempt/x86: Fix voluntary preempt for x86
>>
>> This just for documentation and in case anything else looks odd in
>> the scheduler and this might be a reason. Its now too late to pick those
>> and the hack works without.
>>
>> -Stefan
>>
>> ---
>>
>> From 951bca51ab8456d2a2cbca33f440680ae675b5e4 Mon Sep 17 00:00:00 2001
>> From: Stefan Bader <stefan.bader at canonical.com>
>> Date: Tue, 1 Apr 2014 20:26:00 +0200
>> Subject: [PATCH] UBUNTU: SAUCE: kvm: Use schedule instead of cond_reschedule
>>  on i386
>>
>> Since commit c2daa3bed53a81171cf8c1a36db798e82b91afe8
>>   Author: Peter Zijlstra <peterz at 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 at 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..512b1ce 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -6092,7 +6092,21 @@ 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.
>> +			 */
>> +			schedule();
>> +#else
>>  			kvm_resched(vcpu);
>> +#endif
>>  			vcpu->srcu_idx = srcu_read_lock(&kvm->srcu);
>>  		}
>>  	}
> 
> I am slightly worried that switching to schedule() here is triggering
> more work that kvm_resched() would have done.  As the stated bug is that
> TIF_NEED_RESCHED does not get copied over correctly "sometimes".  Could
> we not make this patch add like:
> 
> 	if (test_tsk_need_resched(current))
> 		set_preempt_need_resched();
> 
> Just before kvm_resched() in the 32bit case?
> 
> -apw
> 

I am working on something equivalent. Will update here when I got it compiled
and tested.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20140402/3a99e9ea/attachment.sig>