KVM: fix apic timer migration when inactive
When local apic timer is inactive or is expired in one shot mode, it should not
be restarted on vcpu and hrtimer migration. This patch fixes this.
Signed-off-by: Qing He <[EMAIL PROTECTED]>
Signed-off-by: Avi Kivity <[EMAIL PROTECTED]>
>-----Original Message-----
>From: Avi Kivity [mailto:[EMAIL PROTECTED]
>Sent: 2007年9月10日 15:27
>To: He, Qing
>Cc: kvm-devel@lists.sourceforge.net
>Subject: Re: [kvm-devel] [PATCH 2/3] KVM: fix apic timer save/migration
>wheninactive
>
>He, Qing wrote:
>> resended, due to wrong attachment
>>
>>
>
>Patch looks good, but needs a changelog comment and a signoff.
>>> -----Original Message-----
>>> From: [EMAIL PROTECTED]
>>> [mailto:[EMAIL PROTECTED] On Behalf Of He, Qing
>>> Sent: 2007年9月10日 13:59
>>> To: Avi Kivity
>>> Cc: kvm-devel@lists.sourceforge.net
>>> Subject: Re: [kvm-devel] [PATCH 2/3] KVM: fix apic timer save/migration
>wheninactive
>>>
>>> Avi,
>>> This is the updated patch, which avoids inactive hrtimers from
>>> unintended
>>> migrations.
>>>
>>> save/restore timer logic is not included because it does not call
>>> hrtimer_cancel
>>> (hrtimer_active is different). It has relatively small impact, for it only
>>> affects
>save/restore
>>> at maybe guest BIOS time, maybe we can defer it to some later time
>>>
>>>
>>>> -----Original Message-----
>>>> From: Avi Kivity [mailto:[EMAIL PROTECTED]
>>>> Sent: 2007年9月9日 15:39
>>>> To: He, Qing
>>>> Cc: kvm-devel@lists.sourceforge.net
>>>> Subject: Re: [kvm-devel] [PATCH 2/3] KVM: fix apic timer save/migration
>>>> when
>inactive
>>>>
>>>> He, Qing wrote:
>>>>
>>>>>>>> + 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(...);
>>>>>>
>>>>>>
>>>>> Thanks for commenting, previously I have thought of something like:
>>>>> if (hrtimer_active(...)) {
>>>>> hrtimer_cancel(...);
>>>>> hrtimer_start(...);
>>>>> }
>>>>>
>>>>> But it is not able to handle pending state this way. Seems the return
>>>>> value of
>>>>>
>>>> hrtimer_cancel is fine. Another question is, when the hrtimer function
>>>> returns
>>>> HRTIMER_NORESTART, does the state finally turn to HRTIMER_STATE_INACTIVE?
>>>>
>>> >From my reading of the hrtimer code, yes. But I would view any code
>>>
>>>> which depends on the state with suspicion.
>>>>
>>>>
>>>> --
>>>> error compiling committee.c: too many arguments to function
>>>>
>>>> ------------------------------------------------------------------------
>>>>
>>>> -------------------------------------------------------------------------
>>>> This SF.net email is sponsored by: Microsoft
>>>> Defy all challenges. Microsoft(R) Visual Studio 2005.
>>>> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
>>>> ------------------------------------------------------------------------
>>>>
>>>> _______________________________________________
>>>> kvm-devel mailing list
>>>> kvm-devel@lists.sourceforge.net
>>>> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>>>>
>
>
>--
>error compiling committee.c: too many arguments to function
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
