[Mesa-dev] [PATCH 06/13] mesa: Replace sampler object locks with atomic inc/dec.

[Ian Romanick]

I know we've talked about this about 100 times, but something in the
back of my mind tells me that we have a pre-existing race.  What happens
if the p_atomic_dec_zero happens on thread A while thread B is between
the _mesa_lookup_renderbuffer call and the _mesa_reference_renderbuffer
call on the same object?  Won't thread A free the memory out from under
thread B?

On 08/06/2015 05:10 PM, Matt Turner wrote:
> ---
>  src/mesa/main/mtypes.h     |  1 -
>  src/mesa/main/samplerobj.c | 16 +++-------------
>  2 files changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 2adfae2..fcc527f 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1153,7 +1153,6 @@ typedef enum
>   */
>  struct gl_sampler_object
>  {
> -   mtx_t Mutex;
>     GLuint Name;
>     GLint RefCount;
>     GLchar *Label;               /**< GL_KHR_debug */
> diff --git a/src/mesa/main/samplerobj.c b/src/mesa/main/samplerobj.c
> index c7b9666..dba2087 100644
> --- a/src/mesa/main/samplerobj.c
> +++ b/src/mesa/main/samplerobj.c
> @@ -38,6 +38,7 @@
>  #include "main/macros.h"
>  #include "main/mtypes.h"
>  #include "main/samplerobj.h"
> +#include "util/u_atomic.h"
>  
>  
>  struct gl_sampler_object *
> @@ -85,16 +86,9 @@ _mesa_reference_sampler_object_(struct gl_context *ctx,
>  
>     if (*ptr) {
>        /* Unreference the old sampler */
> -      GLboolean deleteFlag = GL_FALSE;
>        struct gl_sampler_object *oldSamp = *ptr;
>  
> -      mtx_lock(&oldSamp->Mutex);
> -      assert(oldSamp->RefCount > 0);
> -      oldSamp->RefCount--;
> -      deleteFlag = (oldSamp->RefCount == 0);
> -      mtx_unlock(&oldSamp->Mutex);
> -
> -      if (deleteFlag) {
> +      if (p_atomic_dec_zero(&oldSamp->RefCount)) {
>  	 assert(ctx->Driver.DeleteSamplerObject);
>           ctx->Driver.DeleteSamplerObject(ctx, oldSamp);
>        }
> @@ -105,7 +99,6 @@ _mesa_reference_sampler_object_(struct gl_context *ctx,
>  
>     if (samp) {
>        /* reference new sampler */
> -      mtx_lock(&samp->Mutex);
>        if (samp->RefCount == 0) {
>           /* this sampler's being deleted (look just above) */
>           /* Not sure this can every really happen.  Warn if it does. */
> @@ -113,10 +106,9 @@ _mesa_reference_sampler_object_(struct gl_context *ctx,
>           *ptr = NULL;
>        }
>        else {
> -         samp->RefCount++;
> +         p_atomic_inc(&samp->RefCount);
>           *ptr = samp;
>        }
> -      mtx_unlock(&samp->Mutex);
>     }
>  }
>  
> @@ -127,7 +119,6 @@ _mesa_reference_sampler_object_(struct gl_context *ctx,
>  static void
>  _mesa_init_sampler_object(struct gl_sampler_object *sampObj, GLuint name)
>  {
> -   mtx_init(&sampObj->Mutex, mtx_plain);
>     sampObj->Name = name;
>     sampObj->RefCount = 1;
>     sampObj->WrapS = GL_REPEAT;
> @@ -170,7 +161,6 @@ static void
>  _mesa_delete_sampler_object(struct gl_context *ctx,
>                              struct gl_sampler_object *sampObj)
>  {
> -   mtx_destroy(&sampObj->Mutex);
>     free(sampObj->Label);
>     free(sampObj);
>  }
>