[Mesa-dev] [PATCH] mesa: framebuffer refcounting with atomic ops

[Timothy Arceri]

We seem to revisit this every few months. Please search your email for 
all the details but the short answer is this makes some race conditions 
worse. You will need to address and follow the previous suggestions to 
be able to improve locking here.

On 15/08/17 23:04, Marek Olšák wrote:
> Reviewed-by: Marek Olšák <marek.olsak at amd.com>
> 
> Marek
> 
> On Tue, Aug 15, 2017 at 2:03 PM, Tapani Pälli <tapani.palli at intel.com> wrote:
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/mesa/main/framebuffer.c | 19 +++++--------------
>>   1 file changed, 5 insertions(+), 14 deletions(-)
>>
>> Use atomic ops in same manner as for shader objects, IMO makes
>> code easier to read.
>>
>> diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
>> index 039762a074..69e28c8af7 100644
>> --- a/src/mesa/main/framebuffer.c
>> +++ b/src/mesa/main/framebuffer.c
>> @@ -45,7 +45,7 @@
>>   #include "texobj.h"
>>   #include "glformats.h"
>>   #include "state.h"
>> -
>> +#include "util/u_atomic.h"
>>
>>
>>   /**
>> @@ -243,25 +243,16 @@ _mesa_reference_framebuffer_(struct gl_framebuffer **ptr,
>>   {
>>      if (*ptr) {
>>         /* unreference old renderbuffer */
>> -      GLboolean deleteFlag = GL_FALSE;
>>         struct gl_framebuffer *oldFb = *ptr;
>> -
>> -      mtx_lock(&oldFb->Mutex);
>> -      assert(oldFb->RefCount > 0);
>> -      oldFb->RefCount--;
>> -      deleteFlag = (oldFb->RefCount == 0);
>> -      mtx_unlock(&oldFb->Mutex);
>> -
>> -      if (deleteFlag)
>> -         oldFb->Delete(oldFb);
>> +      assert(p_atomic_read(&oldFb->RefCount) > 0);
>> +      if (p_atomic_dec_zero(&oldFb->RefCount))
>> +          oldFb->Delete(oldFb);
>>
>>         *ptr = NULL;
>>      }
>>
>>      if (fb) {
>> -      mtx_lock(&fb->Mutex);
>> -      fb->RefCount++;
>> -      mtx_unlock(&fb->Mutex);
>> +      p_atomic_inc(&fb->RefCount);
>>         *ptr = fb;
>>      }
>>   }
>> --
>> 2.13.5
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>