[Qemu-devel] Re: [kvm-devel] [PATCH 0/4] Rework alarm timer infrastrucure - take2

[Luca Tettamanti]

Dor Laor ha scritto: 
> >> Hello,
> >> in reply to this mail I will send a serie of 4 patches that cleans up
> >and
> >> expands the alarm timer handling in QEMU. Patches have been rebased
> on
> >QEMU
> >> CVS.
> >>
> >> Patch 1 is mostly a cleanup of the existing code; instead of having
> >multiple
> >> #ifdefs to handle different timers scattered all over the code I've
> >created a
> >> modular infrastructure where each timer type is self-contained and
> >generic code
> >> is more readable. The resulting code is functionally equivalent to
> the
> >old one.
> >>
> >> Patch 2 implements the "-clock" command line option proposed by
> Daniel
> >Berrange
> >> and Avi Kivity. By default QEMU tries RTC and then falls back to unix
> >timer;
> >> user can override the order of the timer through this options. Syntax
> >is pretty
> >> simple: -clock timer1,timer2,etc. (QEMU will pick the first one that
> >works).
> >>
> >> Patch 3 adds support for HPET under Linux (which is basically my old
> >patch). As
> >> suggested HPET takes precedence over other timers, but of course this
> >can be
> >> overridden.
> >>
> >> Patch 4 introduces "dynticks" timer source; patch is mostly based on
> >the work
> >> Dan Kenigsberg. dynticks is now the default alarm timer.
> >
> >Why do you guard dynticks with #ifdef?  Is there any reason why you
> >wouldn't want to use dynticks?
> 
> I think too that it's should be the default.
> There is no regression in performance nor behaviour with this option.

Ok, I've updated the patch. It was pretty easy to implement the same
feature for win32 (slightly tested inside a winxp VM).

> We didn't test qemu dyn-tick with kernels that don't have
> high-res-timer+dyn-tick.

I did ;)

> In this case the dyn-tick minimum res will be 1msec. I believe it should
> work ok since this is the case without any dyn-tick.

Actually minimum resolution depends on host HZ setting, but - yes -
essentially you have the same behaviour of the "unix" timer, plus the
overhead of reprogramming the timer. 

Add support for dynamic ticks.

If the the dynticks alarm timer is used qemu does not attempt to generate
SIGALRM at a constant rate. Rather, the system timer is set to generate SIGALRM
only when it is needed. Dynticks timer reduces the number of SIGALRMs sent to
idle dynamic-ticked guests.
Original patch from Dan Kenigsberg <address@hidden>

Signed-off-by: Luca Tettamanti <address@hidden>

---
 vl.c |  178 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 170 insertions(+), 8 deletions(-)

Index: qemu/vl.c
===================================================================
--- qemu.orig/vl.c      2007-08-18 23:23:47.000000000 +0200
+++ qemu/vl.c   2007-08-18 23:23:53.000000000 +0200
@@ -784,12 +784,31 @@
 
 struct qemu_alarm_timer {
     char const *name;
+    unsigned int flags;
 
     int (*start)(struct qemu_alarm_timer *t);
     void (*stop)(struct qemu_alarm_timer *t);
+    void (*rearm)(struct qemu_alarm_timer *t);
     void *priv;
 };
 
+#define ALARM_FLAG_DYNTICKS  0x1
+
+static inline int alarm_has_dynticks(struct qemu_alarm_timer *t)
+{
+    return t->flags & ALARM_FLAG_DYNTICKS;
+}
+
+static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t) {
+    if (!alarm_has_dynticks(t))
+        return;
+
+    t->rearm(t);
+}
+
+/* TODO: MIN_TIMER_REARM_US should be optimized */
+#define MIN_TIMER_REARM_US 250
+
 static struct qemu_alarm_timer *alarm_timer;
 
 #ifdef _WIN32
@@ -802,12 +821,17 @@
 
 static int win32_start_timer(struct qemu_alarm_timer *t);
 static void win32_stop_timer(struct qemu_alarm_timer *t);
+static void win32_rearm_timer(struct qemu_alarm_timer *t);
 
 #else
 
 static int unix_start_timer(struct qemu_alarm_timer *t);
 static void unix_stop_timer(struct qemu_alarm_timer *t);
 
+static int dynticks_start_timer(struct qemu_alarm_timer *t);
+static void dynticks_stop_timer(struct qemu_alarm_timer *t);
+static void dynticks_rearm_timer(struct qemu_alarm_timer *t);
+
 #ifdef __linux__
 
 static int hpet_start_timer(struct qemu_alarm_timer *t);
