Reduce redundant timer rearming

The dynticks code in qemu sets and gets timers very often. These
are the system calls (strace -c) of qemu/kvm running an idle Linux kernel at 250Hz for 10 seconds:

% time seconds usecs/call calls errors syscall
------ ----------- ----------- --------- --------- ----------------

 99.67    0.014391           1     23248      3187 ioctl
  0.33    0.000047           0     43643           clock_gettime
  0.00    0.000000           0      6375           gettimeofday
  0.00    0.000000           0      6708           select
  0.00    0.000000           0      3187           rt_sigaction
  0.00    0.000000           0      9562      6375 rt_sigtimedwait
  0.00    0.000000           0     10311           timer_settime
  0.00    0.000000           0     12750           timer_gettime

------ ----------- ----------- --------- --------- ----------------
100.00 0.014438 115784 9562 total

The qemu_rearm_alarm_timer() function looks at vm_clock as well as rt_clock timers, but is called from qemu_run_timers() which looks at just one queue.

When an rt_clock timer has expired, the vm_clock iteration will rearm
with MIN_TIMER_REARM_US. This is not needed, since the timer in question will be removed right away when qemu_run_timers() is run on the rt_clock queue.

Moving the rearm call to after the two calls of qemu_run_timers() helps a lot:

% time seconds usecs/call calls errors syscall
------ ----------- ----------- --------- --------- ----------------

 99.74    0.012590           1     22766      2706 ioctl
  0.26    0.000033           0     27044           clock_gettime
  0.00    0.000000           0      5413           gettimeofday
  0.00    0.000000           0      5745           select
  0.00    0.000000           0      2706           rt_sigaction
  0.00    0.000000           0      8119      5413 rt_sigtimedwait
  0.00    0.000000           0      5215           timer_settime
  0.00    0.000000           0      5413           timer_gettime

------ ----------- ----------- --------- --------- ----------------
100.00 0.012623 82421 8119 total

Patch (based on kvm-userspace git) attached.

Cheers,
Anders.

index 80ceb2f..2cd580d 100644
@@ -1063,7 +1063,6 @@ static void qemu_run_timers(QEMUTimer **ptimer_head, int64_t current_time)
         /* 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)

@@ -7216,6 +7215,8 @@ void main_loop_wait(int timeout)
     qemu_run_timers(&active_timers[QEMU_TIMER_REALTIME],
                     qemu_get_clock(rt_clock));
 
+    qemu_rearm_alarm_timer(alarm_timer);
+
     /* Check bottom-halves last in case any of the earlier events triggered
        them.  */
     qemu_bh_poll();

Here is another patch that implements a dirty bit for the timer lists. The bit is set when timers are modified or expire. Rearming clears the bit (and only happens if it is set).

Numbers improve to this:

% time seconds usecs/call calls errors syscall
------ ----------- ----------- --------- --------- ----------------

100.00    0.000013           0     21813           clock_gettime
  0.00    0.000000           0     22789      2729 ioctl
  0.00    0.000000           0      5459           gettimeofday
  0.00    0.000000           0      5790           select
  0.00    0.000000           0      2729           rt_sigaction
  0.00    0.000000           0      8188      5459 rt_sigtimedwait
  0.00    0.000000           0      2729           timer_settime
  0.00    0.000000           0      2752           timer_gettime

------ ----------- ----------- --------- --------- ----------------
100.00 0.000013 72249 8188 total

I am not so sure about this one, so comments are appreciated.

Best regards,
Anders.

index 2cd580d..924736e 100644
@@ -816,6 +816,7 @@ struct qemu_alarm_timer {
 };
 
 #define ALARM_FLAG_DYNTICKS  0x1
+#define ALARM_FLAG_MODIFIED  0x2
 
 static inline int alarm_has_dynticks(struct qemu_alarm_timer *t)
 {

@@ -827,6 +828,11 @@ static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t)
     if (!alarm_has_dynticks(t))
         return;
 
+    if (!(t->flags & ALARM_FLAG_MODIFIED))
+        return;
+
+    t->flags &= ~(ALARM_FLAG_MODIFIED);
+
     t->rearm(t);
 }
 

@@ -989,6 +995,8 @@ void qemu_del_timer(QEMUTimer *ts)
 {
     QEMUTimer **pt, *t;
 
+    alarm_timer->flags |= ALARM_FLAG_MODIFIED;
+
     /* NOTE: this code must be signal safe because
        qemu_timer_expired() can be called from a signal. */
     pt = &active_timers[ts->clock->type];

@@ -1182,6 +1190,7 @@ static void host_alarm_handler(int host_signum)
 #endif
         CPUState *env = cpu_single_env;
         if (env) {
+            alarm_timer->flags |= ALARM_FLAG_MODIFIED;
             /* stop the currently executing cpu because a timer occured */
             cpu_interrupt(env, CPU_INTERRUPT_EXIT);
 #ifdef USE_KQEMU

(resending with the subscribed sender address)

Here is another patch that implements a dirty bit for the timer lists. The bit is set when timers are modified or expire. Rearming clears the bit (and only happens if it is set).

Numbers improve to this:

% time seconds usecs/call calls errors syscall
------ ----------- ----------- --------- --------- ----------------

100.00    0.000013           0     21813           clock_gettime
  0.00    0.000000           0     22789      2729 ioctl
  0.00    0.000000           0      5459           gettimeofday
  0.00    0.000000           0      5790           select
  0.00    0.000000           0      2729           rt_sigaction
  0.00    0.000000           0      8188      5459 rt_sigtimedwait
  0.00    0.000000           0      2729           timer_settime
  0.00    0.000000           0      2752           timer_gettime

------ ----------- ----------- --------- --------- ----------------
100.00 0.000013 72249 8188 total

I am not so sure about this one, so comments are appreciated.

Best regards,
Anders.

index 2cd580d..924736e 100644
@@ -816,6 +816,7 @@ struct qemu_alarm_timer {
 };
 
 #define ALARM_FLAG_DYNTICKS  0x1
+#define ALARM_FLAG_MODIFIED  0x2
 
 static inline int alarm_has_dynticks(struct qemu_alarm_timer *t)
 {

@@ -827,6 +828,11 @@ static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t)
     if (!alarm_has_dynticks(t))
         return;
 
+    if (!(t->flags & ALARM_FLAG_MODIFIED))
+        return;
+
+    t->flags &= ~(ALARM_FLAG_MODIFIED);
+
     t->rearm(t);
 }
 

@@ -989,6 +995,8 @@ void qemu_del_timer(QEMUTimer *ts)
 {
     QEMUTimer **pt, *t;
 
+    alarm_timer->flags |= ALARM_FLAG_MODIFIED;
+
     /* NOTE: this code must be signal safe because
        qemu_timer_expired() can be called from a signal. */
     pt = &active_timers[ts->clock->type];

@@ -1182,6 +1190,7 @@ static void host_alarm_handler(int host_signum)
 #endif
         CPUState *env = cpu_single_env;
         if (env) {
+            alarm_timer->flags |= ALARM_FLAG_MODIFIED;
             /* stop the currently executing cpu because a timer occured */
             cpu_interrupt(env, CPU_INTERRUPT_EXIT);
 #ifdef USE_KQEMU

The VGA refresh (a 33 Hz timer) is running even with -nographics, apparently doing nothing. The vga_hw_update() call ends up just returning, because the display depth is zero.

This patch removes the dummy refresh handler, and thus the GUI refresh timer.

With an idle dyntick Linux guest in qemu/kvm, there are now only two timers per second (those are from the rtc). Another 10 seconds strace:

% time seconds usecs/call calls errors syscall
------ ----------- ----------- --------- --------- ----------------

100.00    0.000012           0      3760        20 ioctl
  0.00    0.000000           0        41           select
  0.00    0.000000           0        20           rt_sigaction
  0.00    0.000000           0        61        41 rt_sigtimedwait
  0.00    0.000000           0        20           timer_settime
  0.00    0.000000           0        20           timer_gettime
  0.00    0.000000           0       162           clock_gettime

------ ----------- ----------- --------- --------- ----------------
100.00 0.000012 4084 61 total

Cheers,
Anders.

index 30c9537..8d67314 100644
@@ -4902,13 +4902,6 @@ static void dumb_resize(DisplayState *ds, int w, int h)
 {
 }
 
-static void dumb_refresh(DisplayState *ds)
-{
-#if defined(CONFIG_SDL)
-    vga_hw_update();
-#endif
-}
-
 static void dumb_display_init(DisplayState *ds)
 {
     ds->data = NULL;

@@ -4916,7 +4909,7 @@ static void dumb_display_init(DisplayState *ds)
     ds->depth = 0;
     ds->dpy_update = dumb_update;
     ds->dpy_resize = dumb_resize;
-    ds->dpy_refresh = dumb_refresh;
+    ds->dpy_refresh = NULL;
 }
 
 /***********************************************************/

The VNC output runs two 33 Hz timers, one for calling vga_hw_update() in dpy_refresh (from vl.c), and another for calling vnc_update_client().

This patch moves the vga_hw_update() call into vnc_update_client(), removing the need for the dpy_refresh timer.

It also makes the remaining timer only fire when a client is actually connected to the VNC server.

Best regards,
Anders.

@@ -493,6 +493,8 @@ static void vnc_update_client(void *opaque)
 	int saved_offset;
 	int has_dirty = 0;
 
+        vga_hw_update();
+
         vnc_set_bits(width_mask, (vs->width / 16), VNC_DIRTY_WORDS);
 
 	/* Walk through the dirty map and eliminate tiles that

@@ -566,22 +568,11 @@ static void vnc_update_client(void *opaque)
 	vnc_flush(vs);
 
     }
-    qemu_mod_timer(vs->timer, qemu_get_clock(rt_clock) + VNC_REFRESH_INTERVAL);
-}
 
-static void vnc_timer_init(VncState *vs)
-{
-    if (vs->timer == NULL) {
-	vs->timer = qemu_new_timer(rt_clock, vnc_update_client, vs);
-	qemu_mod_timer(vs->timer, qemu_get_clock(rt_clock));
+    if (vs->csock != -1) {
+        qemu_mod_timer(vs->timer, qemu_get_clock(rt_clock) + VNC_REFRESH_INTERVAL);
     }
-}
 
-static void vnc_dpy_refresh(DisplayState *ds)
-{
-    VncState *vs = ds->opaque;
-    vnc_timer_init(vs);
-    vga_hw_update();
 }
 
 static int vnc_listen_poll(void *opaque)

@@ -1890,6 +1881,7 @@ static void vnc_listen_read(void *opaque)
 	vs->has_resize = 0;
 	vs->has_hextile = 0;
 	vs->ds->dpy_copy = NULL;
+        vnc_update_client(vs);
     }
 }
 

@@ -1923,10 +1915,12 @@ void vnc_display_init(DisplayState *ds)
     if (!vs->kbd_layout)
 	exit(1);
 
+    vs->timer = qemu_new_timer(rt_clock, vnc_update_client, vs);
+
     vs->ds->data = NULL;
     vs->ds->dpy_update = vnc_dpy_update;
     vs->ds->dpy_resize = vnc_dpy_resize;
-    vs->ds->dpy_refresh = vnc_dpy_refresh;
+    vs->ds->dpy_refresh = NULL;
 
     memset(vs->dirty_row, 0xFF, sizeof(vs->dirty_row));