From a29d63ecf71546c4798c609e37810f0ec81793d8 Mon Sep 17 00:00:00 2001 From: Chuck Atkins Date: Mon, 22 Jan 2018 12:09:28 -0500 Subject: [PATCH] swr: refactor swr_create_screen to allow for proper cleanup on error This makes the following changes to address cleanup issues: - Error conditions now return NULL instead of calling exit() - swr_creen is now freed upon error, rather than leak. - Library handle from dlopen is now closed upon swr_screen destruction v2: Added additional context in commit msg and remove unnecessary "PUBLIC" v3: Fix typo in commit message. Signed-off-by: Chuck Atkins Reviewed-by: Emil Velikov Cc: Bruce Cherniak Cc: Tim Rowley cc: mesa-stable@lists.freedesktop.org --- src/gallium/drivers/swr/swr_loader.cpp | 100 +++++++++++++------------ src/gallium/drivers/swr/swr_public.h | 6 +- src/gallium/drivers/swr/swr_screen.cpp | 26 +++++-- src/gallium/drivers/swr/swr_screen.h | 3 + 4 files changed, 79 insertions(+), 56 deletions(-) diff --git a/src/gallium/drivers/swr/swr_loader.cpp b/src/gallium/drivers/swr/swr_loader.cpp index 7f28bdb5366..01b98046462 100644 --- a/src/gallium/drivers/swr/swr_loader.cpp +++ b/src/gallium/drivers/swr/swr_loader.cpp @@ -29,96 +29,98 @@ #include // Helper function to resolve the backend filename based on architecture -inline void get_swr_arch_filename(const char arch[], char filename[]) +static bool +swr_initialize_screen_interface(struct swr_screen *screen, const char arch[]) { #ifdef HAVE_SWR_BUILTIN - strcpy(filename , "builtin"); + screen->pLibrary = NULL; + screen->pfnSwrGetInterface = SwrGetInterface; + fprintf(stderr, "(using: builtin).\n"); #else + char filename[256] = { 0 }; sprintf(filename, "%sswr%s%s", UTIL_DL_PREFIX, arch, UTIL_DL_EXT); + + screen->pLibrary = util_dl_open(filename); + if (!screen->pLibrary) { + fprintf(stderr, "(skipping: %s).\n", util_dl_error()); + return false; + } + + util_dl_proc pApiProc = util_dl_get_proc_address(screen->pLibrary, + "SwrGetInterface"); + if (!pApiProc) { + fprintf(stderr, "(skipping: %s).\n", util_dl_error()); + util_dl_close(screen->pLibrary); + screen->pLibrary = NULL; + return false; + } + + screen->pfnSwrGetInterface = (PFNSwrGetInterface)pApiProc; + fprintf(stderr, "(using: %s).\n", filename); #endif + return true; } + struct pipe_screen * swr_create_screen(struct sw_winsys *winsys) { - char filename[256] = { 0 }; - bool found = false; - bool is_knl = false; - PFNSwrGetInterface pfnSwrGetInterface = nullptr; + struct pipe_screen *p_screen = swr_create_screen_internal(winsys); + if (!p_screen) { + return NULL; + } + + struct swr_screen *screen = swr_screen(p_screen); + screen->is_knl = false; util_cpu_detect(); - if (!found && util_cpu_caps.has_avx512f && util_cpu_caps.has_avx512er) { + if (util_cpu_caps.has_avx512f && util_cpu_caps.has_avx512er) { fprintf(stderr, "SWR detected KNL instruction support "); #ifndef HAVE_SWR_KNL - fprintf(stderr, "(skipping not built).\n"); + fprintf(stderr, "(skipping: not built).\n"); #else - get_swr_arch_filename("KNL", filename); - found = true; - is_knl = true; + if (swr_initialize_screen_interface(screen, "KNL")) { + screen->is_knl = true; + return p_screen; + } #endif } - if (!found && util_cpu_caps.has_avx512f && util_cpu_caps.has_avx512bw) { + if (util_cpu_caps.has_avx512f && util_cpu_caps.has_avx512bw) { fprintf(stderr, "SWR detected SKX instruction support "); #ifndef HAVE_SWR_SKX fprintf(stderr, "(skipping not built).\n"); #else - get_swr_arch_filename("SKX", filename); - found = true; + if (swr_initialize_screen_interface(screen, "SKX")) + return p_screen; #endif } - if (!found && util_cpu_caps.has_avx2) { + if (util_cpu_caps.has_avx2) { fprintf(stderr, "SWR detected AVX2 instruction support "); #ifndef HAVE_SWR_AVX2 fprintf(stderr, "(skipping not built).\n"); #else - get_swr_arch_filename("AVX2", filename); - found = true; + if (swr_initialize_screen_interface(screen, "AVX2")) + return p_screen; #endif } - if (!found && util_cpu_caps.has_avx) { + if (util_cpu_caps.has_avx) { fprintf(stderr, "SWR detected AVX instruction support "); #ifndef HAVE_SWR_AVX fprintf(stderr, "(skipping not built).\n"); #else - get_swr_arch_filename("AVX", filename); - found = true; + if (swr_initialize_screen_interface(screen, "AVX")) + return p_screen; #endif } - if (!found) { - fprintf(stderr, "SWR could not detect a supported CPU architecture.\n"); - exit(-1); - } - - fprintf(stderr, "(using %s).\n", filename); - -#ifdef HAVE_SWR_BUILTIN - pfnSwrGetInterface = SwrGetInterface; -#else - util_dl_library *pLibrary = util_dl_open(filename); - if (!pLibrary) { - fprintf(stderr, "SWR library load failure: %s\n", util_dl_error()); - exit(-1); - } - - util_dl_proc pApiProc = util_dl_get_proc_address(pLibrary, "SwrGetInterface"); - if (!pApiProc) { - fprintf(stderr, "SWR library search failure: %s\n", util_dl_error()); - exit(-1); - } - - pfnSwrGetInterface = (PFNSwrGetInterface)pApiProc; -#endif - - struct pipe_screen *screen = swr_create_screen_internal(winsys); - swr_screen(screen)->is_knl = is_knl; - swr_screen(screen)->pfnSwrGetInterface = pfnSwrGetInterface; + fprintf(stderr, "SWR could not initialize a supported CPU architecture.\n"); + swr_destroy_screen_internal(&screen); - return screen; + return NULL; } diff --git a/src/gallium/drivers/swr/swr_public.h b/src/gallium/drivers/swr/swr_public.h index 4b150705cd7..07ea6280cd6 100644 --- a/src/gallium/drivers/swr/swr_public.h +++ b/src/gallium/drivers/swr/swr_public.h @@ -25,8 +25,9 @@ #define SWR_PUBLIC_H struct pipe_screen; -struct sw_winsys; struct sw_displaytarget; +struct sw_winsys; +struct swr_screen; #ifdef __cplusplus extern "C" { @@ -38,6 +39,9 @@ struct pipe_screen *swr_create_screen(struct sw_winsys *winsys); // arch-specific dll entry point PUBLIC struct pipe_screen *swr_create_screen_internal(struct sw_winsys *winsys); +// cleanup for failed screen creation +void swr_destroy_screen_internal(struct swr_screen **screen); + #ifdef _WIN32 void swr_gdi_swap(struct pipe_screen *screen, struct pipe_resource *res, diff --git a/src/gallium/drivers/swr/swr_screen.cpp b/src/gallium/drivers/swr/swr_screen.cpp index b67ac25ac89..839179a3043 100644 --- a/src/gallium/drivers/swr/swr_screen.cpp +++ b/src/gallium/drivers/swr/swr_screen.cpp @@ -1052,6 +1052,24 @@ swr_flush_frontbuffer(struct pipe_screen *p_screen, } +void +swr_destroy_screen_internal(struct swr_screen **screen) +{ + struct pipe_screen *p_screen = &(*screen)->base; + + swr_fence_finish(p_screen, NULL, (*screen)->flush_fence, 0); + swr_fence_reference(p_screen, &(*screen)->flush_fence, NULL); + + JitDestroyContext((*screen)->hJitMgr); + + if ((*screen)->pLibrary) + util_dl_close((*screen)->pLibrary); + + FREE(*screen); + *screen = NULL; +} + + static void swr_destroy_screen(struct pipe_screen *p_screen) { @@ -1060,15 +1078,10 @@ swr_destroy_screen(struct pipe_screen *p_screen) fprintf(stderr, "SWR destroy screen!\n"); - swr_fence_finish(p_screen, NULL, screen->flush_fence, 0); - swr_fence_reference(p_screen, &screen->flush_fence, NULL); - - JitDestroyContext(screen->hJitMgr); - if (winsys->destroy) winsys->destroy(winsys); - FREE(screen); + swr_destroy_screen_internal(&screen); } @@ -1119,6 +1132,7 @@ struct pipe_screen * swr_create_screen_internal(struct sw_winsys *winsys) { struct swr_screen *screen = CALLOC_STRUCT(swr_screen); + memset(screen, 0, sizeof(struct swr_screen)); if (!screen) return NULL; diff --git a/src/gallium/drivers/swr/swr_screen.h b/src/gallium/drivers/swr/swr_screen.h index 89faab182c5..b80d8c70ec9 100644 --- a/src/gallium/drivers/swr/swr_screen.h +++ b/src/gallium/drivers/swr/swr_screen.h @@ -28,6 +28,7 @@ #include "pipe/p_screen.h" #include "pipe/p_defines.h" +#include "util/u_dl.h" #include "util/u_format.h" #include "api.h" @@ -50,6 +51,8 @@ struct swr_screen { HANDLE hJitMgr; + /* Dynamic backend implementations */ + util_dl_library *pLibrary; PFNSwrGetInterface pfnSwrGetInterface; /* Do we run on Xeon Phi? */ -- 2.30.2