From 75f7f21a48f3c4054cae024ff769b781481786a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 22:17:12 +0200 Subject: [PATCH 01/10] shm: split up buffer struct into internal/private and public parts --- shm.c | 205 ++++++++++++++++++++++++++++++++++++---------------------- shm.h | 21 +----- 2 files changed, 127 insertions(+), 99 deletions(-) diff --git a/shm.c b/shm.c index 14d14d02..12688445 100644 --- a/shm.c +++ b/shm.c @@ -55,11 +55,34 @@ */ static off_t max_pool_size = 512 * 1024 * 1024; -static tll(struct buffer) buffers; - static bool can_punch_hole = false; static bool can_punch_hole_initialized = false; +struct buffer_pool { + int fd; /* memfd */ + struct wl_shm_pool *wl_pool; + + void *real_mmapped; /* Address returned from mmap */ + size_t mmap_size; /* Size of mmap (>= size) */ + + size_t ref_count; +}; + +struct buffer_private { + struct buffer public; + + bool busy; /* Owned by compositor */ + + unsigned long cookie; + struct buffer_pool *pool; + off_t offset; /* Offset into memfd where data begins */ + + bool scrollable; + bool purge; /* True if this buffer should be destroyed */ +}; + +static tll(struct buffer_private) buffers; + #undef MEASURE_SHM_ALLOCS #if defined(MEASURE_SHM_ALLOCS) static size_t max_alloced = 0; @@ -114,14 +137,14 @@ pool_unref(struct buffer_pool *pool) } static void -buffer_destroy(struct buffer *buf) +buffer_destroy(struct buffer_private *buf) { - buffer_destroy_dont_close(buf); + buffer_destroy_dont_close(&buf->public); pool_unref(buf->pool); buf->pool = NULL; - free(buf->scroll_damage); - pixman_region32_fini(&buf->dirty); + free(buf->public.scroll_damage); + pixman_region32_fini(&buf->public.dirty); } void @@ -140,12 +163,12 @@ shm_fini(void) static void buffer_release(void *data, struct wl_buffer *wl_buffer) { - struct buffer *buffer = data; + struct buffer_private *buffer = data; LOG_DBG("release: cookie=%lx (buf=%p, total buffer count: %zu)", buffer->cookie, (void *)buffer, tll_length(buffers)); - xassert(buffer->wl_buf == wl_buffer); + xassert(buffer->public.wl_buf == wl_buffer); xassert(buffer->busy); buffer->busy = false; } @@ -174,23 +197,24 @@ page_size(void) #endif static bool -instantiate_offset(struct wl_shm *shm, struct buffer *buf, off_t new_offset) +instantiate_offset(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) { - xassert(buf->mmapped == NULL); - xassert(buf->wl_buf == NULL); - xassert(buf->pix == NULL); + xassert(buf->public.mmapped == NULL); + xassert(buf->public.pix == NULL); + xassert(buf->public.wl_buf == NULL); xassert(buf->pool != NULL); const struct buffer_pool *pool = buf->pool; void *mmapped = MAP_FAILED; struct wl_buffer *wl_buf = NULL; - pixman_image_t **pix = xcalloc(buf->pix_instances, sizeof(*pix)); + pixman_image_t **pix = xcalloc(buf->public.pix_instances, sizeof(*pix)); mmapped = (uint8_t *)pool->real_mmapped + new_offset; wl_buf = wl_shm_pool_create_buffer( - pool->wl_pool, new_offset, buf->width, buf->height, buf->stride, + pool->wl_pool, new_offset, + buf->public.width, buf->public.height, buf->public.stride, WL_SHM_FORMAT_ARGB8888); if (wl_buf == NULL) { @@ -199,26 +223,27 @@ instantiate_offset(struct wl_shm *shm, struct buffer *buf, off_t new_offset) } /* One pixman image for each worker thread (do we really need multiple?) */ - for (size_t i = 0; i < buf->pix_instances; i++) { + for (size_t i = 0; i < buf->public.pix_instances; i++) { pix[i] = pixman_image_create_bits_no_clear( - PIXMAN_a8r8g8b8, buf->width, buf->height, (uint32_t *)mmapped, buf->stride); + PIXMAN_a8r8g8b8, buf->public.width, buf->public.height, + (uint32_t *)mmapped, buf->public.stride); if (pix[i] == NULL) { LOG_ERR("failed to create pixman image"); goto err; } } - buf->mmapped = mmapped; + buf->public.mmapped = mmapped; + buf->public.wl_buf = wl_buf; + buf->public.pix = pix; buf->offset = new_offset; - buf->wl_buf = wl_buf; - buf->pix = pix; wl_buffer_add_listener(wl_buf, &buffer_listener, buf); return true; err: if (pix != NULL) { - for (size_t i = 0; i < buf->pix_instances; i++) + for (size_t i = 0; i < buf->public.pix_instances; i++) if (pix[i] != NULL) pixman_image_unref(pix[i]); } @@ -235,7 +260,7 @@ destroy_all_purgeables(void) { /* Purge buffers marked for purging */ tll_foreach(buffers, it) { - if (it->item.locked) + if (it->item.public.locked) continue; if (!it->item.purge) @@ -393,29 +418,31 @@ get_new_buffers(struct wl_shm *shm, size_t count, /* Push to list of available buffers, but marked as 'busy' */ tll_push_front( buffers, - ((struct buffer){ + ((struct buffer_private){ + .public = { + .width = info[i].width, + .height = info[i].height, + .stride = stride[i], + .size = sizes[i], + .pix_instances = pix_instances, + .age = 1234, /* Force a full repaint */ + }, .cookie = info[i].cookie, - .width = info[i].width, - .height = info[i].height, - .stride = stride[i], .busy = true, .purge = immediate_purge, - .size = sizes[i], - .pix_instances = pix_instances, .pool = pool, .scrollable = scrollable, .offset = 0, - .age = 1234, /* Force a full repaint */ })); - struct buffer *buf = &tll_front(buffers); + struct buffer_private *buf = &tll_front(buffers); if (!instantiate_offset(shm, buf, offset)) goto err; - pixman_region32_init(&buf->dirty); + pixman_region32_init(&buf->public.dirty); pool->ref_count++; - offset += buf->size; - bufs[i] = buf; + offset += buf->public.size; + bufs[i] = &buf->public; } #if defined(MEASURE_SHM_ALLOCS) && MEASURE_SHM_ALLOCS @@ -466,17 +493,17 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, { destroy_all_purgeables(); - struct buffer *cached = NULL; + struct buffer_private *cached = NULL; tll_foreach(buffers, it) { - if (it->item.width != width) + if (it->item.public.width != width) continue; - if (it->item.height != height) + if (it->item.public.height != height) continue; if (it->item.cookie != cookie) continue; if (it->item.busy) - it->item.age++; + it->item.public.age++; else #if FORCED_DOUBLE_BUFFERING if (it->item.age == 0) @@ -489,16 +516,16 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, cookie, (void *)&it->item); it->item.busy = true; it->item.purge = false; - pixman_region32_clear(&it->item.dirty); - free(it->item.scroll_damage); - it->item.scroll_damage = NULL; - xassert(it->item.pix_instances == pix_instances); + pixman_region32_clear(&it->item.public.dirty); + free(it->item.public.scroll_damage); + it->item.public.scroll_damage = NULL; + xassert(it->item.public.pix_instances == pix_instances); cached = &it->item; } else { /* We have multiple buffers eligible for * re-use. Pick the “youngest” one, and mark the * other one for purging */ - if (it->item.age < cached->age) { + if (it->item.public.age < cached->public.age) { cached->purge = true; cached = &it->item; } else @@ -508,7 +535,7 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, } if (cached != NULL) - return cached; + return &cached->public; /* Mark old buffers associated with this cookie for purging */ tll_foreach(buffers, it) { @@ -518,7 +545,7 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, if (it->item.busy) continue; - if (it->item.width == width && it->item.height == height) + if (it->item.public.width == width && it->item.public.height == height) continue; LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)&it->item); @@ -532,9 +559,10 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, } bool -shm_can_scroll(const struct buffer *buf) +shm_can_scroll(const struct buffer *_buf) { #if __SIZEOF_POINTER__ == 8 + const struct buffer_private *buf = (const struct buffer_private *)_buf; return can_punch_hole && max_pool_size > 0 && buf->scrollable; #else /* Not enough virtual address space in 32-bit */ @@ -544,17 +572,20 @@ shm_can_scroll(const struct buffer *buf) #if __SIZEOF_POINTER__ == 8 && defined(FALLOC_FL_PUNCH_HOLE) static bool -wrap_buffer(struct wl_shm *shm, struct buffer *buf, off_t new_offset) +wrap_buffer(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) { struct buffer_pool *pool = buf->pool; xassert(pool->ref_count == 1); /* We don't allow overlapping offsets */ - off_t UNUSED diff = - new_offset < buf->offset ? buf->offset - new_offset : new_offset - buf->offset; - xassert(diff > buf->size); + off_t UNUSED diff = new_offset < buf->offset + ? buf->offset - new_offset + : new_offset - buf->offset; + xassert(diff > buf->public.size); - memcpy((uint8_t *)pool->real_mmapped + new_offset, buf->mmapped, buf->size); + memcpy((uint8_t *)pool->real_mmapped + new_offset, + buf->public.mmapped, + buf->public.size); off_t trim_ofs, trim_len; if (new_offset > buf->offset) { @@ -563,7 +594,7 @@ wrap_buffer(struct wl_shm *shm, struct buffer *buf, off_t new_offset) trim_len = new_offset; } else { /* Trim everything *after* the new buffer location */ - trim_ofs = new_offset + buf->size; + trim_ofs = new_offset + buf->public.size; trim_len = pool->mmap_size - trim_ofs; } @@ -577,12 +608,12 @@ wrap_buffer(struct wl_shm *shm, struct buffer *buf, off_t new_offset) } /* Re-instantiate pixman+wl_buffer+raw pointersw */ - buffer_destroy_dont_close(buf); + buffer_destroy_dont_close(&buf->public); return instantiate_offset(shm, buf, new_offset); } static bool -shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, +shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { @@ -590,19 +621,19 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, xassert(can_punch_hole); xassert(buf->busy); - xassert(buf->pix); - xassert(buf->wl_buf); + xassert(buf->public.pix != NULL); + xassert(buf->public.wl_buf != NULL); xassert(pool != NULL); xassert(pool->ref_count == 1); xassert(pool->fd >= 0); LOG_DBG("scrolling %d rows (%d bytes)", rows, rows * buf->stride); - const off_t diff = rows * buf->stride; + const off_t diff = rows * buf->public.stride; xassert(rows > 0); - xassert(diff < buf->size); + xassert(diff < buf->public.size); - if (buf->offset + diff + buf->size > max_pool_size) { + if (buf->offset + diff + buf->public.size > max_pool_size) { LOG_DBG("memfd offset wrap around"); if (!wrap_buffer(shm, buf, 0)) goto err; @@ -610,7 +641,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, off_t new_offset = buf->offset + diff; xassert(new_offset > buf->offset); - xassert(new_offset + buf->size <= max_pool_size); + xassert(new_offset + buf->public.size <= max_pool_size); #if TIME_SCROLL struct timeval time1; @@ -621,10 +652,13 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, if (top_keep_rows > 0) { /* Copy current 'top' region to its new location */ + const int stride = buf->public.stride; + uint8_t *base = buf->public.mmapped; + memmove( - (uint8_t *)buf->mmapped + (top_margin + rows) * buf->stride, - (uint8_t *)buf->mmapped + (top_margin + 0) * buf->stride, - top_keep_rows * buf->stride); + base + (top_margin + rows) * stride, + base + (top_margin + 0) * stride, + top_keep_rows * stride); #if TIME_SCROLL gettimeofday(&time2, NULL); @@ -634,7 +668,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, } /* Destroy old objects (they point to the old offset) */ - buffer_destroy_dont_close(buf); + buffer_destroy_dont_close(&buf->public); /* Free unused memory - everything up until the new offset */ const off_t trim_ofs = 0; @@ -668,10 +702,14 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer *buf, int rows, if (ret && bottom_keep_rows > 0) { /* Copy 'bottom' region to its new location */ + const size_t size = buf->public.size; + const int stride = buf->public.stride; + uint8_t *base = buf->public.mmapped; + memmove( - (uint8_t *)buf->mmapped + buf->size - (bottom_margin + bottom_keep_rows) * buf->stride, - (uint8_t *)buf->mmapped + buf->size - (bottom_margin + rows + bottom_keep_rows) * buf->stride, - bottom_keep_rows * buf->stride); + base + size - (bottom_margin + bottom_keep_rows) * stride, + base + size - (bottom_margin + rows + bottom_keep_rows) * stride, + bottom_keep_rows * stride); #if TIME_SCROLL struct timeval time5; @@ -690,7 +728,7 @@ err: } static bool -shm_scroll_reverse(struct wl_shm *shm, struct buffer *buf, int rows, +shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { @@ -699,10 +737,10 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer *buf, int rows, struct buffer_pool *pool = buf->pool; xassert(pool->ref_count == 1); - const off_t diff = rows * buf->stride; + const off_t diff = rows * buf->public.stride; if (diff > buf->offset) { LOG_DBG("memfd offset reverse wrap-around"); - if (!wrap_buffer(shm, buf, (max_pool_size - buf->size) & ~(page_size() - 1))) + if (!wrap_buffer(shm, buf, (max_pool_size - buf->public.size) & ~(page_size() - 1))) goto err; } @@ -720,10 +758,14 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer *buf, int rows, if (bottom_keep_rows > 0) { /* Copy 'bottom' region to its new location */ + const size_t size = buf->public.size; + const int stride = buf->public.stride; + uint8_t *base = buf->public.mmapped; + memmove( - (uint8_t *)buf->mmapped + buf->size - (bottom_margin + rows + bottom_keep_rows) * buf->stride, - (uint8_t *)buf->mmapped + buf->size - (bottom_margin + bottom_keep_rows) * buf->stride, - bottom_keep_rows * buf->stride); + base + size - (bottom_margin + rows + bottom_keep_rows) * stride, + base + size - (bottom_margin + bottom_keep_rows) * stride, + bottom_keep_rows * stride); #if TIME_SCROLL gettimeofday(&time1, NULL); @@ -733,10 +775,10 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer *buf, int rows, } /* Destroy old objects (they point to the old offset) */ - buffer_destroy_dont_close(buf); + buffer_destroy_dont_close(&buf->public); /* Free unused memory - everything after the relocated buffer */ - const off_t trim_ofs = new_offset + buf->size; + const off_t trim_ofs = new_offset + buf->public.size; const off_t trim_len = pool->mmap_size - trim_ofs; if (fallocate( @@ -766,10 +808,13 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer *buf, int rows, if (ret && top_keep_rows > 0) { /* Copy current 'top' region to its new location */ + const int stride = buf->public.stride; + uint8_t *base = buf->public.mmapped; + memmove( - (uint8_t *)buf->mmapped + (top_margin + 0) * buf->stride, - (uint8_t *)buf->mmapped + (top_margin + rows) * buf->stride, - top_keep_rows * buf->stride); + base + (top_margin + 0) * stride, + base + (top_margin + rows) * stride, + top_keep_rows * stride); #if TIME_SCROLL struct timeval time4; @@ -788,14 +833,16 @@ err: #endif /* FALLOC_FL_PUNCH_HOLE */ bool -shm_scroll(struct wl_shm *shm, struct buffer *buf, int rows, +shm_scroll(struct wl_shm *shm, struct buffer *_buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { #if __SIZEOF_POINTER__ == 8 && defined(FALLOC_FL_PUNCH_HOLE) - if (!shm_can_scroll(buf)) + if (!shm_can_scroll(_buf)) return false; + struct buffer_private *buf = (struct buffer_private *)_buf; + xassert(rows != 0); return rows > 0 ? shm_scroll_forward(shm, buf, rows, top_margin, top_keep_rows, bottom_margin, bottom_keep_rows) @@ -817,7 +864,7 @@ shm_purge(struct wl_shm *shm, unsigned long cookie) if (it->item.busy) { LOG_WARN("deferring purge of 'busy' buffer (width=%d, height=%d)", - it->item.width, it->item.height); + it->item.public.width, it->item.public.height); it->item.purge = true; } else { buffer_destroy(&it->item); diff --git a/shm.h b/shm.h index 610f0311..01f1a711 100644 --- a/shm.h +++ b/shm.h @@ -9,25 +9,13 @@ #include "terminal.h" -struct buffer_pool { - int fd; /* memfd */ - struct wl_shm_pool *wl_pool; - - void *real_mmapped; /* Address returned from mmap */ - size_t mmap_size; /* Size of mmap (>= size) */ - - size_t ref_count; -}; - struct buffer { - unsigned long cookie; + bool locked; /* Caller owned, shm won’t destroy it */ int width; int height; int stride; - bool locked; /* Caller owned, shm won’t destroy it */ - bool busy; /* Owned by compositor */ size_t size; /* Buffer size */ void *mmapped; /* Raw data (TODO: rename) */ @@ -35,13 +23,6 @@ struct buffer { pixman_image_t **pix; size_t pix_instances; - /* Internal */ - struct buffer_pool *pool; - off_t offset; /* Offset into memfd where data begins */ - - bool scrollable; - bool purge; /* True if this buffer should be destroyed */ - unsigned age; struct damage *scroll_damage; size_t scroll_damage_count; From 9b6cee825beb927ec24ef28f0f162623db218fe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 22:18:09 +0200 Subject: [PATCH 02/10] shm: rename buffer.mmapped to buffer.data --- render.c | 6 +++--- shm.c | 18 ++++++++++-------- shm.h | 2 +- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/render.c b/render.c index 41a20226..076a8d54 100644 --- a/render.c +++ b/render.c @@ -954,7 +954,7 @@ grid_render_scroll(struct terminal *term, struct buffer *buf, term, buf, dmg->region.end - dmg->lines, term->rows, false); } else { /* Fallback for when we either cannot do SHM scrolling, or it failed */ - uint8_t *raw = buf->mmapped; + uint8_t *raw = buf->data; memmove(raw + dst_y * buf->stride, raw + src_y * buf->stride, height * buf->stride); @@ -1019,7 +1019,7 @@ grid_render_scroll_reverse(struct terminal *term, struct buffer *buf, term, buf, dmg->region.start, dmg->region.start + dmg->lines, false); } else { /* Fallback for when we either cannot do SHM scrolling, or it failed */ - uint8_t *raw = buf->mmapped; + uint8_t *raw = buf->data; memmove(raw + dst_y * buf->stride, raw + src_y * buf->stride, height * buf->stride); @@ -2162,7 +2162,7 @@ reapply_old_damage(struct terminal *term, struct buffer *new, struct buffer *old } if (new->age > 1) { - memcpy(new->mmapped, old->mmapped, new->size); + memcpy(new->data, old->data, new->size); return; } diff --git a/shm.c b/shm.c index 12688445..26d1b839 100644 --- a/shm.c +++ b/shm.c @@ -108,7 +108,7 @@ buffer_destroy_dont_close(struct buffer *buf) free(buf->pix); buf->pix = NULL; buf->wl_buf = NULL; - buf->mmapped = NULL; + buf->data = NULL; } static void @@ -150,6 +150,8 @@ buffer_destroy(struct buffer_private *buf) void shm_fini(void) { + xassert(tll_length(buffers) == 0); + tll_foreach(buffers, it) { buffer_destroy(&it->item); tll_remove(buffers, it); @@ -199,7 +201,7 @@ page_size(void) static bool instantiate_offset(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) { - xassert(buf->public.mmapped == NULL); + xassert(buf->public.data == NULL); xassert(buf->public.pix == NULL); xassert(buf->public.wl_buf == NULL); xassert(buf->pool != NULL); @@ -233,7 +235,7 @@ instantiate_offset(struct wl_shm *shm, struct buffer_private *buf, off_t new_off } } - buf->public.mmapped = mmapped; + buf->public.data = mmapped; buf->public.wl_buf = wl_buf; buf->public.pix = pix; buf->offset = new_offset; @@ -584,7 +586,7 @@ wrap_buffer(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) xassert(diff > buf->public.size); memcpy((uint8_t *)pool->real_mmapped + new_offset, - buf->public.mmapped, + buf->public.data, buf->public.size); off_t trim_ofs, trim_len; @@ -653,7 +655,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, if (top_keep_rows > 0) { /* Copy current 'top' region to its new location */ const int stride = buf->public.stride; - uint8_t *base = buf->public.mmapped; + uint8_t *base = buf->public.data; memmove( base + (top_margin + rows) * stride, @@ -704,7 +706,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, /* Copy 'bottom' region to its new location */ const size_t size = buf->public.size; const int stride = buf->public.stride; - uint8_t *base = buf->public.mmapped; + uint8_t *base = buf->public.data; memmove( base + size - (bottom_margin + bottom_keep_rows) * stride, @@ -760,7 +762,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, /* Copy 'bottom' region to its new location */ const size_t size = buf->public.size; const int stride = buf->public.stride; - uint8_t *base = buf->public.mmapped; + uint8_t *base = buf->public.data; memmove( base + size - (bottom_margin + rows + bottom_keep_rows) * stride, @@ -809,7 +811,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, if (ret && top_keep_rows > 0) { /* Copy current 'top' region to its new location */ const int stride = buf->public.stride; - uint8_t *base = buf->public.mmapped; + uint8_t *base = buf->public.data; memmove( base + (top_margin + 0) * stride, diff --git a/shm.h b/shm.h index 01f1a711..43398cc9 100644 --- a/shm.h +++ b/shm.h @@ -17,7 +17,7 @@ struct buffer { int stride; size_t size; /* Buffer size */ - void *mmapped; /* Raw data (TODO: rename) */ + void *data; /* Raw data (TODO: rename) */ struct wl_buffer *wl_buf; pixman_image_t **pix; From f5da62c462430551869d830ef6c4302d4c3b4fea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 22:21:37 +0200 Subject: [PATCH 03/10] shm: replace inclusion of terminal.h with a forward declaration of damage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s the only thing we “need” from terminal.h --- shm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shm.h b/shm.h index 43398cc9..dc37f1b7 100644 --- a/shm.h +++ b/shm.h @@ -7,7 +7,7 @@ #include #include -#include "terminal.h" +struct damage; struct buffer { bool locked; /* Caller owned, shm won’t destroy it */ From 99ea47c97ac6884e469d1d2cdb76220f3398a737 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Thu, 15 Jul 2021 22:30:08 +0200 Subject: [PATCH 04/10] =?UTF-8?q?shm:=20move=20=E2=80=98size=E2=80=99=20to?= =?UTF-8?q?=20the=20private=20buffer=20struct?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- render.c | 2 +- shm.c | 31 ++++++++++++++++--------------- shm.h | 3 +-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/render.c b/render.c index 076a8d54..c48c515a 100644 --- a/render.c +++ b/render.c @@ -2162,7 +2162,7 @@ reapply_old_damage(struct terminal *term, struct buffer *new, struct buffer *old } if (new->age > 1) { - memcpy(new->data, old->data, new->size); + memcpy(new->data, old->data, new->height * new->stride); return; } diff --git a/shm.c b/shm.c index 26d1b839..cd26c62e 100644 --- a/shm.c +++ b/shm.c @@ -71,14 +71,15 @@ struct buffer_pool { struct buffer_private { struct buffer public; - bool busy; /* Owned by compositor */ + size_t size; + bool busy; /* Owned by compositor */ unsigned long cookie; struct buffer_pool *pool; - off_t offset; /* Offset into memfd where data begins */ + off_t offset; /* Offset into memfd where data begins */ bool scrollable; - bool purge; /* True if this buffer should be destroyed */ + bool purge; /* True if this buffer should be destroyed */ }; static tll(struct buffer_private) buffers; @@ -425,11 +426,11 @@ get_new_buffers(struct wl_shm *shm, size_t count, .width = info[i].width, .height = info[i].height, .stride = stride[i], - .size = sizes[i], .pix_instances = pix_instances, .age = 1234, /* Force a full repaint */ }, .cookie = info[i].cookie, + .size = sizes[i], .busy = true, .purge = immediate_purge, .pool = pool, @@ -443,7 +444,7 @@ get_new_buffers(struct wl_shm *shm, size_t count, pixman_region32_init(&buf->public.dirty); pool->ref_count++; - offset += buf->public.size; + offset += buf->size; bufs[i] = &buf->public; } @@ -583,11 +584,11 @@ wrap_buffer(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) off_t UNUSED diff = new_offset < buf->offset ? buf->offset - new_offset : new_offset - buf->offset; - xassert(diff > buf->public.size); + xassert(diff > buf->size); memcpy((uint8_t *)pool->real_mmapped + new_offset, buf->public.data, - buf->public.size); + buf->size); off_t trim_ofs, trim_len; if (new_offset > buf->offset) { @@ -596,7 +597,7 @@ wrap_buffer(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) trim_len = new_offset; } else { /* Trim everything *after* the new buffer location */ - trim_ofs = new_offset + buf->public.size; + trim_ofs = new_offset + buf->size; trim_len = pool->mmap_size - trim_ofs; } @@ -633,9 +634,9 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, const off_t diff = rows * buf->public.stride; xassert(rows > 0); - xassert(diff < buf->public.size); + xassert(diff < buf->size); - if (buf->offset + diff + buf->public.size > max_pool_size) { + if (buf->offset + diff + buf->size > max_pool_size) { LOG_DBG("memfd offset wrap around"); if (!wrap_buffer(shm, buf, 0)) goto err; @@ -643,7 +644,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, off_t new_offset = buf->offset + diff; xassert(new_offset > buf->offset); - xassert(new_offset + buf->public.size <= max_pool_size); + xassert(new_offset + buf->size <= max_pool_size); #if TIME_SCROLL struct timeval time1; @@ -704,7 +705,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, if (ret && bottom_keep_rows > 0) { /* Copy 'bottom' region to its new location */ - const size_t size = buf->public.size; + const size_t size = buf->size; const int stride = buf->public.stride; uint8_t *base = buf->public.data; @@ -742,7 +743,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, const off_t diff = rows * buf->public.stride; if (diff > buf->offset) { LOG_DBG("memfd offset reverse wrap-around"); - if (!wrap_buffer(shm, buf, (max_pool_size - buf->public.size) & ~(page_size() - 1))) + if (!wrap_buffer(shm, buf, (max_pool_size - buf->size) & ~(page_size() - 1))) goto err; } @@ -760,7 +761,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, if (bottom_keep_rows > 0) { /* Copy 'bottom' region to its new location */ - const size_t size = buf->public.size; + const size_t size = buf->size; const int stride = buf->public.stride; uint8_t *base = buf->public.data; @@ -780,7 +781,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, buffer_destroy_dont_close(&buf->public); /* Free unused memory - everything after the relocated buffer */ - const off_t trim_ofs = new_offset + buf->public.size; + const off_t trim_ofs = new_offset + buf->size; const off_t trim_len = pool->mmap_size - trim_ofs; if (fallocate( diff --git a/shm.h b/shm.h index dc37f1b7..435f9de4 100644 --- a/shm.h +++ b/shm.h @@ -16,8 +16,7 @@ struct buffer { int height; int stride; - size_t size; /* Buffer size */ - void *data; /* Raw data (TODO: rename) */ + void *data; struct wl_buffer *wl_buf; pixman_image_t **pix; From 69260dd9604f39f5e7270ac44502c7e60618500b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:47:15 +0200 Subject: [PATCH 05/10] shm: we may exit with busy buffers remaining (seen on KDE) --- render.c | 2 +- shm.c | 28 +++++++++++++++++++++------- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/render.c b/render.c index c48c515a..90fdae7b 100644 --- a/render.c +++ b/render.c @@ -2311,7 +2311,7 @@ grid_render(struct terminal *term) } else if (buf->age > 0) { - LOG_DBG("buffer age: %u", buf->age); + LOG_DBG("buffer age: %u (%p)", buf->age, (void *)buf); xassert(term->render.last_buf != NULL); xassert(term->render.last_buf != buf); diff --git a/shm.c b/shm.c index cd26c62e..87900f01 100644 --- a/shm.c +++ b/shm.c @@ -151,13 +151,26 @@ buffer_destroy(struct buffer_private *buf) void shm_fini(void) { - xassert(tll_length(buffers) == 0); + size_t busy_count UNUSED = 0; + size_t non_busy_count UNUSED = 0; tll_foreach(buffers, it) { + if (it->item.busy) + busy_count++; + else + non_busy_count++; + buffer_destroy(&it->item); tll_remove(buffers, it); } + LOG_DBG("buffers left: busy=%zu, non-busy=%zu", busy_count, non_busy_count); + + if (non_busy_count > 0) { + BUG("%zu non-busy buffers remaining (%zu buffers in total)", + non_busy_count, busy_count + non_busy_count); + } + #if defined(MEASURE_SHM_ALLOCS) && MEASURE_SHM_ALLOCS LOG_INFO("max total allocations was: %zu MB", max_alloced / 1024 / 1024); #endif @@ -272,8 +285,9 @@ destroy_all_purgeables(void) if (it->item.busy) continue; - LOG_DBG("cookie=%lx: purging buffer %p (width=%d, height=%d): %zu KB", - cookie, (void *)&it->item, it->item.width, it->item.height, + LOG_DBG("purging buffer %p (width=%d, height=%d): %zu KB", + (void *)&it->item, + it->item.public.width, it->item.public.height, it->item.size / 1024); buffer_destroy(&it->item); @@ -630,7 +644,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, xassert(pool->ref_count == 1); xassert(pool->fd >= 0); - LOG_DBG("scrolling %d rows (%d bytes)", rows, rows * buf->stride); + LOG_DBG("scrolling %d rows (%d bytes)", rows, rows * buf->public.stride); const off_t diff = rows * buf->public.stride; xassert(rows > 0); @@ -865,9 +879,9 @@ shm_purge(struct wl_shm *shm, unsigned long cookie) if (it->item.cookie != cookie) continue; - if (it->item.busy) { - LOG_WARN("deferring purge of 'busy' buffer (width=%d, height=%d)", - it->item.public.width, it->item.public.height); + if (it->item.busy) { + LOG_DBG("deferring purge of 'busy' buffer (width=%d, height=%d)", + it->item.public.width, it->item.public.height); it->item.purge = true; } else { buffer_destroy(&it->item); From 232fb20269b68157874f57d676c1b0b95c2386aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:47:57 +0200 Subject: [PATCH 06/10] shm: replace 'locked' attribute with a ref-counter The initial ref-count is either 1 or 0, depending on whether the buffer is supposed to be released "immeidately" (meaning, as soon as the compositor releases it). Two new user facing functions have been added: shm_addref() and shm_unref(). Our renderer now uses these two functions instead of manually setting and clearing the 'locked' attribute. shm_unref() will decrement the ref-counter, and destroy the buffer when the counter reaches zero. Except if the buffer is currently "busy" (compositor owned), in which case destruction is deferred to the release event. The buffer is still removed from the list though. --- render.c | 12 +++---- shm.c | 94 +++++++++++++++++++++++++++++------------------------- shm.h | 5 +-- terminal.c | 2 ++ 4 files changed, 60 insertions(+), 53 deletions(-) diff --git a/render.c b/render.c index 90fdae7b..c6835664 100644 --- a/render.c +++ b/render.c @@ -2324,19 +2324,18 @@ grid_render(struct terminal *term) } if (term->render.last_buf != NULL) { - term->render.last_buf->locked = false; - free(term->render.last_buf->scroll_damage); - term->render.last_buf->scroll_damage = NULL; + shm_unref(term->render.last_buf); + term->render.last_buf = NULL; } term->render.last_buf = buf; term->render.was_flashing = term->flash.active; term->render.was_searching = term->is_searching; - buf->locked = true; + shm_addref(buf); buf->age = 0; - xassert(buf->scroll_damage == NULL); + free(term->render.last_buf->scroll_damage); buf->scroll_damage_count = tll_length(term->grid->scroll_damage); buf->scroll_damage = xmalloc( buf->scroll_damage_count * sizeof(buf->scroll_damage[0])); @@ -3484,8 +3483,7 @@ damage_view: tll_free(term->normal.scroll_damage); tll_free(term->alt.scroll_damage); - if (term->render.last_buf != NULL) - term->render.last_buf->locked = false; + shm_unref(term->render.last_buf); term->render.last_buf = NULL; term_damage_view(term); render_refresh_csd(term); diff --git a/shm.c b/shm.c index 87900f01..a2f8cffe 100644 --- a/shm.c +++ b/shm.c @@ -71,6 +71,7 @@ struct buffer_pool { struct buffer_private { struct buffer public; + size_t ref_count; size_t size; bool busy; /* Owned by compositor */ @@ -79,7 +80,6 @@ struct buffer_private { off_t offset; /* Offset into memfd where data begins */ bool scrollable; - bool purge; /* True if this buffer should be destroyed */ }; static tll(struct buffer_private) buffers; @@ -148,6 +148,19 @@ buffer_destroy(struct buffer_private *buf) pixman_region32_fini(&buf->public.dirty); } +static bool +buffer_unref_no_remove_from_cache(struct buffer_private *buf) +{ + if (buf->ref_count > 0) + buf->ref_count--; + + if (buf->ref_count > 0 || buf->busy) + return false; + + buffer_destroy(buf); + return true; +} + void shm_fini(void) { @@ -187,6 +200,9 @@ buffer_release(void *data, struct wl_buffer *wl_buffer) xassert(buffer->public.wl_buf == wl_buffer); xassert(buffer->busy); buffer->busy = false; + + if (buffer->ref_count == 0) + shm_unref(&buffer->public); } static const struct wl_buffer_listener buffer_listener = { @@ -271,30 +287,6 @@ err: return false; } -static void NOINLINE -destroy_all_purgeables(void) -{ - /* Purge buffers marked for purging */ - tll_foreach(buffers, it) { - if (it->item.public.locked) - continue; - - if (!it->item.purge) - continue; - - if (it->item.busy) - continue; - - LOG_DBG("purging buffer %p (width=%d, height=%d): %zu KB", - (void *)&it->item, - it->item.public.width, it->item.public.height, - it->item.size / 1024); - - buffer_destroy(&it->item); - tll_remove(buffers, it); - } -} - static void NOINLINE get_new_buffers(struct wl_shm *shm, size_t count, struct buffer_description info[static count], @@ -444,9 +436,9 @@ get_new_buffers(struct wl_shm *shm, size_t count, .age = 1234, /* Force a full repaint */ }, .cookie = info[i].cookie, + .ref_count = immediate_purge ? 0 : 1, .size = sizes[i], .busy = true, - .purge = immediate_purge, .pool = pool, .scrollable = scrollable, .offset = 0, @@ -500,7 +492,6 @@ shm_get_many(struct wl_shm *shm, size_t count, struct buffer *bufs[static count], size_t pix_instances) { - destroy_all_purgeables(); get_new_buffers(shm, count, info, bufs, pix_instances, false, true); } @@ -508,8 +499,6 @@ struct buffer * shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, bool scrollable, size_t pix_instances) { - destroy_all_purgeables(); - struct buffer_private *cached = NULL; tll_foreach(buffers, it) { if (it->item.public.width != width) @@ -532,7 +521,6 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, LOG_DBG("cookie=%lx: re-using buffer from cache (buf=%p)", cookie, (void *)&it->item); it->item.busy = true; - it->item.purge = false; pixman_region32_clear(&it->item.public.dirty); free(it->item.public.scroll_damage); it->item.public.scroll_damage = NULL; @@ -543,10 +531,12 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, * re-use. Pick the “youngest” one, and mark the * other one for purging */ if (it->item.public.age < cached->public.age) { - cached->purge = true; + shm_unref(&cached->public); cached = &it->item; - } else - it->item.purge = true; + } else { + if (buffer_unref_no_remove_from_cache(&it->item)) + tll_remove(buffers, it); + } } } } @@ -559,14 +549,12 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, if (it->item.cookie != cookie) continue; - if (it->item.busy) - continue; - if (it->item.public.width == width && it->item.public.height == height) continue; LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)&it->item); - it->item.purge = true; + if (buffer_unref_no_remove_from_cache(&it->item)) + tll_remove(buffers, it); } struct buffer *ret; @@ -879,13 +867,31 @@ shm_purge(struct wl_shm *shm, unsigned long cookie) if (it->item.cookie != cookie) continue; - if (it->item.busy) { - LOG_DBG("deferring purge of 'busy' buffer (width=%d, height=%d)", - it->item.public.width, it->item.public.height); - it->item.purge = true; - } else { - buffer_destroy(&it->item); + if (buffer_unref_no_remove_from_cache(&it->item)) tll_remove(buffers, it); - } + } +} + +void +shm_addref(struct buffer *_buf) +{ + struct buffer_private *buf = (struct buffer_private *)_buf; + buf->ref_count++; +} + +void +shm_unref(struct buffer *_buf) +{ + if (_buf == NULL) + return; + + struct buffer_private *buf = (struct buffer_private *)_buf; + tll_foreach(buffers, it) { + if (&it->item != buf) + continue; + + if (buffer_unref_no_remove_from_cache(buf)) + tll_remove(buffers, it); + break; } } diff --git a/shm.h b/shm.h index 435f9de4..18941588 100644 --- a/shm.h +++ b/shm.h @@ -10,8 +10,6 @@ struct damage; struct buffer { - bool locked; /* Caller owned, shm won’t destroy it */ - int width; int height; int stride; @@ -23,6 +21,7 @@ struct buffer { size_t pix_instances; unsigned age; + struct damage *scroll_damage; size_t scroll_damage_count; pixman_region32_t dirty; @@ -74,6 +73,8 @@ bool shm_scroll(struct wl_shm *shm, struct buffer *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows); +void shm_addref(struct buffer *buf); +void shm_unref(struct buffer *buf); void shm_purge(struct wl_shm *shm, unsigned long cookie); struct terminal; diff --git a/terminal.c b/terminal.c index 488ac316..c7776f28 100644 --- a/terminal.c +++ b/terminal.c @@ -35,6 +35,7 @@ #include "selection.h" #include "sixel.h" #include "slave.h" +#include "shm.h" #include "spawn.h" #include "url-mode.h" #include "util.h" @@ -1457,6 +1458,7 @@ term_destroy(struct terminal *term) sem_destroy(&term->render.workers.done); xassert(tll_length(term->render.workers.queue) == 0); tll_free(term->render.workers.queue); + shm_unref(term->render.last_buf); tll_free(term->tab_stops); From 4efb34927eb46d8b12fc0d5638653cf23839d7dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:48:18 +0200 Subject: [PATCH 07/10] shm: remove deferred buffers from main list immediately When unref:ing a "busy" buffer, destruction is (still) deferred to the buffer release event. However, we now move the buffer off the buffer list immediately, and instead push it to a 'deferred' list. This prevents buffer re-use of buffers scheduled for destruction. It also means less buffers to iterate through when trying to find a re-usable buffer in shm_get_buffer(), since we no longer have to wade through a potentially long list of to-be-deleted buffers. --- shm.c | 143 ++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 90 insertions(+), 53 deletions(-) diff --git a/shm.c b/shm.c index a2f8cffe..d5c94c8b 100644 --- a/shm.c +++ b/shm.c @@ -82,7 +82,8 @@ struct buffer_private { bool scrollable; }; -static tll(struct buffer_private) buffers; +static tll(struct buffer_private *) buffers; +static tll(struct buffer_private *) deferred; #undef MEASURE_SHM_ALLOCS #if defined(MEASURE_SHM_ALLOCS) @@ -146,18 +147,22 @@ buffer_destroy(struct buffer_private *buf) free(buf->public.scroll_damage); pixman_region32_fini(&buf->public.dirty); + free(buf); } static bool buffer_unref_no_remove_from_cache(struct buffer_private *buf) { - if (buf->ref_count > 0) - buf->ref_count--; + xassert(buf->ref_count > 0); + buf->ref_count--; - if (buf->ref_count > 0 || buf->busy) + if (buf->ref_count > 0) return false; - buffer_destroy(buf); + if (buf->busy) + tll_push_back(deferred, buf); + else + buffer_destroy(buf); return true; } @@ -168,15 +173,23 @@ shm_fini(void) size_t non_busy_count UNUSED = 0; tll_foreach(buffers, it) { - if (it->item.busy) + if (it->item->busy) busy_count++; else non_busy_count++; - buffer_destroy(&it->item); + buffer_destroy(it->item); tll_remove(buffers, it); } + xassert(busy_count == 0); + busy_count = tll_length(deferred); + + tll_foreach(deferred, it) { + buffer_destroy(it->item); + tll_remove(deferred, it); + } + LOG_DBG("buffers left: busy=%zu, non-busy=%zu", busy_count, non_busy_count); if (non_busy_count > 0) { @@ -201,8 +214,21 @@ buffer_release(void *data, struct wl_buffer *wl_buffer) xassert(buffer->busy); buffer->busy = false; - if (buffer->ref_count == 0) - shm_unref(&buffer->public); + if (buffer->ref_count == 0) { + LOG_WARN("deferred delete"); + + bool found = false; + tll_foreach(deferred, it) { + if (it->item == buffer) { + found = true; + buffer_destroy(buffer); + tll_remove(deferred, it); + break; + } + } + + xassert(found); + } } static const struct wl_buffer_listener buffer_listener = { @@ -425,28 +451,33 @@ get_new_buffers(struct wl_shm *shm, size_t count, for (size_t i = 0; i < count; i++) { /* Push to list of available buffers, but marked as 'busy' */ - tll_push_front( - buffers, - ((struct buffer_private){ - .public = { - .width = info[i].width, - .height = info[i].height, - .stride = stride[i], - .pix_instances = pix_instances, - .age = 1234, /* Force a full repaint */ - }, - .cookie = info[i].cookie, - .ref_count = immediate_purge ? 0 : 1, - .size = sizes[i], - .busy = true, - .pool = pool, - .scrollable = scrollable, - .offset = 0, - })); + struct buffer_private *buf = xmalloc(sizeof(*buf)); + *buf = (struct buffer_private){ + .public = { + .width = info[i].width, + .height = info[i].height, + .stride = stride[i], + .pix_instances = pix_instances, + .age = 1234, /* Force a full repaint */ + }, + .cookie = info[i].cookie, + .ref_count = immediate_purge ? 0 : 1, + .size = sizes[i], + .busy = true, + .pool = pool, + .scrollable = scrollable, + .offset = 0, + }; - struct buffer_private *buf = &tll_front(buffers); - if (!instantiate_offset(shm, buf, offset)) + if (!instantiate_offset(shm, buf, offset)) { + free(buf); goto err; + } + + if (immediate_purge) + tll_push_front(deferred, buf); + else + tll_push_front(buffers, buf); pixman_region32_init(&buf->public.dirty); pool->ref_count++; @@ -501,40 +532,42 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, { struct buffer_private *cached = NULL; tll_foreach(buffers, it) { - if (it->item.public.width != width) + struct buffer_private *buf = it->item; + + if (buf->public.width != width) continue; - if (it->item.public.height != height) + if (buf->public.height != height) continue; - if (it->item.cookie != cookie) + if (buf->cookie != cookie) continue; - if (it->item.busy) - it->item.public.age++; + if (buf->busy) + buf->public.age++; else #if FORCED_DOUBLE_BUFFERING - if (it->item.age == 0) - it->item.age++; + if (buf->age == 0) + buf->age++; else #endif { if (cached == NULL) { LOG_DBG("cookie=%lx: re-using buffer from cache (buf=%p)", - cookie, (void *)&it->item); - it->item.busy = true; - pixman_region32_clear(&it->item.public.dirty); - free(it->item.public.scroll_damage); - it->item.public.scroll_damage = NULL; - xassert(it->item.public.pix_instances == pix_instances); - cached = &it->item; + cookie, (void *)buf); + buf->busy = true; + pixman_region32_clear(&buf->public.dirty); + free(buf->public.scroll_damage); + buf->public.scroll_damage = NULL; + xassert(buf->public.pix_instances == pix_instances); + cached = it->item; } else { /* We have multiple buffers eligible for * re-use. Pick the “youngest” one, and mark the * other one for purging */ - if (it->item.public.age < cached->public.age) { + if (buf->public.age < cached->public.age) { shm_unref(&cached->public); - cached = &it->item; + cached = buf; } else { - if (buffer_unref_no_remove_from_cache(&it->item)) + if (buffer_unref_no_remove_from_cache(buf)) tll_remove(buffers, it); } } @@ -546,14 +579,16 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, /* Mark old buffers associated with this cookie for purging */ tll_foreach(buffers, it) { - if (it->item.cookie != cookie) + struct buffer_private *buf = it->item; + + if (buf->cookie != cookie) continue; - if (it->item.public.width == width && it->item.public.height == height) + if (buf->public.width == width && buf->public.height == height) continue; - LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)&it->item); - if (buffer_unref_no_remove_from_cache(&it->item)) + LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)buf); + if (buffer_unref_no_remove_from_cache(buf)) tll_remove(buffers, it); } @@ -864,10 +899,12 @@ shm_purge(struct wl_shm *shm, unsigned long cookie) /* Purge old buffers associated with this cookie */ tll_foreach(buffers, it) { - if (it->item.cookie != cookie) + struct buffer_private *buf = it->item; + + if (buf->cookie != cookie) continue; - if (buffer_unref_no_remove_from_cache(&it->item)) + if (buffer_unref_no_remove_from_cache(buf)) tll_remove(buffers, it); } } @@ -887,7 +924,7 @@ shm_unref(struct buffer *_buf) struct buffer_private *buf = (struct buffer_private *)_buf; tll_foreach(buffers, it) { - if (&it->item != buf) + if (it->item != buf) continue; if (buffer_unref_no_remove_from_cache(buf)) From 53851e13ec976001cdec2077d7963c292bcd6d20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:48:49 +0200 Subject: [PATCH 08/10] shm: refactor: move away from a single, global, buffer list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Up until now, *all* buffers have been tracked in a single, global buffer list. We've used 'cookies' to separate buffers from different contexts (so that shm_get_buffer() doesn't try to re-use e.g. a search-box buffer for the main grid). This patch refactors this, and completely removes the global list. Instead of cookies, we now use 'chains'. A chain tracks both the properties to apply to newly created buffers (scrollable, number of pixman instances to instantiate etc), as well as the instantiated buffers themselves. This means there's strictly speaking not much use for shm_fini() anymore, since its up to the chain owner to call shm_chain_free(), which will also purge all buffers. However, since purging a buffer may be deferred, if the buffer is owned by the compositor at the time of the call to shm_purge() or shm_chain_free(), we still keep a global 'deferred' list, on to which deferred buffers are pushed. shm_fini() iterates this list and destroys the buffers _even_ if they are still owned by the compositor. This only happens at program termination, and not when destroying a terminal instance. I.e. closing a window in a “foot --server” does *not* trigger this. Each terminal instatiates a number of chains, and these chains are destroyed when the terminal instance is destroyed. Note that some buffers may be put on the deferred list, as mentioned above. --- render.c | 49 ++++++------ shm.c | 215 ++++++++++++++++++++++++++++------------------------- shm.h | 39 ++++------ terminal.c | 25 +++++++ terminal.h | 10 +++ wayland.c | 16 ++-- 6 files changed, 190 insertions(+), 164 deletions(-) diff --git a/render.c b/render.c index c6835664..7bcad519 100644 --- a/render.c +++ b/render.c @@ -943,7 +943,7 @@ grid_render_scroll(struct terminal *term, struct buffer *buf, if (try_shm_scroll) { did_shm_scroll = shm_scroll( - term->wl->shm, buf, dmg->lines * term->cell_height, + buf, dmg->lines * term->cell_height, term->margins.top, dmg->region.start * term->cell_height, term->margins.bottom, (term->rows - dmg->region.end) * term->cell_height); } @@ -1008,7 +1008,7 @@ grid_render_scroll_reverse(struct terminal *term, struct buffer *buf, if (try_shm_scroll) { did_shm_scroll = shm_scroll( - term->wl->shm, buf, -dmg->lines * term->cell_height, + buf, -dmg->lines * term->cell_height, term->margins.top, dmg->region.start * term->cell_height, term->margins.bottom, (term->rows - dmg->region.end) * term->cell_height); } @@ -1587,9 +1587,8 @@ render_csd_title(struct terminal *term) xassert(info.width % term->scale == 0); xassert(info.height % term->scale == 0); - unsigned long cookie = shm_cookie_csd(term, CSD_SURF_TITLE); - struct buffer *buf = shm_get_buffer( - term->wl->shm, info.width, info.height, cookie, false, 1); + struct buffer_chain *chain = term->render.chains.csd[CSD_SURF_TITLE]; + struct buffer *buf = shm_get_buffer(chain, info.width, info.height); uint32_t _color = term->conf->colors.fg; uint16_t alpha = 0xffff; @@ -1622,9 +1621,8 @@ render_csd_border(struct terminal *term, enum csd_surface surf_idx) xassert(info.width % term->scale == 0); xassert(info.height % term->scale == 0); - unsigned long cookie = shm_cookie_csd(term, surf_idx); - struct buffer *buf = shm_get_buffer( - term->wl->shm, info.width, info.height, cookie, false, 1); + struct buffer_chain *chain = term->render.chains.csd[surf_idx]; + struct buffer *buf = shm_get_buffer(chain, info.width, info.height); pixman_color_t color = color_hex_to_pixman_with_alpha(0, 0); render_csd_part(term, surf, buf, info.width, info.height, &color); @@ -1808,9 +1806,8 @@ render_csd_button(struct terminal *term, enum csd_surface surf_idx) xassert(info.width % term->scale == 0); xassert(info.height % term->scale == 0); - unsigned long cookie = shm_cookie_csd(term, surf_idx); - struct buffer *buf = shm_get_buffer( - term->wl->shm, info.width, info.height, cookie, false, 1); + struct buffer_chain *chain = term->render.chains.csd[surf_idx]; + struct buffer *buf = shm_get_buffer(chain, info.width, info.height); uint32_t _color; uint16_t alpha = 0xffff; @@ -2084,9 +2081,8 @@ render_scrollback_position(struct terminal *term) return; } - unsigned long cookie = shm_cookie_scrollback_indicator(term); - struct buffer *buf = shm_get_buffer( - term->wl->shm, width, height, cookie, false, 1); + struct buffer_chain *chain = term->render.chains.scrollback_indicator; + struct buffer *buf = shm_get_buffer(chain, width, height); wl_subsurface_set_position( win->scrollback_indicator.sub, x / scale, y / scale); @@ -2117,9 +2113,8 @@ render_render_timer(struct terminal *term, struct timeval render_time) const int height = (2 * margin + term->cell_height + scale - 1) / scale * scale; - unsigned long cookie = shm_cookie_render_timer(term); - struct buffer *buf = shm_get_buffer( - term->wl->shm, width, height, cookie, false, 1); + struct buffer_chain *chain = term->render.chains.render_timer; + struct buffer *buf = shm_get_buffer(chain, width, height); wl_subsurface_set_position( win->render_timer.sub, @@ -2292,9 +2287,8 @@ grid_render(struct terminal *term) xassert(term->width > 0); xassert(term->height > 0); - unsigned long cookie = shm_cookie_grid(term); - struct buffer *buf = shm_get_buffer( - term->wl->shm, term->width, term->height, cookie, true, 1 + term->render.workers.count); + struct buffer_chain *chain = term->render.chains.grid; + struct buffer *buf = shm_get_buffer(chain, term->width, term->height); /* Dirty old and current cursor cell, to ensure they’re repainted */ dirty_old_cursor(term); @@ -2626,8 +2620,8 @@ render_search_box(struct terminal *term) const size_t visible_cells = (visible_width - 2 * margin) / term->cell_width; size_t glyph_offset = term->render.search_glyph_offset; - unsigned long cookie = shm_cookie_search(term); - struct buffer *buf = shm_get_buffer(term->wl->shm, width, height, cookie, false, 1); + struct buffer_chain *chain = term->render.chains.search; + struct buffer *buf = shm_get_buffer(chain, width, height); pixman_region32_t clip; pixman_region32_init_rect(&clip, 0, 0, width, height); @@ -2936,7 +2930,8 @@ render_urls(struct terminal *term) } info[tll_length(win->urls)]; /* For shm_get_many() */ - struct buffer_description shm_desc[tll_length(win->urls)]; + int widths[tll_length(win->urls)]; + int heights[tll_length(win->urls)]; size_t render_count = 0; @@ -3066,15 +3061,15 @@ render_urls(struct terminal *term) info[render_count].x = x; info[render_count].y = y; - shm_desc[render_count].width = width; - shm_desc[render_count].height = height; - shm_desc[render_count].cookie = shm_cookie_url(url); + widths[render_count] = width; + heights[render_count] = height; render_count++; } + struct buffer_chain *chain = term->render.chains.url; struct buffer *bufs[render_count]; - shm_get_many(term->wl->shm, render_count, shm_desc, bufs, 1); + shm_get_many(chain, render_count, widths, heights, bufs); uint32_t fg = term->conf->colors.use_custom.jump_label ? term->conf->colors.jump_label.fg diff --git a/shm.c b/shm.c index d5c94c8b..acc14a68 100644 --- a/shm.c +++ b/shm.c @@ -68,21 +68,28 @@ struct buffer_pool { size_t ref_count; }; +struct buffer_chain; struct buffer_private { struct buffer public; + struct buffer_chain *chain; size_t ref_count; - size_t size; bool busy; /* Owned by compositor */ - unsigned long cookie; struct buffer_pool *pool; off_t offset; /* Offset into memfd where data begins */ + size_t size; bool scrollable; }; -static tll(struct buffer_private *) buffers; +struct buffer_chain { + tll(struct buffer_private *) bufs; + struct wl_shm *shm; + size_t pix_instances; + bool scrollable; +}; + static tll(struct buffer_private *) deferred; #undef MEASURE_SHM_ALLOCS @@ -151,7 +158,7 @@ buffer_destroy(struct buffer_private *buf) } static bool -buffer_unref_no_remove_from_cache(struct buffer_private *buf) +buffer_unref_no_remove_from_chain(struct buffer_private *buf) { xassert(buf->ref_count > 0); buf->ref_count--; @@ -169,34 +176,13 @@ buffer_unref_no_remove_from_cache(struct buffer_private *buf) void shm_fini(void) { - size_t busy_count UNUSED = 0; - size_t non_busy_count UNUSED = 0; - - tll_foreach(buffers, it) { - if (it->item->busy) - busy_count++; - else - non_busy_count++; - - buffer_destroy(it->item); - tll_remove(buffers, it); - } - - xassert(busy_count == 0); - busy_count = tll_length(deferred); + LOG_DBG("deferred buffers: %zu", tll_length(deferred)); tll_foreach(deferred, it) { buffer_destroy(it->item); tll_remove(deferred, it); } - LOG_DBG("buffers left: busy=%zu, non-busy=%zu", busy_count, non_busy_count); - - if (non_busy_count > 0) { - BUG("%zu non-busy buffers remaining (%zu buffers in total)", - non_busy_count, busy_count + non_busy_count); - } - #if defined(MEASURE_SHM_ALLOCS) && MEASURE_SHM_ALLOCS LOG_INFO("max total allocations was: %zu MB", max_alloced / 1024 / 1024); #endif @@ -207,27 +193,25 @@ buffer_release(void *data, struct wl_buffer *wl_buffer) { struct buffer_private *buffer = data; - LOG_DBG("release: cookie=%lx (buf=%p, total buffer count: %zu)", - buffer->cookie, (void *)buffer, tll_length(buffers)); - xassert(buffer->public.wl_buf == wl_buffer); xassert(buffer->busy); buffer->busy = false; if (buffer->ref_count == 0) { - LOG_WARN("deferred delete"); - bool found = false; tll_foreach(deferred, it) { if (it->item == buffer) { found = true; - buffer_destroy(buffer); tll_remove(deferred, it); break; } } + buffer_destroy(buffer); + xassert(found); + if (!found) + LOG_WARN("deferred delete: buffer not on the 'deferred' list"); } } @@ -255,7 +239,7 @@ page_size(void) #endif static bool -instantiate_offset(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) +instantiate_offset(struct buffer_private *buf, off_t new_offset) { xassert(buf->public.data == NULL); xassert(buf->public.pix == NULL); @@ -314,12 +298,11 @@ err: } static void NOINLINE -get_new_buffers(struct wl_shm *shm, size_t count, - struct buffer_description info[static count], - struct buffer *bufs[static count], - size_t pix_instances, bool scrollable, bool immediate_purge) +get_new_buffers(struct buffer_chain *chain, size_t count, + int widths[static count], int heights[static count], + struct buffer *bufs[static count], bool immediate_purge) { - xassert(count == 1 || !scrollable); + xassert(count == 1 || !chain->scrollable); /* * No existing buffer available. Create a new one by: * @@ -335,8 +318,8 @@ get_new_buffers(struct wl_shm *shm, size_t count, size_t total_size = 0; for (size_t i = 0; i < count; i++) { - stride[i] = stride_for_format_and_width(PIXMAN_a8r8g8b8, info[i].width); - sizes[i] = stride[i] * info[i].height; + stride[i] = stride_for_format_and_width(PIXMAN_a8r8g8b8, widths[i]); + sizes[i] = stride[i] * heights[i]; total_size += sizes[i]; } @@ -363,14 +346,18 @@ get_new_buffers(struct wl_shm *shm, size_t count, } #if __SIZEOF_POINTER__ == 8 - off_t offset = scrollable && max_pool_size > 0 ? (max_pool_size / 4) & ~(page_size() - 1) : 0; - off_t memfd_size = scrollable && max_pool_size > 0 ? max_pool_size : total_size; + off_t offset = chain->scrollable && max_pool_size > 0 + ? (max_pool_size / 4) & ~(page_size() - 1) + : 0; + off_t memfd_size = chain->scrollable && max_pool_size > 0 + ? max_pool_size + : total_size; #else off_t offset = 0; off_t memfd_size = total_size; #endif - xassert(scrollable || (offset == 0 && memfd_size == total_size)); + xassert(chain->scrollable || (offset == 0 && memfd_size == total_size)); LOG_DBG("memfd-size: %lu, initial offset: %lu", memfd_size, offset); @@ -397,10 +384,10 @@ get_new_buffers(struct wl_shm *shm, size_t count, #endif } - if (scrollable && !can_punch_hole) { + if (chain->scrollable && !can_punch_hole) { offset = 0; memfd_size = total_size; - scrollable = false; + chain->scrollable = false; if (ftruncate(pool_fd, memfd_size) < 0) { LOG_ERRNO("failed to set size of SHM backing memory file"); @@ -428,13 +415,13 @@ get_new_buffers(struct wl_shm *shm, size_t count, } #endif - wl_pool = wl_shm_create_pool(shm, pool_fd, memfd_size); + wl_pool = wl_shm_create_pool(chain->shm, pool_fd, memfd_size); if (wl_pool == NULL) { LOG_ERR("failed to create SHM pool"); goto err; } - pool = malloc(sizeof(*pool)); + pool = xmalloc(sizeof(*pool)); if (pool == NULL) { LOG_ERRNO("failed to allocate buffer pool"); goto err; @@ -454,22 +441,22 @@ get_new_buffers(struct wl_shm *shm, size_t count, struct buffer_private *buf = xmalloc(sizeof(*buf)); *buf = (struct buffer_private){ .public = { - .width = info[i].width, - .height = info[i].height, + .width = widths[i], + .height = heights[i], .stride = stride[i], - .pix_instances = pix_instances, + .pix_instances = chain->pix_instances, .age = 1234, /* Force a full repaint */ }, - .cookie = info[i].cookie, + .chain = chain, .ref_count = immediate_purge ? 0 : 1, - .size = sizes[i], .busy = true, .pool = pool, - .scrollable = scrollable, .offset = 0, + .size = sizes[i], + .scrollable = chain->scrollable, }; - if (!instantiate_offset(shm, buf, offset)) { + if (!instantiate_offset(buf, offset)) { free(buf); goto err; } @@ -477,7 +464,7 @@ get_new_buffers(struct wl_shm *shm, size_t count, if (immediate_purge) tll_push_front(deferred, buf); else - tll_push_front(buffers, buf); + tll_push_front(chain->bufs, buf); pixman_region32_init(&buf->public.dirty); pool->ref_count++; @@ -518,28 +505,24 @@ err: } void -shm_get_many(struct wl_shm *shm, size_t count, - struct buffer_description info[static count], - struct buffer *bufs[static count], - size_t pix_instances) +shm_get_many(struct buffer_chain *chain, size_t count, + int widths[static count], int heights[static count], + struct buffer *bufs[static count]) { - get_new_buffers(shm, count, info, bufs, pix_instances, false, true); + get_new_buffers(chain, count, widths, heights, bufs, true); } struct buffer * -shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, - bool scrollable, size_t pix_instances) +shm_get_buffer(struct buffer_chain *chain, int width, int height) { struct buffer_private *cached = NULL; - tll_foreach(buffers, it) { + tll_foreach(chain->bufs, it) { struct buffer_private *buf = it->item; if (buf->public.width != width) continue; if (buf->public.height != height) continue; - if (buf->cookie != cookie) - continue; if (buf->busy) buf->public.age++; @@ -551,13 +534,12 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, #endif { if (cached == NULL) { - LOG_DBG("cookie=%lx: re-using buffer from cache (buf=%p)", - cookie, (void *)buf); + LOG_DBG("re-using buffer %p from cache", (void *)buf); buf->busy = true; pixman_region32_clear(&buf->public.dirty); free(buf->public.scroll_damage); buf->public.scroll_damage = NULL; - xassert(buf->public.pix_instances == pix_instances); + xassert(buf->public.pix_instances == chain->pix_instances); cached = it->item; } else { /* We have multiple buffers eligible for @@ -567,8 +549,8 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, shm_unref(&cached->public); cached = buf; } else { - if (buffer_unref_no_remove_from_cache(buf)) - tll_remove(buffers, it); + if (buffer_unref_no_remove_from_chain(buf)) + tll_remove(chain->bufs, it); } } } @@ -578,23 +560,19 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie, return &cached->public; /* Mark old buffers associated with this cookie for purging */ - tll_foreach(buffers, it) { + tll_foreach(chain->bufs, it) { struct buffer_private *buf = it->item; - if (buf->cookie != cookie) - continue; - if (buf->public.width == width && buf->public.height == height) continue; - LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)buf); - if (buffer_unref_no_remove_from_cache(buf)) - tll_remove(buffers, it); + LOG_DBG("marking buffer %p for purging", (void *)buf); + if (buffer_unref_no_remove_from_chain(buf)) + tll_remove(chain->bufs, it); } struct buffer *ret; - get_new_buffers(shm, 1, &(struct buffer_description){width, height, cookie}, - &ret, pix_instances, scrollable, false); + get_new_buffers(chain, 1, &width, &height, &ret, false); return ret; } @@ -612,7 +590,7 @@ shm_can_scroll(const struct buffer *_buf) #if __SIZEOF_POINTER__ == 8 && defined(FALLOC_FL_PUNCH_HOLE) static bool -wrap_buffer(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) +wrap_buffer(struct buffer_private *buf, off_t new_offset) { struct buffer_pool *pool = buf->pool; xassert(pool->ref_count == 1); @@ -649,11 +627,11 @@ wrap_buffer(struct wl_shm *shm, struct buffer_private *buf, off_t new_offset) /* Re-instantiate pixman+wl_buffer+raw pointersw */ buffer_destroy_dont_close(&buf->public); - return instantiate_offset(shm, buf, new_offset); + return instantiate_offset(buf, new_offset); } static bool -shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, +shm_scroll_forward(struct buffer_private *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { @@ -675,7 +653,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, if (buf->offset + diff + buf->size > max_pool_size) { LOG_DBG("memfd offset wrap around"); - if (!wrap_buffer(shm, buf, 0)) + if (!wrap_buffer(buf, 0)) goto err; } @@ -684,6 +662,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, xassert(new_offset + buf->size <= max_pool_size); #if TIME_SCROLL + struct timeval tot; struct timeval time1; gettimeofday(&time1, NULL); @@ -731,7 +710,7 @@ shm_scroll_forward(struct wl_shm *shm, struct buffer_private *buf, int rows, #endif /* Re-instantiate pixman+wl_buffer+raw pointersw */ - bool ret = instantiate_offset(shm, buf, new_offset); + bool ret = instantiate_offset(buf, new_offset); #if TIME_SCROLL struct timeval time4; @@ -768,7 +747,7 @@ err: } static bool -shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, +shm_scroll_reverse(struct buffer_private *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { @@ -780,7 +759,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, const off_t diff = rows * buf->public.stride; if (diff > buf->offset) { LOG_DBG("memfd offset reverse wrap-around"); - if (!wrap_buffer(shm, buf, (max_pool_size - buf->size) & ~(page_size() - 1))) + if (!wrap_buffer(buf, (max_pool_size - buf->size) & ~(page_size() - 1))) goto err; } @@ -837,7 +816,7 @@ shm_scroll_reverse(struct wl_shm *shm, struct buffer_private *buf, int rows, #endif /* Re-instantiate pixman+wl_buffer+raw pointers */ - bool ret = instantiate_offset(shm, buf, new_offset); + bool ret = instantiate_offset(buf, new_offset); #if TIME_SCROLL struct timeval time3; @@ -873,7 +852,7 @@ err: #endif /* FALLOC_FL_PUNCH_HOLE */ bool -shm_scroll(struct wl_shm *shm, struct buffer *_buf, int rows, +shm_scroll(struct buffer *_buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows) { @@ -885,27 +864,22 @@ shm_scroll(struct wl_shm *shm, struct buffer *_buf, int rows, xassert(rows != 0); return rows > 0 - ? shm_scroll_forward(shm, buf, rows, top_margin, top_keep_rows, bottom_margin, bottom_keep_rows) - : shm_scroll_reverse(shm, buf, -rows, top_margin, top_keep_rows, bottom_margin, bottom_keep_rows); + ? shm_scroll_forward(buf, rows, top_margin, top_keep_rows, bottom_margin, bottom_keep_rows) + : shm_scroll_reverse(buf, -rows, top_margin, top_keep_rows, bottom_margin, bottom_keep_rows); #else return false; #endif } void -shm_purge(struct wl_shm *shm, unsigned long cookie) +shm_purge(struct buffer_chain *chain) { - LOG_DBG("cookie=%lx: purging all buffers", cookie); + LOG_DBG("chain: %p: purging all buffers", (void *)chain); /* Purge old buffers associated with this cookie */ - tll_foreach(buffers, it) { - struct buffer_private *buf = it->item; - - if (buf->cookie != cookie) - continue; - - if (buffer_unref_no_remove_from_cache(buf)) - tll_remove(buffers, it); + tll_foreach(chain->bufs, it) { + if (buffer_unref_no_remove_from_chain(it->item)) + tll_remove(chain->bufs, it); } } @@ -923,12 +897,47 @@ shm_unref(struct buffer *_buf) return; struct buffer_private *buf = (struct buffer_private *)_buf; - tll_foreach(buffers, it) { + struct buffer_chain *chain = buf->chain; + + tll_foreach(chain->bufs, it) { if (it->item != buf) continue; - if (buffer_unref_no_remove_from_cache(buf)) - tll_remove(buffers, it); + if (buffer_unref_no_remove_from_chain(buf)) + tll_remove(chain->bufs, it); break; } } + +void +shm_chain_purge(struct buffer_chain *chain) +{ + tll_foreach(chain->bufs, it) { + struct buffer_private *buf = it->item; + if (buffer_unref_no_remove_from_chain(buf)) + tll_remove(chain->bufs, it); + } +} + +struct buffer_chain * +shm_chain_new(struct wl_shm *shm, bool scrollable, size_t pix_instances) +{ + struct buffer_chain *chain = xmalloc(sizeof(*chain)); + *chain = (struct buffer_chain){ + .bufs = tll_init(), + .shm = shm, + .pix_instances = pix_instances, + .scrollable = scrollable, + }; + return chain; +} + +void +shm_chain_free(struct buffer_chain *chain) +{ + if (chain == NULL) + return; + + shm_chain_purge(chain); + free(chain); +} diff --git a/shm.h b/shm.h index 18941588..180c8525 100644 --- a/shm.h +++ b/shm.h @@ -7,6 +7,8 @@ #include #include +#include + struct damage; struct buffer { @@ -27,28 +29,24 @@ struct buffer { pixman_region32_t dirty; }; -struct buffer_description { - int width; - int height; - unsigned long cookie; -}; - void shm_fini(void); void shm_set_max_pool_size(off_t max_pool_size); +struct buffer_chain; +struct buffer_chain *shm_chain_new( + struct wl_shm *shm, bool scrollable, size_t pix_instances); +void shm_chain_free(struct buffer_chain *chain); + /* * Returns a single buffer. * * May returned a cached buffer. If so, the buffer’s age indicates how * many shm_get_buffer() calls have been made for the same - * width/height/cookie while the buffer was still busy. + * width/height while the buffer was still busy. * * A newly allocated buffer has an age of 1234. */ -struct buffer *shm_get_buffer( - struct wl_shm *shm, int width, int height, unsigned long cookie, - bool scrollable, size_t pix_instances); - +struct buffer *shm_get_buffer(struct buffer_chain *chain, int width, int height); /* * Returns many buffers, described by ‘info’, all sharing the same SHM * buffer pool. @@ -64,25 +62,16 @@ struct buffer *shm_get_buffer( * soon as the compositor releases them. */ void shm_get_many( - struct wl_shm *shm, size_t count, - struct buffer_description info[static count], - struct buffer *bufs[static count], size_t pix_instances); + struct buffer_chain *chain, size_t count, + int widths[static count], int heights[static count], + struct buffer *bufs[static count]); bool shm_can_scroll(const struct buffer *buf); -bool shm_scroll(struct wl_shm *shm, struct buffer *buf, int rows, +bool shm_scroll(struct buffer *buf, int rows, int top_margin, int top_keep_rows, int bottom_margin, int bottom_keep_rows); void shm_addref(struct buffer *buf); void shm_unref(struct buffer *buf); -void shm_purge(struct wl_shm *shm, unsigned long cookie); -struct terminal; -static inline unsigned long shm_cookie_grid(const struct terminal *term) { return (unsigned long)((uintptr_t)term + 0); } -static inline unsigned long shm_cookie_search(const struct terminal *term) { return (unsigned long)((uintptr_t)term + 1); } -static inline unsigned long shm_cookie_scrollback_indicator(const struct terminal *term) { return (unsigned long)(uintptr_t)term + 2; } -static inline unsigned long shm_cookie_render_timer(const struct terminal *term) { return (unsigned long)(uintptr_t)term + 3; } -static inline unsigned long shm_cookie_csd(const struct terminal *term, int n) { return (unsigned long)((uintptr_t)term + 4 + (n)); } - -struct url; -static inline unsigned long shm_cookie_url(const struct url *url) { return (unsigned long)(uintptr_t)url; } +void shm_purge(struct buffer_chain *chain); diff --git a/terminal.c b/terminal.c index c7776f28..72ab1f2b 100644 --- a/terminal.c +++ b/terminal.c @@ -1144,6 +1144,23 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, .tab_stops = tll_init(), .wl = wayl, .render = { + .chains = { + .grid = shm_chain_new(wayl->shm, true, 1 + conf->render_worker_count), + .search = shm_chain_new(wayl->shm, false, 1), + .scrollback_indicator = shm_chain_new(wayl->shm, false, 1), + .render_timer = shm_chain_new(wayl->shm, false, 1), + .url = shm_chain_new(wayl->shm, false, 1), + .csd = { + [CSD_SURF_TITLE] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_LEFT] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_RIGHT] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_TOP] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_BOTTOM] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_MINIMIZE] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_MAXIMIZE] = shm_chain_new(wayl->shm, false, 1), + [CSD_SURF_CLOSE] = shm_chain_new(wayl->shm, false, 1), + }, + }, .scrollback_lines = conf->scrollback.lines, .app_sync_updates.timer_fd = app_sync_updates_fd, .title = { @@ -1458,7 +1475,15 @@ term_destroy(struct terminal *term) sem_destroy(&term->render.workers.done); xassert(tll_length(term->render.workers.queue) == 0); tll_free(term->render.workers.queue); + shm_unref(term->render.last_buf); + shm_chain_free(term->render.chains.grid); + shm_chain_free(term->render.chains.search); + shm_chain_free(term->render.chains.scrollback_indicator); + shm_chain_free(term->render.chains.render_timer); + shm_chain_free(term->render.chains.url); + for (size_t i = 0; i < CSD_SURF_COUNT; i++) + shm_chain_free(term->render.chains.csd[i]); tll_free(term->tab_stops); diff --git a/terminal.h b/terminal.h index 5ffa6154..b02036a2 100644 --- a/terminal.h +++ b/terminal.h @@ -21,6 +21,7 @@ #include "fdm.h" #include "macros.h" #include "reaper.h" +#include "shm.h" #include "wayland.h" /* @@ -475,6 +476,15 @@ struct terminal { enum term_surface active_surface; struct { + struct { + struct buffer_chain *grid; + struct buffer_chain *search; + struct buffer_chain *scrollback_indicator; + struct buffer_chain *render_timer; + struct buffer_chain *url; + struct buffer_chain *csd[CSD_SURF_COUNT]; + } chains; + /* Scheduled for rendering, as soon-as-possible */ struct { bool grid; diff --git a/wayland.c b/wayland.c index 880a4cf3..38c80999 100644 --- a/wayland.c +++ b/wayland.c @@ -52,11 +52,10 @@ static void csd_destroy(struct wl_window *win) { struct terminal *term = win->term; - struct wl_shm *shm = term->wl->shm; for (size_t i = 0; i < ALEN(win->csd.surface); i++) { wayl_win_subsurface_destroy(&win->csd.surface[i]); - shm_purge(shm, shm_cookie_csd(term, i)); + shm_purge(term->render.chains.csd[i]); } } @@ -1417,7 +1416,6 @@ wayl_win_destroy(struct wl_window *win) return; struct terminal *term = win->term; - struct wl_shm *shm = term->wl->shm; if (win->csd.move_timeout_fd != -1) close(win->csd.move_timeout_fd); @@ -1472,7 +1470,7 @@ wayl_win_destroy(struct wl_window *win) tll_foreach(win->urls, it) { wayl_win_subsurface_destroy(&it->item.surf); - shm_purge(shm, shm_cookie_url(it->item.url)); + shm_purge(term->render.chains.url); tll_remove(win->urls, it); } @@ -1481,13 +1479,13 @@ wayl_win_destroy(struct wl_window *win) wayl_win_subsurface_destroy(&win->scrollback_indicator); wayl_win_subsurface_destroy(&win->render_timer); - shm_purge(shm, shm_cookie_search(term)); - shm_purge(shm, shm_cookie_scrollback_indicator(term)); - shm_purge(shm, shm_cookie_render_timer(term)); - shm_purge(shm, shm_cookie_grid(term)); + shm_purge(term->render.chains.search); + shm_purge(term->render.chains.scrollback_indicator); + shm_purge(term->render.chains.render_timer); + shm_purge(term->render.chains.grid); for (size_t i = 0; i < ALEN(win->csd.surface); i++) - shm_purge(shm, shm_cookie_csd(term, i)); + shm_purge(term->render.chains.csd[i]); #if defined(HAVE_XDG_ACTIVATION) if (win->xdg_activation_token != NULL) From 0751172b922e32a9c6d54b52e721c934cb88fe9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:49:27 +0200 Subject: [PATCH 09/10] shm: get_buffer: purge mismatching buffers in first buffer iteration There's no longer any need to defer purging of mismatching buffer (i.e. buffers whose width/height doesn't match the requested ones) to after the cache lookup loop. --- shm.c | 32 ++++++-------------------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/shm.c b/shm.c index acc14a68..f7e7a4c5 100644 --- a/shm.c +++ b/shm.c @@ -519,10 +519,12 @@ shm_get_buffer(struct buffer_chain *chain, int width, int height) tll_foreach(chain->bufs, it) { struct buffer_private *buf = it->item; - if (buf->public.width != width) - continue; - if (buf->public.height != height) + if (buf->public.width != width || buf->public.height != height) { + LOG_DBG("purging mismatching buffer %p", (void *)buf); + if (buffer_unref_no_remove_from_chain(buf)) + tll_remove(chain->bufs, it); continue; + } if (buf->busy) buf->public.age++; @@ -559,18 +561,6 @@ shm_get_buffer(struct buffer_chain *chain, int width, int height) if (cached != NULL) return &cached->public; - /* Mark old buffers associated with this cookie for purging */ - tll_foreach(chain->bufs, it) { - struct buffer_private *buf = it->item; - - if (buf->public.width == width && buf->public.height == height) - continue; - - LOG_DBG("marking buffer %p for purging", (void *)buf); - if (buffer_unref_no_remove_from_chain(buf)) - tll_remove(chain->bufs, it); - } - struct buffer *ret; get_new_buffers(chain, 1, &width, &height, &ret, false); return ret; @@ -909,16 +899,6 @@ shm_unref(struct buffer *_buf) } } -void -shm_chain_purge(struct buffer_chain *chain) -{ - tll_foreach(chain->bufs, it) { - struct buffer_private *buf = it->item; - if (buffer_unref_no_remove_from_chain(buf)) - tll_remove(chain->bufs, it); - } -} - struct buffer_chain * shm_chain_new(struct wl_shm *shm, bool scrollable, size_t pix_instances) { @@ -938,6 +918,6 @@ shm_chain_free(struct buffer_chain *chain) if (chain == NULL) return; - shm_chain_purge(chain); + shm_purge(chain); free(chain); } From 6657146a207834ac44425c01703935dee6d61d41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 16 Jul 2021 16:49:52 +0200 Subject: [PATCH 10/10] shm: chain_free: BUG() if there are buffers remaining after purge There may be buffers left, if their destruction has been deferred. However, they should be on the 'deferred' list, not the chain's buffer list. If there are buffers left on the chain's list, that means someone forgot to call shm_unref(). --- shm.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/shm.c b/shm.c index f7e7a4c5..0e55011d 100644 --- a/shm.c +++ b/shm.c @@ -515,6 +515,11 @@ shm_get_many(struct buffer_chain *chain, size_t count, struct buffer * shm_get_buffer(struct buffer_chain *chain, int width, int height) { + LOG_DBG( + "chain=%p: looking for a re-usable %dx%d buffer " + "among %zu potential buffers", + (void *)chain, width, height, tll_length(chain->bufs)); + struct buffer_private *cached = NULL; tll_foreach(chain->bufs, it) { struct buffer_private *buf = it->item; @@ -919,5 +924,11 @@ shm_chain_free(struct buffer_chain *chain) return; shm_purge(chain); + + if (tll_length(chain->bufs) > 0) { + BUG("chain=%p: there are buffers remaining; " + "is there a missing call to shm_unref()?", (void *)chain); + } + free(chain); }