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

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

[-- Attachment #1: Type: text/plain, Size: 3207 bytes --]

He, Qing wrote:
> resended, due to wrong attachment
>
>   

Patch looks good, but needs a changelog comment and a signoff.
>> -----Original Message-----
>> From: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>> [mailto:kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org] On Behalf Of He, Qing
>> Sent: 2007年9月10日 13:59
>> To: Avi Kivity
>> Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>> 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:avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org]
>>> Sent: 2007年9月9日 15:39
>>> To: He, Qing
>>> Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>>> 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-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>>> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>>>       


-- 
error compiling committee.c: too many arguments to function



[-- Attachment #2: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
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/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel