winsys/radeon: fix a race condition between winsys_create and winsys_destroy

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 <[email protected]>
Reviewed-by: Christian König <[email protected]>

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)
 {