On 5/18/21 1:18 PM, Maarten Lankhorst wrote:
Op 18-05-2021 om 10:26 schreef Thomas Hellström:
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 take additional
measures to make sure objects sharing the GGTT reservation lock are
freed at GGTT takedown
Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
---
v2: Try harder to make sure objects sharing the GGTT reservation lock are
freed at GGTT takedown.
---
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 | 19 ++++++--
drivers/gpu/drm/i915/gt/intel_gtt.c | 45 +++++++++++++++----
drivers/gpu/drm/i915/gt/intel_gtt.h | 30 ++++++++++++-
drivers/gpu/drm/i915/gt/intel_ppgtt.c | 2 +-
drivers/gpu/drm/i915/i915_drv.c | 5 +++
7 files changed, 92 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;
Since this can only be a vm object, would it make sense to make this a pointer to the vm address space, so we can call vm_resv_put on it directly?
The current pointer type and name makes it look generic, but if you try to use it with anything but an address space, it will blow up.
Otherwise looks good. I guess we cannot force all bo's to be deleted before the vm is freed. :-)
So with that fixed
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
Thanks, I'll take a look at that.
Thomas
