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

Avi Kivity
2009-04-30T13:25:38Z

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