[PATCH] xwayland: Group multiple cursor buffers per shm pool
Kristian Høgsberg
krh at bitplanet.net
Fri Dec 4 16:30:20 PST 2015
On Tue, Dec 1, 2015 at 7:48 AM, Rui Matos <tiagomatos at gmail.com> wrote:
> 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.
Nice work. I think it would be about the same amount of work to do
this for shm pixmaps instead of cursors. That way, an application that
creates a lot of small pixmaps on shm-Xwayland (non-glamor) will also
not crash the server either.
Kristian
> 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
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
More information about the xorg-devel
mailing list