From ac330d4130cb005c75972da2a701b674413456ba Mon Sep 17 00:00:00 2001 From: =?utf8?q?Marek=20Ol=C5=A1=C3=A1k?= Date: Wed, 9 Apr 2014 00:26:32 +0200 Subject: [PATCH] winsys/radeon: fix a race condition between winsys_create and winsys_destroy MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This also hides the reference count from drivers. v2: update the reference count while the mutex is locked in winsys_create Reviewed-by: Michel Dänzer Reviewed-by: Christian König --- src/gallium/drivers/r300/r300_screen.c | 2 +- src/gallium/drivers/r600/r600_pipe.c | 2 +- src/gallium/drivers/radeonsi/si_pipe.c | 2 +- .../winsys/radeon/drm/radeon_drm_winsys.c | 30 ++++++++++++++----- .../winsys/radeon/drm/radeon_drm_winsys.h | 1 + src/gallium/winsys/radeon/drm/radeon_winsys.h | 20 +++++-------- 6 files changed, 33 insertions(+), 24 deletions(-) diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c index 70c9cdf468a..8e601e3d37f 100644 --- a/src/gallium/drivers/r300/r300_screen.c +++ b/src/gallium/drivers/r300/r300_screen.c @@ -552,7 +552,7 @@ static void r300_destroy_screen(struct pipe_screen* pscreen) struct r300_screen* r300screen = r300_screen(pscreen); struct radeon_winsys *rws = radeon_winsys(pscreen); - if (rws && !radeon_winsys_unref(rws)) + if (rws && !rws->unref(rws)) return; pipe_mutex_destroy(r300screen->cmask_mutex); diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c index 8d69358d802..18fde7453ba 100644 --- a/src/gallium/drivers/r600/r600_pipe.c +++ b/src/gallium/drivers/r600/r600_pipe.c @@ -514,7 +514,7 @@ static void r600_destroy_screen(struct pipe_screen* pscreen) if (rscreen == NULL) return; - if (!radeon_winsys_unref(rscreen->b.ws)) + if (!rscreen->b.ws->unref(rscreen->b.ws)) return; if (rscreen->global_pool) { diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c index 09ec60343fe..7dac2872664 100644 --- a/src/gallium/drivers/radeonsi/si_pipe.c +++ b/src/gallium/drivers/radeonsi/si_pipe.c @@ -424,7 +424,7 @@ static void si_destroy_screen(struct pipe_screen* pscreen) if (sscreen == NULL) return; - if (!radeon_winsys_unref(sscreen->b.ws)) + if (!sscreen->b.ws->unref(sscreen->b.ws)) return; r600_destroy_common_screen(&sscreen->b); diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c index ebf7697ac33..07b3c05f2b7 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c @@ -392,12 +392,6 @@ static void radeon_winsys_destroy(struct radeon_winsys *rws) { struct radeon_drm_winsys *ws = (struct radeon_drm_winsys*)rws; - pipe_mutex_lock(fd_tab_mutex); - if (fd_tab) { - util_hash_table_remove(fd_tab, intptr_to_pointer(ws->fd)); - } - pipe_mutex_unlock(fd_tab_mutex); - if (ws->thread) { ws->kill_thread = 1; pipe_semaphore_signal(&ws->cs_queued); @@ -573,6 +567,25 @@ static PIPE_THREAD_ROUTINE(radeon_drm_cs_emit_ioctl, param) DEBUG_GET_ONCE_BOOL_OPTION(thread, "RADEON_THREAD", TRUE) static PIPE_THREAD_ROUTINE(radeon_drm_cs_emit_ioctl, param); +static bool radeon_winsys_unref(struct radeon_winsys *ws) +{ + struct radeon_drm_winsys *rws = (struct radeon_drm_winsys*)ws; + bool destroy; + + /* When the reference counter drops to zero, remove the fd from the table. + * This must happen while the mutex is locked, so that + * radeon_drm_winsys_create in another thread doesn't get the winsys + * from the table when the counter drops to 0. */ + pipe_mutex_lock(fd_tab_mutex); + + destroy = pipe_reference(&rws->reference, NULL); + if (destroy && fd_tab) + util_hash_table_remove(fd_tab, intptr_to_pointer(rws->fd)); + + pipe_mutex_unlock(fd_tab_mutex); + return destroy; +} + PUBLIC struct radeon_winsys *radeon_drm_winsys_create(int fd) { struct radeon_drm_winsys *ws; @@ -584,8 +597,8 @@ PUBLIC struct radeon_winsys *radeon_drm_winsys_create(int fd) ws = util_hash_table_get(fd_tab, intptr_to_pointer(fd)); if (ws) { + pipe_reference(NULL, &ws->reference); pipe_mutex_unlock(fd_tab_mutex); - pipe_reference(NULL, &ws->base.reference); return &ws->base; } @@ -616,9 +629,10 @@ PUBLIC struct radeon_winsys *radeon_drm_winsys_create(int fd) } /* init reference */ - pipe_reference_init(&ws->base.reference, 1); + pipe_reference_init(&ws->reference, 1); /* Set functions. */ + ws->base.unref = radeon_winsys_unref; ws->base.destroy = radeon_winsys_destroy; ws->base.query_info = radeon_query_info; ws->base.cs_request_feature = radeon_cs_request_feature; diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h index 1aa9cf41288..18fe0aeab10 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h @@ -43,6 +43,7 @@ enum radeon_generation { struct radeon_drm_winsys { struct radeon_winsys base; + struct pipe_reference reference; int fd; /* DRM file descriptor */ int num_cs; /* The number of command streams created. */ diff --git a/src/gallium/winsys/radeon/drm/radeon_winsys.h b/src/gallium/winsys/radeon/drm/radeon_winsys.h index eeae724d042..485e9259be3 100644 --- a/src/gallium/winsys/radeon/drm/radeon_winsys.h +++ b/src/gallium/winsys/radeon/drm/radeon_winsys.h @@ -232,14 +232,17 @@ enum radeon_feature_id { struct radeon_winsys { /** - * Reference counting + * The screen object this winsys was created for */ - struct pipe_reference reference; + struct pipe_screen *screen; /** - * The screen object this winsys was created for + * Decrement the winsys reference count. + * + * \param ws The winsys this function is called for. + * \return True if the winsys and screen should be destroyed. */ - struct pipe_screen *screen; + bool (*unref)(struct radeon_winsys *ws); /** * Destroy this winsys. @@ -568,15 +571,6 @@ struct radeon_winsys { enum radeon_value_id value); }; -/** - * Decrement the winsys reference count. - * - * \param ws The winsys this function is called for. - */ -static INLINE boolean radeon_winsys_unref(struct radeon_winsys *ws) -{ - return pipe_reference(&ws->reference, NULL); -} static INLINE void radeon_emit(struct radeon_winsys_cs *cs, uint32_t value) { -- 2.30.2