[Mesa-dev] [PATCH v2 07/18] anv/allocator: Add a BO cache

[Chris Wilson]

On Wed, Mar 15, 2017 at 08:28:24AM -0700, Jason Ekstrand wrote:
>    On Wed, Mar 15, 2017 at 5:21 AM, Chris Wilson
>    <[1]chris at chris-wilson.co.uk> wrote:
> 
>      On Wed, Mar 15, 2017 at 09:50:57AM +0000, Chris Wilson wrote:
>      > On Tue, Mar 14, 2017 at 10:43:07PM -0700, Jason Ekstrand wrote:
>      > > +void
>      > > +anv_bo_cache_release(struct anv_device *device,
>      > > +                     struct anv_bo_cache *cache,
>      > > +                     struct anv_bo *bo_in,
>      > > +                     VkAllocationCallbacks *alloc)
>      > > +{
>      > > +   assert(anv_bo_cache_lookup(cache, bo_in->gem_handle) == bo_in);
>      > > +   struct anv_cached_bo *bo = (struct anv_cached_bo *)bo_in;
>      > > +
>      > > +   uint32_t count = __sync_fetch_and_add(&bo->refcount, -1);
>      > > +   assert(count > 0);
>      > > +   if (count > 1)
>      > > +      return;
>      > > +   assert(count == 1);
>      > > +   assert(bo->refcount == 0);
>      >
>      > There's a window here for a second thread to acquire a reference to
>      the
>      > bo, just before you then go and free it.
>      >
>      > The trick is the final unref has to be inside the mutex, earlier can
>      be
>      > outside. See refcount_dec_and_lock() in the kernel.
> 
>      The alternative is to check against the dead bo when retrieving from the
>      cache (under the mutex) -- refcount_add_not_zero. That is probably the
>      cheaper option.
> 
>    How about if I just check the refcount again after I've entered the mutex
>    and release the mutex and bail if it isn't zero?  The only time the
>    reference count is ever imported is in import and it's done inside the
>    mutex.  That's the cheapest option.

The second thread (that acquired the ref from the cache) may also drop
it before you acquire the mutex. You now have two threads that see
bo->refcount as 0 inside anv_bo_cache_release().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre