<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Apr 3, 2017 at 5:19 PM, Chad Versace <span dir="ltr"><<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed 15 Mar 2017, Jason Ekstrand wrote:<br>
</span><span class="">> This cache allows us to easily ensure that we have a unique anv_bo for<br>
> each gem handle.  We'll need this in order to support multiple-import of<br>
> memory objects and semaphores.<br>
><br>
> v2 (Jason Ekstrand):<br>
>  - Reject BO imports if the size doesn't match the prime fd size as<br>
>    reported by lseek().<br>
><br>
> v3 (Jason Ekstrand):<br>
>  - Fix reference counting around cache_release (Chris Willson)<br>
>  - Move the mutex_unlock() later in cache_release<br>
> ---<br>
>  src/intel/vulkan/anv_<wbr>allocator.c | 261 ++++++++++++++++++++++++++++++<wbr>+++++++++<br>
>  src/intel/vulkan/anv_private.h   |  26 ++++<br>
>  2 files changed, 287 insertions(+)<br>
<br>
<br>
<br>
</span><span class="">> +VkResult anv_bo_cache_alloc(struct anv_device *device,<br>
> +                            struct anv_bo_cache *cache,<br>
> +                            uint64_t size, struct anv_bo **bo,<br>
> +                            VkAllocationCallbacks *alloc);<br>
<br>
> +VkResult anv_bo_cache_import(struct anv_device *device,<br>
> +                             struct anv_bo_cache *cache,<br>
> +                             int fd, uint64_t size, struct anv_bo **bo,<br>
> +                             VkAllocationCallbacks *alloc);<br>
<br>
</span><span class="">> +void anv_bo_cache_release(struct anv_device *device,<br>
> +                          struct anv_bo_cache *cache,<br>
> +                          struct anv_bo *bo,<br>
> +                          VkAllocationCallbacks *alloc);<br>
> +<br>
<br>
</span>The app may do this:<br>
<br>
  // fd1 != fd2<br>
  // fd1 and fd2 wrap the same gem handle<br>
<br>
  mem1 = vkAllocateMemory(import fd1, pAllocator=alloc1); // anv_cached_bo is allocated here<br>
  mem2 = vkAllocateMemory(import fd2, pAllocator=alloc2);<br>
<br>
  ...<br>
<br>
  vkFreeMemory(mem1, alloc1);<br>
  vkFreeMemory(mem2, alloc2); // anv_cached_bo is freed here<br>
<br>
So we can't use either pAllocator to allocate/free the anv_cached_bo,<br>
unless anv_cached_bo also cached its original allocator.<br>
If we don't cache the allocator (and please, let's not), then the<br>
'alloc' params to anv_bo_cache_alloc/import/<wbr>release are dead params.<br>
We must use an allocator that is not bound<br>
to the anv_cached_bo, such as device->alloc.<br>
<br>
<br>
Patch 8/18 correctly passes device->alloc into these functions. But,<br>
since the 'alloc' params aren't really used, they should be dropped to<br>
prevent accidental misuse.<br>
</blockquote></div><br></div><div class="gmail_extra">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.<br><br></div><div class="gmail_extra">--Jason<br></div></div>