From 425095809218470fecaab65cfd2bbbff476376ac Mon Sep 17 00:00:00 2001
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
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
Git-commit: 6945c53bc712cf4a28a46fe46c2bd8526ea261d1
Patch-mainline: v5.18-rc1
References: jsc#PED-1166 jsc#PED-1168 jsc#PED-1170 jsc#PED-1218 jsc#PED-1220 jsc#PED-1222 jsc#PED-1223 jsc#PED-1225
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 <maarten.lankhorst@linux.intel.com>
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
[mlankhorst: Fix up doc warning.]
Link: https://patchwork.freedesktop.org/patch/msgid/20220117075604.131477-1-maarten.lankhorst@linux.intel.com
Acked-by: Patrik Jakobsson <pjakobsson@suse.de>
---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 3 +-
drivers/gpu/drm/i915/i915_gem_evict.c | 34 +++++++++++++++++--
drivers/gpu/drm/i915/i915_vma.c | 7 +++-
.../gpu/drm/i915/selftests/i915_gem_evict.c | 12 ++++---
5 files changed, 48 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 48537289305b..8d0744efaa1a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -757,7 +757,7 @@ static int eb_reserve(struct i915_execbuffer *eb)
case 1:
/* Too fragmented, unbind everything and retry */
mutex_lock(&eb->context->vm->mutex);
- err = i915_gem_evict_vm(eb->context->vm);
+ err = i915_gem_evict_vm(eb->context->vm, &eb->ww);
mutex_unlock(&eb->context->vm->mutex);
if (err)
return err;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 48be1abb7da4..efe69d6b86f4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -24,7 +24,6 @@
#include "i915_trace.h"
#include "i915_user_extensions.h"
#include "i915_gem_ttm.h"
-#include "i915_gem_evict.h"
#include "i915_vma.h"
static inline bool
@@ -370,7 +369,7 @@ static vm_fault_t vm_fault_gtt(struct vm_fault *vmf)
if (vma == ERR_PTR(-ENOSPC)) {
ret = mutex_lock_interruptible(&ggtt->vm.mutex);
if (!ret) {
- ret = i915_gem_evict_vm(&ggtt->vm);
+ ret = i915_gem_evict_vm(&ggtt->vm, &ww);
mutex_unlock(&ggtt->vm.mutex);
}
if (ret)
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 24eee0c2055f..b8e71a91c562 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -359,6 +359,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.
*
@@ -368,7 +370,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;
@@ -389,24 +391,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);
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 5c7273834d74..9792176c3b45 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1538,7 +1538,12 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
/* Unlike i915_vma_pin, we don't take no for an answer! */
flush_idle_contexts(vm->gt);
if (mutex_lock_interruptible(&vm->mutex) == 0) {
- i915_gem_evict_vm(vm);
+ /*
+ * We pass NULL ww here, as we don't want to unbind
+ * locked objects when called from execbuf when pinning
+ * is removed. This would probably regress badly.
+ */
+ i915_gem_evict_vm(vm, NULL);
mutex_unlock(&vm->mutex);
}
} while (1);
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
index 74a1b2ecf48f..9ec8ccb7c599 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
@@ -332,6 +332,7 @@ static int igt_evict_vm(void *arg)
{
struct intel_gt *gt = arg;
struct i915_ggtt *ggtt = gt->ggtt;
+ struct i915_gem_ww_ctx ww;
LIST_HEAD(objects);
int err;
@@ -343,7 +344,7 @@ static int igt_evict_vm(void *arg)
/* Everything is pinned, nothing should happen */
mutex_lock(&ggtt->vm.mutex);
- err = i915_gem_evict_vm(&ggtt->vm);
+ err = i915_gem_evict_vm(&ggtt->vm, NULL);
mutex_unlock(&ggtt->vm.mutex);
if (err) {
pr_err("i915_gem_evict_vm on a full GGTT returned err=%d]\n",
@@ -353,9 +354,12 @@ static int igt_evict_vm(void *arg)
unpin_ggtt(ggtt);
- mutex_lock(&ggtt->vm.mutex);
- err = i915_gem_evict_vm(&ggtt->vm);
- mutex_unlock(&ggtt->vm.mutex);
+ for_i915_gem_ww(&ww, err, false) {
+ mutex_lock(&ggtt->vm.mutex);
+ err = i915_gem_evict_vm(&ggtt->vm, &ww);
+ mutex_unlock(&ggtt->vm.mutex);
+ }
+
if (err) {
pr_err("i915_gem_evict_vm on a full GGTT returned err=%d]\n",
err);
--
2.38.1