Re: [patch 2/2] KVM: PIT: provide an option to disable interrupt reinjection

[Marcelo Tosatti <mtosatti@redhat.com>]

On Tue, Dec 30, 2008 at 12:08:01PM +0200, Avi Kivity wrote:
> Marcelo Tosatti wrote:
>> Certain clocks (such as TSC) in older 2.6 guests overaccount for lost
>> ticks, causing severe time drift. Interrupt reinjection magnifies the
>> problem.
>>
>> Provide an option to disable it.
>>
>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>
>> Index: kvm/arch/x86/kvm/i8254.c
>> ===================================================================
>> --- kvm.orig/arch/x86/kvm/i8254.c
>> +++ kvm/arch/x86/kvm/i8254.c
>> @@ -201,6 +201,9 @@ static int __pit_timer_fn(struct kvm_kpi
>>  	if (!atomic_inc_and_test(&pt->pending))
>>  		set_bit(KVM_REQ_PENDING_TIMER, &vcpu0->requests);
>>  +	if (pt->no_reinject)
>> +		atomic_set(&pt->pending, 1);
>> +
>>   
>
> What about moving this to the place where we atomic_inc()?  Any reason  
> not to?

The atomic_inc_and_test is right above, and we want to set
KVM_REQ_PENDING_TIMER on 0->1 transitions, so better keep it
separate. BTW the KVM_REQ_PENDING_TIMER optimization is broken
(atomic_inc_and_test returns true if the _new_ value is zero) but thats
another issue.

Can't see the improvement you're suggesting.

>> --- kvm.orig/arch/x86/kvm/i8254.h
>> +++ kvm/arch/x86/kvm/i8254.h
>> @@ -9,6 +9,7 @@ struct kvm_kpit_timer {
>>  	s64 period; /* unit: ns */
>>  	s64 scheduled;
>>  	atomic_t pending;
>> +	bool no_reinject;
>>  };
>>   
>
> Negative logic = !good.  Call it reinject and init to true.

Sure.