Commit 7dd5c565 authored by Chris Wilson's avatar Chris Wilson Committed by Matthew Auld

drm/i915/gem: Remove shared locking on freeing objects

The obj->base.resv may be shared across many objects, some of which may
still be live and locked, preventing objects from being freed
indefintely. We could individualise the lock during the free, or rely on
a freed object having no contention and being able to immediately free
the pages it owns.

References: https://gitlab.freedesktop.org/drm/intel/-/issues/6469
Fixes: be7612fd ("drm/i915: Require object lock when freeing pages during destruction")
Fixes: 6cb12fbd ("drm/i915: Use trylock instead of blocking lock for __i915_gem_free_objects.")
Cc: <stable@vger.kernel.org> # v5.17+
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Tested-by: default avatarNirmoy Das <nirmoy.das@intel.com>
Acked-by: default avatarNirmoy Das <nirmoy.das@intel.com>
Signed-off-by: default avatarNirmoy Das <nirmoy.das@intel.com>
Reviewed-by: default avatarMatthew Auld <matthew.auld@intel.com>
Signed-off-by: default avatarMatthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220726144844.18429-1-nirmoy.das@intel.com
parent ae5a3d2c
...@@ -268,7 +268,7 @@ static void __i915_gem_object_free_mmaps(struct drm_i915_gem_object *obj) ...@@ -268,7 +268,7 @@ static void __i915_gem_object_free_mmaps(struct drm_i915_gem_object *obj)
*/ */
void __i915_gem_object_pages_fini(struct drm_i915_gem_object *obj) void __i915_gem_object_pages_fini(struct drm_i915_gem_object *obj)
{ {
assert_object_held(obj); assert_object_held_shared(obj);
if (!list_empty(&obj->vma.list)) { if (!list_empty(&obj->vma.list)) {
struct i915_vma *vma; struct i915_vma *vma;
...@@ -331,15 +331,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, ...@@ -331,15 +331,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
continue; continue;
} }
if (!i915_gem_object_trylock(obj, NULL)) {
/* busy, toss it back to the pile */
if (llist_add(&obj->freed, &i915->mm.free_list))
queue_delayed_work(i915->wq, &i915->mm.free_work, msecs_to_jiffies(10));
continue;
}
__i915_gem_object_pages_fini(obj); __i915_gem_object_pages_fini(obj);
i915_gem_object_unlock(obj);
__i915_gem_free_object(obj); __i915_gem_free_object(obj);
/* But keep the pointer alive for RCU-protected lookups */ /* But keep the pointer alive for RCU-protected lookups */
...@@ -359,7 +351,7 @@ void i915_gem_flush_free_objects(struct drm_i915_private *i915) ...@@ -359,7 +351,7 @@ void i915_gem_flush_free_objects(struct drm_i915_private *i915)
static void __i915_gem_free_work(struct work_struct *work) static void __i915_gem_free_work(struct work_struct *work)
{ {
struct drm_i915_private *i915 = struct drm_i915_private *i915 =
container_of(work, struct drm_i915_private, mm.free_work.work); container_of(work, struct drm_i915_private, mm.free_work);
i915_gem_flush_free_objects(i915); i915_gem_flush_free_objects(i915);
} }
...@@ -391,7 +383,7 @@ static void i915_gem_free_object(struct drm_gem_object *gem_obj) ...@@ -391,7 +383,7 @@ static void i915_gem_free_object(struct drm_gem_object *gem_obj)
*/ */
if (llist_add(&obj->freed, &i915->mm.free_list)) if (llist_add(&obj->freed, &i915->mm.free_list))
queue_delayed_work(i915->wq, &i915->mm.free_work, 0); queue_work(i915->wq, &i915->mm.free_work);
} }
void __i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj, void __i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj,
...@@ -745,7 +737,7 @@ bool i915_gem_object_needs_ccs_pages(struct drm_i915_gem_object *obj) ...@@ -745,7 +737,7 @@ bool i915_gem_object_needs_ccs_pages(struct drm_i915_gem_object *obj)
void i915_gem_init__objects(struct drm_i915_private *i915) void i915_gem_init__objects(struct drm_i915_private *i915)
{ {
INIT_DELAYED_WORK(&i915->mm.free_work, __i915_gem_free_work); INIT_WORK(&i915->mm.free_work, __i915_gem_free_work);
} }
void i915_objects_module_exit(void) void i915_objects_module_exit(void)
......
...@@ -254,7 +254,7 @@ struct i915_gem_mm { ...@@ -254,7 +254,7 @@ struct i915_gem_mm {
* List of objects which are pending destruction. * List of objects which are pending destruction.
*/ */
struct llist_head free_list; struct llist_head free_list;
struct delayed_work free_work; struct work_struct free_work;
/** /**
* Count of objects pending destructions. Used to skip needlessly * Count of objects pending destructions. Used to skip needlessly
* waiting on an RCU barrier if no objects are waiting to be freed. * waiting on an RCU barrier if no objects are waiting to be freed.
...@@ -1440,7 +1440,7 @@ static inline void i915_gem_drain_freed_objects(struct drm_i915_private *i915) ...@@ -1440,7 +1440,7 @@ static inline void i915_gem_drain_freed_objects(struct drm_i915_private *i915)
* armed the work again. * armed the work again.
*/ */
while (atomic_read(&i915->mm.free_count)) { while (atomic_read(&i915->mm.free_count)) {
flush_delayed_work(&i915->mm.free_work); flush_work(&i915->mm.free_work);
flush_delayed_work(&i915->bdev.wq); flush_delayed_work(&i915->bdev.wq);
rcu_barrier(); rcu_barrier();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment