[Qemu-devel] [PATCH] Reduce redundant timer rearming

[Qemu-devel] [PATCH] Reduce redundant timer rearming

[Anders]

[-- Attachment #1: Type: text/plain, Size: 2227 bytes --]

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.

[-- Attachment #2: qemu-single-rearm.diff --]
[-- Type: text/x-patch, Size: 707 bytes --]

diff --git a/qemu/vl.c b/qemu/vl.c
index 80ceb2f..2cd580d 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -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();

Re: [Qemu-devel] [PATCH] Reduce redundant timer rearming

[Anders Melchiorsen]

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]


> % 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

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.


[-- Attachment #2: qemu-rearm-when-modified.diff --]
[-- Type: text/x-patch, Size: 1186 bytes --]

diff --git a/qemu/vl.c b/qemu/vl.c
index 2cd580d..924736e 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -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

Re: [Qemu-devel] [PATCH 2] Reduce redundant timer rearming

[Anders]

[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]

(resending with the subscribed sender address)

 > % 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

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.



[-- Attachment #2: qemu-rearm-when-modified.diff --]
[-- Type: text/x-patch, Size: 1186 bytes --]

diff --git a/qemu/vl.c b/qemu/vl.c
index 2cd580d..924736e 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -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

Re: [Qemu-devel] [PATCH 3] Reduce redundant timer rearming

[Anders]

[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]

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.

[-- Attachment #2: qemu-disable-dummy-vga-refresh.patch --]
[-- Type: text/x-patch, Size: 659 bytes --]

diff --git a/qemu/vl.c b/qemu/vl.c
index 30c9537..8d67314 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -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;
 }
 
 /***********************************************************/

Re: [Qemu-devel] [PATCH 4] Reduce redundant timer rearming

[Anders]

[-- Attachment #1: Type: text/plain, Size: 384 bytes --]

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.

[-- Attachment #2: qemu-dynamic-vnc-refresh.diff --]
[-- Type: text/x-patch, Size: 1569 bytes --]

--- a/qemu/vnc.c
+++ b/qemu/vnc.c
@@ -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));