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

[Timothy Arceri]

On 16/08/17 00:23, Tapani Pälli wrote:
> On 08/15/2017 05:14 PM, Timothy Arceri wrote:
>> 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.
>>
> 
> ok will take a look,
> 
> Thanks;

I think I tried to address this and failed in my last attempt with the 
patch.

mesa: only lock framebuffer in compat profile

See Fredrik's comments.

> 
>> 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
>>>
>