[Qemu-devel] [PULL 0/9] vnc patches

[Qemu-devel] [PULL 0/9] vnc patches

[Gerd Hoffmann]

  Hi,

Picked up a collection of vnc patches send to the list over the last
months.  Reviewed, picked into a branch, smoke-tested.  Here is the
pull req for them.

If anyone has pending vnc bits not included here please resend & Cc /me.

please pull,
  Gerd

The following changes since commit f53f3d0a00b6df39ce8dfca942608e5b6a9a4f71:

  Merge remote-tracking branch 'remotes/kvaneesh/for-upstream' into staging (2014-03-08 12:38:43 +0000)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-vnc-1

for you to fetch changes up to e22492d332c5c80ea8cb1755c29ca9b6240c7716:

  ui/vnc: disable adaptive update calculations if not needed (2014-03-10 12:35:04 +0100)

----------------------------------------------------------------
vnc dirty tracking optinizations.
various vnc bugfixes.

----------------------------------------------------------------
Gerd Hoffmann (1):
      vnc: fix use-after-free in vnc_update_client_sync

Gonglei (Arei) (1):
      vnc: Fix qemu crashed when vnc client disconnect suddenly

Markus Armbruster (1):
      vnc: Fix tight_detect_smooth_image() for lossless case

Peter Lieven (6):
      ui/vnc: introduce VNC_DIRTY_PIXELS_PER_BIT macro
      ui/vnc: derive cmp_bytes from VNC_DIRTY_PIXELS_PER_BIT
      ui/vnc: optimize dirty bitmap tracking
      ui/vnc: optimize clearing in find_and_clear_dirty_height()
      ui/vnc: optimize setting in vnc_dpy_update()
      ui/vnc: disable adaptive update calculations if not needed

 ui/vnc-enc-tight.c |   2 +-
 ui/vnc-jobs.c      |   5 ++
 ui/vnc.c           | 227 +++++++++++++++++++++++++++++------------------------
 ui/vnc.h           |  10 ++-
 4 files changed, 141 insertions(+), 103 deletions(-)

[Qemu-devel] [PULL 1/9] vnc: Fix tight_detect_smooth_image() for lossless case

[Gerd Hoffmann]

From: Markus Armbruster <armbru@redhat.com>

VncTight member uint8_t quality is either (uint8_t)-1 for lossless or
less than 10 for lossy.

tight_detect_smooth_image() first promotes it to int, then compares
with -1.  Always unequal, so we always execute the lossy code.  Reads
beyond tight_conf[] and returns crap when quality is actually
lossless.

Compare to (uint8_t)-1 instead, like we do elsewhere.

Spotted by Coverity.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc-enc-tight.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index e6966ae..59b59c0 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -330,7 +330,7 @@ tight_detect_smooth_image(VncState *vs, int w, int h)
     } else {
         errors = tight_detect_smooth_image16(vs, w, h);
     }
