From 2f81c845b6b35ee40aced51bef18e66c6ec73765 Mon Sep 17 00:00:00 2001
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Date: Fri, 14 Jan 2022 14:23:18 +0100
Subject: drm/i915: Add i915_vma_unbind_unlocked, and take obj lock for
i915_vma_unbind, v2.
Git-commit: 0f341974cbc2a4efe074dd24c153e439b8430afe
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
We want to remove more members of i915_vma, which requires the locking to
be held more often.
Start requiring gem object lock for i915_vma_unbind, as it's one of the
callers that may unpin pages.
Some special care is needed when evicting, because the last reference to
the object may be held by the VMA, so after __i915_vma_unbind, vma may be
garbage, and we need to cache vma->obj before unlocking.
Changes since v1:
- Make trylock failing a WARN. (Matt)
- Remove double i915_vma_wait_for_bind() (Matt)
- Move atomic_set to right before mutex_unlock(), to make it more clear
they belong together. (Matt)
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220114132320.109030-5-maarten.lankhorst@linux.intel.com
Acked-by: Patrik Jakobsson <pjakobsson@suse.de>
---
drivers/gpu/drm/i915/display/intel_fb_pin.c | 2 +-
.../gpu/drm/i915/gem/selftests/huge_pages.c | 2 +-
.../i915/gem/selftests/i915_gem_client_blt.c | 2 +-
.../drm/i915/gem/selftests/i915_gem_mman.c | 6 +++
drivers/gpu/drm/i915/gt/intel_ggtt.c | 45 +++++++++++++++++--
drivers/gpu/drm/i915/i915_gem.c | 2 +
drivers/gpu/drm/i915/i915_vma.c | 27 ++++++++++-
drivers/gpu/drm/i915/i915_vma.h | 1 +
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 22 ++++-----
drivers/gpu/drm/i915/selftests/i915_vma.c | 8 ++--
10 files changed, 94 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c
index e60046d90124..8913254b5dcb 100644
--- a/drivers/gpu/drm/i915/display/intel_fb_pin.c
+++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c
@@ -48,7 +48,7 @@ intel_pin_fb_obj_dpt(struct drm_framebuffer *fb,
goto err;
if (i915_vma_misplaced(vma, 0, alignment, 0)) {
- ret = i915_vma_unbind(vma);
+ ret = i915_vma_unbind_unlocked(vma);
if (ret) {
vma = ERR_PTR(ret);
goto err;
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index 41a020de9feb..8424ee8c5eb8 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -642,7 +642,7 @@ static int igt_mock_ppgtt_misaligned_dma(void *arg)
* pages.
*/
for (offset = 4096; offset < page_size; offset += 4096) {
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
if (err)
goto out_unpin;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
index 71d21aff15b4..8f28e46e8ee5 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
@@ -319,7 +319,7 @@ static int pin_buffer(struct i915_vma *vma, u64 addr)
int err;
if (drm_mm_node_allocated(&vma->node) && vma->node.start != addr) {
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
if (err)
return err;
}
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index dc69de3f2b6d..8ae1a1530bd8 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -168,7 +168,9 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj,
kunmap(p);
out:
+ i915_gem_object_lock(obj, NULL);
__i915_vma_put(vma);
+ i915_gem_object_unlock(obj);
return err;
}
@@ -263,7 +265,9 @@ static int check_partial_mappings(struct drm_i915_gem_object *obj,
if (err)
return err;
+ i915_gem_object_lock(obj, NULL);
__i915_vma_put(vma);
+ i915_gem_object_unlock(obj);
if (igt_timeout(end_time,
"%s: timed out after tiling=%d stride=%d\n",
@@ -1354,7 +1358,9 @@ static int __igt_mmap_revoke(struct drm_i915_private *i915,
* for other objects. Ergo we have to revoke the previous mmap PTE
* access as it no longer points to the same object.
*/
+ i915_gem_object_lock(obj, NULL);
err = i915_gem_object_unbind(obj, I915_GEM_OBJECT_UNBIND_ACTIVE);
+ i915_gem_object_unlock(obj);
if (err) {
pr_err("Failed to unbind object!\n");
goto out_unmap;
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 880fbdcc20e8..8850d4e0f9cc 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -130,22 +130,51 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
drm_WARN_ON(&vm->i915->drm, !vm->is_ggtt && !vm->is_dpt);
+retry:
+ i915_gem_drain_freed_objects(vm->i915);
+
mutex_lock(&vm->mutex);
/* Skip rewriting PTE on VMA unbind. */
open = atomic_xchg(&vm->open, 0);
list_for_each_entry_safe(vma, vn, &vm->bound_list, vm_link) {
+ struct drm_i915_gem_object *obj = vma->obj;
+
GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
- i915_vma_wait_for_bind(vma);
- if (i915_vma_is_pinned(vma))
+ if (i915_vma_is_pinned(vma) || !i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND))
continue;
+ /* unlikely to race when GPU is idle, so no worry about slowpath.. */
+ if (WARN_ON(!i915_gem_object_trylock(obj, NULL))) {
+ /*
+ * No dead objects should appear here, GPU should be
+ * completely idle, and userspace suspended
+ */
+ i915_gem_object_get(obj);
+
+ atomic_set(&vm->open, open);
+ mutex_unlock(&vm->mutex);
+
+ i915_gem_object_lock(obj, NULL);
+ open = i915_vma_unbind(vma);
+ i915_gem_object_unlock(obj);
+
+ GEM_WARN_ON(open);
+
+ i915_gem_object_put(obj);
+ goto retry;
+ }
+
if (!i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND)) {
+ i915_vma_wait_for_bind(vma);
+
__i915_vma_evict(vma, false);
drm_mm_remove_node(&vma->node);
}
+
+ i915_gem_object_unlock(obj);
}
vm->clear_range(vm, 0, vm->total);
@@ -747,11 +776,21 @@ static void ggtt_cleanup_hw(struct i915_ggtt *ggtt)
atomic_set(&ggtt->vm.open, 0);
flush_workqueue(ggtt->vm.i915->wq);
+ i915_gem_drain_freed_objects(ggtt->vm.i915);
mutex_lock(&ggtt->vm.mutex);
- list_for_each_entry_safe(vma, vn, &ggtt->vm.bound_list, vm_link)
+ list_for_each_entry_safe(vma, vn, &ggtt->vm.bound_list, vm_link) {
+ struct drm_i915_gem_object *obj = vma->obj;
+ bool trylock;
+
+ trylock = i915_gem_object_trylock(obj, NULL);
+ WARN_ON(!trylock);
+
WARN_ON(__i915_vma_unbind(vma));
+ if (trylock)
+ i915_gem_object_unlock(obj);
+ }
if (drm_mm_node_allocated(&ggtt->error_capture))
drm_mm_remove_node(&ggtt->error_capture);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7dfebcc6063f..2e10187cd0a0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -123,6 +123,8 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
struct i915_vma *vma;
int ret;
+ assert_object_held(obj);
+
if (list_empty(&obj->vma.list))
return 0;
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index f5a5fee0a76c..e40da26cb518 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1681,8 +1681,16 @@ void i915_vma_parked(struct intel_gt *gt)
struct drm_i915_gem_object *obj = vma->obj;
struct i915_address_space *vm = vma->vm;
- INIT_LIST_HEAD(&vma->closed_link);
- __i915_vma_put(vma);
+ if (i915_gem_object_trylock(obj, NULL)) {
+ INIT_LIST_HEAD(&vma->closed_link);
+ __i915_vma_put(vma);
+ i915_gem_object_unlock(obj);
+ } else {
+ /* back you go.. */
+ spin_lock_irq(>->closed_lock);
+ list_add(&vma->closed_link, >->closed_vma);
+ spin_unlock_irq(>->closed_lock);
+ }
i915_gem_object_put(obj);
i915_vm_close(vm);
@@ -1801,6 +1809,7 @@ struct dma_fence *__i915_vma_evict(struct i915_vma *vma, bool async)
struct dma_fence *unbind_fence;
GEM_BUG_ON(i915_vma_is_pinned(vma));
+ assert_object_held_shared(vma->obj);
if (i915_vma_is_map_and_fenceable(vma)) {
/* Force a pagefault for domain tracking on next user access */
@@ -1867,6 +1876,7 @@ int __i915_vma_unbind(struct i915_vma *vma)
int ret;
lockdep_assert_held(&vma->vm->mutex);
+ assert_object_held_shared(vma->obj);
if (!drm_mm_node_allocated(&vma->node))
return 0;
@@ -1933,6 +1943,8 @@ int i915_vma_unbind(struct i915_vma *vma)
intel_wakeref_t wakeref = 0;
int err;
+ assert_object_held_shared(vma->obj);
+
/* Optimistic wait before taking the mutex */
err = i915_vma_sync(vma);
if (err)
@@ -2025,6 +2037,17 @@ int i915_vma_unbind_async(struct i915_vma *vma, bool trylock_vm)
return err;
}
+int i915_vma_unbind_unlocked(struct i915_vma *vma)
+{
+ int err;
+
+ i915_gem_object_lock(vma->obj, NULL);
+ err = i915_vma_unbind(vma);
+ i915_gem_object_unlock(vma->obj);
+
+ 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 a560bae04e7e..011af044ad4f 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -217,6 +217,7 @@ 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);
+int __must_check i915_vma_unbind_unlocked(struct i915_vma *vma);
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/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 518d11dc50d1..e7e6c4b2c81d 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -387,7 +387,7 @@ static void close_object_list(struct list_head *objects,
vma = i915_vma_instance(obj, vm, NULL);
if (!IS_ERR(vma))
- ignored = i915_vma_unbind(vma);
+ ignored = i915_vma_unbind_unlocked(vma);
list_del(&obj->st_link);
i915_gem_object_put(obj);
@@ -498,7 +498,7 @@ static int fill_hole(struct i915_address_space *vm,
goto err;
}
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
if (err) {
pr_err("%s(%s) (forward) unbind of vma.node=%llx + %llx failed with err=%d\n",
__func__, p->name, vma->node.start, vma->node.size,
@@ -571,7 +571,7 @@ static int fill_hole(struct i915_address_space *vm,
goto err;
}
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
if (err) {
pr_err("%s(%s) (backward) unbind of vma.node=%llx + %llx failed with err=%d\n",
__func__, p->name, vma->node.start, vma->node.size,
@@ -657,7 +657,7 @@ static int walk_hole(struct i915_address_space *vm,
goto err_put;
}
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
if (err) {
pr_err("%s unbind failed at %llx + %llx with err=%d\n",
__func__, addr, vma->size, err);
@@ -734,13 +734,13 @@ static int pot_hole(struct i915_address_space *vm,
pr_err("%s incorrect at %llx + %llx\n",
__func__, addr, vma->size);
i915_vma_unpin(vma);
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
err = -EINVAL;
goto err_obj;
}
i915_vma_unpin(vma);
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
GEM_BUG_ON(err);
}
@@ -834,13 +834,13 @@ static int drunk_hole(struct i915_address_space *vm,
pr_err("%s incorrect at %llx + %llx\n",
__func__, addr, BIT_ULL(size));
i915_vma_unpin(vma);
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
err = -EINVAL;
goto err_obj;
}
i915_vma_unpin(vma);
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
GEM_BUG_ON(err);
if (igt_timeout(end_time,
@@ -908,7 +908,7 @@ static int __shrink_hole(struct i915_address_space *vm,
pr_err("%s incorrect at %llx + %llx\n",
__func__, addr, size);
i915_vma_unpin(vma);
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
err = -EINVAL;
break;
}
@@ -1482,7 +1482,7 @@ static int igt_gtt_reserve(void *arg)
goto out;
}
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
if (err) {
pr_err("i915_vma_unbind failed with err=%d!\n", err);
goto out;
@@ -1678,7 +1678,7 @@ static int igt_gtt_insert(void *arg)
GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
offset = vma->node.start;
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
if (err) {
pr_err("i915_vma_unbind failed with err=%d!\n", err);
goto out;
diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c
index 753b133ee4c6..6921ba128015 100644
--- a/drivers/gpu/drm/i915/selftests/i915_vma.c
+++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
@@ -341,7 +341,7 @@ static int igt_vma_pin1(void *arg)
if (!err) {
i915_vma_unpin(vma);
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
if (err) {
pr_err("Failed to unbind single page from GGTT, err=%d\n", err);
goto out;
@@ -692,7 +692,7 @@ static int igt_vma_rotate_remap(void *arg)
}
i915_vma_unpin(vma);
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
if (err) {
pr_err("Unbinding returned %i\n", err);
goto out_object;
@@ -853,7 +853,7 @@ static int igt_vma_partial(void *arg)
i915_vma_unpin(vma);
nvma++;
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
if (err) {
pr_err("Unbinding returned %i\n", err);
goto out_object;
@@ -892,7 +892,7 @@ static int igt_vma_partial(void *arg)
i915_vma_unpin(vma);
- err = i915_vma_unbind(vma);
+ err = i915_vma_unbind_unlocked(vma);
if (err) {
pr_err("Unbinding returned %i\n", err);
goto out_object;
--
2.38.1