@@ -816,21 +840,23 @@
 static int rtc_start_timer(struct qemu_alarm_timer *t);
 static void rtc_stop_timer(struct qemu_alarm_timer *t);
 
-#endif
+#endif /* __linux__ */
 
 #endif /* _WIN32 */
 
 static struct qemu_alarm_timer alarm_timers[] = {
+#ifndef _WIN32
+    {"dynticks", ALARM_FLAG_DYNTICKS, dynticks_start_timer, 
dynticks_stop_timer, dynticks_rearm_timer, NULL},
 #ifdef __linux__
     /* HPET - if available - is preferred */
-    {"hpet", hpet_start_timer, hpet_stop_timer, NULL},
+    {"hpet", 0, hpet_start_timer, hpet_stop_timer, NULL, NULL},
     /* ...otherwise try RTC */
-    {"rtc", rtc_start_timer, rtc_stop_timer, NULL},
+    {"rtc", 0, rtc_start_timer, rtc_stop_timer, NULL, NULL},
 #endif
-#ifndef _WIN32
-    {"unix", unix_start_timer, unix_stop_timer, NULL},
+    {"unix", 0, unix_start_timer, unix_stop_timer, NULL, NULL},
 #else
-    {"win32", win32_start_timer, win32_stop_timer, &alarm_win32_data},
+    {"dynticks", ALARM_FLAG_DYNTICKS, win32_start_timer, win32_stop_timer, 
win32_rearm_timer, &alarm_win32_data},
+    {"win32", 0, win32_start_timer, win32_stop_timer, NULL, &alarm_win32_data},
 #endif
     {NULL, }
 };
@@ -949,6 +975,8 @@
         }
         pt = &t->next;
     }
+
+    qemu_rearm_alarm_timer(alarm_timer);
 }
 
 /* modify the current timer so that it will be fired when current_time
@@ -1008,6 +1036,7 @@
         /* run the callback (the timer list can be modified) */
         ts->cb(ts->opaque);
     }
+    qemu_rearm_alarm_timer(alarm_timer);
 }
 
 int64_t qemu_get_clock(QEMUClock *clock)
@@ -1115,7 +1144,8 @@
         last_clock = ti;
     }
 #endif