-    if (quality != -1) {
+    if (quality != (uint8_t)-1) {
         return (errors < tight_conf[quality].jpeg_threshold);
     }
     return (errors < tight_conf[compression].gradient_threshold);
-- 
1.8.3.1

[Qemu-devel] [PULL 2/9] vnc: Fix qemu crashed when vnc client disconnect suddenly

[Gerd Hoffmann]

From: "Gonglei (Arei)" <arei.gonglei@huawei.com>

Hi,

When I use RealVNC viewer client (http://www.realvnc.com/) to connect vnc server,
the client disconnect suddenly, and I click reconnect button immediately, then the Qemu crashed.

In the function vnc_worker_thread_loop, will call vnc_async_encoding_start
to set the local vs->output buffer by global queue's buffer. Then send rectangles to
the vnc client call function vnc_send_framebuffer_update. Finally, Under normal circumstances,
call vnc_async_encoding_end to set the global queue'buffer by the local vs->output conversely.

When the vnc client disconnect, the job->vs->csock will be set to -1. And the current prcoess
logic will goto disconnected partion without call function vnc_async_encoding_end.
But, the function vnc_send_framebuffer_update will call buffer_reserve, which
maybe call g_realloc reset the local vs's buffer, meaning the global queue's buffer is modified also.
If anyone use the original global queue's buffer memory will cause corruption and then crash qemu.

This patch assure the function vnc_async_encoding_end being called
even though the vnc client disconnect suddenly.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc-jobs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 2d3fce8..a141f40 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -252,6 +252,8 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 
         if (job->vs->csock == -1) {
             vnc_unlock_display(job->vs->vd);
+            /* Copy persistent encoding data */
+            vnc_async_encoding_end(job->vs, &vs);
             goto disconnected;
         }
 
@@ -278,6 +280,9 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
         vnc_async_encoding_end(job->vs, &vs);
 
 	qemu_bh_schedule(job->vs->bh);
+    }  else {
+        /* Copy persistent encoding data */
+        vnc_async_encoding_end(job->vs, &vs);
     }
     vnc_unlock_output(job->vs);
 
-- 
1.8.3.1

[Qemu-devel] [PULL 3/9] vnc: fix use-after-free in vnc_update_client_sync

[Gerd Hoffmann]

Spotted by Coverity:

876     static int vnc_update_client_sync(VncState *vs, int has_dirty)
877     {

(1) Event freed_arg:    "vnc_update_client(VncState *, int)" frees "vs".  [details]
Also see events:        [deref_arg]

878         int ret = vnc_update_client(vs, has_dirty);

(2) Event deref_arg:    Calling "vnc_jobs_join(VncState *)" dereferences freed pointer "vs". [details]
Also see events:        [freed_arg]

879         vnc_jobs_join(vs);
880         return ret;
881     }

Remove vnc_update_client_sync wrapper, replace it with an additional
argument to vnc_update_client, so we can so the sync properly in
vnc_update_client (i.e. skip it in case of a client disconnect).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 ui/vnc.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 7dfc94a..b053d0e 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -417,8 +417,7 @@ out_error:
    3) resolutions > 1024
 */
 
-static int vnc_update_client(VncState *vs, int has_dirty);
-static int vnc_update_client_sync(VncState *vs, int has_dirty);
+static int vnc_update_client(VncState *vs, int has_dirty, bool sync);
 static void vnc_disconnect_start(VncState *vs);
 
 static void vnc_colordepth(VncState *vs);
@@ -751,7 +750,7 @@ static void vnc_dpy_copy(DisplayChangeListener *dcl,
     QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) {
         if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
             vs->force_update = 1;
-            vnc_update_client_sync(vs, 1);
+            vnc_update_client(vs, 1, true);
             /* vs might be free()ed here */
         }
     }
@@ -874,14 +873,7 @@ static int find_and_clear_dirty_height(struct VncState *vs,
     return h;
 }
 
-static int vnc_update_client_sync(VncState *vs, int has_dirty)
-{
-    int ret = vnc_update_client(vs, has_dirty);
-    vnc_jobs_join(vs);
-    return ret;
-}
-
-static int vnc_update_client(VncState *vs, int has_dirty)
+static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
 {
     if (vs->need_update && vs->csock != -1) {
         VncDisplay *vd = vs->vd;
@@ -940,8 +932,11 @@ static int vnc_update_client(VncState *vs, int has_dirty)
         return n;
     }
 
-    if (vs->csock == -1)
+    if (vs->csock == -1) {
         vnc_disconnect_finish(vs);
+    } else if (sync) {
+        vnc_jobs_join(vs);
+    }
 
     return 0;
 }
@@ -2734,7 +2729,7 @@ static void vnc_refresh(DisplayChangeListener *dcl)
     vnc_unlock_display(vd);
 
     QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) {
-        rects += vnc_update_client(vs, has_dirty);
+        rects += vnc_update_client(vs, has_dirty, false);
         /* vs might be free()ed here */
     }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 4/9] ui/vnc: introduce VNC_DIRTY_PIXELS_PER_BIT macro

[Gerd Hoffmann]

