Re: [PATCH 18/21] Remove host_alarm_timer hacks.

[Avi Kivity <avi@redhat.com>]

Anthony Liguori wrote:
> Avi Kivity wrote:
>> Anthony Liguori wrote:
>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>> ---
>>>  vl.c |    3 +--
>>>  1 files changed, 1 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/vl.c b/vl.c
>>> index 3b0e3dc..848a8f8 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -1367,8 +1367,7 @@ static void host_alarm_handler(int host_signum)
>>>          last_clock = ti;
>>>      }
>>>  #endif
>>> -    if (1 ||
>>> -        alarm_has_dynticks(alarm_timer) ||
>>> +    if (alarm_has_dynticks(alarm_timer) ||
>>>          (!use_icount &&
>>>              qemu_timer_expired(active_timers[QEMU_TIMER_VIRTUAL],
>>>                                 qemu_get_clock(vm_clock))) ||
>>>   
>>
>> This was added to fix a problem.  Have you tested it?
>
> Do you know what problem it fixes?
>
> This goes back a very long time.  IIUC, this was added prior to the IO 
> thread as an "optimization".  This ensures that any time there's a 
> timer, the vcpu is interrupted to allow IO to run.  With non-dynticks, 
> there can be spurious timer signals because we problem the timer with 
> a fixed frequency.  It's necessary to take this path with dynticks 
> because we need to rearm the timer which happens in the IO path.  It's 
> not necessary to take this path with a non-dynticks timer unless 
> there's been an expiration.
>
> In modern KVM, the IO thread is capable of interrupting the CPU 
> whenever it needs to process IO.  Therefore this "problem" no longer 
> exists.
>

It would still be good to verify that the problem no longer exists.  
This is not a cosmetic change; some testing is needed to verify it 
doesn't introduce new latencies.

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