[Qemu-devel] [PATCH] vnc-tight: fix regression with libxenstore

[Qemu-devel] [PATCH] vnc-tight: fix regression with libxenstore

[Peter Lieven]

commit 095497ff added thread local storage for the color counting
palette. Unfortunately, a VncPalette is about 7kB on a x86_64 system.
This memory is reserved from the stack of every thread and it
exhausted the stack space of a libxenstore thread.

Fix this by allocating memory only for the VNC encoding thread.

Fixes: 095497ffc66b7f031ff2a17f1e50f5cb105ce588
Reported-by: Juergen Gross <jgross@suse.com>
Tested-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
---
 ui/vnc-enc-tight.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index b8581dd..2b58739 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -1457,11 +1457,17 @@ static int send_sub_rect_jpeg(VncState *vs, int x, int y, int w, int h,
 }
 #endif
 
-static __thread VncPalette color_count_palette;
+static __thread VncPalette *color_count_palette;
+static __thread Notifier vnc_tight_cleanup_notifier;
+
+static void vnc_tight_cleanup(Notifier *n, void *value)
+{
+    g_free(color_count_palette);
+    color_count_palette = NULL;
+}
 
 static int send_sub_rect(VncState *vs, int x, int y, int w, int h)
 {
-    VncPalette *palette = &color_count_palette;
     uint32_t bg = 0, fg = 0;
     int colors;
     int ret = 0;
@@ -1470,6 +1476,12 @@ static int send_sub_rect(VncState *vs, int x, int y, int w, int h)
     bool allow_jpeg = true;
 #endif
 
+    if (!color_count_palette) {
+        color_count_palette = g_malloc(sizeof(VncPalette));
+        vnc_tight_cleanup_notifier.notify = vnc_tight_cleanup;
+        qemu_thread_atexit_add(&vnc_tight_cleanup_notifier);
+    }
+
     vnc_framebuffer_update(vs, x, y, w, h, vs->tight.type);
 
     vnc_tight_start(vs);
@@ -1490,17 +1502,19 @@ static int send_sub_rect(VncState *vs, int x, int y, int w, int h)
     }
 #endif
 
-    colors = tight_fill_palette(vs, x, y, w * h, &bg, &fg, palette);
+    colors = tight_fill_palette(vs, x, y, w * h, &bg, &fg, color_count_palette);
 
 #ifdef CONFIG_VNC_JPEG
     if (allow_jpeg && vs->tight.quality != (uint8_t)-1) {
-        ret = send_sub_rect_jpeg(vs, x, y, w, h, bg, fg, colors, palette,
-                                 force_jpeg);
+        ret = send_sub_rect_jpeg(vs, x, y, w, h, bg, fg, colors,
+                                 color_count_palette, force_jpeg);
     } else {
-        ret = send_sub_rect_nojpeg(vs, x, y, w, h, bg, fg, colors, palette);
+        ret = send_sub_rect_nojpeg(vs, x, y, w, h, bg, fg, colors,
+                                   color_count_palette);
     }
 #else
-    ret = send_sub_rect_nojpeg(vs, x, y, w, h, bg, fg, colors, palette);
+    ret = send_sub_rect_nojpeg(vs, x, y, w, h, bg, fg, colors,
+                               color_count_palette);
 #endif
 
     return ret;
-- 
1.9.1

Re: [Qemu-devel] [PATCH] vnc-tight: fix regression with libxenstore

[Gerd Hoffmann]

On Fr, 2016-07-15 at 11:45 +0200, Peter Lieven wrote:
> commit 095497ff added thread local storage for the color counting
> palette. Unfortunately, a VncPalette is about 7kB on a x86_64 system.
> This memory is reserved from the stack of every thread and it
> exhausted the stack space of a libxenstore thread.
> 
> Fix this by allocating memory only for the VNC encoding thread.

Added to vnc queue.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH] vnc-tight: fix regression with libxenstore

[Peter Lieven]

Am 15.07.2016 um 12:07 schrieb Gerd Hoffmann:
> On Fr, 2016-07-15 at 11:45 +0200, Peter Lieven wrote:
>> commit 095497ff added thread local storage for the color counting
>> palette. Unfortunately, a VncPalette is about 7kB on a x86_64 system.
>> This memory is reserved from the stack of every thread and it
>> exhausted the stack space of a libxenstore thread.
>>
>> Fix this by allocating memory only for the VNC encoding thread.
> Added to vnc queue.

Please wait. Paolo mentioned that TLS is not allocated from the stack.
Maybe this patch is ok, but we need a different commit message then.

Peter

Re: [Qemu-devel] [PATCH] vnc-tight: fix regression with libxenstore

[Paolo Bonzini]

On 15/07/2016 12:10, Peter Lieven wrote:
> Am 15.07.2016 um 12:07 schrieb Gerd Hoffmann:
>> On Fr, 2016-07-15 at 11:45 +0200, Peter Lieven wrote:
>>> commit 095497ff added thread local storage for the color counting
>>> palette. Unfortunately, a VncPalette is about 7kB on a x86_64 system.
>>> This memory is reserved from the stack of every thread and it
>>> exhausted the stack space of a libxenstore thread.
>>>
>>> Fix this by allocating memory only for the VNC encoding thread.
>> Added to vnc queue.
> 
> Please wait. Paolo mentioned that TLS is not allocated from the stack.

Actually it does---which is not a problem, but then the stack size from
pthread attributes should be increased IMHO.  Anyway, the patch is okay.

Paolo

> Maybe this patch is ok, but we need a different commit message then.