[PATCH 58/60] drm/i915: Remove struct_mutex protected active reference
Chris Wilson
chris at chris-wilson.co.uk
Mon Jul 2 11:24:55 UTC 2018
Instead of tracking when to add an active reference, refuse to free
objects that are still active, instead deferring them to the zombie list
for a later pass.
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gvt/scheduler.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 35 ++++++++++++-------
drivers/gpu/drm/i915/i915_gem_batch_pool.c | 2 +-
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_gem_object.h | 30 ----------------
drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +-
drivers/gpu/drm/i915/i915_vma.c | 24 ++++++-------
drivers/gpu/drm/i915/intel_engine_cs.c | 2 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--
drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +-
.../drm/i915/selftests/i915_gem_coherency.c | 4 +--
.../gpu/drm/i915/selftests/i915_gem_context.c | 2 +-
.../gpu/drm/i915/selftests/i915_gem_object.c | 1 -
drivers/gpu/drm/i915/selftests/i915_request.c | 8 -----
.../gpu/drm/i915/selftests/intel_hangcheck.c | 10 ------
drivers/gpu/drm/i915/selftests/intel_lrc.c | 10 ------
.../drm/i915/selftests/intel_workarounds.c | 3 --
18 files changed, 44 insertions(+), 100 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 16c244f5cbd5..95e41a2a776b 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -560,7 +560,7 @@ static void release_shadow_batch_buffer(struct intel_vgpu_workload *workload)
i915_vma_unpin(bb->vma);
i915_vma_close(bb->vma);
}
- __i915_gem_object_release_unless_active(bb->obj);
+ i915_gem_object_put(bb->obj);
}
list_del(&bb->list);
kfree(bb);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 398da8014430..788681af6ec8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -944,6 +944,7 @@ struct i915_gem_mm {
* List of objects which are pending destruction.
*/
struct llist_head free_list;
+ struct llist_head zombie_list;
struct work_struct free_work;
spinlock_t free_lock;
/**
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ee514490d7a6..ee4ad468c715 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -51,6 +51,7 @@
#include "intel_workarounds.h"
static void i915_gem_flush_free_objects(struct drm_i915_private *i915);
+static void free_zombies(struct drm_i915_private *i915);
static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj)
{
@@ -162,6 +163,9 @@ static u32 __i915_gem_park(struct drm_i915_private *i915)
i915_pmu_gt_parked(i915);
i915_vma_parked(i915);
+ free_zombies(i915);
+ GEM_BUG_ON(!llist_empty(&i915->mm.zombie_list));
+
i915->gt.awake = false;
if (INTEL_GEN(i915) >= 6)
@@ -3194,7 +3198,7 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file)
list_del(&lut->ctx_link);
kmem_cache_free(i915->luts, lut);
- __i915_gem_object_release_unless_active(obj);
+ i915_gem_object_put(obj);
}
mutex_unlock(&i915->drm.struct_mutex);
@@ -4364,9 +4368,13 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
llist_for_each_entry_safe(obj, on, freed, freed) {
struct i915_vma *vma, *vn;
+ if (i915_gem_object_is_active(obj)) {
+ llist_add(&obj->freed, &i915->mm.zombie_list);
+ continue;
+ }
+
trace_i915_gem_object_destroy(obj);
- GEM_BUG_ON(i915_gem_object_is_active(obj));
list_for_each_entry_safe(vma, vn,
&obj->vma.list, obj_link) {
GEM_BUG_ON(i915_vma_is_active(vma));
@@ -4438,6 +4446,15 @@ static void i915_gem_flush_free_objects(struct drm_i915_private *i915)
}
}
+static void free_zombies(struct drm_i915_private *i915)
+{
+ struct llist_node *freed;
+
+ freed = llist_del_all(&i915->mm.zombie_list);
+ if (freed)
+ __i915_gem_free_objects(i915, freed);
+}
+
static void __i915_gem_free_work(struct work_struct *work)
{
struct drm_i915_private *i915 =
@@ -4452,6 +4469,7 @@ static void __i915_gem_free_work(struct work_struct *work)
* the GTT either for the user or for scanout). Those VMA still need to
* unbound now.
*/
+ free_zombies(i915);
spin_lock(&i915->mm.free_lock);
while ((freed = llist_del_all(&i915->mm.free_list))) {
@@ -4515,17 +4533,6 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
call_rcu(&obj->rcu, __i915_gem_free_object_rcu);
}
-void __i915_gem_object_release_unless_active(struct drm_i915_gem_object *obj)
-{
- lockdep_assert_held(&obj->base.dev->struct_mutex);
-
- if (!i915_gem_object_has_active_reference(obj) &&
- i915_gem_object_is_active(obj))
- i915_gem_object_set_active_reference(obj);
- else
- i915_gem_object_put(obj);
-}
-
void i915_gem_sanitize(struct drm_i915_private *i915)
{
int err;
@@ -5178,6 +5185,7 @@ static void i915_gem_init__mm(struct drm_i915_private *i915)
spin_lock_init(&i915->mm.free_lock);
init_llist_head(&i915->mm.free_list);
+ init_llist_head(&i915->mm.zombie_list);
INIT_LIST_HEAD(&i915->mm.purge_list);
INIT_LIST_HEAD(&i915->mm.unbound_list);
@@ -5261,6 +5269,7 @@ void i915_gem_cleanup_early(struct drm_i915_private *dev_priv)
{
i915_gem_drain_freed_objects(dev_priv);
GEM_BUG_ON(!llist_empty(&dev_priv->mm.free_list));
+ GEM_BUG_ON(!llist_empty(&dev_priv->mm.zombie_list));
GEM_BUG_ON(atomic_read(&dev_priv->mm.free_count));
WARN_ON(dev_priv->mm.shrink_count);
WARN_ON(!list_empty(&dev_priv->gt.timelines));
diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
index f3890b664e3f..56adfdcaed3e 100644
--- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c
+++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
@@ -55,7 +55,7 @@ void i915_gem_batch_pool_fini(struct i915_gem_batch_pool *pool)
list_for_each_entry_safe(obj, next,
&pool->cache_list[n],
batch_pool_link)
- __i915_gem_object_release_unless_active(obj);
+ i915_gem_object_put(obj);
INIT_LIST_HEAD(&pool->cache_list[n]);
}
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index d6a350269684..f394adc9f078 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -114,7 +114,7 @@ static void lut_close(struct i915_gem_context *ctx)
radix_tree_iter_delete(&ctx->handles_vma, &iter, slot);
vma->open_count--;
- __i915_gem_object_release_unless_active(vma->obj);
+ i915_gem_object_put(vma->obj);
}
rcu_read_unlock();
}
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index dfdd365b6163..5d153e0f1601 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -138,14 +138,6 @@ struct drm_i915_gem_object {
struct list_head batch_pool_link;
I915_SELFTEST_DECLARE(struct list_head st_link);
- unsigned long flags;
-
- /**
- * Have we taken a reference for the object for incomplete GPU
- * activity?
- */
-#define I915_BO_ACTIVE_REF 0
-
/*
* Is the object to be mapped as read-only to the GPU
* Only honoured if hardware has relevant pte bit
@@ -404,28 +396,6 @@ i915_gem_object_is_active(const struct drm_i915_gem_object *obj)
return obj->active_count;
}
-static inline bool
-i915_gem_object_has_active_reference(const struct drm_i915_gem_object *obj)
-{
- return test_bit(I915_BO_ACTIVE_REF, &obj->flags);
-}
-
-static inline void
-i915_gem_object_set_active_reference(struct drm_i915_gem_object *obj)
-{
- lockdep_assert_held(&obj->base.dev->struct_mutex);
- __set_bit(I915_BO_ACTIVE_REF, &obj->flags);
-}
-
-static inline void
-i915_gem_object_clear_active_reference(struct drm_i915_gem_object *obj)
-{
- lockdep_assert_held(&obj->base.dev->struct_mutex);
- __clear_bit(I915_BO_ACTIVE_REF, &obj->flags);
-}
-
-void __i915_gem_object_release_unless_active(struct drm_i915_gem_object *obj);
-
static inline bool
i915_gem_object_is_framebuffer(const struct drm_i915_gem_object *obj)
{
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 5f62361cf035..fcc221b260d2 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -239,6 +239,6 @@ int i915_gem_render_state_emit(struct i915_request *rq)
i915_vma_close(so.vma);
mutex_unlock(&ggtt->vm.mutex);
err_obj:
- __i915_gem_object_release_unless_active(so.obj);
+ i915_gem_object_put(so.obj);
return err;
}
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 35ab3d108d3f..fab3a995949e 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -79,9 +79,11 @@ __i915_vma_retire(struct i915_vma *vma, struct i915_request *rq)
if (--vma->active_count)
return;
+ rcu_read_lock();
+
GEM_BUG_ON(!i915_gem_object_is_active(obj));
- if (--obj->active_count)
- return;
+ if (obj->active_count > 1)
+ goto unlock;
/* Prune the shared fence arrays iff completely idle (inc. external) */
if (reservation_object_trylock(obj->resv)) {
@@ -90,7 +92,8 @@ __i915_vma_retire(struct i915_vma *vma, struct i915_request *rq)
reservation_object_unlock(obj->resv);
}
- /* Bump our place on the bound list to keep it roughly in LRU order
+ /*
+ * Bump our place on the bound list to keep it roughly in LRU order
* so that we don't steal from recently used but inactive objects
* (unless we are forced to ofc!)
*/
@@ -103,10 +106,10 @@ __i915_vma_retire(struct i915_vma *vma, struct i915_request *rq)
obj->mm.dirty = true; /* be paranoid */
- if (i915_gem_object_has_active_reference(obj)) {
- i915_gem_object_clear_active_reference(obj);
- i915_gem_object_put(obj);
- }
+ smp_wmb();
+unlock:
+ --obj->active_count;
+ rcu_read_unlock();
}
static void
@@ -427,19 +430,14 @@ void i915_vma_unpin_iomap(struct i915_vma *vma)
void i915_vma_unpin_and_release(struct i915_vma **p_vma)
{
struct i915_vma *vma;
- struct drm_i915_gem_object *obj;
vma = fetch_and_zero(p_vma);
if (!vma)
return;
- obj = vma->obj;
- GEM_BUG_ON(!obj);
-
i915_vma_unpin(vma);
i915_vma_close(vma);
-
- __i915_gem_object_release_unless_active(obj);
+ i915_vma_put(vma);
}
bool i915_vma_misplaced(const struct i915_vma *vma,
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 7ebd6dca26a1..4ae04144c30a 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -555,7 +555,7 @@ static void cleanup_status_page(struct intel_engine_cs *engine)
i915_vma_close(vma);
i915_gem_object_unpin_map(obj);
- __i915_gem_object_release_unless_active(obj);
+ i915_gem_object_put(obj);
}
static int init_status_page(struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 41e6efc662d9..fa947f374e6f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1194,10 +1194,8 @@ intel_engine_create_ring(struct intel_engine_cs *engine,
void
intel_ring_free(struct intel_ring *ring)
{
- struct drm_i915_gem_object *obj = ring->vma->obj;
-
i915_vma_close(ring->vma);
- __i915_gem_object_release_unless_active(obj);
+ i915_vma_put(ring->vma);
i915_timeline_put(ring->timeline);
kfree(ring);
diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c
index 28b47ed677c3..3da7b2aec2d9 100644
--- a/drivers/gpu/drm/i915/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/selftests/huge_pages.c
@@ -991,9 +991,9 @@ static int gpu_write(struct i915_vma *vma,
if (err)
goto err_request;
- i915_gem_object_set_active_reference(batch->obj);
i915_vma_unpin(batch);
i915_vma_close(batch);
+ i915_vma_put(batch);
err = engine->emit_bb_start(rq,
batch->node.start, batch->node.size,
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c b/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c
index 52894bb5a7b0..b29cd40e89fd 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c
@@ -357,7 +357,7 @@ static int igt_gem_coherency(void *arg)
}
}
- __i915_gem_object_release_unless_active(obj);
+ i915_gem_object_put(obj);
}
}
}
@@ -368,7 +368,7 @@ static int igt_gem_coherency(void *arg)
return err;
put_object:
- __i915_gem_object_release_unless_active(obj);
+ i915_gem_object_put(obj);
goto unlock;
}
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index cde26923b797..e981c34ff03b 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -183,9 +183,9 @@ static int gpu_fill(struct drm_i915_gem_object *obj,
if (err)
goto skip_request;
- i915_gem_object_set_active_reference(batch->obj);
i915_vma_unpin(batch);
i915_vma_close(batch);
+ i915_vma_put(batch);
i915_vma_unpin(vma);
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/selftests/i915_gem_object.c
index e82e23690c21..7bd740351210 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_object.c
@@ -464,7 +464,6 @@ static int make_obj_busy(struct drm_i915_gem_object *obj)
i915_request_add(rq);
- __i915_gem_object_release_unless_active(obj);
i915_vma_unpin(vma);
return err;
diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c
index 3d663fb30d93..2148239ae0fc 100644
--- a/drivers/gpu/drm/i915/selftests/i915_request.c
+++ b/drivers/gpu/drm/i915/selftests/i915_request.c
@@ -682,11 +682,6 @@ static int live_all_engines(void *arg)
GEM_BUG_ON(err);
request[id]->batch = batch;
- if (!i915_gem_object_has_active_reference(batch->obj)) {
- i915_gem_object_get(batch->obj);
- i915_gem_object_set_active_reference(batch->obj);
- }
-
i915_vma_lock(batch);
err = i915_vma_move_to_active(batch, request[id], 0);
i915_vma_unlock(batch);
@@ -808,9 +803,6 @@ static int live_sequential_engines(void *arg)
i915_vma_unlock(batch);
GEM_BUG_ON(err);
- i915_gem_object_set_active_reference(batch->obj);
- i915_vma_get(batch);
-
i915_request_get(request[id]);
i915_request_add(request[id]);
diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
index 31d0e428dcb7..2a12a6279520 100644
--- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
@@ -139,22 +139,12 @@ static int emit_recurse_batch(struct hang *h,
if (err)
goto unpin_hws;
- if (!i915_gem_object_has_active_reference(vma->obj)) {
- i915_gem_object_get(vma->obj);
- i915_gem_object_set_active_reference(vma->obj);
- }
-
i915_vma_lock(hws);
err = i915_vma_move_to_active(hws, rq, 0);
i915_vma_unlock(hws);
if (err)
goto unpin_hws;
- if (!i915_gem_object_has_active_reference(hws->obj)) {
- i915_gem_object_get(hws->obj);
- i915_gem_object_set_active_reference(hws->obj);
- }
-
batch = h->batch;
if (INTEL_GEN(i915) >= 8) {
*batch++ = MI_STORE_DWORD_IMM_GEN4;
diff --git a/drivers/gpu/drm/i915/selftests/intel_lrc.c b/drivers/gpu/drm/i915/selftests/intel_lrc.c
index 0149025f583b..f11b906f6230 100644
--- a/drivers/gpu/drm/i915/selftests/intel_lrc.c
+++ b/drivers/gpu/drm/i915/selftests/intel_lrc.c
@@ -115,22 +115,12 @@ static int emit_recurse_batch(struct spinner *spin,
if (err)
goto unpin_hws;
- if (!i915_gem_object_has_active_reference(vma->obj)) {
- i915_gem_object_get(vma->obj);
- i915_gem_object_set_active_reference(vma->obj);
- }
-
i915_vma_lock(hws);
err = i915_vma_move_to_active(hws, rq, 0);
i915_vma_unlock(hws);
if (err)
goto unpin_hws;
- if (!i915_gem_object_has_active_reference(hws->obj)) {
- i915_gem_object_get(hws->obj);
- i915_gem_object_set_active_reference(hws->obj);
- }
-
batch = spin->batch;
*batch++ = MI_STORE_DWORD_IMM_GEN4;
diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
index 3662ff569f5b..d3b1f96cbb39 100644
--- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
@@ -80,9 +80,6 @@ read_nonprivs(struct i915_gem_context *ctx, struct intel_engine_cs *engine)
reservation_object_add_excl_fence(vma->resv, &rq->fence);
reservation_object_unlock(vma->resv);
- i915_gem_object_get(result);
- i915_gem_object_set_active_reference(result);
-
i915_request_add(rq);
i915_vma_unpin(vma);
--
2.18.0
More information about the Intel-gfx-trybot
mailing list