[Mesa-dev] [v4, 18/23] anv/allocator: Support softpin in the BO cache

[Jason Ekstrand]

On Thu, May 31, 2018 at 2:21 PM, Scott D Phillips <
scott.d.phillips at intel.com> wrote:

> Jason Ekstrand <jason at jlekstrand.net> writes:
>
> > ---
> >  src/intel/vulkan/anv_allocator.c | 54 ++++++++++++++++++++++++++++++
> +++++++++-
> >  1 file changed, 53 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_
> allocator.c
> > index 697da5f..f18e015 100644
> > --- a/src/intel/vulkan/anv_allocator.c
> > +++ b/src/intel/vulkan/anv_allocator.c
> > @@ -1240,7 +1240,8 @@ anv_bo_cache_lookup(struct anv_bo_cache *cache,
> uint32_t gem_handle)
> >  #define ANV_BO_CACHE_SUPPORTED_FLAGS \
> >     (EXEC_OBJECT_WRITE | \
> >      EXEC_OBJECT_ASYNC | \
> > -    EXEC_OBJECT_SUPPORTS_48B_ADDRESS)
> > +    EXEC_OBJECT_SUPPORTS_48B_ADDRESS | \
> > +    EXEC_OBJECT_PINNED)
> >
> >  VkResult
> >  anv_bo_cache_alloc(struct anv_device *device,
> > @@ -1269,6 +1270,14 @@ anv_bo_cache_alloc(struct anv_device *device,
> >
> >     bo->bo.flags = bo_flags;
> >
> > +   if (!anv_vma_alloc(device, &bo->bo)) {
> > +      anv_gem_close(device, bo->bo.gem_handle);
> > +      vk_free(&device->alloc, bo);
> > +      return vk_errorf(device->instance, NULL,
> > +                       VK_ERROR_OUT_OF_DEVICE_MEMORY,
> > +                       "failed to allocate virtual address for BO");
> > +   }
> > +
> >     assert(bo->bo.gem_handle);
> >
> >     pthread_mutex_lock(&cache->mutex);
> > @@ -1310,6 +1319,38 @@ anv_bo_cache_import(struct anv_device *device,
> >        new_flags |= (bo->bo.flags | bo_flags) & EXEC_OBJECT_WRITE;
> >        new_flags |= (bo->bo.flags & bo_flags) & EXEC_OBJECT_ASYNC;
> >        new_flags |= (bo->bo.flags & bo_flags) & EXEC_OBJECT_SUPPORTS_48B_
> ADDRESS;
> > +      new_flags |= (bo->bo.flags | bo_flags) & EXEC_OBJECT_PINNED;
> > +
> > +      /* It's theoretically possible for a BO to get imported such that
> it's
> > +       * both pinned and not pinned.  The only way this can happen is
> if it
> > +       * gets imported as both a semaphore and a memory object and that
> would
> > +       * be an application error.  Just fail out in that case.
> > +       */
> > +      if ((bo->bo.flags & EXEC_OBJECT_PINNED) !=
> > +          (bo_flags & EXEC_OBJECT_PINNED)) {
> > +         pthread_mutex_unlock(&cache->mutex);
> > +         return vk_errorf(device->instance, NULL,
> > +                          VK_ERROR_INVALID_EXTERNAL_HANDLE,
> > +                          "The same BO was imported two different
> ways");
> > +      }
> > +
> > +      /* It's also theoretically possible that someone could export a
> BO from
> > +       * one heap and import it into another or to import the same BO
> into two
> > +       * different heaps.  If this happens, we could potentially end up
> both
> > +       * allowing and disallowing 48-bit addresses.  There's not much
> we can
> > +       * do about it if we're pinning so we just throw a warning and
> hope no
> > +       * app is actually that stupid.
> > +       */
> > +      if ((new_flags & EXEC_OBJECT_PINNED) &&
> > +          (bo->bo.flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS) &&
> > +          !(bo_flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS)) {
> > +         vk_debug_report(&device->instance->debug_report_callbacks,
> > +                         VK_DEBUG_REPORT_WARNING_BIT_EXT,
> > +                         VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT,
> > +                         (uint64_t) (uintptr_t) device,
> > +                         0, 0, "anv",
> > +                         "Import of the same BO on multiple heaps");
> > +      }
>
> Hmm, it seems like we should error here too. If the BO is getting used
> in a way that doesn't support 48 bit addresses then at best we can hit
> one of our known iffy cases, and at worst we might cram a 64 bit address
> into a narrower offset field and then possibly get GPU hangs. Or if
> there is no such case like this then I think it probably needs some more
> explanation about why this isn't an error.
>

I think the worst case is that someone will try to make a vertex buffer out
of it and maybe get cache corruption.  I think that case can theoretically
lead to hangs (if it's used for the gpu_memcpy of surface states) but it's
pretty hard to hit.  I think leaving it as a warning is probably ok.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180531/d8658a5d/attachment.html>