-    if (qemu_timer_expired(active_timers[QEMU_TIMER_VIRTUAL],
+    if (alarm_has_dynticks(alarm_timer) ||
+        qemu_timer_expired(active_timers[QEMU_TIMER_VIRTUAL],
                            qemu_get_clock(vm_clock)) ||
         qemu_timer_expired(active_timers[QEMU_TIMER_REALTIME],
                            qemu_get_clock(rt_clock))) {
@@ -1136,6 +1166,27 @@
     }
 }
 
+static uint64_t qemu_next_deadline(void) {
+    uint64_t nearest_delta_us = ULLONG_MAX;
+    uint64_t vmdelta_us;
+
+    if (active_timers[QEMU_TIMER_REALTIME])
+        nearest_delta_us = (active_timers[QEMU_TIMER_REALTIME]->expire_time - 
qemu_get_clock(rt_clock))*1000;
+
+    if (active_timers[QEMU_TIMER_VIRTUAL]) {
+        /* round up */
+        vmdelta_us = (active_timers[QEMU_TIMER_VIRTUAL]->expire_time - 
qemu_get_clock(vm_clock)+999)/1000;
+        if (vmdelta_us < nearest_delta_us)
+            nearest_delta_us = vmdelta_us;
+    }
+
+    /* Avoid arming the timer to negative, zero, or too low values */
+    if (nearest_delta_us <= MIN_TIMER_REARM_US)
+        nearest_delta_us = MIN_TIMER_REARM_US;
+
+    return nearest_delta_us;
+}
+
 #ifndef _WIN32
 
 #if defined(__linux__)
@@ -1243,6 +1294,80 @@
 
 #endif /* !defined(__linux__) */
 
+static int dynticks_start_timer(struct qemu_alarm_timer *t)
+{
+    struct sigevent ev;
+    timer_t host_timer;
+    struct sigaction act;
+
+    sigfillset(&act.sa_mask);
+    act.sa_flags = 0;
+#if defined(TARGET_I386) && defined(USE_CODE_COPY)
+    act.sa_flags |= SA_ONSTACK;
+#endif
+    act.sa_handler = host_alarm_handler;
+
+    sigaction(SIGALRM, &act, NULL);
+
+    ev.sigev_value.sival_int = 0;
+    ev.sigev_notify = SIGEV_SIGNAL;
+    ev.sigev_signo = SIGALRM;
+
+    if (timer_create(CLOCK_REALTIME, &ev, &host_timer)) {
+        perror("timer_create");
+
+        /* disable dynticks */
+        fprintf(stderr, "Dynamic Ticks disabled\n");
+
+        return -1;
+    }
+
+    t->priv = (void *)host_timer;
+
+    return 0;
+}
+
+static void dynticks_stop_timer(struct qemu_alarm_timer *t)
+{
+    timer_t host_timer = (timer_t)t->priv;
+
+    timer_delete(host_timer);
+}
+
+static void dynticks_rearm_timer(struct qemu_alarm_timer *t)
+{
+    timer_t host_timer = (timer_t)t->priv;
+    struct itimerspec timeout;
+    int64_t nearest_delta_us = INT64_MAX;
+    int64_t current_us;
+
+    if (!active_timers[QEMU_TIMER_REALTIME] &&
+                !active_timers[QEMU_TIMER_VIRTUAL])
+            return;
+
+    nearest_delta_us = qemu_next_deadline();
+
+    /* check whether a timer is already running */
+    if (timer_gettime(host_timer, &timeout)) {
+        perror("gettime");
+        fprintf(stderr, "Internal timer error: aborting\n");
+        exit(1);
+    }
+    current_us = timeout.it_value.tv_sec * 1000000 + 
timeout.it_value.tv_nsec/1000;
+    if (current_us && current_us <= nearest_delta_us)
+        return;
+
+    timeout.it_interval.tv_sec = 0;
+    timeout.it_interval.tv_nsec = 0; /* 0 for one-shot timer */
+    timeout.it_value.tv_sec =  nearest_delta_us / 1000000;
+    timeout.it_value.tv_nsec = (nearest_delta_us % 1000000) * 1000;
+    if (timer_settime(host_timer, 0 /* RELATIVE */, &timeout, NULL)) {
+        perror("settime");
+        fprintf(stderr, "Internal timer error: aborting\n");
+        exit(1);
+    }
+}
+
 static int unix_start_timer(struct qemu_alarm_timer *t)
 {
     struct sigaction act;
@@ -1288,6 +1413,7 @@
 {
     TIMECAPS tc;
     struct qemu_alarm_win32 *data = t->priv;
+    UINT flags;
 
     data->host_alarm = CreateEvent(NULL, FALSE, FALSE, NULL);
     if (!data->host_alarm) {
@@ -1304,11 +1430,17 @@
 
     timeBeginPeriod(data->period);
 
+    flags = TIME_CALLBACK_FUNCTION;
+    if (alarm_has_dynticks(t))
+        flags |= TIME_ONESHOT;
+    else
+        flags |= TIME_PERIODIC;
+
     data->timerId = timeSetEvent(1,         // interval (ms)
                         data->period,       // resolution
                         host_alarm_handler, // function
                         (DWORD)t,           // parameter
-                        TIME_PERIODIC | TIME_CALLBACK_FUNCTION);
+                        flags);
 
     if (!data->timerId) {
         perror("Failed to initialize win32 alarm timer");
@@ -1333,6 +1465,35 @@
     CloseHandle(data->host_alarm);
 }
 
+static void win32_rearm_timer(struct qemu_alarm_timer *t)
+{
+    struct qemu_alarm_win32 *data = t->priv;
+    uint64_t nearest_delta_us;
+
+    if (!active_timers[QEMU_TIMER_REALTIME] &&
+                !active_timers[QEMU_TIMER_VIRTUAL])
+            return;
+
+    nearest_delta_us = qemu_next_deadline();
+    nearest_delta_us /= 1000;
+
+    timeKillEvent(data->timerId);
+
+    data->timerId = timeSetEvent(1,
+                        data->period,
+                        host_alarm_handler,
+                        (DWORD)t,
+                        TIME_ONESHOT | TIME_PERIODIC);
+
+    if (!data->timerId) {
+        perror("Failed to re-arm win32 alarm timer");
+
+        timeEndPeriod(data->period);
+        CloseHandle(data->host_alarm);
+        exit(1);
+    }
+}
+
 #endif /* _WIN32 */
 
 static void init_timer_alarm(void)
@@ -6491,6 +6652,7 @@
         cpu_enable_ticks();
         vm_running = 1;
         vm_state_notify(1);
+        qemu_rearm_alarm_timer(alarm_timer);
     }
 }
 

Luca
-- 
Let me make your mind, leave yourself behind
Be not afraid