[Mesa-dev] [PATCH 5/7] EGL: Record the debug object label in _EGLThreadInfo

[Emil Velikov]

On 8 September 2016 at 18:47, Adam Jackson <ajax at redhat.com> wrote:
> From: Kyle Brenneman <kbrenneman at nvidia.com>
>
> Added a field to _EGLThreadInfo to hold the object label for the current
> EGL function call.
>
> Changed the _EGL_FUNC_START macro and _eglSetFuncName function to take
> an object type enum and an _EGLResource pointer, which it uses to fill
> in the object label.
>
> Removed the command name and object label parameters from
> _eglDebugReport, and made it look them up from the current
> _EGLThreadInfo.
>
Some command name and object label were only added to be removed. What
is the obstacle of adding them to _EGLThreadInfo in the first place ?

> Added a separate _eglDebugReportFull function to allow the caller to
> specify the command and label.
>
> Reviewed-by: Adam Jackson <ajax at redhat.com>
> ---
>  src/egl/main/eglapi.c     | 183 ++++++++++++++++++++++++++--------------------
>  src/egl/main/eglcurrent.c |  47 ++++++++----
>  src/egl/main/eglcurrent.h |  23 +++---
>  3 files changed, 149 insertions(+), 104 deletions(-)
>
This patch fixes a few lockups/undefined behaviour cases which must be
squashed in previous patch. Namely: we use _EGL_FUNC_START() w/o a
locked display.



> @@ -577,13 +595,13 @@ eglQueryString(EGLDisplay dpy, EGLint name)
>     _EGLDisplay *disp;
>     _EGLDriver *drv;
>
> -   _EGL_FUNC_START(NULL, NULL);
> -
>     if (dpy == EGL_NO_DISPLAY && name == EGL_EXTENSIONS) {
>        RETURN_EGL_SUCCESS(NULL, _eglGlobal.ClientExtensionString);
>     }
>
>     disp = _eglLockDisplay(dpy);
> +
> +   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, NULL);
The _EGL_FUNC_START movement should be in the previous patch.


> @@ -1136,11 +1154,11 @@ eglWaitClient(void)
>     if (!ctx)
>        RETURN_EGL_SUCCESS(NULL, EGL_TRUE);
>
> -   _EGL_FUNC_START(NULL, EGL_FALSE);
> -
>     disp = ctx->Resource.Display;
>     mtx_lock(&disp->Mutex);
>
> +   _EGL_FUNC_START(disp, EGL_OBJECT_CONTEXT_KHR, ctx, EGL_FALSE);
> +
Ditto.

> @@ -1434,11 +1453,10 @@ eglDestroyImage(EGLDisplay dpy, EGLImage image)
>
>
>  static EGLSync
> -_eglCreateSync(EGLDisplay dpy, EGLenum type, const EGLint *attrib_list,
> +_eglCreateSync(_EGLDisplay *disp, EGLenum type, const EGLint *attrib_list,
>                 const EGLAttrib *attrib_list64, EGLBoolean is64,
>                 EGLenum invalid_type_error)
>  {
> -   _EGLDisplay *disp = _eglLockDisplay(dpy);
>     _EGLContext *ctx = _eglGetCurrentContext();
>     _EGLDriver *drv;
>     _EGLSync *sync;
> @@ -1450,7 +1468,7 @@ _eglCreateSync(EGLDisplay dpy, EGLenum type, const EGLint *attrib_list,
>        RETURN_EGL_EVAL(disp, EGL_NO_SYNC_KHR);
>
>     /* return an error if the client API doesn't support GL_OES_EGL_sync */
> -   if (!ctx || ctx->Resource.Display != dpy ||
> +   if (!ctx || ctx->Resource.Display != disp ||
>         ctx->ClientAPI != EGL_OPENGL_ES_API)
>        RETURN_EGL_ERROR(disp, EGL_BAD_MATCH, EGL_NO_SYNC_KHR);
>
> @@ -1481,8 +1499,9 @@ _eglCreateSync(EGLDisplay dpy, EGLenum type, const EGLint *attrib_list,
>  static EGLSync EGLAPIENTRY
>  eglCreateSyncKHR(EGLDisplay dpy, EGLenum type, const EGLint *attrib_list)
>  {
> -   _EGL_FUNC_START(NULL, EGL_NO_SYNC_KHR);
> -   return _eglCreateSync(dpy, type, attrib_list, NULL, EGL_FALSE,
> +   _EGLDisplay *disp = _eglLockDisplay(dpy);
> +   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, EGL_FALSE);
> +   return _eglCreateSync(disp, type, attrib_list, NULL, EGL_FALSE,
>                           EGL_BAD_ATTRIBUTE);
>  }
>
> @@ -1490,8 +1509,9 @@ eglCreateSyncKHR(EGLDisplay dpy, EGLenum type, const EGLint *attrib_list)
>  static EGLSync EGLAPIENTRY
>  eglCreateSync64KHR(EGLDisplay dpy, EGLenum type, const EGLAttrib *attrib_list)
>  {
> -   _EGL_FUNC_START(NULL, EGL_NO_SYNC_KHR);
> -   return _eglCreateSync(dpy, type, NULL, attrib_list, EGL_TRUE,
> +   _EGLDisplay *disp = _eglLockDisplay(dpy);
> +   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, EGL_FALSE);
> +   return _eglCreateSync(disp, type, NULL, attrib_list, EGL_TRUE,
>                           EGL_BAD_ATTRIBUTE);
>  }
>
> @@ -1499,8 +1519,9 @@ eglCreateSync64KHR(EGLDisplay dpy, EGLenum type, const EGLAttrib *attrib_list)
>  EGLSync EGLAPIENTRY
>  eglCreateSync(EGLDisplay dpy, EGLenum type, const EGLAttrib *attrib_list)
>  {
> -   _EGL_FUNC_START(NULL, EGL_NO_SYNC_KHR);
> -   return _eglCreateSync(dpy, type, NULL, attrib_list, EGL_TRUE,
> +   _EGLDisplay *disp = _eglLockDisplay(dpy);
> +   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, EGL_FALSE);
> +   return _eglCreateSync(disp, type, NULL, attrib_list, EGL_TRUE,
>                           EGL_BAD_PARAMETER);
>  }
>
Ditto.

> @@ -1922,14 +1943,18 @@ eglLabelObjectKHR(
>        EGLObjectKHR object,
>        EGLLabelKHR label)
>  {
> -   _EGL_FUNC_START(NULL, EGL_BAD_ALLOC);
> +   _EGL_FUNC_START(NULL, EGL_NONE, NULL, EGL_BAD_ALLOC);
>
>     if (objectType == EGL_OBJECT_THREAD_KHR) {
>        _EGLThreadInfo *t = _eglGetCurrentThread();
>        if (!_eglIsCurrentThreadDummy()) {
>           t->Label = label;
> +         return EGL_SUCCESS;
> +      } else {
> +         _eglDebugReportFull(EGL_BAD_ALLOC, __func__, __func__,
> +               EGL_DEBUG_MSG_CRITICAL_KHR, NULL, NULL);
> +         return EGL_BAD_ALLOC;
And again ?

Thanks
Emil