[PATCH 2/7] drm/i915: Don't free shared locks while shared
Thomas Hellström
thomas.hellstrom at linux.intel.com
Tue May 11 13:25:20 UTC 2021
We are currently sharing the VM reservation locks across a number of
gem objects with page-table memory. Since TTM will individiualize the
reservation locks when freeing objects, including accessing the shared
locks, make sure that the shared locks are not freed until that is done.
For PPGTT we add an additional refcount, for GGTT we flush the object
freeing workqueue before freeing the shared lock.
Signed-off-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 3 ++
.../gpu/drm/i915/gem/i915_gem_object_types.h | 1 +
drivers/gpu/drm/i915/gt/intel_ggtt.c | 13 ++++--
drivers/gpu/drm/i915/gt/intel_gtt.c | 45 +++++++++++++++----
drivers/gpu/drm/i915/gt/intel_gtt.h | 29 +++++++++++-
drivers/gpu/drm/i915/gt/intel_ppgtt.c | 2 +-
6 files changed, 80 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 28144410df86..abadf0994ad0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -252,6 +252,9 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
if (obj->mm.n_placements > 1)
kfree(obj->mm.placements);
+ if (obj->resv_shared_from)
+ i915_vm_resv_put(obj->resv_shared_from);
+
/* But keep the pointer alive for RCU-protected lookups */
call_rcu(&obj->rcu, __i915_gem_free_object_rcu);
cond_resched();
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 0727d0c76aa0..450340a73186 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -149,6 +149,7 @@ struct drm_i915_gem_object {
* when i915_gem_ww_ctx_backoff() or i915_gem_ww_ctx_fini() are called.
*/
struct list_head obj_link;
+ struct dma_resv *resv_shared_from;
union {
struct rcu_head rcu;
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 35069ca5d7de..128d781e429f 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -746,7 +746,13 @@ static void ggtt_cleanup_hw(struct i915_ggtt *ggtt)
mutex_unlock(&ggtt->vm.mutex);
i915_address_space_fini(&ggtt->vm);
- dma_resv_fini(&ggtt->vm.resv);
+ /*
+ * Make sure our pagetable gem objects have been freed,
+ * so that nobody shares our reservation object anymore.
+ */
+ i915_gem_flush_free_objects(ggtt->vm.i915);
+ GEM_WARN_ON(kref_read(&ggtt->vm.resv_ref) != 1);
+ dma_resv_fini(&ggtt->vm._resv);
arch_phys_wc_del(ggtt->mtrr);
@@ -829,6 +835,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
return -ENOMEM;
}
+ kref_init(&ggtt->vm.resv_ref);
ret = setup_scratch_page(&ggtt->vm);
if (ret) {
drm_err(&i915->drm, "Scratch setup failed\n");
@@ -1135,7 +1142,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt)
ggtt->vm.gt = gt;
ggtt->vm.i915 = i915;
ggtt->vm.dma = i915->drm.dev;
- dma_resv_init(&ggtt->vm.resv);
+ dma_resv_init(&ggtt->vm._resv);
if (INTEL_GEN(i915) <= 5)
ret = i915_gmch_probe(ggtt);
@@ -1144,7 +1151,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt)
else
ret = gen8_gmch_probe(ggtt);
if (ret) {
- dma_resv_fini(&ggtt->vm.resv);
+ dma_resv_fini(&ggtt->vm._resv);
return ret;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 9b98f9d9faa3..695b22b17644 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -22,8 +22,11 @@ struct drm_i915_gem_object *alloc_pt_lmem(struct i915_address_space *vm, int sz)
* object underneath, with the idea that one object_lock() will lock
* them all at once.
*/
- if (!IS_ERR(obj))
- obj->base.resv = &vm->resv;
+ if (!IS_ERR(obj)) {
+ obj->base.resv = i915_vm_resv_get(vm);
+ obj->resv_shared_from = obj->base.resv;
+ }
+
return obj;
}
@@ -40,8 +43,11 @@ struct drm_i915_gem_object *alloc_pt_dma(struct i915_address_space *vm, int sz)
* object underneath, with the idea that one object_lock() will lock
* them all at once.
*/
- if (!IS_ERR(obj))
- obj->base.resv = &vm->resv;
+ if (!IS_ERR(obj)) {
+ obj->base.resv = i915_vm_resv_get(vm);
+ obj->resv_shared_from = obj->base.resv;
+ }
+
return obj;
}
@@ -102,7 +108,7 @@ void __i915_vm_close(struct i915_address_space *vm)
int i915_vm_lock_objects(struct i915_address_space *vm,
struct i915_gem_ww_ctx *ww)
{
- if (vm->scratch[0]->base.resv == &vm->resv) {
+ if (vm->scratch[0]->base.resv == &vm->_resv) {
return i915_gem_object_lock(vm->scratch[0], ww);
} else {
struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
@@ -118,6 +124,22 @@ void i915_address_space_fini(struct i915_address_space *vm)
mutex_destroy(&vm->mutex);
}
+/**
+ * i915_vm_resv_release - Final struct i915_address_space destructor
+ * @kref: Pointer to the &i915_address_space.resv_ref member.
+ *
+ * This function is called when the last lock sharer no longer shares the
+ * &i915_address_space._resv lock.
+ */
+void i915_vm_resv_release(struct kref *kref)
+{
+ struct i915_address_space *vm =
+ container_of(kref, typeof(*vm), resv_ref);
+
+ dma_resv_fini(&vm->_resv);
+ kfree(vm);
+}
+
static void __i915_vm_release(struct work_struct *work)
{
struct i915_address_space *vm =
@@ -125,9 +147,8 @@ static void __i915_vm_release(struct work_struct *work)
vm->cleanup(vm);
i915_address_space_fini(vm);
- dma_resv_fini(&vm->resv);
- kfree(vm);
+ i915_vm_resv_put(&vm->_resv);
}
void i915_vm_release(struct kref *kref)
@@ -144,6 +165,14 @@ void i915_vm_release(struct kref *kref)
void i915_address_space_init(struct i915_address_space *vm, int subclass)
{
kref_init(&vm->ref);
+
+ /*
+ * Special case for GGTT that has already done an early
+ * kref_init here.
+ */
+ if (!kref_read(&vm->resv_ref))
+ kref_init(&vm->resv_ref);
+
INIT_RCU_WORK(&vm->rcu, __i915_vm_release);
atomic_set(&vm->open, 1);
@@ -170,7 +199,7 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass)
might_alloc(GFP_KERNEL);
mutex_release(&vm->mutex.dep_map, _THIS_IP_);
}
- dma_resv_init(&vm->resv);
+ dma_resv_init(&vm->_resv);
GEM_BUG_ON(!vm->total);
drm_mm_init(&vm->mm, 0, vm->total);
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
index ca00b45827b7..e760d4fadb43 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -245,7 +245,9 @@ struct i915_address_space {
atomic_t open;
struct mutex mutex; /* protects vma and our lists */
- struct dma_resv resv; /* reservation lock for all pd objects, and buffer pool */
+
+ struct kref resv_ref; /* kref to keep the reservation lock alive. */
+ struct dma_resv _resv; /* reservation lock for all pd objects, and buffer pool */
#define VM_CLASS_GGTT 0
#define VM_CLASS_PPGTT 1
#define VM_CLASS_DPT 2
@@ -404,13 +406,38 @@ i915_vm_get(struct i915_address_space *vm)
return vm;
}
+/**
+ * i915_vm_resv_get - Obtain a reference on the vm's reservation lock
+ * @vm: The vm whose reservation lock we want to share.
+ *
+ * Return: A pointer to the vm's reservation lock.
+ */
+static inline struct dma_resv *i915_vm_resv_get(struct i915_address_space *vm)
+{
+ kref_get(&vm->resv_ref);
+ return &vm->_resv;
+}
+
void i915_vm_release(struct kref *kref);
+void i915_vm_resv_release(struct kref *kref);
+
static inline void i915_vm_put(struct i915_address_space *vm)
{
kref_put(&vm->ref, i915_vm_release);
}
+/**
+ * i915_vm_resv_put - Release a reference on the vm's reservation lock
+ * @resv: Pointer to a reservation lock obtained from i915_vm_resv_get()
+ */
+static inline void i915_vm_resv_put(struct dma_resv *resv)
+{
+ struct i915_address_space *vm = container_of(resv, typeof(*vm), _resv);
+
+ kref_put(&vm->resv_ref, i915_vm_resv_release);
+}
+
static inline struct i915_address_space *
i915_vm_open(struct i915_address_space *vm)
{
diff --git a/drivers/gpu/drm/i915/gt/intel_ppgtt.c b/drivers/gpu/drm/i915/gt/intel_ppgtt.c
index 4e3d80c2295c..aee3a8929245 100644
--- a/drivers/gpu/drm/i915/gt/intel_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ppgtt.c
@@ -307,7 +307,7 @@ void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt)
ppgtt->vm.dma = i915->drm.dev;
ppgtt->vm.total = BIT_ULL(INTEL_INFO(i915)->ppgtt_size);
- dma_resv_init(&ppgtt->vm.resv);
+ dma_resv_init(&ppgtt->vm._resv);
i915_address_space_init(&ppgtt->vm, VM_CLASS_PPGTT);
ppgtt->vm.vma_ops.bind_vma = ppgtt_bind_vma;
--
2.30.2
More information about the dri-devel
mailing list