From 6945c53bc712cf4a28a46fe46c2bd8526ea261d1 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 17 Jan 2022 08:56:04 +0100 Subject: drm/i915: Add locking to i915_gem_evict_vm(), v3. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit i915_gem_evict_vm will need to be able to evict objects that are locked by the current ctx. By testing if the current context already locked the object, we can do this correctly. This allows us to evict the entire vm even if we already hold some objects' locks. Previously, this was spread over several commits, but it makes more sense to commit the changes to i915_gem_evict_vm separately from the changes to i915_gem_evict_something() and i915_gem_evict_for_node(). Changes since v1: - Handle evicting dead objects better. Changes since v2: - Use for_i915_gem_ww in igt_evict_vm. (Thomas) Signed-off-by: Maarten Lankhorst Reviewed-by: Thomas Hellström [mlankhorst: Fix up doc warning.] Link: https://patchwork.freedesktop.org/patch/msgid/20220117075604.131477-1-maarten.lankhorst@linux.intel.com --- drivers/gpu/drm/i915/i915_gem_evict.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_evict.c') diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c index 2b73ddb11c66..670dceaa9b24 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -358,6 +358,8 @@ int i915_gem_evict_for_node(struct i915_address_space *vm, /** * i915_gem_evict_vm - Evict all idle vmas from a vm * @vm: Address space to cleanse + * @ww: An optional struct i915_gem_ww_ctx. If not NULL, i915_gem_evict_vm + * will be able to evict vma's locked by the ww as well. * * This function evicts all vmas from a vm. * @@ -367,7 +369,7 @@ int i915_gem_evict_for_node(struct i915_address_space *vm, * To clarify: This is for freeing up virtual address space, not for freeing * memory in e.g. the shrinker. */ -int i915_gem_evict_vm(struct i915_address_space *vm) +int i915_gem_evict_vm(struct i915_address_space *vm, struct i915_gem_ww_ctx *ww) { int ret = 0; @@ -388,24 +390,52 @@ int i915_gem_evict_vm(struct i915_address_space *vm) do { struct i915_vma *vma, *vn; LIST_HEAD(eviction_list); + LIST_HEAD(locked_eviction_list); list_for_each_entry(vma, &vm->bound_list, vm_link) { if (i915_vma_is_pinned(vma)) continue; + /* + * If we already own the lock, trylock fails. In case + * the resv is shared among multiple objects, we still + * need the object ref. + */ + if (!kref_read(&vma->obj->base.refcount) || + (ww && (dma_resv_locking_ctx(vma->obj->base.resv) == &ww->ctx))) { + __i915_vma_pin(vma); + list_add(&vma->evict_link, &locked_eviction_list); + continue; + } + + if (!i915_gem_object_trylock(vma->obj, ww)) + continue; + __i915_vma_pin(vma); list_add(&vma->evict_link, &eviction_list); } - if (list_empty(&eviction_list)) + if (list_empty(&eviction_list) && list_empty(&locked_eviction_list)) break; ret = 0; + /* Unbind locked objects first, before unlocking the eviction_list */ + list_for_each_entry_safe(vma, vn, &locked_eviction_list, evict_link) { + __i915_vma_unpin(vma); + + if (ret == 0) + ret = __i915_vma_unbind(vma); + if (ret != -EINTR) /* "Get me out of here!" */ + ret = 0; + } + list_for_each_entry_safe(vma, vn, &eviction_list, evict_link) { __i915_vma_unpin(vma); if (ret == 0) ret = __i915_vma_unbind(vma); if (ret != -EINTR) /* "Get me out of here!" */ ret = 0; + + i915_gem_object_unlock(vma->obj); } } while (ret == 0); -- cgit From 7e00897be8bf13ef9c68c95a8e386b714c29ad95 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Fri, 14 Jan 2022 14:23:17 +0100 Subject: drm/i915: Add object locking to i915_gem_evict_for_node and i915_gem_evict_something, v2. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because we will start to require the obj->resv lock for unbinding, ensure these vma eviction utility functions also take the lock. This requires some function signature changes, to ensure that the ww context is passed around, but is mostly straightforward. Previously this was split up into several patches, but reworking should allow for easier bisection. Changes since v1: - Handle evicting dead objects better. Signed-off-by: Maarten Lankhorst Reviewed-by: Thomas Hellström Link: https://patchwork.freedesktop.org/patch/msgid/20220114132320.109030-4-maarten.lankhorst@linux.intel.com --- drivers/gpu/drm/i915/i915_gem_evict.c | 69 +++++++++++++++++++++++++++++++---- 1 file changed, 62 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_evict.c') diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c index 670dceaa9b24..3483d4ee235b 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -37,6 +37,11 @@ I915_SELFTEST_DECLARE(static struct igt_evict_ctl { bool fail_if_busy:1; } igt_evict_ctl;) +static bool dying_vma(struct i915_vma *vma) +{ + return !kref_read(&vma->obj->base.refcount); +} + static int ggtt_flush(struct intel_gt *gt) { /* @@ -49,8 +54,37 @@ static int ggtt_flush(struct intel_gt *gt) return intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT); } +static bool grab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx *ww) +{ + /* + * We add the extra refcount so the object doesn't drop to zero until + * after ungrab_vma(), this way trylock is always paired with unlock. + */ + if (i915_gem_object_get_rcu(vma->obj)) { + if (!i915_gem_object_trylock(vma->obj, ww)) { + i915_gem_object_put(vma->obj); + return false; + } + } else { + /* Dead objects don't need pins */ + atomic_and(~I915_VMA_PIN_MASK, &vma->flags); + } + + return true; +} + +static void ungrab_vma(struct i915_vma *vma) +{ + if (dying_vma(vma)) + return; + + i915_gem_object_unlock(vma->obj); + i915_gem_object_put(vma->obj); +} + static bool mark_free(struct drm_mm_scan *scan, + struct i915_gem_ww_ctx *ww, struct i915_vma *vma, unsigned int flags, struct list_head *unwind) @@ -58,6 +92,9 @@ mark_free(struct drm_mm_scan *scan, if (i915_vma_is_pinned(vma)) return false; + if (!grab_vma(vma, ww)) + return false; + list_add(&vma->evict_link, unwind); return drm_mm_scan_add_block(scan, &vma->node); } @@ -76,6 +113,7 @@ static bool defer_evict(struct i915_vma *vma) /** * i915_gem_evict_something - Evict vmas to make room for binding a new one * @vm: address space to evict from + * @ww: An optional struct i915_gem_ww_ctx. * @min_size: size of the desired free space * @alignment: alignment constraint of the desired free space * @color: color for the desired space @@ -98,6 +136,7 @@ static bool defer_evict(struct i915_vma *vma) */ int i915_gem_evict_something(struct i915_address_space *vm, + struct i915_gem_ww_ctx *ww, u64 min_size, u64 alignment, unsigned long color, u64 start, u64 end, @@ -170,7 +209,7 @@ search_again: continue; } - if (mark_free(&scan, vma, flags, &eviction_list)) + if (mark_free(&scan, ww, vma, flags, &eviction_list)) goto found; } @@ -178,6 +217,7 @@ search_again: list_for_each_entry_safe(vma, next, &eviction_list, evict_link) { ret = drm_mm_scan_remove_block(&scan, &vma->node); BUG_ON(ret); + ungrab_vma(vma); } /* @@ -222,10 +262,12 @@ found: * of any of our objects, thus corrupting the list). */ list_for_each_entry_safe(vma, next, &eviction_list, evict_link) { - if (drm_mm_scan_remove_block(&scan, &vma->node)) + if (drm_mm_scan_remove_block(&scan, &vma->node)) { __i915_vma_pin(vma); - else + } else { list_del(&vma->evict_link); + ungrab_vma(vma); + } } /* Unbinding will emit any required flushes */ @@ -234,16 +276,20 @@ found: __i915_vma_unpin(vma); if (ret == 0) ret = __i915_vma_unbind(vma); + ungrab_vma(vma); } while (ret == 0 && (node = drm_mm_scan_color_evict(&scan))) { vma = container_of(node, struct i915_vma, node); /* If we find any non-objects (!vma), we cannot evict them */ - if (vma->node.color != I915_COLOR_UNEVICTABLE) + if (vma->node.color != I915_COLOR_UNEVICTABLE && + grab_vma(vma, ww)) { ret = __i915_vma_unbind(vma); - else - ret = -ENOSPC; /* XXX search failed, try again? */ + ungrab_vma(vma); + } else { + ret = -ENOSPC; + } } return ret; @@ -252,6 +298,7 @@ found: /** * i915_gem_evict_for_node - Evict vmas to make room for binding a new one * @vm: address space to evict from + * @ww: An optional struct i915_gem_ww_ctx. * @target: range (and color) to evict for * @flags: additional flags to control the eviction algorithm * @@ -261,6 +308,7 @@ found: * memory in e.g. the shrinker. */ int i915_gem_evict_for_node(struct i915_address_space *vm, + struct i915_gem_ww_ctx *ww, struct drm_mm_node *target, unsigned int flags) { @@ -333,6 +381,11 @@ int i915_gem_evict_for_node(struct i915_address_space *vm, break; } + if (!grab_vma(vma, ww)) { + ret = -ENOSPC; + break; + } + /* * Never show fear in the face of dragons! * @@ -350,6 +403,8 @@ int i915_gem_evict_for_node(struct i915_address_space *vm, __i915_vma_unpin(vma); if (ret == 0) ret = __i915_vma_unbind(vma); + + ungrab_vma(vma); } return ret; @@ -401,7 +456,7 @@ int i915_gem_evict_vm(struct i915_address_space *vm, struct i915_gem_ww_ctx *ww) * the resv is shared among multiple objects, we still * need the object ref. */ - if (!kref_read(&vma->obj->base.refcount) || + if (dying_vma(vma) || (ww && (dma_resv_locking_ctx(vma->obj->base.resv) == &ww->ctx))) { __i915_vma_pin(vma); list_add(&vma->evict_link, &locked_eviction_list); -- cgit