[Mesa-dev] [PATCH 6/7] winsys/radeon: fix a race condition between winsys_create and winsys_destroy
From: Marek Olšák marek.ol...@amd.com
This also hides the reference count from drivers.
---
src/gallium/drivers/r300/r300_screen.c| 2 +-
src/gallium/drivers/r600/r600_pipe.c | 2 +-
src/gallium/drivers/radeonsi/si_pipe.c| 2 +-
src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 30 +--
src/gallium/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 70c9cdf..8e601e3 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 8d69358..18fde74 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 09ec603..7dac287 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 ebf7697..b968bd6 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;
@@ -585,7 +598,7 @@ PUBLIC struct radeon_winsys *radeon_drm_winsys_create(int
fd)
ws = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
if (ws) {
pipe_mutex_unlock(fd_tab_mutex);
-pipe_reference(NULL, ws-base.reference);
+pipe_reference(NULL, ws-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 1aa9cf4..18fe0ae 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
[Mesa-dev] [PATCH 6/7] winsys/radeon: fix a race condition between winsys_create and winsys_destroy
From: Marek Olšák marek.ol...@amd.com
This also hides the reference count from drivers.
v2: update the reference count while the mutex is locked in winsys_create
---
src/gallium/drivers/r300/r300_screen.c| 2 +-
src/gallium/drivers/r600/r600_pipe.c | 2 +-
src/gallium/drivers/radeonsi/si_pipe.c| 2 +-
src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 30 +--
src/gallium/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 70c9cdf..8e601e3 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 8d69358..18fde74 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 09ec603..7dac287 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 ebf7697..07b3c05 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 1aa9cf4..18fe0ae 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;
