[Mesa-dev] [PATCH] st/mesa: create framebuffer iface hash table per st manager
Brian Paul
brianp at vmware.com
Mon Jul 24 02:41:56 UTC 2017
Can you explain in the comment how/why this fixes the crash? It's not
obvious to me.
Patch looks OK, AFAICT.
Reviewed-by: Brian Paul <brianp at vmware.com>
On 07/23/2017 05:37 PM, Charmaine Lee wrote:
> With this patch, framebuffer interface hash table is created
> per state tracker manager.
>
> Fixes crash with steam.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101876
> Fixes: 5124bf98239 ("st/mesa: add destroy_drawable interface")
> Tested-by: Christoph Haag <haagch at frickel.club>
> ---
> src/gallium/include/state_tracker/st_api.h | 21 +++++
> src/gallium/state_trackers/dri/dri_drawable.c | 1 +
> src/gallium/state_trackers/dri/dri_screen.c | 3 +
> src/gallium/state_trackers/glx/xlib/xm_api.c | 3 +
> src/gallium/state_trackers/glx/xlib/xm_st.c | 1 +
> src/gallium/state_trackers/wgl/stw_device.c | 3 +
> src/gallium/state_trackers/wgl/stw_st.c | 1 +
> src/mesa/state_tracker/st_manager.c | 107 +++++++++++++++++++-------
> 8 files changed, 113 insertions(+), 27 deletions(-)
>
> diff --git a/src/gallium/include/state_tracker/st_api.h b/src/gallium/include/state_tracker/st_api.h
> index 9b660f7..bc62a69 100644
> --- a/src/gallium/include/state_tracker/st_api.h
> +++ b/src/gallium/include/state_tracker/st_api.h
> @@ -284,6 +284,7 @@ struct st_context_attribs
> };
>
> struct st_context_iface;
> +struct st_manager;
>
> /**
> * Represent a windowing system drawable.
> @@ -317,6 +318,11 @@ struct st_framebuffer_iface
> uint32_t ID;
>
> /**
> + * The state tracker manager that manages this object.
> + */
> + struct st_manager *state_manager;
> +
> + /**
> * Available for the state tracker manager to use.
> */
> void *st_manager_private;
> @@ -376,6 +382,11 @@ struct st_context_iface
> void *st_manager_private;
>
> /**
> + * The state tracker manager that manages this object.
> + */
> + struct st_manager *state_manager;
> +
> + /**
> * The CSO context associated with this context in case we need to draw
> * something before swap buffers.
> */
> @@ -483,6 +494,16 @@ struct st_manager
> */
> void (*set_background_context)(struct st_context_iface *stctxi,
> struct util_queue_monitoring *queue_info);
> +
> + /**
> + * Destroy any private data used by the state tracker manager.
> + */
> + void (*destroy)(struct st_manager *smapi);
> +
> + /**
> + * Available for the state tracker manager to use.
> + */
> + void *st_manager_private;
> };
>
> /**
> diff --git a/src/gallium/state_trackers/dri/dri_drawable.c b/src/gallium/state_trackers/dri/dri_drawable.c
> index c7df0f6..9e0dd6b 100644
> --- a/src/gallium/state_trackers/dri/dri_drawable.c
> +++ b/src/gallium/state_trackers/dri/dri_drawable.c
> @@ -158,6 +158,7 @@ dri_create_buffer(__DRIscreen * sPriv,
> dPriv->driverPrivate = (void *)drawable;
> p_atomic_set(&drawable->base.stamp, 1);
> drawable->base.ID = p_atomic_inc_return(&drifb_ID);
> + drawable->base.state_manager = &screen->base;
>
> return GL_TRUE;
> fail:
> diff --git a/src/gallium/state_trackers/dri/dri_screen.c b/src/gallium/state_trackers/dri/dri_screen.c
> index 1dd7bd3..59a850b 100644
> --- a/src/gallium/state_trackers/dri/dri_screen.c
> +++ b/src/gallium/state_trackers/dri/dri_screen.c
> @@ -457,6 +457,9 @@ dri_destroy_option_cache(struct dri_screen * screen)
> void
> dri_destroy_screen_helper(struct dri_screen * screen)
> {
> + if (screen->base.destroy)
> + screen->base.destroy(&screen->base);
> +
> if (screen->st_api && screen->st_api->destroy)
> screen->st_api->destroy(screen->st_api);
>
> diff --git a/src/gallium/state_trackers/glx/xlib/xm_api.c b/src/gallium/state_trackers/glx/xlib/xm_api.c
> index e4b1e9d..828253b 100644
> --- a/src/gallium/state_trackers/glx/xlib/xm_api.c
> +++ b/src/gallium/state_trackers/glx/xlib/xm_api.c
> @@ -181,6 +181,9 @@ xmesa_close_display(Display *display)
> * xmdpy->screen->destroy(xmdpy->screen);
> * }
> */
> +
> + if (xmdpy->smapi->destroy)
> + xmdpy->smapi->destroy(xmdpy->smapi);
> free(xmdpy->smapi);
>
> XFree((char *) info);
> diff --git a/src/gallium/state_trackers/glx/xlib/xm_st.c b/src/gallium/state_trackers/glx/xlib/xm_st.c
> index 6a0f4aa..0c42e65 100644
> --- a/src/gallium/state_trackers/glx/xlib/xm_st.c
> +++ b/src/gallium/state_trackers/glx/xlib/xm_st.c
> @@ -304,6 +304,7 @@ xmesa_create_st_framebuffer(XMesaDisplay xmdpy, XMesaBuffer b)
> stfbi->flush_front = xmesa_st_framebuffer_flush_front;
> stfbi->validate = xmesa_st_framebuffer_validate;
> stfbi->ID = p_atomic_inc_return(&xmesa_stfbi_ID);
> + stfbi->state_manager = xmdpy->smapi;
> p_atomic_set(&stfbi->stamp, 1);
> stfbi->st_manager_private = (void *) xstfb;
>
> diff --git a/src/gallium/state_trackers/wgl/stw_device.c b/src/gallium/state_trackers/wgl/stw_device.c
> index 6c0f14d..b88e110 100644
> --- a/src/gallium/state_trackers/wgl/stw_device.c
> +++ b/src/gallium/state_trackers/wgl/stw_device.c
> @@ -199,6 +199,9 @@ stw_cleanup(void)
> DeleteCriticalSection(&stw_dev->fb_mutex);
> DeleteCriticalSection(&stw_dev->ctx_mutex);
>
> + if (stw_dev->smapi->destroy)
> + stw_dev->smapi->destroy(stw_dev->smapi);
> +
> FREE(stw_dev->smapi);
> stw_dev->stapi->destroy(stw_dev->stapi);
>
> diff --git a/src/gallium/state_trackers/wgl/stw_st.c b/src/gallium/state_trackers/wgl/stw_st.c
> index 85a8b17..5e165c8 100644
> --- a/src/gallium/state_trackers/wgl/stw_st.c
> +++ b/src/gallium/state_trackers/wgl/stw_st.c
> @@ -235,6 +235,7 @@ stw_st_create_framebuffer(struct stw_framebuffer *fb)
> stwfb->fb = fb;
> stwfb->stvis = fb->pfi->stvis;
> stwfb->base.ID = p_atomic_inc_return(&stwfb_ID);
> + stwfb->base.state_manager = stw_dev->smapi;
>
> stwfb->base.visual = &stwfb->stvis;
> p_atomic_set(&stwfb->base.stamp, 1);
> diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c
> index ebc7ca8..834bcc9 100644
> --- a/src/mesa/state_tracker/st_manager.c
> +++ b/src/mesa/state_tracker/st_manager.c
> @@ -61,9 +61,13 @@
> #include "util/list.h"
>
> struct hash_table;
> -static struct hash_table *st_fbi_ht; /* framebuffer iface objects hash table */
> +struct st_manager_private
> +{
> + struct hash_table *stfbi_ht; /* framebuffer iface objects hash table */
> + mtx_t st_mutex;
> +};
>
> -static mtx_t st_mutex = _MTX_INITIALIZER_NP;
> +static void st_manager_destroy(struct st_manager *);
>
> /**
> * Map an attachment to a buffer index.
> @@ -511,45 +515,63 @@ st_framebuffer_iface_equal(const void *a, const void *b)
>
>
> static boolean
> -st_framebuffer_iface_lookup(const struct st_framebuffer_iface *stfbi)
> +st_framebuffer_iface_lookup(struct st_manager *smapi,
> + const struct st_framebuffer_iface *stfbi)
> {
> + struct st_manager_private *smPriv =
> + (struct st_manager_private *)smapi->st_manager_private;
> struct hash_entry *entry;
>
> - mtx_lock(&st_mutex);
> - entry = _mesa_hash_table_search(st_fbi_ht, stfbi);
> - mtx_unlock(&st_mutex);
> + assert(smPriv);
> + assert(smPriv->stfbi_ht);
> +
> + mtx_lock(&smPriv->st_mutex);
> + entry = _mesa_hash_table_search(smPriv->stfbi_ht, stfbi);
> + mtx_unlock(&smPriv->st_mutex);
>
> return entry != NULL;
> }
>
>
> static boolean
> -st_framebuffer_iface_insert(struct st_framebuffer_iface *stfbi)
> +st_framebuffer_iface_insert(struct st_manager *smapi,
> + struct st_framebuffer_iface *stfbi)
> {
> + struct st_manager_private *smPriv =
> + (struct st_manager_private *)smapi->st_manager_private;
> struct hash_entry *entry;
>
> - mtx_lock(&st_mutex);
> - entry = _mesa_hash_table_insert(st_fbi_ht, stfbi, stfbi);
> - mtx_unlock(&st_mutex);
> + assert(smPriv);
> + assert(smPriv->stfbi_ht);
> +
> + mtx_lock(&smPriv->st_mutex);
> + entry = _mesa_hash_table_insert(smPriv->stfbi_ht, stfbi, stfbi);
> + mtx_unlock(&smPriv->st_mutex);
>
> return entry != NULL;
> }
>
>
> static void
> -st_framebuffer_iface_remove(struct st_framebuffer_iface *stfbi)
> +st_framebuffer_iface_remove(struct st_manager *smapi,
> + struct st_framebuffer_iface *stfbi)
> {
> + struct st_manager_private *smPriv =
> + (struct st_manager_private *)smapi->st_manager_private;
> struct hash_entry *entry;
>
> - mtx_lock(&st_mutex);
> - entry = _mesa_hash_table_search(st_fbi_ht, stfbi);
> + if (!smPriv || !smPriv->stfbi_ht);
> + return;
> +
> + mtx_lock(&smPriv->st_mutex);
> + entry = _mesa_hash_table_search(smPriv->stfbi_ht, stfbi);
> if (!entry)
> goto unlock;
>
> - _mesa_hash_table_remove(st_fbi_ht, entry);
> + _mesa_hash_table_remove(smPriv->stfbi_ht, entry);
>
> unlock:
> - mtx_unlock(&st_mutex);
> + mtx_unlock(&smPriv->st_mutex);
> }
>
>
> @@ -561,7 +583,10 @@ static void
> st_api_destroy_drawable(struct st_api *stapi,
> struct st_framebuffer_iface *stfbi)
> {
> - st_framebuffer_iface_remove(stfbi);
> + if (stfbi)
> + return;
> +
> + st_framebuffer_iface_remove(stfbi->state_manager, stfbi);
> }
>
>
> @@ -572,16 +597,24 @@ st_api_destroy_drawable(struct st_api *stapi,
> static void
> st_framebuffers_purge(struct st_context *st)
> {
> + struct st_context_iface *st_iface = &st->iface;
> + struct st_manager *smapi = st_iface->state_manager;
> struct st_framebuffer *stfb, *next;
>
> + assert(smapi);
> +
> LIST_FOR_EACH_ENTRY_SAFE_REV(stfb, next, &st->winsys_buffers, head) {
> + struct st_framebuffer_iface *stfbi = stfb->iface;
> +
> + assert(stfbi);
> +
> /**
> * If the corresponding framebuffer interface object no longer exists,
> * remove the framebuffer object from the context's winsys buffers list,
> * and unreference the framebuffer object, so its resources can be
> * deleted.
> */
> - if (!st_framebuffer_iface_lookup(stfb->iface)) {
> + if (!st_framebuffer_iface_lookup(smapi, stfbi)) {
> LIST_DEL(&stfb->head);
> st_framebuffer_reference(&stfb, NULL);
> }
> @@ -778,6 +811,21 @@ st_api_create_context(struct st_api *stapi, struct st_manager *smapi,
> return NULL;
> }
>
> + /* Create a hash table for the framebuffer interface objects
> + * if it has not been created for this st manager.
> + */
> + if (smapi->st_manager_private == NULL) {
> + struct st_manager_private *smPriv;
> +
> + smPriv = CALLOC_STRUCT(st_manager_private);
> + mtx_init(&smPriv->st_mutex, mtx_plain);
> + smPriv->stfbi_ht = _mesa_hash_table_create(NULL,
> + st_framebuffer_iface_hash,
> + st_framebuffer_iface_equal);
> + smapi->st_manager_private = smPriv;
> + smapi->destroy = st_manager_destroy;
> + }
> +
> if (attribs->flags & ST_CONTEXT_FLAG_ROBUST_ACCESS)
> ctx_flags |= PIPE_CONTEXT_ROBUST_BUFFER_ACCESS;
>
> @@ -846,6 +894,7 @@ st_api_create_context(struct st_api *stapi, struct st_manager *smapi,
> st->iface.st_context_private = (void *) smapi;
> st->iface.cso_context = st->cso_context;
> st->iface.pipe = st->pipe;
> + st->iface.state_manager = smapi;
>
> *error = ST_CONTEXT_SUCCESS;
> return &st->iface;
> @@ -888,7 +937,7 @@ st_framebuffer_reuse_or_create(struct st_context *st,
> /* add the referenced framebuffer interface object to
> * the framebuffer interface object hash table.
> */
> - if (!st_framebuffer_iface_insert(stfbi)) {
> + if (!st_framebuffer_iface_insert(stfbi->state_manager, stfbi)) {
> st_framebuffer_reference(&cur, NULL);
> return NULL;
> }
> @@ -964,8 +1013,6 @@ st_api_make_current(struct st_api *stapi, struct st_context_iface *stctxi,
> static void
> st_api_destroy(struct st_api *stapi)
> {
> - _mesa_hash_table_destroy(st_fbi_ht, NULL);
> - mtx_destroy(&st_mutex);
> }
>
> /**
> @@ -1051,6 +1098,19 @@ st_manager_add_color_renderbuffer(struct st_context *st,
> return TRUE;
> }
>
> +static void
> +st_manager_destroy(struct st_manager *smapi)
> +{
> + struct st_manager_private *smPriv = smapi->st_manager_private;
> +
> + if (smPriv && smPriv->stfbi_ht) {
> + _mesa_hash_table_destroy(smPriv->stfbi_ht, NULL);
> + mtx_destroy(&smPriv->st_mutex);
> + free(smPriv);
> + smapi->st_manager_private = NULL;
> + }
> +}
> +
> static unsigned
> get_version(struct pipe_screen *screen,
> struct st_config_options *options, gl_api api)
> @@ -1106,12 +1166,5 @@ static const struct st_api st_gl_api = {
> struct st_api *
> st_gl_api_create(void)
> {
> - /* Create a hash table for all the framebuffer interface objects */
> -
> - mtx_init(&st_mutex, mtx_plain);
> - st_fbi_ht = _mesa_hash_table_create(NULL,
> - st_framebuffer_iface_hash,
> - st_framebuffer_iface_equal);
> -
> return (struct st_api *) &st_gl_api;
> }
>
More information about the mesa-dev
mailing list