Re: [Qemu-devel] [PATCH] qemu-clock: add an alarm timer based on timerfd

[Anthony Liguori]

Jan Kiszka <address@hidden> writes:

> On 2012-09-19 09:26, Paolo Bonzini wrote:
>> Il 18/09/2012 22:37, Anthony Liguori ha scritto:
>>> Unfortunately, there's a lot of Windows code in qemu-timer.c and main-loop.c
>>> right now otherwise the refactoring would be trivial.  I'll leave that for
>>> another day.
>>>
>>> Cc: Paolo Bonzini <address@hidden>
>>> Cc: Jan Kiszka <address@hidden>
>>> Signed-off-by: Anthony Liguori <address@hidden>
>>> ---
>>> Please note, this is lightly tested.  Since this is such a fundamental 
>>> change,
>>> I'd like to do some performance analysis before committing but wanted to 
>>> share
>>> early.
>> 
>> Looks good.  I think Peter Portante tested something similar, and found no 
>> big
>> difference between the two.  But it's a good thing and, in my opinion, for
>> non-timerfd OSes we should simply adjust the select() timeout and not bother
>> with signals.
>
> What would be the advantage of timerfd over select? On Linux, both use
> hrtimers (and low slack for RT processes). I'm starting to like the
> select/WaitForMultipleObjects pattern as it would allow to consolidate
> over basically two versions of timers and simplify the code.

I don't think it's an either or.  Ideally we get to a complete glib main
loop and our timer implementation could fall back to use the timeout if
timerfd wasn't available.

Practically speaking, timerfd allows selecting the clock to use which
select() doesn't and it also supports absolute events instead of
relative events.  Not sure either matters all that much, but no doubt
it's better than using signals.

Regarding microsecond granularity, note that while select() provides
microsecond granualrity, poll() only provides millisecond granularity.

I think there are a lot of good reasons to evaluate moving to poll()
(glib uses poll by default).  With poll(), we definitely need timerfd.

Regards,

Anthony Liguori

>
> Jan
>
>> 
>> I'm not sure if the same can be done for Windows, but I think it's possible 
>> as long
>> as you keep the timeBeginPeriod/timeEndPeriod calls.  As a start, Stefan, 
>> can you
>> check if the win32 timer works for you with the calls added?  Like this:
>> 
>> diff --git a/qemu-timer.c b/qemu-timer.c
>> index c7a1551..721c769 100644
>> --- a/qemu-timer.c
>> +++ b/qemu-timer.c
>> @@ -673,6 +673,10 @@ static int win32_start_timer(struct qemu_alarm_timer *t)
>>      HANDLE hTimer;
>>      BOOLEAN success;
>>  
>> +    timeGetDevCaps(&mm_tc, sizeof(mm_tc));
>> +
>> +    timeBeginPeriod(mm_tc.wPeriodMin);
>> +
>>      /* If you call ChangeTimerQueueTimer on a one-shot timer (its period
>>         is zero) that has already expired, the timer is not updated.  Since
>>         creating a new timer is relatively expensive, set a bogus one-hour
>> @@ -688,6 +692,7 @@ static int win32_start_timer(struct qemu_alarm_timer *t)
>>      if (!success) {
>>          fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n",
>>                  GetLastError());
>> +        timeEndPeriod(mm_tc.wPeriodMin);
>>          return -1;
>>      }
>>  
>> @@ -702,6 +707,7 @@ static void win32_stop_timer(struct qemu_alarm_timer *t)
>>      if (hTimer) {
>>          DeleteTimerQueueTimer(NULL, hTimer, NULL);
>>      }
>> +    timeEndPeriod(mm_tc.wPeriodMin);
>>  }
>>  
>>  static void win32_rearm_timer(struct qemu_alarm_timer *t,
>> 
>> Paolo
>> 
>
> -- 
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux