On Tue, Dec 1, 2015 at 7:48 AM, Rui Matos <tiagoma...@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 <tiagoma...@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@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel