[PATCH] xwayland: Group multiple cursor buffers per shm pool

Rui Matos tiagomatos at gmail.com
Tue Dec 1 07:48:43 PST 2015


Each shm pool implies a file descriptor which means that currently, we
can quickly exhaust the available FDs since we create a shm pool per
cursor buffer. Instead, this patch creates shm pools big enough to
contain multiple cursor buffers to avoid hitting the FD limit on
reasonable workloads.

On my testing, most cursors require 2304 bytes so I made the pools be
256 kB meaning that each can hold around 100 buffers.

Signed-off-by: Rui Matos <tiagomatos at gmail.com>
---

This fixes bug https://bugzilla.gnome.org/show_bug.cgi?id=758255 . The
use case reported there, which is running two instances of
ImageMagick's display tool, makes xwayland realize exactly 1024
cursors which breaks due to FD exhaustion before this patch.

Note that there's no smart allocation algorithm here, it just
allocates more pools as needed and only frees them after all cursors
inside a pool are freed which, I'm afraid, will lead to terrible
fragmentation over time.

Our shm pixmap usage suffers from the same problem of using one pool
per pixmap which seems problematic too but since most setups should be
able to use drm for pixmaps it's not as pressing a problem to fix. At
first I attempted to use drm buffers for cursors too but failed to get
it working since I got lost finding a way to copy the cursor data into
the buffers. I'm happy to do that instead if someone points me to how
to do it.

 hw/xwayland/xwayland-cursor.c | 68 +++++++++++++++++++++++++++++++-------
 hw/xwayland/xwayland-shm.c    | 76 +++++++++++++++++++++++++++++++++++++++++++
 hw/xwayland/xwayland.h        |  6 ++++
 3 files changed, 138 insertions(+), 12 deletions(-)

diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
index 76729db..e5b88a5 100644
--- a/hw/xwayland/xwayland-cursor.c
+++ b/hw/xwayland/xwayland-cursor.c
@@ -28,7 +28,17 @@
 
 #include <mipointer.h>
 
+#define SHM_POOL_SIZE (1 << 18) /* 256 kB */
+
+struct xwl_cursor {
+    struct xorg_list link;
+    struct xwl_shm_pool *pool;
+    struct wl_buffer *buffer;
+    char *data;
+};
+
 static DevPrivateKeyRec xwl_cursor_private_key;
+static struct xorg_list xwl_cursor_list;
 
 static void
 expand_source_and_mask(CursorPtr cursor, CARD32 *data)
@@ -63,23 +73,55 @@ expand_source_and_mask(CursorPtr cursor, CARD32 *data)
 static Bool
 xwl_realize_cursor(DeviceIntPtr device, ScreenPtr screen, CursorPtr cursor)
 {
-    PixmapPtr pixmap;
+    struct xwl_cursor *xwl_cursor;
+    struct xwl_shm_pool *pool = NULL;
+
+    xwl_cursor = malloc(sizeof *xwl_cursor);
+    if (!xwl_cursor)
+        return FALSE;
+
+    if (!xorg_list_is_empty(&xwl_cursor_list)) {
+        pool = (xorg_list_first_entry(&xwl_cursor_list, struct xwl_cursor, link))->pool;
+        xwl_cursor->buffer = xwl_shm_pool_allocate(pool, cursor->bits->width,
+                                                   cursor->bits->height, &xwl_cursor->data);
+        if (!xwl_cursor->buffer)
+            pool = NULL;
+    }
 
-    pixmap = xwl_shm_create_pixmap(screen, cursor->bits->width,
-                                   cursor->bits->height, 32, 0);
-    dixSetPrivate(&cursor->devPrivates, &xwl_cursor_private_key, pixmap);
+    if (!pool) {
+        pool = xwl_shm_pool_create(xwl_screen_get(screen)->shm, SHM_POOL_SIZE);
+        if (!pool)
+            goto err_free;
+        xwl_cursor->buffer = xwl_shm_pool_allocate(pool, cursor->bits->width,
+                                                   cursor->bits->height, &xwl_cursor->data);
+        if (!xwl_cursor->buffer)
+            goto err_free;
+    }
+
+    xwl_cursor->pool = pool;
+
+    xorg_list_add(&xwl_cursor->link, &xwl_cursor_list);
+    dixSetPrivate(&cursor->devPrivates, &xwl_cursor_private_key, xwl_cursor);
 
     return TRUE;
+
+ err_free:
+    free(xwl_cursor);
+    return FALSE;
 }
 
 static Bool
 xwl_unrealize_cursor(DeviceIntPtr device, ScreenPtr screen, CursorPtr cursor)
 {
-    PixmapPtr pixmap;
+    struct xwl_cursor *xwl_cursor;
 
-    pixmap = dixGetPrivate(&cursor->devPrivates, &xwl_cursor_private_key);
+    xwl_cursor = dixGetPrivate(&cursor->devPrivates, &xwl_cursor_private_key);
+    wl_buffer_destroy(xwl_cursor->buffer);
+    xwl_shm_pool_unref(xwl_cursor->pool);
+    xorg_list_del(&xwl_cursor->link);
+    free(xwl_cursor);
 
-    return xwl_shm_destroy_pixmap(pixmap);
+    return TRUE;
 }
 
 static void
@@ -102,7 +144,7 @@ static const struct wl_callback_listener frame_listener = {
 void
 xwl_seat_set_cursor(struct xwl_seat *xwl_seat)
 {
-    PixmapPtr pixmap;
+    struct xwl_cursor *xwl_cursor;
     CursorPtr cursor;
     int stride;
 
@@ -121,13 +163,13 @@ xwl_seat_set_cursor(struct xwl_seat *xwl_seat)
     }
 
     cursor = xwl_seat->x_cursor;
-    pixmap = dixGetPrivate(&cursor->devPrivates, &xwl_cursor_private_key);
+    xwl_cursor = dixGetPrivate(&cursor->devPrivates, &xwl_cursor_private_key);
     stride = cursor->bits->width * 4;
     if (cursor->bits->argb)
-        memcpy(pixmap->devPrivate.ptr,
+        memcpy(xwl_cursor->data,
                cursor->bits->argb, cursor->bits->height * stride);
     else
-        expand_source_and_mask(cursor, pixmap->devPrivate.ptr);
+        expand_source_and_mask(cursor, (CARD32 *) xwl_cursor->data);
 
     wl_pointer_set_cursor(xwl_seat->wl_pointer,
                           xwl_seat->pointer_enter_serial,
@@ -135,7 +177,7 @@ xwl_seat_set_cursor(struct xwl_seat *xwl_seat)
                           xwl_seat->x_cursor->bits->xhot,
                           xwl_seat->x_cursor->bits->yhot);
     wl_surface_attach(xwl_seat->cursor,
-                      xwl_shm_pixmap_get_wl_buffer(pixmap), 0, 0);
+                      xwl_cursor->buffer, 0, 0);
     wl_surface_damage(xwl_seat->cursor, 0, 0,
                       xwl_seat->x_cursor->bits->width,
                       xwl_seat->x_cursor->bits->height);
@@ -214,6 +256,8 @@ xwl_screen_init_cursor(struct xwl_screen *xwl_screen)
     if (!dixRegisterPrivateKey(&xwl_cursor_private_key, PRIVATE_CURSOR_BITS, 0))
         return FALSE;
 
+    xorg_list_init(&xwl_cursor_list);
+
     return miPointerInitialize(xwl_screen->screen,
                                &xwl_pointer_sprite_funcs,
                                &xwl_pointer_screen_funcs, TRUE);
diff --git a/hw/xwayland/xwayland-shm.c b/hw/xwayland/xwayland-shm.c
index 1022c0d..1c53c11 100644
--- a/hw/xwayland/xwayland-shm.c
+++ b/hw/xwayland/xwayland-shm.c
@@ -290,3 +290,79 @@ xwl_shm_create_screen_resources(ScreenPtr screen)
 
     return screen->devPrivate != NULL;
 }
+
+
+struct xwl_shm_pool {
+    struct wl_shm_pool *pool;
+    int fd;
+    size_t size;
+    size_t used;
+    char *data;
+    int refcnt;
+};
+
+struct xwl_shm_pool *
+xwl_shm_pool_create(struct wl_shm *shm, size_t size)
+{
+    struct xwl_shm_pool *pool;
+
+    pool = malloc(sizeof *pool);
+    if (!pool)
+        return NULL;
+
+    pool->fd = os_create_anonymous_file(size);
+    if (pool->fd < 0)
+        goto err_free;
+
+    pool->data = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED,
+                      pool->fd, 0);
+
+    if (pool->data == MAP_FAILED)
+        goto err_close;
+
+    pool->pool = wl_shm_create_pool(shm, pool->fd, size);
+    pool->size = size;
+    pool->used = 0;
+    pool->refcnt = 0;
+
+    return pool;
+
+ err_close:
+    close(pool->fd);
+ err_free:
+    free(pool);
+    return NULL;
+}
+
+struct wl_buffer *
+xwl_shm_pool_allocate(struct xwl_shm_pool *pool, size_t width, size_t height, char **data)
+{
+    size_t size, stride;
+
+    stride = width * 4;
+    size = stride * height;
+
+    if (pool->used + size > pool->size)
+        return NULL;
+
+    *data = pool->data + pool->used;
+    pool->used += size;
+    pool->refcnt += 1;
+
+    return wl_shm_pool_create_buffer(pool->pool, *data - pool->data,
+                                     width, height, stride, WL_SHM_FORMAT_ARGB8888);
+}
+
+void
+xwl_shm_pool_unref(struct xwl_shm_pool *pool)
+{
+    if (pool->refcnt > 1) {
+        pool->refcnt -= 1;
+        return;
+    }
+
+    munmap(pool->data, pool->size);
+    wl_shm_pool_destroy(pool->pool);
+    close(pool->fd);
+    free(pool);
+}
diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
index a7d7119..6093b88 100644
--- a/hw/xwayland/xwayland.h
+++ b/hw/xwayland/xwayland.h
@@ -151,6 +151,7 @@ struct xwl_output {
 };
 
 struct xwl_pixmap;
+struct xwl_shm_pool;
 
 Bool xwl_screen_init_cursor(struct xwl_screen *xwl_screen);
 
@@ -182,6 +183,11 @@ PixmapPtr xwl_shm_create_pixmap(ScreenPtr screen, int width, int height,
 Bool xwl_shm_destroy_pixmap(PixmapPtr pixmap);
 struct wl_buffer *xwl_shm_pixmap_get_wl_buffer(PixmapPtr pixmap);
 
+struct xwl_shm_pool *xwl_shm_pool_create(struct wl_shm *shm, size_t size);
+struct wl_buffer *xwl_shm_pool_allocate(struct xwl_shm_pool *pool, size_t widht,
+                                        size_t height, char **data);
+void xwl_shm_pool_unref(struct xwl_shm_pool *pool);
+
 
 Bool xwl_glamor_init(struct xwl_screen *xwl_screen);
 
-- 
2.5.0



More information about the xorg-devel mailing list