Re: [PATCH 2/3] KVM: fix apic timer save/migration when inactive

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Avi Kivity wrote:
> He, Qing wrote:
>>         When local apic timer is inactive or is expired in oneshot 
>> mode, it
>>     should not be restarted in save/restore or hrtimer migration. This
>>     patch fixes this.
>>
>> diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
>> index 5f97e25..68d454c 100644
>> --- a/drivers/kvm/irq.h
>> +++ b/drivers/kvm/irq.h
>> @@ -110,6 +110,7 @@ struct kvm_lapic {
>>      struct kvm_io_device dev;
>>      struct {
>>          atomic_t pending;
>> +        atomic_t active;
>>   
>
> This is atomic, but you never use read-modify-write instructions (read 
> and write are atomic on a simple int).
>
>> +
>> +    if (atomic_read(&apic->timer.active))
>>   
>
> What if the timer fires here?
>
>> +        apic_set_reg(apic, APIC_TMCCT, tmict);
>> +    else
>> +        apic_set_reg(apic, APIC_TMCCT, 0);
>> +}
>> +
>>  void kvm_migrate_apic_timer(struct kvm_vcpu *vcpu)
>> @@ -1036,11 +1062,14 @@ void kvm_migrate_apic_timer(struct kvm_vcpu
>> *vcpu)
>>      struct kvm_lapic *apic = vcpu->apic;
>>      struct hrtimer *timer;
>>  
>> -    if (apic) {
>> -        timer = &apic->timer.dev;
>> -        hrtimer_cancel(timer);
>> -        hrtimer_start(timer, timer->expires, HRTIMER_MODE_ABS);
>> -    }
>> +    if (!apic)
>> +        return;
>> +
>> +    timer = &apic->timer.dev;
>> +    hrtimer_cancel(timer);
>> +    if (atomic_read(&apic->timer.active))
>>   
>
> Or here?
>
>> +        hrtimer_start(timer, timer->expires,
>> +                  HRTIMER_MODE_ABS);
>>  }
>>  EXPORT_SYMBOL_GPL(kvm_migrate_apic_timer);
>>  
>>   
>

I think you could use the return value of hrtimer_cancel() here:

    if (hrtimer_cancel(...))
        hrtimer_start(...);



-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/