Module: Mesa Branch: main Commit: 5d99e8cc0322aa9132826cbfe00e9525790a8b5d URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=5d99e8cc0322aa9132826cbfe00e9525790a8b5d
Author: Rob Clark <[email protected]> Date: Mon Aug 29 10:28:11 2022 -0700 egl: Introduce rwlock to protect eglTerminate() eglTerminate() must be serialized against all other EGL calls. But in most cases, other EGL calls do not need to be serialized against each other. Which fits rather well with a rwlock. One would be tempted to simply replace the existing BDL with a rwlock, but several portability and debuggability limitations of the rwlock implementation prevent that, as described in the TerminateLock comment block. Signed-off-by: Rob Clark <[email protected]> Acked-by: Eric Engestrom <[email protected]> Reviewed-by: Adam Jackson <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18050> --- src/egl/drivers/dri2/egl_dri2.c | 4 ++-- src/egl/drivers/wgl/egl_wgl.c | 4 ++-- src/egl/main/eglapi.c | 22 ++++++++++++++-------- src/egl/main/egldisplay.c | 1 + src/egl/main/egldisplay.h | 40 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c index ce25cf8a9d3..7614ff5a798 100644 --- a/src/egl/drivers/dri2/egl_dri2.c +++ b/src/egl/drivers/dri2/egl_dri2.c @@ -656,9 +656,9 @@ dri2_validate_egl_image(void *image, void *data) _EGLDisplay *disp = data; _EGLImage *img; - simple_mtx_lock(&disp->Mutex); + egl_lock(disp); img = _eglLookupImage(image, disp); - simple_mtx_unlock(&disp->Mutex); + egl_unlock(disp); if (img == NULL) { _eglError(EGL_BAD_PARAMETER, "dri2_validate_egl_image"); diff --git a/src/egl/drivers/wgl/egl_wgl.c b/src/egl/drivers/wgl/egl_wgl.c index 0d8b627e94b..c2d9efa85c4 100644 --- a/src/egl/drivers/wgl/egl_wgl.c +++ b/src/egl/drivers/wgl/egl_wgl.c @@ -215,9 +215,9 @@ wgl_validate_egl_image(struct st_manager *smapi, void *image) _EGLDisplay *disp = wgl_dpy->parent; _EGLImage *img; - simple_mtx_lock(&disp->Mutex); + egl_lock(disp); img = _eglLookupImage(image, disp); - simple_mtx_unlock(&disp->Mutex); + egl_unlock(disp); if (img == NULL) { _eglError(EGL_BAD_PARAMETER, "wgl_validate_egl_image"); diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c index 13f055355e8..595841b5f27 100644 --- a/src/egl/main/eglapi.c +++ b/src/egl/main/eglapi.c @@ -250,7 +250,7 @@ _eglLockDisplay(EGLDisplay dpy) { _EGLDisplay *disp = _eglLookupDisplay(dpy); if (disp) - simple_mtx_lock(&disp->Mutex); + egl_lock(disp); return disp; } @@ -261,7 +261,7 @@ _eglLockDisplay(EGLDisplay dpy) static inline void _eglUnlockDisplay(_EGLDisplay *disp) { - simple_mtx_unlock(&disp->Mutex); + egl_unlock(disp); } static void @@ -689,13 +689,16 @@ eglInitialize(EGLDisplay dpy, EGLint *major, EGLint *minor) EGLBoolean EGLAPIENTRY eglTerminate(EGLDisplay dpy) { - _EGLDisplay *disp = _eglLockDisplay(dpy); + _EGLDisplay *disp = _eglLookupDisplay(dpy); _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL); if (!disp) RETURN_EGL_ERROR(NULL, EGL_BAD_DISPLAY, EGL_FALSE); + u_rwlock_wrlock(&disp->TerminateLock); + simple_mtx_lock(&disp->Mutex); + if (disp->Initialized) { disp->Driver->Terminate(disp); /* do not reset disp->Driver */ @@ -707,7 +710,10 @@ eglTerminate(EGLDisplay dpy) disp->BlobCacheGet = NULL; } - RETURN_EGL_SUCCESS(disp, EGL_TRUE); + simple_mtx_unlock(&disp->Mutex); + u_rwlock_wrunlock(&disp->TerminateLock); + + RETURN_EGL_SUCCESS(NULL, EGL_TRUE); } @@ -1523,7 +1529,7 @@ _eglWaitClientCommon(void) RETURN_EGL_SUCCESS(NULL, EGL_TRUE); disp = ctx->Resource.Display; - simple_mtx_lock(&disp->Mutex); + egl_lock(disp); /* let bad current context imply bad current surface */ if (_eglGetContextHandle(ctx) == EGL_NO_CONTEXT || @@ -1566,7 +1572,7 @@ eglWaitNative(EGLint engine) _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL); disp = ctx->Resource.Display; - simple_mtx_lock(&disp->Mutex); + egl_lock(disp); /* let bad current context imply bad current surface */ if (_eglGetContextHandle(ctx) == EGL_NO_CONTEXT || @@ -1725,9 +1731,9 @@ eglReleaseThread(void) if (ctx) { _EGLDisplay *disp = ctx->Resource.Display; - simple_mtx_lock(&disp->Mutex); + egl_lock(disp); (void) disp->Driver->MakeCurrent(disp, NULL, NULL, NULL); - simple_mtx_unlock(&disp->Mutex); + egl_unlock(disp); } _eglDestroyCurrentThread(); diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c index ddbcbf0061d..9d49ec55dad 100644 --- a/src/egl/main/egldisplay.c +++ b/src/egl/main/egldisplay.c @@ -279,6 +279,7 @@ _eglFindDisplay(_EGLPlatformType plat, void *plat_dpy, goto out; simple_mtx_init(&disp->Mutex, mtx_plain); + u_rwlock_init(&disp->TerminateLock); disp->Platform = plat; disp->PlatformDisplay = plat_dpy; num_attribs = _eglNumAttribs(attrib_list); diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h index cf7cfb1a82b..54838764040 100644 --- a/src/egl/main/egldisplay.h +++ b/src/egl/main/egldisplay.h @@ -32,6 +32,7 @@ #define EGLDISPLAY_INCLUDED #include "util/simple_mtx.h" +#include "util/rwlock.h" #include "egltypedefs.h" #include "egldefines.h" @@ -159,8 +160,34 @@ struct _egl_display /* used to link displays */ _EGLDisplay *Next; + /** + * The big-display-lock (BDL) which protects our internal state. EGL + * drivers should use their own locking, as needed, to protect their + * own state, rather than relying on this. + */ simple_mtx_t Mutex; + /** + * The spec appears to allow eglTerminate() to race with more or less + * any other egl call. To allow for this, while relaxing the BDL to + * allow other egl calls to happen in parallel, a rwlock is used. All + * points where the BDL lock is acquired also acquire TerminateLock + * for reading, while eglTerminate() itself acquires the TerminateLock + * for writing. + * + * Note, we could conceivably just replace the BDL with a single + * rwlock. But there are a couple shortcomings of u_rwlock: + * + * 1) The WIN32 implementation does not allow promoting a read- + * lock to write-lock, nor recursive locking, whereas the + * pthread based implementation does. Because of this, it + * would be difficult to keep the eglapi layer portable if + * we depended on any less-than-trivial rwlock usage. + * + * 2) We'd lose simple_mtx_assert_locked(). + */ + struct u_rwlock TerminateLock; + _EGLPlatformType Platform; /**< The type of the platform display */ void *PlatformDisplay; /**< A pointer to the platform display */ @@ -198,6 +225,19 @@ struct _egl_display EGLGetBlobFuncANDROID BlobCacheGet; }; +static inline void +egl_lock(_EGLDisplay *disp) +{ + u_rwlock_rdlock(&disp->TerminateLock); + simple_mtx_lock(&disp->Mutex); +} + +static inline void +egl_unlock(_EGLDisplay *disp) +{ + simple_mtx_unlock(&disp->Mutex); + u_rwlock_rdunlock(&disp->TerminateLock); +} extern _EGLPlatformType _eglGetNativePlatform(void *nativeDisplay);
