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]ch...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