From: Peter Lieven <pl@kamp.de>

Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 65 ++++++++++++++++++++++++++++++++++++++++------------------------
 ui/vnc.h |  6 +++++-
 2 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index b053d0e..54e97a4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -442,17 +442,19 @@ static void vnc_dpy_update(DisplayChangeListener *dcl,
        iteration.  otherwise, if (x % 16) != 0, the last iteration may span
        two 16-pixel blocks but we only mark the first as dirty
     */
-    w += (x % 16);
-    x -= (x % 16);
+    w += (x % VNC_DIRTY_PIXELS_PER_BIT);
+    x -= (x % VNC_DIRTY_PIXELS_PER_BIT);
 
     x = MIN(x, width);
     y = MIN(y, height);
     w = MIN(x + w, width) - x;
     h = MIN(h, height);
 
-    for (; y < h; y++)
-        for (i = 0; i < w; i += 16)
-            set_bit((x + i) / 16, s->dirty[y]);
+    for (; y < h; y++) {
+        for (i = 0; i < w; i += VNC_DIRTY_PIXELS_PER_BIT) {
+            set_bit((x + i) / VNC_DIRTY_PIXELS_PER_BIT, s->dirty[y]);
+        }
+    }
 }
 
 void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
@@ -769,11 +771,12 @@ static void vnc_dpy_copy(DisplayChangeListener *dcl,
         y = dst_y + h - 1;
         inc = -1;
     }
