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

[Chad Versace]

On Mon 03 Apr 2017, Jason Ekstrand wrote:
> On Mon, Apr 3, 2017 at 5:19 PM, Chad Versace <chadversary at chromium.org>
> wrote:
> 
> > On Wed 15 Mar 2017, Jason Ekstrand wrote:
> > > This cache allows us to easily ensure that we have a unique anv_bo for
> > > each gem handle.  We'll need this in order to support multiple-import of
> > > memory objects and semaphores.
> > >
> > > v2 (Jason Ekstrand):
> > >  - Reject BO imports if the size doesn't match the prime fd size as
> > >    reported by lseek().
> > >
> > > v3 (Jason Ekstrand):
> > >  - Fix reference counting around cache_release (Chris Willson)
> > >  - Move the mutex_unlock() later in cache_release
> > > ---
> > >  src/intel/vulkan/anv_allocator.c | 261 ++++++++++++++++++++++++++++++
> > +++++++++
> > >  src/intel/vulkan/anv_private.h   |  26 ++++
> > >  2 files changed, 287 insertions(+)
> >
> >
> >
> > > +VkResult anv_bo_cache_alloc(struct anv_device *device,
> > > +                            struct anv_bo_cache *cache,
> > > +                            uint64_t size, struct anv_bo **bo,
> > > +                            VkAllocationCallbacks *alloc);
> >
> > > +VkResult anv_bo_cache_import(struct anv_device *device,
> > > +                             struct anv_bo_cache *cache,
> > > +                             int fd, uint64_t size, struct anv_bo **bo,
> > > +                             VkAllocationCallbacks *alloc);
> >
> > > +void anv_bo_cache_release(struct anv_device *device,
> > > +                          struct anv_bo_cache *cache,
> > > +                          struct anv_bo *bo,
> > > +                          VkAllocationCallbacks *alloc);
> > > +
> >
> > The app may do this:
> >
> >   // fd1 != fd2
> >   // fd1 and fd2 wrap the same gem handle
> >
> >   mem1 = vkAllocateMemory(import fd1, pAllocator=alloc1); // anv_cached_bo
> > is allocated here
> >   mem2 = vkAllocateMemory(import fd2, pAllocator=alloc2);
> >
> >   ...
> >
> >   vkFreeMemory(mem1, alloc1);
> >   vkFreeMemory(mem2, alloc2); // anv_cached_bo is freed here
> >
> > So we can't use either pAllocator to allocate/free the anv_cached_bo,
> > unless anv_cached_bo also cached its original allocator.
> > If we don't cache the allocator (and please, let's not), then the
> > 'alloc' params to anv_bo_cache_alloc/import/release are dead params.
> > We must use an allocator that is not bound
> > to the anv_cached_bo, such as device->alloc.
> >
> >
> > Patch 8/18 correctly passes device->alloc into these functions. But,
> > since the 'alloc' params aren't really used, they should be dropped to
> > prevent accidental misuse.
> >
> 
> I'm not quite sure what you're asking for here.  Do you want me to just use
> device->alloc and drop the parameter?  I'm happy to do that.

Yeah, I meant that.