| From 2826d447fbd60e6a05e53d5f918bceb8c04e315c Mon Sep 17 00:00:00 2001 |
| From: Chris Wilson <chris@chris-wilson.co.uk> |
| Date: Tue, 26 Jul 2022 16:48:44 +0200 |
| Subject: drm/i915/gem: Remove shared locking on freeing objects |
| |
| From: Chris Wilson <chris@chris-wilson.co.uk> |
| |
| commit 2826d447fbd60e6a05e53d5f918bceb8c04e315c upstream. |
| |
| 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: be7612fd6665 ("drm/i915: Require object lock when freeing pages during destruction") |
| Fixes: 6cb12fbda1c2 ("drm/i915: Use trylock instead of blocking lock for __i915_gem_free_objects.") |
| Cc: <stable@vger.kernel.org> # v5.17+ |
| Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> |
| Tested-by: Nirmoy Das <nirmoy.das@intel.com> |
| Acked-by: Nirmoy Das <nirmoy.das@intel.com> |
| Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> |
| Reviewed-by: Matthew Auld <matthew.auld@intel.com> |
| Signed-off-by: Matthew Auld <matthew.auld@intel.com> |
| Link: https://patchwork.freedesktop.org/patch/msgid/20220726144844.18429-1-nirmoy.das@intel.com |
| (cherry picked from commit 7dd5c56531eb03696acdb17774721de5ef481c0b) |
| Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/gpu/drm/i915/gem/i915_gem_object.c | 16 ++++------------ |
| drivers/gpu/drm/i915/i915_drv.h | 4 ++-- |
| 2 files changed, 6 insertions(+), 14 deletions(-) |
| |
| --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c |
| +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c |
| @@ -268,7 +268,7 @@ static void __i915_gem_object_free_mmaps |
| */ |
| 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)) { |
| struct i915_vma *vma; |
| @@ -331,15 +331,7 @@ static void __i915_gem_free_objects(stru |
| 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_unlock(obj); |
| __i915_gem_free_object(obj); |
| |
| /* But keep the pointer alive for RCU-protected lookups */ |
| @@ -359,7 +351,7 @@ void i915_gem_flush_free_objects(struct |
| static void __i915_gem_free_work(struct work_struct *work) |
| { |
| 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); |
| } |
| @@ -391,7 +383,7 @@ static void i915_gem_free_object(struct |
| */ |
| |
| 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, |
| @@ -719,7 +711,7 @@ bool i915_gem_object_placement_possible( |
| |
| 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) |
| --- a/drivers/gpu/drm/i915/i915_drv.h |
| +++ b/drivers/gpu/drm/i915/i915_drv.h |
| @@ -254,7 +254,7 @@ struct i915_gem_mm { |
| * List of objects which are pending destruction. |
| */ |
| struct llist_head free_list; |
| - struct delayed_work free_work; |
| + struct work_struct free_work; |
| /** |
| * Count of objects pending destructions. Used to skip needlessly |
| * waiting on an RCU barrier if no objects are waiting to be freed. |
| @@ -1415,7 +1415,7 @@ static inline void i915_gem_drain_freed_ |
| * armed the work again. |
| */ |
| 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); |
| rcu_barrier(); |
| } |