-    w_lim = w - (16 - (dst_x % 16));
-    if (w_lim < 0)
+    w_lim = w - (VNC_DIRTY_PIXELS_PER_BIT - (dst_x % VNC_DIRTY_PIXELS_PER_BIT));
+    if (w_lim < 0) {
         w_lim = w;
-    else
-        w_lim = w - (w_lim % 16);
+    } else {
+        w_lim = w - (w_lim % VNC_DIRTY_PIXELS_PER_BIT);
+    }
     for (i = 0; i < h; i++) {
         for (x = 0; x <= w_lim;
                 x += s, src_row += cmp_bytes, dst_row += cmp_bytes) {
@@ -781,10 +784,11 @@ static void vnc_dpy_copy(DisplayChangeListener *dcl,
                 if ((s = w - w_lim) == 0)
                     break;
             } else if (!x) {
-                s = (16 - (dst_x % 16));
+                s = (VNC_DIRTY_PIXELS_PER_BIT -
+                    (dst_x % VNC_DIRTY_PIXELS_PER_BIT));
                 s = MIN(s, w_lim);
             } else {
-                s = 16;
+                s = VNC_DIRTY_PIXELS_PER_BIT;
             }
             cmp_bytes = s * VNC_SERVER_FB_BYTES;
             if (memcmp(src_row, dst_row, cmp_bytes) == 0)
@@ -792,7 +796,8 @@ static void vnc_dpy_copy(DisplayChangeListener *dcl,
             memmove(dst_row, src_row, cmp_bytes);
             QTAILQ_FOREACH(vs, &vd->clients, next) {
                 if (!vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
-                    set_bit(((x + dst_x) / 16), vs->dirty[y]);
+                    set_bit(((x + dst_x) / VNC_DIRTY_PIXELS_PER_BIT),
+                            vs->dirty[y]);
                 }
             }
         }
@@ -904,7 +909,7 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
         for (y = 0; y < height; y++) {
             int x;
             int last_x = -1;
-            for (x = 0; x < width / 16; x++) {
+            for (x = 0; x < width / VNC_DIRTY_PIXELS_PER_BIT; x++) {
                 if (test_and_clear_bit(x, vs->dirty[y])) {
                     if (last_x == -1) {
                         last_x = x;
@@ -914,16 +919,22 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
                         int h = find_and_clear_dirty_height(vs, y, last_x, x,
                                                             height);
 
-                        n += vnc_job_add_rect(job, last_x * 16, y,
-                                              (x - last_x) * 16, h);
+                        n += vnc_job_add_rect(job,
+                                              last_x * VNC_DIRTY_PIXELS_PER_BIT,
+                                              y,
+                                              (x - last_x) *
+                                              VNC_DIRTY_PIXELS_PER_BIT,
+                                              h);
                     }
                     last_x = -1;
                 }
             }
             if (last_x != -1) {
                 int h = find_and_clear_dirty_height(vs, y, last_x, x, height);
-                n += vnc_job_add_rect(job, last_x * 16, y,
-                                      (x - last_x) * 16, h);
+                n += vnc_job_add_rect(job, last_x * VNC_DIRTY_PIXELS_PER_BIT,
+                                      y,
+                                      (x - last_x) * VNC_DIRTY_PIXELS_PER_BIT,
+                                      h);
             }
         }
 
@@ -1841,7 +1852,7 @@ static void framebuffer_update_request(VncState *vs, int incremental,
                                        int w, int h)
 {
     int i;
-    const size_t width = surface_width(vs->vd->ds) / 16;
+    const size_t width = surface_width(vs->vd->ds) / VNC_DIRTY_PIXELS_PER_BIT;
     const size_t height = surface_height(vs->vd->ds);
 
     if (y_position > height) {
@@ -2543,7 +2554,9 @@ static int vnc_refresh_lossy_rect(VncDisplay *vd, int x, int y)
 
         vs->lossy_rect[sty][stx] = 0;
         for (j = 0; j < VNC_STAT_RECT; ++j) {
-            bitmap_set(vs->dirty[y + j], x / 16, VNC_STAT_RECT / 16);
+            bitmap_set(vs->dirty[y + j],
+                       x / VNC_DIRTY_PIXELS_PER_BIT,
+                       VNC_STAT_RECT / VNC_DIRTY_PIXELS_PER_BIT);
         }
         has_dirty++;
     }
@@ -2690,17 +2703,21 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
             }
             server_ptr = server_row;
 
-            for (x = 0; x + 15 < width;
-                    x += 16, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) {
-                if (!test_and_clear_bit((x / 16), vd->guest.dirty[y]))
+            for (x = 0; x + VNC_DIRTY_PIXELS_PER_BIT - 1 < width;
+                 x += VNC_DIRTY_PIXELS_PER_BIT, guest_ptr += cmp_bytes,
+                 server_ptr += cmp_bytes) {
+                if (!test_and_clear_bit((x / VNC_DIRTY_PIXELS_PER_BIT),
+                    vd->guest.dirty[y])) {
                     continue;
-                if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0)
+                }
+                if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0) {
                     continue;
+                }
                 memcpy(server_ptr, guest_ptr, cmp_bytes);
                 if (!vd->non_adaptive)
                     vnc_rect_updated(vd, x, y, &tv);
                 QTAILQ_FOREACH(vs, &vd->clients, next) {
-                    set_bit((x / 16), vs->dirty[y]);
+                    set_bit((x / VNC_DIRTY_PIXELS_PER_BIT), vs->dirty[y]);
                 }
                 has_dirty++;
             }
diff --git a/ui/vnc.h b/ui/vnc.h
index e63c142..a379aab 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -81,8 +81,12 @@ typedef void VncSendHextileTile(VncState *vs,
 #define VNC_MAX_WIDTH 2560
 #define VNC_MAX_HEIGHT 2048
 
+/* VNC_DIRTY_PIXELS_PER_BIT is the number of dirty pixels represented
+ * by one bit in the dirty bitmap */
+#define VNC_DIRTY_PIXELS_PER_BIT 16
+
 /* VNC_DIRTY_BITS is the number of bits in the dirty bitmap. */
-#define VNC_DIRTY_BITS (VNC_MAX_WIDTH / 16)
+#define VNC_DIRTY_BITS (VNC_MAX_WIDTH / VNC_DIRTY_PIXELS_PER_BIT)
 
 #define VNC_STAT_RECT  64
 #define VNC_STAT_COLS (VNC_MAX_WIDTH / VNC_STAT_RECT)
-- 
1.8.3.1

[Qemu-devel] [PULL 5/9] ui/vnc: derive cmp_bytes from VNC_DIRTY_PIXELS_PER_BIT

[Gerd Hoffmann]

From: Peter Lieven <pl@kamp.de>

this allows for setting VNC_DIRTY_PIXELS_PER_BIT to different
values than 16 if desired.

Reviewed-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 54e97a4..9a64f4b 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2679,7 +2679,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
      * Check and copy modified bits from guest to server surface.
      * Update server dirty map.
      */
-    cmp_bytes = 64;
+    cmp_bytes = VNC_DIRTY_PIXELS_PER_BIT * VNC_SERVER_FB_BYTES;
     if (cmp_bytes > vnc_server_fb_stride(vd)) {
         cmp_bytes = vnc_server_fb_stride(vd);
     }
-- 
1.8.3.1

[Qemu-devel] [PULL 6/9] ui/vnc: optimize dirty bitmap tracking

[Gerd Hoffmann]

From: Peter Lieven <pl@kamp.de>

vnc_update_client currently scans the dirty bitmap of each client
bitwise which is a very costly operation if only few bits are dirty.
vnc_refresh_server_surface does almost the same.
this patch optimizes both by utilizing the heavily optimized
function find_next_bit to find the offset of the next dirty
bit in the dirty bitmaps.

The following artifical test (just the bitmap operation part) running
vnc_update_client 65536 times on a 2560x2048 surface illustrates the
performance difference:

All bits clean - vnc_update_client_new: 0.07 secs
 vnc_update_client_old: 10.98 secs

All bits dirty - vnc_update_client_new: 11.26 secs
 vnc_update_client_old: 20.19 secs

Few bits dirty - vnc_update_client_new: 0.08 secs
 vnc_update_client_old: 10.98 secs

The case for all bits dirty is still rather slow, this
is due to the implementation of find_and_clear_dirty_height.
This will be addresses in a separate patch.

Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 155 ++++++++++++++++++++++++++++++++++-----------------------------
 ui/vnc.h |   4 ++
 2 files changed, 88 insertions(+), 71 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 9a64f4b..1ed360a 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -572,6 +572,15 @@ void *vnc_server_fb_ptr(VncDisplay *vd, int x, int y)
     ptr += x * VNC_SERVER_FB_BYTES;
     return ptr;
 }
+/* this sets only the visible pixels of a dirty bitmap */
+#define VNC_SET_VISIBLE_PIXELS_DIRTY(bitmap, w, h) {\
+        int y;\
+        memset(bitmap, 0x00, sizeof(bitmap));\
+        for (y = 0; y < h; y++) {\
+            bitmap_set(bitmap[y], 0,\
+                       DIV_ROUND_UP(w, VNC_DIRTY_PIXELS_PER_BIT));\
+        } \
+    }
 
 static void vnc_dpy_switch(DisplayChangeListener *dcl,
                            DisplaySurface *surface)
@@ -597,7 +606,9 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
     qemu_pixman_image_unref(vd->guest.fb);
     vd->guest.fb = pixman_image_ref(surface->image);
     vd->guest.format = surface->format;
-    memset(vd->guest.dirty, 0xFF, sizeof(vd->guest.dirty));
+    VNC_SET_VISIBLE_PIXELS_DIRTY(vd->guest.dirty,
+                                 surface_width(vd->ds),
+                                 surface_height(vd->ds));
 
     QTAILQ_FOREACH(vs, &vd->clients, next) {
         vnc_colordepth(vs);
@@ -605,7 +616,9 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
         if (vs->vd->cursor) {
             vnc_cursor_define(vs);
         }
-        memset(vs->dirty, 0xFF, sizeof(vs->dirty));
+        VNC_SET_VISIBLE_PIXELS_DIRTY(vs->dirty,
+                                     surface_width(vd->ds),
+                                     surface_height(vd->ds));
     }
 }
 
