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

[Jason Ekstrand]

On Wed, Mar 15, 2017 at 5:21 AM, Chris Wilson <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.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170315/6149a8de/attachment-0001.html>