[PATCH v6 4/6] drm/i915: Use vma resources for async unbinding
Matthew Auld
matthew.auld at intel.com
Mon Jan 10 13:21:06 UTC 2022
On 07/01/2022 14:23, Thomas Hellström wrote:
> Implement async (non-blocking) unbinding by not syncing the vma before
> calling unbind on the vma_resource.
> Add the resulting unbind fence to the object's dma_resv from where it is
> picked up by the ttm migration code.
> Ideally these unbind fences should be coalesced with the migration blit
> fence to avoid stalling the migration blit waiting for unbind, as they
> can certainly go on in parallel, but since we don't yet have a
> reasonable data structure to use to coalesce fences and attach the
> resulting fence to a timeline, we defer that for now.
>
> Note that with async unbinding, even while the unbind waits for the
> preceding bind to complete before unbinding, the vma itself might have been
> destroyed in the process, clearing the vma pages. Therefore we can
> only allow async unbinding if we have a refcounted sg-list and keep a
> refcount on that for the vma resource pages to stay intact until
> binding occurs. If this condition is not met, a request for an async
> unbind is diverted to a sync unbind.
>
> v2:
> - Use a separate kmem_cache for vma resources for now to isolate their
> memory allocation and aid debugging.
> - Move the check for vm closed to the actual unbinding thread. Regardless
> of whether the vm is closed, we need the unbind fence to properly wait
> for capture.
> - Clear vma_res::vm on unbind and update its documentation.
> v4:
> - Take cache coloring into account when searching for vma resources
> pending unbind. (Matthew Auld)
> v5:
> - Fix timeout and error check in i915_vma_resource_bind_dep_await().
> - Avoid taking a reference on the object for async binding if
> async unbind capable.
> - Fix braces around a single-line if statement.
> v6:
> - Fix up the cache coloring adjustment. (Kernel test robot <lkp at intel.com>)
> - Don't allow async unbinding if the vma_res pages are not the same as
> the object pages.
>
> Signed-off-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 11 +-
> drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_gtt.c | 4 +
> drivers/gpu/drm/i915/gt/intel_gtt.h | 3 +
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_gem.c | 12 +-
> drivers/gpu/drm/i915/i915_module.c | 3 +
> drivers/gpu/drm/i915/i915_vma.c | 205 +++++++++--
> drivers/gpu/drm/i915/i915_vma.h | 3 +-
> drivers/gpu/drm/i915/i915_vma_resource.c | 354 +++++++++++++++++--
> drivers/gpu/drm/i915/i915_vma_resource.h | 48 +++
> 11 files changed, 579 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> index 8653855d808b..1de306c03aaf 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> @@ -142,7 +142,16 @@ int i915_ttm_move_notify(struct ttm_buffer_object *bo)
> struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
> int ret;
>
> - ret = i915_gem_object_unbind(obj, I915_GEM_OBJECT_UNBIND_ACTIVE);
> + /*
> + * Note: The async unbinding here will actually transform the
> + * blocking wait for unbind into a wait before finally submitting
> + * evict / migration blit and thus stall the migration timeline
> + * which may not be good for overall throughput. We should make
> + * sure we await the unbind fences *after* the migration blit
> + * instead of *before* as we currently do.
> + */
> + ret = i915_gem_object_unbind(obj, I915_GEM_OBJECT_UNBIND_ACTIVE |
> + I915_GEM_OBJECT_UNBIND_ASYNC);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> index e49b6250c4b7..a1b2761bc16e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> @@ -142,7 +142,7 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
> continue;
>
> if (!i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND)) {
> - __i915_vma_evict(vma);
> + __i915_vma_evict(vma, false);
> drm_mm_remove_node(&vma->node);
> }
> }
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
> index a94be0306464..46be4197b93f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
> @@ -161,6 +161,9 @@ static void __i915_vm_release(struct work_struct *work)
> struct i915_address_space *vm =
> container_of(work, struct i915_address_space, release_work);
>
> + /* Synchronize async unbinds. */
> + i915_vma_resource_bind_dep_sync_all(vm);
> +
> vm->cleanup(vm);
> i915_address_space_fini(vm);
>
> @@ -189,6 +192,7 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass)
> if (!kref_read(&vm->resv_ref))
> kref_init(&vm->resv_ref);
>
> + vm->pending_unbind = RB_ROOT_CACHED;
> INIT_WORK(&vm->release_work, __i915_vm_release);
> atomic_set(&vm->open, 1);
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
> index 676b839d1a34..8073438b67c8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> @@ -265,6 +265,9 @@ struct i915_address_space {
> /* Flags used when creating page-table objects for this vm */
> unsigned long lmem_pt_obj_flags;
>
> + /* Interval tree for pending unbind vma resources */
> + struct rb_root_cached pending_unbind;
> +
> struct drm_i915_gem_object *
> (*alloc_pt_dma)(struct i915_address_space *vm, int sz);
> struct drm_i915_gem_object *
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2f9336302e6c..f7de0a509b82 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1667,6 +1667,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> #define I915_GEM_OBJECT_UNBIND_BARRIER BIT(1)
> #define I915_GEM_OBJECT_UNBIND_TEST BIT(2)
> #define I915_GEM_OBJECT_UNBIND_VM_TRYLOCK BIT(3)
> +#define I915_GEM_OBJECT_UNBIND_ASYNC BIT(4)
>
> void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e3730096abd9..3d6c00f845a3 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -156,10 +156,16 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> spin_unlock(&obj->vma.lock);
>
> if (vma) {
> + bool vm_trylock = !!(flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK);
> ret = -EBUSY;
> - if (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
> - !i915_vma_is_active(vma)) {
> - if (flags & I915_GEM_OBJECT_UNBIND_VM_TRYLOCK) {
> + if (flags & I915_GEM_OBJECT_UNBIND_ASYNC) {
> + assert_object_held(vma->obj);
> + ret = i915_vma_unbind_async(vma, vm_trylock);
> + }
> +
> + if (ret == -EBUSY && (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
> + !i915_vma_is_active(vma))) {
> + if (vm_trylock) {
> if (mutex_trylock(&vma->vm->mutex)) {
> ret = __i915_vma_unbind(vma);
> mutex_unlock(&vma->vm->mutex);
> diff --git a/drivers/gpu/drm/i915/i915_module.c b/drivers/gpu/drm/i915/i915_module.c
> index f6bcd2f89257..a8f175960b34 100644
> --- a/drivers/gpu/drm/i915/i915_module.c
> +++ b/drivers/gpu/drm/i915/i915_module.c
> @@ -17,6 +17,7 @@
> #include "i915_scheduler.h"
> #include "i915_selftest.h"
> #include "i915_vma.h"
> +#include "i915_vma_resource.h"
>
> static int i915_check_nomodeset(void)
> {
> @@ -64,6 +65,8 @@ static const struct {
> .exit = i915_scheduler_module_exit },
> { .init = i915_vma_module_init,
> .exit = i915_vma_module_exit },
> + { .init = i915_vma_resource_module_init,
> + .exit = i915_vma_resource_module_exit },
> { .init = i915_mock_selftests },
> { .init = i915_pmu_init,
> .exit = i915_pmu_exit },
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 8fa3e0b2fe26..a2592605ba5c 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -285,9 +285,10 @@ struct i915_vma_work {
> struct dma_fence_work base;
> struct i915_address_space *vm;
> struct i915_vm_pt_stash stash;
> - struct i915_vma *vma;
> + struct i915_vma_resource *vma_res;
> struct drm_i915_gem_object *pinned;
> struct i915_sw_dma_fence_cb cb;
> + struct i915_refct_sgt *rsgt;
> enum i915_cache_level cache_level;
> unsigned int flags;
> };
> @@ -295,10 +296,11 @@ struct i915_vma_work {
> static void __vma_bind(struct dma_fence_work *work)
> {
> struct i915_vma_work *vw = container_of(work, typeof(*vw), base);
> - struct i915_vma *vma = vw->vma;
> + struct i915_vma_resource *vma_res = vw->vma_res;
> +
> + vma_res->ops->bind_vma(vma_res->vm, &vw->stash,
> + vma_res, vw->cache_level, vw->flags);
>
> - vma->ops->bind_vma(vw->vm, &vw->stash,
> - vma->resource, vw->cache_level, vw->flags);
> }
>
> static void __vma_release(struct dma_fence_work *work)
> @@ -310,6 +312,10 @@ static void __vma_release(struct dma_fence_work *work)
>
> i915_vm_free_pt_stash(vw->vm, &vw->stash);
> i915_vm_put(vw->vm);
> + if (vw->vma_res)
> + i915_vma_resource_put(vw->vma_res);
> + if (vw->rsgt)
> + i915_refct_sgt_put(vw->rsgt);
> }
>
> static const struct dma_fence_work_ops bind_ops = {
> @@ -379,13 +385,11 @@ i915_vma_resource_init_from_vma(struct i915_vma_resource *vma_res,
> {
> struct drm_i915_gem_object *obj = vma->obj;
>
> - i915_vma_resource_init(vma_res, vma->pages, &vma->page_sizes,
> + i915_vma_resource_init(vma_res, vma->vm, vma->pages, &vma->page_sizes,
> i915_gem_object_is_readonly(obj),
> i915_gem_object_is_lmem(obj),
> - vma->private,
> - vma->node.start,
> - vma->node.size,
> - vma->size);
> + vma->ops, vma->private, vma->node.start,
> + vma->node.size, vma->size);
> }
>
> /**
> @@ -409,6 +413,7 @@ int i915_vma_bind(struct i915_vma *vma,
> {
> u32 bind_flags;
> u32 vma_flags;
> + int ret;
>
> lockdep_assert_held(&vma->vm->mutex);
> GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
> @@ -417,12 +422,12 @@ int i915_vma_bind(struct i915_vma *vma,
> if (GEM_DEBUG_WARN_ON(range_overflows(vma->node.start,
> vma->node.size,
> vma->vm->total))) {
> - kfree(vma_res);
> + i915_vma_resource_free(vma_res);
> return -ENODEV;
> }
>
> if (GEM_DEBUG_WARN_ON(!flags)) {
> - kfree(vma_res);
> + i915_vma_resource_free(vma_res);
> return -EINVAL;
> }
>
> @@ -434,12 +439,30 @@ int i915_vma_bind(struct i915_vma *vma,
>
> bind_flags &= ~vma_flags;
> if (bind_flags == 0) {
> - kfree(vma_res);
> + i915_vma_resource_free(vma_res);
> return 0;
> }
>
> GEM_BUG_ON(!atomic_read(&vma->pages_count));
>
> + /* Wait for or await async unbinds touching our range */
> + if (work && bind_flags & vma->vm->bind_async_flags)
> + ret = i915_vma_resource_bind_dep_await(vma->vm,
> + &work->base.chain,
> + vma->node.start,
> + vma->node.size,
> + true,
> + GFP_NOWAIT |
> + __GFP_RETRY_MAYFAIL |
> + __GFP_NOWARN);
> + else
> + ret = i915_vma_resource_bind_dep_sync(vma->vm, vma->node.start,
> + vma->node.size, true);
> + if (ret) {
> + i915_vma_resource_free(vma_res);
> + return ret;
> + }
> +
> if (vma->resource || !vma_res) {
> /* Rebinding with an additional I915_VMA_*_BIND */
> GEM_WARN_ON(!vma_flags);
> @@ -452,9 +475,11 @@ int i915_vma_bind(struct i915_vma *vma,
> if (work && bind_flags & vma->vm->bind_async_flags) {
> struct dma_fence *prev;
>
> - work->vma = vma;
> + work->vma_res = i915_vma_resource_get(vma->resource);
> work->cache_level = cache_level;
> work->flags = bind_flags;
> + if (vma->obj->mm.rsgt)
> + work->rsgt = i915_refct_sgt_get(vma->obj->mm.rsgt);
>
> /*
> * Note we only want to chain up to the migration fence on
> @@ -475,14 +500,24 @@ int i915_vma_bind(struct i915_vma *vma,
>
> work->base.dma.error = 0; /* enable the queue_work() */
>
> - work->pinned = i915_gem_object_get(vma->obj);
> + /*
> + * If we don't have the refcounted pages list, keep a reference
> + * on the object to avoid waiting for the async bind to
> + * complete in the object destruction path.
> + */
> + if (!work->rsgt)
> + work->pinned = i915_gem_object_get(vma->obj);
> } else {
> if (vma->obj) {
> int ret;
>
> ret = i915_gem_object_wait_moving_fence(vma->obj, true);
> - if (ret)
> + if (ret) {
> + i915_vma_resource_free(vma->resource);
> + vma->resource = NULL;
> +
> return ret;
> + }
> }
> vma->ops->bind_vma(vma->vm, NULL, vma->resource, cache_level,
> bind_flags);
> @@ -1755,8 +1790,9 @@ int _i915_vma_move_to_active(struct i915_vma *vma,
> return 0;
> }
>
> -void __i915_vma_evict(struct i915_vma *vma)
> +struct dma_fence *__i915_vma_evict(struct i915_vma *vma, bool async)
> {
> + struct i915_vma_resource *vma_res = vma->resource;
> struct dma_fence *unbind_fence;
>
> GEM_BUG_ON(i915_vma_is_pinned(vma));
> @@ -1789,27 +1825,39 @@ void __i915_vma_evict(struct i915_vma *vma)
> GEM_BUG_ON(vma->fence);
> GEM_BUG_ON(i915_vma_has_userfault(vma));
>
> - if (likely(atomic_read(&vma->vm->open))) {
> - trace_i915_vma_unbind(vma);
> - vma->ops->unbind_vma(vma->vm, vma->resource);
> - }
> + /* Object backend must be async capable. */
> + GEM_WARN_ON(async && !vma->obj->mm.rsgt);
> +
> + /* If vm is not open, unbind is a nop. */
> + vma_res->needs_wakeref = i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND) &&
> + atomic_read(&vma->vm->open);
> + trace_i915_vma_unbind(vma);
> +
> + unbind_fence = i915_vma_resource_unbind(vma_res);
> + vma->resource = NULL;
> +
> atomic_and(~(I915_VMA_BIND_MASK | I915_VMA_ERROR | I915_VMA_GGTT_WRITE),
> &vma->flags);
>
> - unbind_fence = i915_vma_resource_unbind(vma->resource);
> - i915_vma_resource_put(vma->resource);
> - vma->resource = NULL;
> + /* Object backend must be async capable. */
> + GEM_WARN_ON(async && !vma->obj->mm.rsgt);
>
> i915_vma_detach(vma);
> - vma_unbind_pages(vma);
> +
> + if (!async && unbind_fence) {
> + dma_fence_wait(unbind_fence, false);
> + dma_fence_put(unbind_fence);
> + unbind_fence = NULL;
> + }
>
> /*
> - * This uninterruptible wait under the vm mutex is currently
> - * only ever blocking while the vma is being captured from.
> - * With async unbinding, this wait here will be removed.
> + * Binding itself may not have completed until the unbind fence signals,
> + * so don't drop the pages until that happens, unless the resource is
> + * async_capable.
> */
> - dma_fence_wait(unbind_fence, false);
> - dma_fence_put(unbind_fence);
> +
> + vma_unbind_pages(vma);
> + return unbind_fence;
> }
>
> int __i915_vma_unbind(struct i915_vma *vma)
> @@ -1836,12 +1884,47 @@ int __i915_vma_unbind(struct i915_vma *vma)
> return ret;
>
> GEM_BUG_ON(i915_vma_is_active(vma));
> - __i915_vma_evict(vma);
> + __i915_vma_evict(vma, false);
>
> drm_mm_remove_node(&vma->node); /* pairs with i915_vma_release() */
> return 0;
> }
>
> +static struct dma_fence *__i915_vma_unbind_async(struct i915_vma *vma)
> +{
> + struct dma_fence *fence;
> +
> + lockdep_assert_held(&vma->vm->mutex);
> +
> + if (!drm_mm_node_allocated(&vma->node))
> + return NULL;
> +
> + if (i915_vma_is_pinned(vma) ||
> + &vma->obj->mm.rsgt->table != vma->resource->bi.pages)
> + return ERR_PTR(-EAGAIN);
> +
> + /*
> + * We probably need to replace this with awaiting the fences of the
> + * object's dma_resv when the vma active goes away. When doing that
> + * we need to be careful to not add the vma_resource unbind fence
> + * immediately to the object's dma_resv, because then unbinding
> + * the next vma from the object, in case there are many, will
> + * actually await the unbinding of the previous vmas, which is
> + * undesirable.
> + */
> + if (i915_sw_fence_await_active(&vma->resource->chain, &vma->active,
> + I915_ACTIVE_AWAIT_EXCL |
> + I915_ACTIVE_AWAIT_ACTIVE) < 0) {
> + return ERR_PTR(-EBUSY);
> + }
> +
> + fence = __i915_vma_evict(vma, true);
> +
> + drm_mm_remove_node(&vma->node); /* pairs with i915_vma_release() */
> +
> + return fence;
> +}
> +
> int i915_vma_unbind(struct i915_vma *vma)
> {
> struct i915_address_space *vm = vma->vm;
> @@ -1878,6 +1961,68 @@ int i915_vma_unbind(struct i915_vma *vma)
> return err;
> }
>
> +int i915_vma_unbind_async(struct i915_vma *vma, bool trylock_vm)
> +{
> + struct drm_i915_gem_object *obj = vma->obj;
> + struct i915_address_space *vm = vma->vm;
> + intel_wakeref_t wakeref = 0;
> + struct dma_fence *fence;
> + int err;
> +
> + /*
> + * We need the dma-resv lock since we add the
> + * unbind fence to the dma-resv object.
> + */
> + assert_object_held(obj);
> +
> + if (!drm_mm_node_allocated(&vma->node))
> + return 0;
> +
> + if (i915_vma_is_pinned(vma)) {
> + vma_print_allocator(vma, "is pinned");
> + return -EAGAIN;
> + }
> +
> + if (!obj->mm.rsgt)
> + return -EBUSY;
> +
> + err = dma_resv_reserve_shared(obj->base.resv, 1);
> + if (err)
> + return -EBUSY;
> +
> + /*
> + * It would be great if we could grab this wakeref from the
> + * async unbind work if needed, but we can't because it uses
> + * kmalloc and it's in the dma-fence signalling critical path.
> + */
> + if (i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND))
> + wakeref = intel_runtime_pm_get(&vm->i915->runtime_pm);
> +
> + if (trylock_vm && !mutex_trylock(&vm->mutex)) {
> + err = -EBUSY;
> + goto out_rpm;
> + } else if (!trylock_vm) {
> + err = mutex_lock_interruptible_nested(&vm->mutex, !wakeref);
> + if (err)
> + goto out_rpm;
> + }
> +
> + fence = __i915_vma_unbind_async(vma);
> + mutex_unlock(&vm->mutex);
> + if (IS_ERR_OR_NULL(fence)) {
> + err = PTR_ERR_OR_ZERO(fence);
> + goto out_rpm;
> + }
> +
> + dma_resv_add_shared_fence(obj->base.resv, fence);
> + dma_fence_put(fence);
> +
> +out_rpm:
> + if (wakeref)
> + intel_runtime_pm_put(&vm->i915->runtime_pm, wakeref);
> + return err;
> +}
> +
> struct i915_vma *i915_vma_make_unshrinkable(struct i915_vma *vma)
> {
> i915_gem_object_make_unshrinkable(vma->obj);
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index 1df57ec832bd..a560bae04e7e 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -213,9 +213,10 @@ bool i915_vma_misplaced(const struct i915_vma *vma,
> u64 size, u64 alignment, u64 flags);
> void __i915_vma_set_map_and_fenceable(struct i915_vma *vma);
> void i915_vma_revoke_mmap(struct i915_vma *vma);
> -void __i915_vma_evict(struct i915_vma *vma);
> +struct dma_fence *__i915_vma_evict(struct i915_vma *vma, bool async);
> int __i915_vma_unbind(struct i915_vma *vma);
> int __must_check i915_vma_unbind(struct i915_vma *vma);
> +int __must_check i915_vma_unbind_async(struct i915_vma *vma, bool trylock_vm);
> void i915_vma_unlink_ctx(struct i915_vma *vma);
> void i915_vma_close(struct i915_vma *vma);
> void i915_vma_reopen(struct i915_vma *vma);
> diff --git a/drivers/gpu/drm/i915/i915_vma_resource.c b/drivers/gpu/drm/i915/i915_vma_resource.c
> index b50e67035d15..745f1b1d7885 100644
> --- a/drivers/gpu/drm/i915/i915_vma_resource.c
> +++ b/drivers/gpu/drm/i915/i915_vma_resource.c
> @@ -2,39 +2,44 @@
> /*
> * Copyright © 2021 Intel Corporation
> */
> +
> +#include <linux/interval_tree_generic.h>
> #include <linux/slab.h>
>
> +#include "i915_sw_fence.h"
> #include "i915_vma_resource.h"
> +#include "i915_drv.h"
>
> -/* Callbacks for the unbind dma-fence. */
> -static const char *get_driver_name(struct dma_fence *fence)
> -{
> - return "vma unbind fence";
> -}
> +#include "gt/intel_gtt.h"
>
> -static const char *get_timeline_name(struct dma_fence *fence)
> -{
> - return "unbound";
> -}
> -
> -static struct dma_fence_ops unbind_fence_ops = {
> - .get_driver_name = get_driver_name,
> - .get_timeline_name = get_timeline_name,
> -};
> +static struct kmem_cache *slab_vma_resources;
>
> /**
> - * __i915_vma_resource_init - Initialize a vma resource.
> - * @vma_res: The vma resource to initialize
> + * DOC:
> + * We use a per-vm interval tree to keep track of vma_resources
> + * scheduled for unbind but not yet unbound. The tree is protected by
> + * the vm mutex, and nodes are removed just after the unbind fence signals.
> + * The removal takes the vm mutex from a kernel thread which we need to
> + * keep in mind so that we don't grab the mutex and try to wait for all
> + * pending unbinds to complete, because that will temporaryily block many
> + * of the workqueue threads, and people will get angry.
> *
> - * Initializes the private members of a vma resource.
> + * We should consider using a single ordered fence per VM instead but that
> + * requires ordering the unbinds and might introduce unnecessary waiting
> + * for unrelated unbinds. Amount of code will probably be roughly the same
> + * due to the simplicity of using the interval tree interface.
> + *
> + * Another drawback of this interval tree is that the complexity of insertion
> + * and removal of fences increases as O(ln(pending_unbinds)) instead of
> + * O(1) for a single fence without interval tree.
> */
> -void __i915_vma_resource_init(struct i915_vma_resource *vma_res)
> -{
> - spin_lock_init(&vma_res->lock);
> - dma_fence_init(&vma_res->unbind_fence, &unbind_fence_ops,
> - &vma_res->lock, 0, 0);
> - refcount_set(&vma_res->hold_count, 1);
> -}
> +#define VMA_RES_START(_node) ((_node)->start)
> +#define VMA_RES_LAST(_node) ((_node)->start + (_node)->node_size - 1)
> +INTERVAL_TREE_DEFINE(struct i915_vma_resource, rb,
> + unsigned long, __subtree_last,
> + VMA_RES_START, VMA_RES_LAST, static, vma_res_itree);
> +
> +/* Callbacks for the unbind dma-fence. */
>
> /**
> * i915_vma_resource_alloc - Allocate a vma resource
> @@ -45,15 +50,73 @@ void __i915_vma_resource_init(struct i915_vma_resource *vma_res)
> struct i915_vma_resource *i915_vma_resource_alloc(void)
> {
> struct i915_vma_resource *vma_res =
> - kzalloc(sizeof(*vma_res), GFP_KERNEL);
> + kmem_cache_zalloc(slab_vma_resources, GFP_KERNEL);
>
> return vma_res ? vma_res : ERR_PTR(-ENOMEM);
> }
>
> +/**
> + * i915_vma_resource_free - Free a vma resource
> + * @vma_res: The vma resource to free.
> + */
> +void i915_vma_resource_free(struct i915_vma_resource *vma_res)
> +{
> + kmem_cache_free(slab_vma_resources, vma_res);
> +}
> +
> +static const char *get_driver_name(struct dma_fence *fence)
> +{
> + return "vma unbind fence";
> +}
> +
> +static const char *get_timeline_name(struct dma_fence *fence)
> +{
> + return "unbound";
> +}
> +
> +static void unbind_fence_free_rcu(struct rcu_head *head)
> +{
> + struct i915_vma_resource *vma_res =
> + container_of(head, typeof(*vma_res), unbind_fence.rcu);
> +
> + i915_vma_resource_free(vma_res);
> +}
> +
> +static void unbind_fence_release(struct dma_fence *fence)
> +{
> + struct i915_vma_resource *vma_res =
> + container_of(fence, typeof(*vma_res), unbind_fence);
> +
> + i915_sw_fence_fini(&vma_res->chain);
> +
> + call_rcu(&fence->rcu, unbind_fence_free_rcu);
> +}
> +
> +static struct dma_fence_ops unbind_fence_ops = {
> + .get_driver_name = get_driver_name,
> + .get_timeline_name = get_timeline_name,
> + .release = unbind_fence_release,
> +};
> +
> static void __i915_vma_resource_unhold(struct i915_vma_resource *vma_res)
> {
> - if (refcount_dec_and_test(&vma_res->hold_count))
> - dma_fence_signal(&vma_res->unbind_fence);
> + struct i915_address_space *vm;
> +
> + if (!refcount_dec_and_test(&vma_res->hold_count))
> + return;
> +
> + dma_fence_signal(&vma_res->unbind_fence);
> +
> + vm = vma_res->vm;
> + if (vma_res->wakeref)
> + intel_runtime_pm_put(&vm->i915->runtime_pm, vma_res->wakeref);
> +
> + vma_res->vm = NULL;
> + if (!RB_EMPTY_NODE(&vma_res->rb)) {
> + mutex_lock(&vm->mutex);
> + vma_res_itree_remove(vma_res, &vm->pending_unbind);
> + mutex_unlock(&vm->mutex);
> + }
> }
>
> /**
> @@ -102,6 +165,49 @@ bool i915_vma_resource_hold(struct i915_vma_resource *vma_res,
> return held;
> }
>
> +static void i915_vma_resource_unbind_work(struct work_struct *work)
> +{
> + struct i915_vma_resource *vma_res =
> + container_of(work, typeof(*vma_res), work);
> + struct i915_address_space *vm = vma_res->vm;
> + bool lockdep_cookie;
> +
> + lockdep_cookie = dma_fence_begin_signalling();
> + if (likely(atomic_read(&vm->open)))
> + vma_res->ops->unbind_vma(vm, vma_res);
> +
> + dma_fence_end_signalling(lockdep_cookie);
> + __i915_vma_resource_unhold(vma_res);
> + i915_vma_resource_put(vma_res);
> +}
> +
> +static int
> +i915_vma_resource_fence_notify(struct i915_sw_fence *fence,
> + enum i915_sw_fence_notify state)
> +{
> + struct i915_vma_resource *vma_res =
> + container_of(fence, typeof(*vma_res), chain);
> + struct dma_fence *unbind_fence =
> + &vma_res->unbind_fence;
> +
> + switch (state) {
> + case FENCE_COMPLETE:
> + dma_fence_get(unbind_fence);
> + if (vma_res->immediate_unbind) {
> + i915_vma_resource_unbind_work(&vma_res->work);
> + } else {
> + INIT_WORK(&vma_res->work, i915_vma_resource_unbind_work);
> + queue_work(system_unbound_wq, &vma_res->work);
> + }
> + break;
> + case FENCE_FREE:
> + i915_vma_resource_put(vma_res);
> + break;
> + }
> +
> + return NOTIFY_DONE;
> +}
> +
> /**
> * i915_vma_resource_unbind - Unbind a vma resource
> * @vma_res: The vma resource to unbind.
> @@ -112,10 +218,196 @@ bool i915_vma_resource_hold(struct i915_vma_resource *vma_res,
> * Return: A refcounted pointer to a dma-fence that signals when unbinding is
> * complete.
> */
> -struct dma_fence *
> -i915_vma_resource_unbind(struct i915_vma_resource *vma_res)
> +struct dma_fence *i915_vma_resource_unbind(struct i915_vma_resource *vma_res)
> {
> - __i915_vma_resource_unhold(vma_res);
> - dma_fence_get(&vma_res->unbind_fence);
> + struct i915_address_space *vm = vma_res->vm;
> +
> + /* Reference for the sw fence */
> + i915_vma_resource_get(vma_res);
> +
> + /* Caller must already have a wakeref in this case. */
> + if (vma_res->needs_wakeref)
> + vma_res->wakeref = intel_runtime_pm_get_if_in_use(&vm->i915->runtime_pm);
> +
> + if (atomic_read(&vma_res->chain.pending) <= 1) {
> + RB_CLEAR_NODE(&vma_res->rb);
> + vma_res->immediate_unbind = 1;
> + } else {
> + vma_res_itree_insert(vma_res, &vma_res->vm->pending_unbind);
> + }
> +
> + i915_sw_fence_commit(&vma_res->chain);
> +
> return &vma_res->unbind_fence;
> }
> +
> +/**
> + * __i915_vma_resource_init - Initialize a vma resource.
> + * @vma_res: The vma resource to initialize
> + *
> + * Initializes the private members of a vma resource.
> + */
> +void __i915_vma_resource_init(struct i915_vma_resource *vma_res)
> +{
> + spin_lock_init(&vma_res->lock);
> + dma_fence_init(&vma_res->unbind_fence, &unbind_fence_ops,
> + &vma_res->lock, 0, 0);
> + refcount_set(&vma_res->hold_count, 1);
> + i915_sw_fence_init(&vma_res->chain, i915_vma_resource_fence_notify);
> +}
> +
> +static void
> +i915_vma_resource_color_adjust_range(struct i915_address_space *vm,
> + unsigned long *start,
> + unsigned long *end)
u64
> +{
> + if (i915_vm_has_cache_coloring(vm)) {
> + if (*start)
> + *start -= I915_GTT_PAGE_SIZE;
> + *end += I915_GTT_PAGE_SIZE;
> + }
else {
WARN_ON_ONCE(vm->cache_coloring);
}
?
> +}
> +
> +/**
> + * i915_vma_resource_bind_dep_sync - Wait for / sync all unbinds touching a
> + * certain vm range.
> + * @vm: The vm to look at.
> + * @offset: The range start.
> + * @size: The range size.
> + * @intr: Whether to wait interrubtible.
> + *
> + * The function needs to be called with the vm lock held.
> + *
> + * Return: Zero on success, -ERESTARTSYS if interrupted and @intr==true
> + */
> +int i915_vma_resource_bind_dep_sync(struct i915_address_space *vm,
> + unsigned long offset,
> + unsigned long size,
u64
> + bool intr)
> +{
> + struct i915_vma_resource *node;
> + unsigned long last = offset + size - 1;
> +
> + lockdep_assert_held(&vm->mutex);
> + might_sleep();
> +
> + i915_vma_resource_color_adjust_range(vm, &offset, &last);
> + node = vma_res_itree_iter_first(&vm->pending_unbind, offset, last);
> + while (node) {
> + int ret = dma_fence_wait(&node->unbind_fence, intr);
> +
> + if (ret)
> + return ret;
> +
> + node = vma_res_itree_iter_next(node, offset, last);
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * i915_vma_resource_bind_dep_sync_all - Wait for / sync all unbinds of a vm,
> + * releasing the vm lock while waiting.
> + * @vm: The vm to look at.
> + *
> + * The function may not be called with the vm lock held.
> + * Typically this is called at vm destruction to finish any pending
> + * unbind operations. The vm mutex is released while waiting to avoid
> + * stalling kernel workqueues trying to grab the mutex.
> + */
> +void i915_vma_resource_bind_dep_sync_all(struct i915_address_space *vm)
> +{
> + struct i915_vma_resource *node;
> + struct dma_fence *fence;
> +
> + do {
> + fence = NULL;
> + mutex_lock(&vm->mutex);
> + node = vma_res_itree_iter_first(&vm->pending_unbind, 0,
> + ULONG_MAX);
U64_MAX?
> + if (node)
> + fence = dma_fence_get_rcu(&node->unbind_fence);
> + mutex_unlock(&vm->mutex);
> +
> + if (fence) {
> + /*
> + * The wait makes sure the node eventually removes
> + * itself from the tree.
> + */
> + dma_fence_wait(fence, false);
> + dma_fence_put(fence);
> + }
> + } while (node);
> +}
> +
> +/**
> + * i915_vma_resource_bind_dep_await - Have a struct i915_sw_fence await all
> + * pending unbinds in a certain range of a vm.
> + * @vm: The vm to look at.
> + * @sw_fence: The struct i915_sw_fence that will be awaiting the unbinds.
> + * @offset: The range start.
> + * @size: The range size.
> + * @intr: Whether to wait interrubtible.
> + * @gfp: Allocation mode for memory allocations.
> + *
> + * The function makes @sw_fence await all pending unbinds in a certain
> + * vm range before calling the complete notifier. To be able to await
> + * each individual unbind, the function needs to allocate memory using
> + * the @gpf allocation mode. If that fails, the function will instead
> + * wait for the unbind fence to signal, using @intr to judge whether to
> + * wait interruptible or not. Note that @gfp should ideally be selected so
> + * as to avoid any expensive memory allocation stalls and rather fail and
> + * synchronize itself. For now the vm mutex is required when calling this
> + * function with means that @gfp can't call into direct reclaim. In reality
> + * this means that during heavy memory pressure, we will sync in this
> + * function.
> + *
> + * Return: Zero on success, -ERESTARTSYS if interrupted and @intr==true
> + */
> +int i915_vma_resource_bind_dep_await(struct i915_address_space *vm,
> + struct i915_sw_fence *sw_fence,
> + unsigned long offset,
> + unsigned long size,
u64
> + bool intr,
> + gfp_t gfp)
> +{
> + struct i915_vma_resource *node;
> + unsigned long last = offset + size - 1;
> +
> + lockdep_assert_held(&vm->mutex);
> + might_alloc(gfp);
> + might_sleep();
> +
> + i915_vma_resource_color_adjust_range(vm, &offset, &last);
> + node = vma_res_itree_iter_first(&vm->pending_unbind, offset, last);
> + while (node) {
> + int ret;
> +
> + ret = i915_sw_fence_await_dma_fence(sw_fence,
> + &node->unbind_fence,
> + 0, gfp);
> + if (ret < 0) {
> + ret = dma_fence_wait(&node->unbind_fence, intr);
> + if (ret)
> + return ret;
> + }
> +
> + node = vma_res_itree_iter_next(node, offset, last);
> + }
> +
> + return 0;
> +}
> +
> +void i915_vma_resource_module_exit(void)
> +{
> + kmem_cache_destroy(slab_vma_resources);
> +}
> +
> +int __init i915_vma_resource_module_init(void)
> +{
> + slab_vma_resources = KMEM_CACHE(i915_vma_resource, SLAB_HWCACHE_ALIGN);
> + if (!slab_vma_resources)
> + return -ENOMEM;
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/i915_vma_resource.h b/drivers/gpu/drm/i915/i915_vma_resource.h
> index 9288a91f0038..58a81b438cc5 100644
> --- a/drivers/gpu/drm/i915/i915_vma_resource.h
> +++ b/drivers/gpu/drm/i915/i915_vma_resource.h
> @@ -10,6 +10,8 @@
> #include <linux/refcount.h>
>
> #include "i915_gem.h"
> +#include "i915_sw_fence.h"
> +#include "intel_runtime_pm.h"
>
> struct i915_page_sizes {
> /**
> @@ -38,6 +40,13 @@ struct i915_page_sizes {
> * @hold_count: Number of holders blocking the fence from finishing.
> * The vma itself is keeping a hold, which is released when unbind
> * is scheduled.
> + * @work: Work struct for deferred unbind work.
> + * @chain: Pointer to struct i915_sw_fence used to await dependencies.
> + * @rb: Rb node for the vm's pending unbind interval tree.
> + * @__subtree_last: Interval tree private member.
> + * @vm: non-refcounted pointer to the vm. This is for internal use only and
> + * this member is cleared after vm_resource unbind.
> + * @ops: Pointer to the backend i915_vma_ops.
> * @private: Bind backend private info.
> * @start: Offset into the address space of bind range start.
> * @node_size: Size of the allocated range manager node.
> @@ -45,6 +54,8 @@ struct i915_page_sizes {
> * @page_sizes_gtt: Resulting page sizes from the bind operation.
> * @bound_flags: Flags indicating binding status.
> * @allocated: Backend private data. TODO: Should move into @private.
> + * @immediate_unbind: Unbind can be done immediately and don't need to be
> + * deferred to a work item awaiting unsignaled fences.
> *
> * The lifetime of a struct i915_vma_resource is from a binding request to
> * the actual possible asynchronous unbind has completed.
> @@ -54,6 +65,12 @@ struct i915_vma_resource {
> /* See above for description of the lock. */
> spinlock_t lock;
> refcount_t hold_count;
> + struct work_struct work;
> + struct i915_sw_fence chain;
> + struct rb_node rb;
> + unsigned long __subtree_last;
> + struct i915_address_space *vm;
> + intel_wakeref_t wakeref;
>
> /**
> * struct i915_vma_bindinfo - Information needed for async bind
> @@ -73,13 +90,17 @@ struct i915_vma_resource {
> bool lmem:1;
> } bi;
>
> + const struct i915_vma_ops *ops;
> void *private;
> u64 start;
> u64 node_size;
> u64 vma_size;
> u32 page_sizes_gtt;
> +
> u32 bound_flags;
> bool allocated:1;
> + bool immediate_unbind:1;
> + bool needs_wakeref:1;
> };
>
> bool i915_vma_resource_hold(struct i915_vma_resource *vma_res,
> @@ -90,6 +111,8 @@ void i915_vma_resource_unhold(struct i915_vma_resource *vma_res,
>
> struct i915_vma_resource *i915_vma_resource_alloc(void);
>
> +void i915_vma_resource_free(struct i915_vma_resource *vma_res);
> +
> struct dma_fence *i915_vma_resource_unbind(struct i915_vma_resource *vma_res);
>
> void __i915_vma_resource_init(struct i915_vma_resource *vma_res);
> @@ -119,10 +142,12 @@ static inline void i915_vma_resource_put(struct i915_vma_resource *vma_res)
> /**
> * i915_vma_resource_init - Initialize a vma resource.
> * @vma_res: The vma resource to initialize
> + * @vm: Pointer to the vm.
> * @pages: The pages sg-table.
> * @page_sizes: Page sizes of the pages.
> * @readonly: Whether the vma should be bound read-only.
> * @lmem: Whether the vma points to lmem.
> + * @ops: The backend ops.
> * @private: Bind backend private info.
> * @start: Offset into the address space of bind range start.
> * @node_size: Size of the allocated range manager node.
> @@ -134,20 +159,24 @@ static inline void i915_vma_resource_put(struct i915_vma_resource *vma_res)
> * allocation is not allowed.
> */
> static inline void i915_vma_resource_init(struct i915_vma_resource *vma_res,
> + struct i915_address_space *vm,
> struct sg_table *pages,
> const struct i915_page_sizes *page_sizes,
> bool readonly,
> bool lmem,
> + const struct i915_vma_ops *ops,
> void *private,
> unsigned long start,
> unsigned long node_size,
> unsigned long size)
u64
Reviewed-by: Matthew Auld <matthew.auld at intel.com>
> {
> __i915_vma_resource_init(vma_res);
> + vma_res->vm = vm;
> vma_res->bi.pages = pages;
> vma_res->bi.page_sizes = *page_sizes;
> vma_res->bi.readonly = readonly;
> vma_res->bi.lmem = lmem;
> + vma_res->ops = ops;
> vma_res->private = private;
> vma_res->start = start;
> vma_res->node_size = node_size;
> @@ -157,6 +186,25 @@ static inline void i915_vma_resource_init(struct i915_vma_resource *vma_res,
> static inline void i915_vma_resource_fini(struct i915_vma_resource *vma_res)
> {
> GEM_BUG_ON(refcount_read(&vma_res->hold_count) != 1);
> + i915_sw_fence_fini(&vma_res->chain);
> }
>
> +int i915_vma_resource_bind_dep_sync(struct i915_address_space *vm,
> + unsigned long first,
> + unsigned long last,
> + bool intr);
> +
> +int i915_vma_resource_bind_dep_await(struct i915_address_space *vm,
> + struct i915_sw_fence *sw_fence,
> + unsigned long first,
> + unsigned long last,
> + bool intr,
> + gfp_t gfp);
> +
> +void i915_vma_resource_bind_dep_sync_all(struct i915_address_space *vm);
> +
> +void i915_vma_resource_module_exit(void);
> +
> +int i915_vma_resource_module_init(void);
> +
> #endif
>
More information about the dri-devel
mailing list