@@ -884,10 +897,9 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
         VncDisplay *vd = vs->vd;
         VncJob *job;
         int y;
-        int width, height;
+        int height;
         int n = 0;
 
-
         if (vs->output.offset && !vs->audio_cap && !vs->force_update)
             /* kernel send buffers are full -> drop frames to throttle */
             return 0;
@@ -903,39 +915,27 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync)
          */
         job = vnc_job_new(vs);
 
-        width = MIN(pixman_image_get_width(vd->server), vs->client_width);
         height = MIN(pixman_image_get_height(vd->server), vs->client_height);
 
-        for (y = 0; y < height; y++) {
-            int x;
-            int last_x = -1;
-            for (x = 0; x < width / VNC_DIRTY_PIXELS_PER_BIT; x++) {
-                if (test_and_clear_bit(x, vs->dirty[y])) {
-                    if (last_x == -1) {
-                        last_x = x;
-                    }
-                } else {
-                    if (last_x != -1) {
-                        int h = find_and_clear_dirty_height(vs, y, last_x, x,
-                                                            height);
-
-                        n += vnc_job_add_rect(job,
-                                              last_x * VNC_DIRTY_PIXELS_PER_BIT,
-                                              y,
-                                              (x - last_x) *
-                                              VNC_DIRTY_PIXELS_PER_BIT,
-                                              h);
-                    }
-                    last_x = -1;
-                }
-            }
-            if (last_x != -1) {
-                int h = find_and_clear_dirty_height(vs, y, last_x, x, height);
-                n += vnc_job_add_rect(job, last_x * VNC_DIRTY_PIXELS_PER_BIT,
-                                      y,
-                                      (x - last_x) * VNC_DIRTY_PIXELS_PER_BIT,
-                                      h);
+        y = 0;
+        for (;;) {
+            int x, h;
+            unsigned long x2;
+            unsigned long offset = find_next_bit((unsigned long *) &vs->dirty,
+                                                 height * VNC_DIRTY_BPL(vs),
+                                                 y * VNC_DIRTY_BPL(vs));
+            if (offset == height * VNC_DIRTY_BPL(vs)) {
+                /* no more dirty bits */
+                break;
             }
+            y = offset / VNC_DIRTY_BPL(vs);
+            x = offset % VNC_DIRTY_BPL(vs);
+            x2 = find_next_zero_bit((unsigned long *) &vs->dirty[y],
+                                    VNC_DIRTY_BPL(vs), x);
+            bitmap_clear(vs->dirty[y], x, x2 - x);
+            h = find_and_clear_dirty_height(vs, y, x, x2, height);
+            n += vnc_job_add_rect(job, x * VNC_DIRTY_PIXELS_PER_BIT, y,
+                                  (x2 - x) * VNC_DIRTY_PIXELS_PER_BIT, h);
         }
 
         vnc_job_push(job);
@@ -2660,8 +2660,8 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
     int width = pixman_image_get_width(vd->guest.fb);
     int height = pixman_image_get_height(vd->guest.fb);
     int y;
-    uint8_t *guest_row;
-    uint8_t *server_row;
+    uint8_t *guest_row0 = NULL, *server_row0;
+    int guest_stride = 0, server_stride;
     int cmp_bytes;
     VncState *vs;
     int has_dirty = 0;
@@ -2686,44 +2686,57 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
     if (vd->guest.format != VNC_SERVER_FB_FORMAT) {
         int width = pixman_image_get_width(vd->server);
         tmpbuf = qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, width);
-    }
-    guest_row = (uint8_t *)pixman_image_get_data(vd->guest.fb);
-    server_row = (uint8_t *)pixman_image_get_data(vd->server);
-    for (y = 0; y < height; y++) {
-        if (!bitmap_empty(vd->guest.dirty[y], VNC_DIRTY_BITS)) {
-            int x;
-            uint8_t *guest_ptr;
-            uint8_t *server_ptr;
-
-            if (vd->guest.format != VNC_SERVER_FB_FORMAT) {
-                qemu_pixman_linebuf_fill(tmpbuf, vd->guest.fb, width, 0, y);
-                guest_ptr = (uint8_t *)pixman_image_get_data(tmpbuf);
-            } else {
-                guest_ptr = guest_row;
-            }
-            server_ptr = server_row;
+    } else {
+        guest_row0 = (uint8_t *)pixman_image_get_data(vd->guest.fb);
+        guest_stride = pixman_image_get_stride(vd->guest.fb);
+    }
+    server_row0 = (uint8_t *)pixman_image_get_data(vd->server);
+    server_stride = pixman_image_get_stride(vd->server);
+
+    y = 0;
+    for (;;) {
+        int x;
+        uint8_t *guest_ptr, *server_ptr;
+        unsigned long offset = find_next_bit((unsigned long *) &vd->guest.dirty,
+                                             height * VNC_DIRTY_BPL(&vd->guest),
+                                             y * VNC_DIRTY_BPL(&vd->guest));
+        if (offset == height * VNC_DIRTY_BPL(&vd->guest)) {
+            /* no more dirty bits */
+            break;
+        }
+        y = offset / VNC_DIRTY_BPL(&vd->guest);
+        x = offset % VNC_DIRTY_BPL(&vd->guest);
 
-            for (x = 0; x + VNC_DIRTY_PIXELS_PER_BIT - 1 < width;
-                 x += VNC_DIRTY_PIXELS_PER_BIT, guest_ptr += cmp_bytes,
-                 server_ptr += cmp_bytes) {
-                if (!test_and_clear_bit((x / VNC_DIRTY_PIXELS_PER_BIT),
-                    vd->guest.dirty[y])) {
-                    continue;
-                }
-                if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0) {
-                    continue;
-                }
-                memcpy(server_ptr, guest_ptr, cmp_bytes);
-                if (!vd->non_adaptive)
-                    vnc_rect_updated(vd, x, y, &tv);
-                QTAILQ_FOREACH(vs, &vd->clients, next) {
-                    set_bit((x / VNC_DIRTY_PIXELS_PER_BIT), vs->dirty[y]);
-                }
-                has_dirty++;
+        server_ptr = server_row0 + y * server_stride + x * cmp_bytes;
+
+        if (vd->guest.format != VNC_SERVER_FB_FORMAT) {
+            qemu_pixman_linebuf_fill(tmpbuf, vd->guest.fb, width, 0, y);
+            guest_ptr = (uint8_t *)pixman_image_get_data(tmpbuf);
+        } else {
+            guest_ptr = guest_row0 + y * guest_stride;
+        }
+        guest_ptr += x * cmp_bytes;
+
+        for (; x < DIV_ROUND_UP(width, VNC_DIRTY_PIXELS_PER_BIT);
+             x++, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) {
+            if (!test_and_clear_bit(x, vd->guest.dirty[y])) {
+                continue;
+            }
+            if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0) {
+                continue;
+            }
+            memcpy(server_ptr, guest_ptr, cmp_bytes);
+            if (!vd->non_adaptive) {
+                vnc_rect_updated(vd, x * VNC_DIRTY_PIXELS_PER_BIT,
+                                 y, &tv);
             }
+            QTAILQ_FOREACH(vs, &vd->clients, next) {
+                set_bit(x, vs->dirty[y]);
+            }
+            has_dirty++;
         }
-        guest_row  += pixman_image_get_stride(vd->guest.fb);
-        server_row += pixman_image_get_stride(vd->server);
+
+        y++;
     }
     qemu_pixman_image_unref(tmpbuf);
     return has_dirty;
diff --git a/ui/vnc.h b/ui/vnc.h
index a379aab..8da81b8 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -88,6 +88,10 @@ typedef void VncSendHextileTile(VncState *vs,
 /* VNC_DIRTY_BITS is the number of bits in the dirty bitmap. */
 #define VNC_DIRTY_BITS (VNC_MAX_WIDTH / VNC_DIRTY_PIXELS_PER_BIT)
 
+/* VNC_DIRTY_BPL (BPL = bits per line) might be greater than
+ * VNC_DIRTY_BITS due to alignment */
+#define VNC_DIRTY_BPL(x) (sizeof((x)->dirty) / VNC_MAX_HEIGHT * BITS_PER_BYTE)
+
 #define VNC_STAT_RECT  64
 #define VNC_STAT_COLS (VNC_MAX_WIDTH / VNC_STAT_RECT)
 #define VNC_STAT_ROWS (VNC_MAX_HEIGHT / VNC_STAT_RECT)
-- 
1.8.3.1

[Qemu-devel] [PULL 7/9] ui/vnc: optimize clearing in find_and_clear_dirty_height()

[Gerd Hoffmann]

From: Peter Lieven <pl@kamp.de>

The following artifical test (just the bitmap operation part) running
vnc_update_client 65536 times on a 2560x2048 surface illustrates the
performance difference:

All bits clean - vnc_update_client_new: 0.07 secs
                 vnc_update_client_new2: 0.07 secs
                 vnc_update_client_old: 10.98 secs

All bits dirty - vnc_update_client_new: 11.26 secs
               - vnc_update_client_new2: 0.29 secs
                 vnc_update_client_old: 20.19 secs

Few bits dirty - vnc_update_client_new: 0.07 secs
               - vnc_update_client_new2: 0.07 secs
                 vnc_update_client_old: 10.98 secs

vnc_update_client_new2 shows the performance of vnc_update_client
with this patch added.

Comparing with the test run of the last patch the performance
is at least unchanged while it is significantly improved
for the all bits dirty case.

Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 1ed360a..e1d6ca3 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -879,13 +879,10 @@ static int find_and_clear_dirty_height(struct VncState *vs,
     int h;
 
     for (h = 1; h < (height - y); h++) {
-        int tmp_x;
         if (!test_bit(last_x, vs->dirty[y + h])) {
             break;
         }
-        for (tmp_x = last_x; tmp_x < x; tmp_x++) {
-            clear_bit(tmp_x, vs->dirty[y + h]);
-        }
+        bitmap_clear(vs->dirty[y + h], last_x, x - last_x);
     }
 
     return h;
-- 
1.8.3.1

[Qemu-devel] [PULL 8/9] ui/vnc: optimize setting in vnc_dpy_update()

[Gerd Hoffmann]

From: Peter Lieven <pl@kamp.de>

Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index e1d6ca3..25e4380 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -430,30 +430,24 @@ static int vnc_refresh_server_surface(VncDisplay *vd);
 static void vnc_dpy_update(DisplayChangeListener *dcl,
                            int x, int y, int w, int h)
 {
-    int i;
     VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
     struct VncSurface *s = &vd->guest;
     int width = surface_width(vd->ds);
     int height = surface_height(vd->ds);
 
-    h += y;
-
-    /* round x down to ensure the loop only spans one 16-pixel block per,
-       iteration.  otherwise, if (x % 16) != 0, the last iteration may span
-       two 16-pixel blocks but we only mark the first as dirty
-    */
+    /* this is needed this to ensure we updated all affected
+     * blocks if x % VNC_DIRTY_PIXELS_PER_BIT != 0 */
     w += (x % VNC_DIRTY_PIXELS_PER_BIT);
     x -= (x % VNC_DIRTY_PIXELS_PER_BIT);
 
     x = MIN(x, width);
     y = MIN(y, height);
     w = MIN(x + w, width) - x;
-    h = MIN(h, height);
+    h = MIN(y + h, height);
 
     for (; y < h; y++) {
-        for (i = 0; i < w; i += VNC_DIRTY_PIXELS_PER_BIT) {
-            set_bit((x + i) / VNC_DIRTY_PIXELS_PER_BIT, s->dirty[y]);
-        }
+        bitmap_set(s->dirty[y], x / VNC_DIRTY_PIXELS_PER_BIT,
+                   DIV_ROUND_UP(w, VNC_DIRTY_PIXELS_PER_BIT));
     }
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 9/9] ui/vnc: disable adaptive update calculations if not needed

[Gerd Hoffmann]

From: Peter Lieven <pl@kamp.de>

Signed-off-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/ui/vnc.c b/ui/vnc.c
index 25e4380..9c84b3e 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3150,7 +3150,9 @@ void vnc_display_open(DisplayState *ds, const char *display, Error **errp)
             acl = 1;
 #endif
         } else if (strncmp(options, "lossy", 5) == 0) {
+#ifdef CONFIG_VNC_JPEG
             vs->lossy = true;
+#endif
         } else if (strncmp(options, "non-adaptive", 12) == 0) {
             vs->non_adaptive = true;
         } else if (strncmp(options, "share=", 6) == 0) {
@@ -3167,6 +3169,13 @@ void vnc_display_open(DisplayState *ds, const char *display, Error **errp)
         }
     }
 
+    /* adaptive updates are only used with tight encoding and
+     * if lossy updates are enabled so we can disable all the
+     * calculations otherwise */
+    if (!vs->lossy) {
+        vs->non_adaptive = true;
+    }
+
 #ifdef CONFIG_VNC_TLS
     if (acl && x509 && vs->tls.x509verify) {
         if (!(vs->tls.acl = qemu_acl_init("vnc.x509dname"))) {
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 0/9] vnc patches

[Peter Maydell]

On 10 March 2014 12:49, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> Picked up a collection of vnc patches send to the list over the last
> months.  Reviewed, picked into a branch, smoke-tested.  Here is the
> pull req for them.
>
> If anyone has pending vnc bits not included here please resend & Cc /me.
>
> please pull,
>   Gerd
>
> The following changes since commit f53f3d0a00b6df39ce8dfca942608e5b6a9a4f71:
>
>   Merge remote-tracking branch 'remotes/kvaneesh/for-upstream' into staging (2014-03-08 12:38:43 +0000)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-vnc-1
>
> for you to fetch changes up to e22492d332c5c80ea8cb1755c29ca9b6240c7716:
>
>   ui/vnc: disable adaptive update calculations if not needed (2014-03-10 12:35:04 +0100)

Applied, thanks.

-- PMM