[Mesa-dev] [PATCH 7/7] winsys/radeon: fix a race condition in initialization of radeon_winsys::screen
Marek Olšák
maraeo at gmail.com
Tue Apr 8 17:15:03 PDT 2014
From: Marek Olšák <marek.olsak at amd.com>
Create the screen in the winsys while the mutex is locked.
This also results in a nice code cleanup!
---
src/gallium/targets/egl-static/egl_pipe.c | 42 ++++-------------------
src/gallium/targets/pipe-loader/pipe_r300.c | 14 ++------
src/gallium/targets/pipe-loader/pipe_r600.c | 14 ++------
src/gallium/targets/pipe-loader/pipe_radeonsi.c | 14 ++------
src/gallium/targets/r300/common/drm_target.c | 15 ++------
src/gallium/targets/r600/common/drm_target.c | 15 ++------
src/gallium/targets/radeonsi/common/drm_target.c | 15 ++------
src/gallium/winsys/radeon/drm/radeon_drm_public.h | 6 +++-
src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 18 ++++++++--
9 files changed, 39 insertions(+), 114 deletions(-)
diff --git a/src/gallium/targets/egl-static/egl_pipe.c b/src/gallium/targets/egl-static/egl_pipe.c
index eb1cff9..ce734fb 100644
--- a/src/gallium/targets/egl-static/egl_pipe.c
+++ b/src/gallium/targets/egl-static/egl_pipe.c
@@ -119,19 +119,9 @@ pipe_r300_create_screen(int fd)
{
#if _EGL_PIPE_R300
struct radeon_winsys *sws;
- struct pipe_screen *screen;
-
- sws = radeon_drm_winsys_create(fd);
- if (!sws)
- return NULL;
-
- screen = r300_screen_create(sws);
- if (!screen)
- return NULL;
- screen = debug_screen_wrap(screen);
-
- return screen;
+ sws = radeon_drm_winsys_create(fd, r300_screen_create);
+ return sws ? debug_screen_wrap(sws->screen) : NULL;
#else
return NULL;
#endif
@@ -142,19 +132,9 @@ pipe_r600_create_screen(int fd)
{
#if _EGL_PIPE_R600
struct radeon_winsys *rw;
- struct pipe_screen *screen;
-
- rw = radeon_drm_winsys_create(fd);
- if (!rw)
- return NULL;
-
- screen = r600_screen_create(rw);
- if (!screen)
- return NULL;
-
- screen = debug_screen_wrap(screen);
- return screen;
+ rw = radeon_drm_winsys_create(fd, r600_screen_create);
+ return rw ? debug_screen_wrap(rw->screen) : NULL;
#else
return NULL;
#endif
@@ -165,19 +145,9 @@ pipe_radeonsi_create_screen(int fd)
{
#if _EGL_PIPE_RADEONSI
struct radeon_winsys *rw;
- struct pipe_screen *screen;
-
- rw = radeon_drm_winsys_create(fd);
- if (!rw)
- return NULL;
-
- screen = radeonsi_screen_create(rw);
- if (!screen)
- return NULL;
- screen = debug_screen_wrap(screen);
-
- return screen;
+ rw = radeon_drm_winsys_create(fd, radeonsi_screen_create);
+ return rw ? debug_screen_wrap(rw->screen) : NULL;
#else
return NULL;
#endif
diff --git a/src/gallium/targets/pipe-loader/pipe_r300.c b/src/gallium/targets/pipe-loader/pipe_r300.c
index 0556859..388b091 100644
--- a/src/gallium/targets/pipe-loader/pipe_r300.c
+++ b/src/gallium/targets/pipe-loader/pipe_r300.c
@@ -8,19 +8,9 @@ static struct pipe_screen *
create_screen(int fd)
{
struct radeon_winsys *sws;
- struct pipe_screen *screen;
- sws = radeon_drm_winsys_create(fd);
- if (!sws)
- return NULL;
-
- screen = r300_screen_create(sws);
- if (!screen)
- return NULL;
-
- screen = debug_screen_wrap(screen);
-
- return screen;
+ sws = radeon_drm_winsys_create(fd, r300_screen_create);
+ return sws ? debug_screen_wrap(sws->screen) : NULL;
}
PUBLIC
diff --git a/src/gallium/targets/pipe-loader/pipe_r600.c b/src/gallium/targets/pipe-loader/pipe_r600.c
index 5d89aca..0c59087 100644
--- a/src/gallium/targets/pipe-loader/pipe_r600.c
+++ b/src/gallium/targets/pipe-loader/pipe_r600.c
@@ -7,19 +7,9 @@ static struct pipe_screen *
create_screen(int fd)
{
struct radeon_winsys *rw;
- struct pipe_screen *screen;
- rw = radeon_drm_winsys_create(fd);
- if (!rw)
- return NULL;
-
- screen = r600_screen_create(rw);
- if (!screen)
- return NULL;
-
- screen = debug_screen_wrap(screen);
-
- return screen;
+ rw = radeon_drm_winsys_create(fd, r600_screen_create);
+ return rw ? debug_screen_wrap(rw->screen) : NULL;
}
PUBLIC
diff --git a/src/gallium/targets/pipe-loader/pipe_radeonsi.c b/src/gallium/targets/pipe-loader/pipe_radeonsi.c
index 48b2b5d..406ba1e 100644
--- a/src/gallium/targets/pipe-loader/pipe_radeonsi.c
+++ b/src/gallium/targets/pipe-loader/pipe_radeonsi.c
@@ -7,19 +7,9 @@ static struct pipe_screen *
create_screen(int fd)
{
struct radeon_winsys *rw;
- struct pipe_screen *screen;
- rw = radeon_drm_winsys_create(fd);
- if (!rw)
- return NULL;
-
- screen = radeonsi_screen_create(rw);
- if (!screen)
- return NULL;
-
- screen = debug_screen_wrap(screen);
-
- return screen;
+ rw = radeon_drm_winsys_create(fd, radeonsi_screen_create);
+ return rw ? debug_screen_wrap(rw->screen) : NULL;
}
PUBLIC
diff --git a/src/gallium/targets/r300/common/drm_target.c b/src/gallium/targets/r300/common/drm_target.c
index 9b48446..dff83da 100644
--- a/src/gallium/targets/r300/common/drm_target.c
+++ b/src/gallium/targets/r300/common/drm_target.c
@@ -36,19 +36,8 @@ create_screen(int fd)
{
struct radeon_winsys *sws;
- sws = radeon_drm_winsys_create(fd);
- if (!sws)
- return NULL;
-
- if (!sws->screen) {
- sws->screen = r300_screen_create(sws);
- if (!sws->screen)
- return NULL;
-
- sws->screen = debug_screen_wrap(sws->screen);
- }
-
- return sws->screen;
+ sws = radeon_drm_winsys_create(fd, r300_screen_create);
+ return sws ? debug_screen_wrap(sws->screen) : NULL;
}
/* Technically this is only true for kernels >= 3.12, which
diff --git a/src/gallium/targets/r600/common/drm_target.c b/src/gallium/targets/r600/common/drm_target.c
index ab1eec6..09250c7 100644
--- a/src/gallium/targets/r600/common/drm_target.c
+++ b/src/gallium/targets/r600/common/drm_target.c
@@ -35,19 +35,8 @@ static struct pipe_screen *create_screen(int fd)
{
struct radeon_winsys *radeon;
- radeon = radeon_drm_winsys_create(fd);
- if (!radeon)
- return NULL;
-
- if (!radeon->screen) {
- radeon->screen = r600_screen_create(radeon);
- if (!radeon->screen)
- return NULL;
-
- radeon->screen = debug_screen_wrap(radeon->screen);
- }
-
- return radeon->screen;
+ radeon = radeon_drm_winsys_create(fd, r600_screen_create);
+ return radeon ? debug_screen_wrap(radeon->screen) : NULL;
}
static const struct drm_conf_ret throttle_ret = {
diff --git a/src/gallium/targets/radeonsi/common/drm_target.c b/src/gallium/targets/radeonsi/common/drm_target.c
index c695dde..74980af 100644
--- a/src/gallium/targets/radeonsi/common/drm_target.c
+++ b/src/gallium/targets/radeonsi/common/drm_target.c
@@ -35,19 +35,8 @@ static struct pipe_screen *create_screen(int fd)
{
struct radeon_winsys *radeon;
- radeon = radeon_drm_winsys_create(fd);
- if (!radeon)
- return NULL;
-
- if (!radeon->screen) {
- radeon->screen = radeonsi_screen_create(radeon);
- if (!radeon->screen)
- return NULL;
-
- radeon->screen = debug_screen_wrap(radeon->screen);
- }
-
- return radeon->screen;
+ radeon = radeon_drm_winsys_create(fd, radeonsi_screen_create);
+ return radeon ? debug_screen_wrap(radeon->screen) : NULL;
}
static const struct drm_conf_ret throttle_ret = {
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_public.h b/src/gallium/winsys/radeon/drm/radeon_drm_public.h
index 4fc62f1..dfcaaa4 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_public.h
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_public.h
@@ -4,7 +4,11 @@
#include "pipe/p_defines.h"
struct radeon_winsys;
+struct pipe_screen;
-struct radeon_winsys *radeon_drm_winsys_create(int fd);
+typedef struct pipe_screen *(*radeon_screen_create_t)(struct radeon_winsys *);
+
+struct radeon_winsys *
+radeon_drm_winsys_create(int fd, radeon_screen_create_t screen_create);
#endif
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
index b968bd6..866af22 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
@@ -586,7 +586,8 @@ static bool radeon_winsys_unref(struct radeon_winsys *ws)
return destroy;
}
-PUBLIC struct radeon_winsys *radeon_drm_winsys_create(int fd)
+PUBLIC struct radeon_winsys *
+radeon_drm_winsys_create(int fd, radeon_screen_create_t screen_create)
{
struct radeon_drm_winsys *ws;
@@ -609,7 +610,6 @@ PUBLIC struct radeon_winsys *radeon_drm_winsys_create(int fd)
}
ws->fd = fd;
- util_hash_table_set(fd_tab, intptr_to_pointer(fd), ws);
if (!do_winsys_init(ws))
goto fail;
@@ -652,6 +652,20 @@ PUBLIC struct radeon_winsys *radeon_drm_winsys_create(int fd)
if (ws->num_cpus > 1 && debug_get_option_thread())
ws->thread = pipe_thread_create(radeon_drm_cs_emit_ioctl, ws);
+ /* Create the screen at the end. The winsys must be initialized
+ * completely.
+ *
+ * Alternatively, we could create the screen based on "ws->gen"
+ * and link all drivers into one binary blob. */
+ ws->base.screen = screen_create(&ws->base);
+ if (!ws->base.screen) {
+ radeon_winsys_destroy(&ws->base);
+ pipe_mutex_unlock(fd_tab_mutex);
+ return NULL;
+ }
+
+ util_hash_table_set(fd_tab, intptr_to_pointer(fd), ws);
+
/* We must unlock the mutex once the winsys is fully initialized, so that
* other threads attempting to create the winsys from the same fd will
* get a fully initialized winsys and not just half-way initialized. */
--
1.8.3.2
More information about the mesa-dev
mailing list