drm/i915: Provide an i915_active.acquire callback
authorChris Wilson <chris@chris-wilson.co.uk>
Fri, 21 Jun 2019 18:38:00 +0000 (19:38 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Fri, 21 Jun 2019 18:47:55 +0000 (19:47 +0100)
If we introduce a callback for i915_active that is only called the first
time we use the i915_active and is symmetrically paired with the
i915_active.retire callback, we can replace the open-coded and
non-atomic implementations -- which will be very fragile (i.e. broken)
upon removing the struct_mutex serialisation.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190621183801.23252-4-chris@chris-wilson.co.uk
13 files changed:
drivers/gpu/drm/i915/gem/i915_gem_context.c
drivers/gpu/drm/i915/gt/intel_context.c
drivers/gpu/drm/i915/gt/intel_context.h
drivers/gpu/drm/i915/gt/intel_lrc.c
drivers/gpu/drm/i915/gt/intel_ringbuffer.c
drivers/gpu/drm/i915/gt/intel_timeline.c
drivers/gpu/drm/i915/gt/mock_engine.c
drivers/gpu/drm/i915/i915_active.c
drivers/gpu/drm/i915/i915_active.h
drivers/gpu/drm/i915/i915_active_types.h
drivers/gpu/drm/i915/i915_gem_gtt.c
drivers/gpu/drm/i915/i915_vma.c
drivers/gpu/drm/i915/selftests/i915_active.c

index 628673d1d7f85d1a2982586e5c8f7e4d0403e5d8..8a9787cf0cd05718b9625b992cf2f23d394f0848 100644 (file)
@@ -923,8 +923,12 @@ static int context_barrier_task(struct i915_gem_context *ctx,
        if (!cb)
                return -ENOMEM;
 
-       i915_active_init(i915, &cb->base, cb_retire);
-       i915_active_acquire(&cb->base);
+       i915_active_init(i915, &cb->base, NULL, cb_retire);
+       err = i915_active_acquire(&cb->base);
+       if (err) {
+               kfree(cb);
+               return err;
+       }
 
        for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
                struct i915_request *rq;
index 23120901c55f410140f0b6029547de14014bb96e..938dd032b8201da25c89cb6d26f2329c49a0f1d5 100644 (file)
@@ -95,11 +95,15 @@ void intel_context_unpin(struct intel_context *ce)
        intel_context_put(ce);
 }
 
-static int __context_pin_state(struct i915_vma *vma, unsigned long flags)
+static int __context_pin_state(struct i915_vma *vma)
 {
+       u64 flags;
        int err;
 
-       err = i915_vma_pin(vma, 0, 0, flags | PIN_GLOBAL);
+       flags = i915_ggtt_pin_bias(vma) | PIN_OFFSET_BIAS;
+       flags |= PIN_HIGH | PIN_GLOBAL;
+
+       err = i915_vma_pin(vma, 0, 0, flags);
        if (err)
                return err;
 
@@ -119,7 +123,7 @@ static void __context_unpin_state(struct i915_vma *vma)
        __i915_vma_unpin(vma);
 }
 
-static void intel_context_retire(struct i915_active *active)
+static void __intel_context_retire(struct i915_active *active)
 {
        struct intel_context *ce = container_of(active, typeof(*ce), active);
 
@@ -130,35 +134,11 @@ static void intel_context_retire(struct i915_active *active)
        intel_context_put(ce);
 }
 
-void
-intel_context_init(struct intel_context *ce,
-                  struct i915_gem_context *ctx,
-                  struct intel_engine_cs *engine)
-{
-       GEM_BUG_ON(!engine->cops);
-
-       kref_init(&ce->ref);
-
-       ce->gem_context = ctx;
-       ce->engine = engine;
-       ce->ops = engine->cops;
-       ce->sseu = engine->sseu;
-
-       INIT_LIST_HEAD(&ce->signal_link);
-       INIT_LIST_HEAD(&ce->signals);
-
-       mutex_init(&ce->pin_mutex);
-
-       i915_active_init(ctx->i915, &ce->active, intel_context_retire);
-}
-
-int intel_context_active_acquire(struct intel_context *ce, unsigned long flags)
+static int __intel_context_active(struct i915_active *active)
 {
+       struct intel_context *ce = container_of(active, typeof(*ce), active);
        int err;
 
-       if (!i915_active_acquire(&ce->active))
-               return 0;
-
        intel_context_get(ce);
 
        err = intel_ring_pin(ce->ring);
@@ -168,7 +148,7 @@ int intel_context_active_acquire(struct intel_context *ce, unsigned long flags)
        if (!ce->state)
                return 0;
 
-       err = __context_pin_state(ce->state, flags);
+       err = __context_pin_state(ce->state);
        if (err)
                goto err_ring;
 
@@ -188,15 +168,30 @@ err_ring:
        intel_ring_unpin(ce->ring);
 err_put:
        intel_context_put(ce);
-       i915_active_cancel(&ce->active);
        return err;
 }
 
-void intel_context_active_release(struct intel_context *ce)
+void
+intel_context_init(struct intel_context *ce,
+                  struct i915_gem_context *ctx,
+                  struct intel_engine_cs *engine)
 {
-       /* Nodes preallocated in intel_context_active() */
-       i915_active_acquire_barrier(&ce->active);
-       i915_active_release(&ce->active);
+       GEM_BUG_ON(!engine->cops);
+
+       kref_init(&ce->ref);
+
+       ce->gem_context = ctx;
+       ce->engine = engine;
+       ce->ops = engine->cops;
+       ce->sseu = engine->sseu;
+
+       INIT_LIST_HEAD(&ce->signal_link);
+       INIT_LIST_HEAD(&ce->signals);
+
+       mutex_init(&ce->pin_mutex);
+
+       i915_active_init(ctx->i915, &ce->active,
+                        __intel_context_active, __intel_context_retire);
 }
 
 static void i915_global_context_shrink(void)
index a47275bc4f0101e506d4ed740fb261b3a0c04bfa..40cd8320fcc39b3310b943cc40a3f7d7315eeaf6 100644 (file)
@@ -9,6 +9,7 @@
 
 #include <linux/lockdep.h>
 
+#include "i915_active.h"
 #include "intel_context_types.h"
 #include "intel_engine_types.h"
 
@@ -102,8 +103,17 @@ static inline void intel_context_exit(struct intel_context *ce)
                ce->ops->exit(ce);
 }
 
-int intel_context_active_acquire(struct intel_context *ce, unsigned long flags);
-void intel_context_active_release(struct intel_context *ce);
+static inline int intel_context_active_acquire(struct intel_context *ce)
+{
+       return i915_active_acquire(&ce->active);
+}
+
+static inline void intel_context_active_release(struct intel_context *ce)
+{
+       /* Nodes preallocated in intel_context_active() */
+       i915_active_acquire_barrier(&ce->active);
+       i915_active_release(&ce->active);
+}
 
 static inline struct intel_context *intel_context_get(struct intel_context *ce)
 {
index ad7638da785d4ef3a889ca8b96cf7b22e29a380e..c8a0c9b327646433be99e5bf8e63d36f6a60f5f3 100644 (file)
@@ -1542,12 +1542,10 @@ __execlists_context_pin(struct intel_context *ce,
                goto err;
        GEM_BUG_ON(!ce->state);
 
-       ret = intel_context_active_acquire(ce,
-                                          engine->i915->ggtt.pin_bias |
-                                          PIN_OFFSET_BIAS |
-                                          PIN_HIGH);
+       ret = intel_context_active_acquire(ce);
        if (ret)
                goto err;
+       GEM_BUG_ON(!i915_vma_is_pinned(ce->state));
 
        vaddr = i915_gem_object_pin_map(ce->state->obj,
                                        i915_coherent_map_type(engine->i915) |
index c9337e4b5ee0845b1d0decd1672da27dd1c73eb8..f094406dcc56c5ceb8b6bc985c26fcc7f1a00273 100644 (file)
@@ -1455,7 +1455,7 @@ static int ring_context_pin(struct intel_context *ce)
                ce->state = vma;
        }
 
-       err = intel_context_active_acquire(ce, PIN_HIGH);
+       err = intel_context_active_acquire(ce);
        if (err)
                return err;
 
index 44273b7c96f8ff21bfdf3dc8e335c5efb08d9b3f..47825827498679b82380f003d36b216e3d16cdc0 100644 (file)
@@ -146,6 +146,15 @@ static void __cacheline_retire(struct i915_active *active)
                __idle_cacheline_free(cl);
 }
 
+static int __cacheline_active(struct i915_active *active)
+{
+       struct intel_timeline_cacheline *cl =
+               container_of(active, typeof(*cl), active);
+
+       __i915_vma_pin(cl->hwsp->vma);
+       return 0;
+}
+
 static struct intel_timeline_cacheline *
 cacheline_alloc(struct intel_timeline_hwsp *hwsp, unsigned int cacheline)
 {
@@ -168,15 +177,16 @@ cacheline_alloc(struct intel_timeline_hwsp *hwsp, unsigned int cacheline)
        cl->hwsp = hwsp;
        cl->vaddr = page_pack_bits(vaddr, cacheline);
 
-       i915_active_init(hwsp->gt->i915, &cl->active, __cacheline_retire);
+       i915_active_init(hwsp->gt->i915, &cl->active,
+                        __cacheline_active, __cacheline_retire);
 
        return cl;
 }
 
 static void cacheline_acquire(struct intel_timeline_cacheline *cl)
 {
-       if (cl && i915_active_acquire(&cl->active))
-               __i915_vma_pin(cl->hwsp->vma);
+       if (cl)
+               i915_active_acquire(&cl->active);
 }
 
 static void cacheline_release(struct intel_timeline_cacheline *cl)
index bf0974b12f3d6c811aa337e0b31e571bd3f48fdd..490ebd121f4c99878b18d324352d77344d8fbe05 100644 (file)
@@ -155,7 +155,7 @@ static int mock_context_pin(struct intel_context *ce)
                        return -ENOMEM;
        }
 
-       ret = intel_context_active_acquire(ce, PIN_HIGH);
+       ret = intel_context_active_acquire(ce);
        if (ret)
                return ret;
 
index eb91a625c71fdad0d7b786e3d120c1f44119d264..cb6a1eadf7df82e926685492cbee690cd22876a2 100644 (file)
@@ -39,7 +39,7 @@ static void *active_debug_hint(void *addr)
 {
        struct i915_active *ref = addr;
 
-       return (void *)ref->retire ?: (void *)ref;
+       return (void *)ref->active ?: (void *)ref->retire ?: (void *)ref;
 }
 
 static struct debug_obj_descr active_debug_desc = {
@@ -83,50 +83,58 @@ static inline void debug_active_assert(struct i915_active *ref) { }
 #endif
 
 static void
-__active_park(struct i915_active *ref)
+__active_retire(struct i915_active *ref)
 {
        struct active_node *it, *n;
+       struct rb_root root;
+       bool retire = false;
+
+       lockdep_assert_held(&ref->mutex);
+
+       /* return the unused nodes to our slabcache -- flushing the allocator */
+       if (atomic_dec_and_test(&ref->count)) {
+               debug_active_deactivate(ref);
+               root = ref->tree;
+               ref->tree = RB_ROOT;
+               ref->cache = NULL;
+               retire = true;
+       }
 
-       rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
+       mutex_unlock(&ref->mutex);
+       if (!retire)
+               return;
+
+       ref->retire(ref);
+
+       rbtree_postorder_for_each_entry_safe(it, n, &root, node) {
                GEM_BUG_ON(i915_active_request_isset(&it->base));
                kmem_cache_free(global.slab_cache, it);
        }
-       ref->tree = RB_ROOT;
 }
 
 static void
-__active_retire(struct i915_active *ref)
+active_retire(struct i915_active *ref)
 {
-       GEM_BUG_ON(!ref->count);
-       if (--ref->count)
+       GEM_BUG_ON(!atomic_read(&ref->count));
+       if (atomic_add_unless(&ref->count, -1, 1))
                return;
 
-       debug_active_deactivate(ref);
-
-       /* return the unused nodes to our slabcache */
-       __active_park(ref);
-
-       ref->retire(ref);
+       /* One active may be flushed from inside the acquire of another */
+       mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING);
+       __active_retire(ref);
 }
 
 static void
 node_retire(struct i915_active_request *base, struct i915_request *rq)
 {
-       __active_retire(container_of(base, struct active_node, base)->ref);
-}
-
-static void
-last_retire(struct i915_active_request *base, struct i915_request *rq)
-{
-       __active_retire(container_of(base, struct i915_active, last));
+       active_retire(container_of(base, struct active_node, base)->ref);
 }
 
 static struct i915_active_request *
 active_instance(struct i915_active *ref, u64 idx)
 {
-       struct active_node *node;
+       struct active_node *node, *prealloc;
        struct rb_node **p, *parent;
-       struct i915_request *old;
 
        /*
         * We track the most recently used timeline to skip a rbtree search
@@ -134,20 +142,18 @@ active_instance(struct i915_active *ref, u64 idx)
         * at all. We can reuse the last slot if it is empty, that is
         * after the previous activity has been retired, or if it matches the
         * current timeline.
-        *
-        * Note that we allow the timeline to be active simultaneously in
-        * the rbtree and the last cache. We do this to avoid having
-        * to search and replace the rbtree element for a new timeline, with
-        * the cost being that we must be aware that the ref may be retired
-        * twice for the same timeline (as the older rbtree element will be
-        * retired before the new request added to last).
         */
-       old = i915_active_request_raw(&ref->last, BKL(ref));
-       if (!old || old->fence.context == idx)
-               goto out;
+       node = READ_ONCE(ref->cache);
+       if (node && node->timeline == idx)
+               return &node->base;
 
-       /* Move the currently active fence into the rbtree */
-       idx = old->fence.context;
+       /* Preallocate a replacement, just in case */
+       prealloc = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
+       if (!prealloc)
+               return NULL;
+
+       mutex_lock(&ref->mutex);
+       GEM_BUG_ON(i915_active_is_idle(ref));
 
        parent = NULL;
        p = &ref->tree.rb_node;
@@ -155,8 +161,10 @@ active_instance(struct i915_active *ref, u64 idx)
                parent = *p;
 
                node = rb_entry(parent, struct active_node, node);
-               if (node->timeline == idx)
-                       goto replace;
+               if (node->timeline == idx) {
+                       kmem_cache_free(global.slab_cache, prealloc);
+                       goto out;
+               }
 
                if (node->timeline < idx)
                        p = &parent->rb_right;
@@ -164,17 +172,7 @@ active_instance(struct i915_active *ref, u64 idx)
                        p = &parent->rb_left;
        }
 
-       node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
-
-       /* kmalloc may retire the ref->last (thanks shrinker)! */
-       if (unlikely(!i915_active_request_raw(&ref->last, BKL(ref)))) {
-               kmem_cache_free(global.slab_cache, node);
-               goto out;
-       }
-
-       if (unlikely(!node))
-               return ERR_PTR(-ENOMEM);
-
+       node = prealloc;
        i915_active_request_init(&node->base, NULL, node_retire);
        node->ref = ref;
        node->timeline = idx;
@@ -182,40 +180,29 @@ active_instance(struct i915_active *ref, u64 idx)
        rb_link_node(&node->node, parent, p);
        rb_insert_color(&node->node, &ref->tree);
 
-replace:
-       /*
-        * Overwrite the previous active slot in the rbtree with last,
-        * leaving last zeroed. If the previous slot is still active,
-        * we must be careful as we now only expect to receive one retire
-        * callback not two, and so much undo the active counting for the
-        * overwritten slot.
-        */
-       if (i915_active_request_isset(&node->base)) {
-               /* Retire ourselves from the old rq->active_list */
-               __list_del_entry(&node->base.link);
-               ref->count--;
-               GEM_BUG_ON(!ref->count);
-       }
-       GEM_BUG_ON(list_empty(&ref->last.link));
-       list_replace_init(&ref->last.link, &node->base.link);
-       node->base.request = fetch_and_zero(&ref->last.request);
-
 out:
-       return &ref->last;
+       ref->cache = node;
+       mutex_unlock(&ref->mutex);
+
+       return &node->base;
 }
 
-void i915_active_init(struct drm_i915_private *i915,
-                     struct i915_active *ref,
-                     void (*retire)(struct i915_active *ref))
+void __i915_active_init(struct drm_i915_private *i915,
+                       struct i915_active *ref,
+                       int (*active)(struct i915_active *ref),
+                       void (*retire)(struct i915_active *ref),
+                       struct lock_class_key *key)
 {
        debug_active_init(ref);
 
        ref->i915 = i915;
+       ref->active = active;
        ref->retire = retire;
        ref->tree = RB_ROOT;
-       i915_active_request_init(&ref->last, NULL, last_retire);
+       ref->cache = NULL;
        init_llist_head(&ref->barriers);
-       ref->count = 0;
+       atomic_set(&ref->count, 0);
+       __mutex_init(&ref->mutex, "i915_active", key);
 }
 
 int i915_active_ref(struct i915_active *ref,
@@ -223,68 +210,84 @@ int i915_active_ref(struct i915_active *ref,
                    struct i915_request *rq)
 {
        struct i915_active_request *active;
-       int err = 0;
+       int err;
 
        /* Prevent reaping in case we malloc/wait while building the tree */
-       i915_active_acquire(ref);
+       err = i915_active_acquire(ref);
+       if (err)
+               return err;
 
        active = active_instance(ref, timeline);
-       if (IS_ERR(active)) {
-               err = PTR_ERR(active);
+       if (!active) {
+               err = -ENOMEM;
                goto out;
        }
 
        if (!i915_active_request_isset(active))
-               ref->count++;
+               atomic_inc(&ref->count);
        __i915_active_request_set(active, rq);
 
-       GEM_BUG_ON(!ref->count);
 out:
        i915_active_release(ref);
        return err;
 }
 
-bool i915_active_acquire(struct i915_active *ref)
+int i915_active_acquire(struct i915_active *ref)
 {
+       int err;
+
        debug_active_assert(ref);
-       lockdep_assert_held(BKL(ref));
+       if (atomic_add_unless(&ref->count, 1, 0))
+               return 0;
+
+       err = mutex_lock_interruptible(&ref->mutex);
+       if (err)
+               return err;
 
-       if (ref->count++)
-               return false;
+       if (!atomic_read(&ref->count) && ref->active)
+               err = ref->active(ref);
+       if (!err) {
+               debug_active_activate(ref);
+               atomic_inc(&ref->count);
+       }
+
+       mutex_unlock(&ref->mutex);
 
-       debug_active_activate(ref);
-       return true;
+       return err;
 }
 
 void i915_active_release(struct i915_active *ref)
 {
        debug_active_assert(ref);
-       lockdep_assert_held(BKL(ref));
-
-       __active_retire(ref);
+       active_retire(ref);
 }
 
 int i915_active_wait(struct i915_active *ref)
 {
        struct active_node *it, *n;
-       int ret = 0;
+       int err;
 
-       if (i915_active_acquire(ref))
-               goto out_release;
+       might_sleep();
+       if (RB_EMPTY_ROOT(&ref->tree))
+               return 0;
 
-       ret = i915_active_request_retire(&ref->last, BKL(ref));
-       if (ret)
-               goto out_release;
+       err = mutex_lock_interruptible(&ref->mutex);
+       if (err)
+               return err;
+
+       if (!atomic_add_unless(&ref->count, 1, 0)) {
+               mutex_unlock(&ref->mutex);
+               return 0;
+       }
 
        rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
-               ret = i915_active_request_retire(&it->base, BKL(ref));
-               if (ret)
+               err = i915_active_request_retire(&it->base, BKL(ref));
+               if (err)
                        break;
        }
 
-out_release:
-       i915_active_release(ref);
-       return ret;
+       __active_retire(ref);
+       return err;
 }
 
 int i915_request_await_active_request(struct i915_request *rq,
@@ -299,23 +302,24 @@ int i915_request_await_active_request(struct i915_request *rq,
 int i915_request_await_active(struct i915_request *rq, struct i915_active *ref)
 {
        struct active_node *it, *n;
-       int err = 0;
+       int err;
 
-       /* await allocates and so we need to avoid hitting the shrinker */
-       if (i915_active_acquire(ref))
-               goto out; /* was idle */
+       if (RB_EMPTY_ROOT(&ref->tree))
+               return 0;
 
-       err = i915_request_await_active_request(rq, &ref->last);
+       /* await allocates and so we need to avoid hitting the shrinker */
+       err = i915_active_acquire(ref);
        if (err)
-               goto out;
+               return err;
 
+       mutex_lock(&ref->mutex);
        rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
                err = i915_request_await_active_request(rq, &it->base);
                if (err)
-                       goto out;
+                       break;
        }
+       mutex_unlock(&ref->mutex);
 
-out:
        i915_active_release(ref);
        return err;
 }
@@ -324,9 +328,9 @@ out:
 void i915_active_fini(struct i915_active *ref)
 {
        debug_active_fini(ref);
-       GEM_BUG_ON(i915_active_request_isset(&ref->last));
        GEM_BUG_ON(!RB_EMPTY_ROOT(&ref->tree));
-       GEM_BUG_ON(ref->count);
+       GEM_BUG_ON(atomic_read(&ref->count));
+       mutex_destroy(&ref->mutex);
 }
 #endif
 
@@ -353,7 +357,7 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
                                         (void *)engine, node_retire);
                node->timeline = kctx->ring->timeline->fence_context;
                node->ref = ref;
-               ref->count++;
+               atomic_inc(&ref->count);
 
                intel_engine_pm_get(engine);
                llist_add((struct llist_node *)&node->base.link,
@@ -380,8 +384,9 @@ void i915_active_acquire_barrier(struct i915_active *ref)
 {
        struct llist_node *pos, *next;
 
-       i915_active_acquire(ref);
+       GEM_BUG_ON(i915_active_is_idle(ref));
 
+       mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING);
        llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
                struct intel_engine_cs *engine;
                struct active_node *node;
@@ -411,7 +416,7 @@ void i915_active_acquire_barrier(struct i915_active *ref)
                          &engine->barrier_tasks);
                intel_engine_pm_put(engine);
        }
-       i915_active_release(ref);
+       mutex_unlock(&ref->mutex);
 }
 
 void i915_request_add_barriers(struct i915_request *rq)
index c14eebf6d07477756d4e6d5e97a3b3d53d935232..134166d3125124adfaa82500cda74839c2052c00 100644 (file)
@@ -369,9 +369,16 @@ i915_active_request_retire(struct i915_active_request *active,
  * synchronisation.
  */
 
-void i915_active_init(struct drm_i915_private *i915,
-                     struct i915_active *ref,
-                     void (*retire)(struct i915_active *ref));
+void __i915_active_init(struct drm_i915_private *i915,
+                       struct i915_active *ref,
+                       int (*active)(struct i915_active *ref),
+                       void (*retire)(struct i915_active *ref),
+                       struct lock_class_key *key);
+#define i915_active_init(i915, ref, active, retire) do {               \
+       static struct lock_class_key __key;                             \
+                                                                       \
+       __i915_active_init(i915, ref, active, retire, &__key);          \
+} while (0)
 
 int i915_active_ref(struct i915_active *ref,
                    u64 timeline,
@@ -384,20 +391,14 @@ int i915_request_await_active(struct i915_request *rq,
 int i915_request_await_active_request(struct i915_request *rq,
                                      struct i915_active_request *active);
 
-bool i915_active_acquire(struct i915_active *ref);
-
-static inline void i915_active_cancel(struct i915_active *ref)
-{
-       GEM_BUG_ON(ref->count != 1);
-       ref->count = 0;
-}
-
+int i915_active_acquire(struct i915_active *ref);
 void i915_active_release(struct i915_active *ref);
+void __i915_active_release_nested(struct i915_active *ref, int subclass);
 
 static inline bool
 i915_active_is_idle(const struct i915_active *ref)
 {
-       return !ref->count;
+       return !atomic_read(&ref->count);
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
index c025991b9233e2b92d4b5f9a616a0f131bebc718..5b0a3024ce2421cb5b1a8d573ef1dcff24bf9736 100644 (file)
@@ -7,7 +7,9 @@
 #ifndef _I915_ACTIVE_TYPES_H_
 #define _I915_ACTIVE_TYPES_H_
 
+#include <linux/atomic.h>
 #include <linux/llist.h>
+#include <linux/mutex.h>
 #include <linux/rbtree.h>
 #include <linux/rcupdate.h>
 
@@ -24,13 +26,17 @@ struct i915_active_request {
        i915_active_retire_fn retire;
 };
 
+struct active_node;
+
 struct i915_active {
        struct drm_i915_private *i915;
 
+       struct active_node *cache;
        struct rb_root tree;
-       struct i915_active_request last;
-       unsigned int count;
+       struct mutex mutex;
+       atomic_t count;
 
+       int (*active)(struct i915_active *ref);
        void (*retire)(struct i915_active *ref);
 
        struct llist_head barriers;
index 90f36739765645f42d52b0d99f488efc97507613..ff1d5008a256a00da38520abfb58744561966ad2 100644 (file)
@@ -2055,7 +2055,7 @@ static struct i915_vma *pd_vma_create(struct gen6_ppgtt *ppgtt, int size)
        if (!vma)
                return ERR_PTR(-ENOMEM);
 
-       i915_active_init(i915, &vma->active, NULL);
+       i915_active_init(i915, &vma->active, NULL, NULL);
        INIT_ACTIVE_REQUEST(&vma->last_fence);
 
        vma->vm = &ggtt->vm;
index c13b86e6ef1f97387b98695d27e63b6f021a6695..c20a3022cd80ee227788186d8707793df4af7b9f 100644 (file)
@@ -78,11 +78,20 @@ static void vma_print_allocator(struct i915_vma *vma, const char *reason)
 
 #endif
 
-static void __i915_vma_retire(struct i915_active *ref)
+static inline struct i915_vma *active_to_vma(struct i915_active *ref)
 {
-       struct i915_vma *vma = container_of(ref, typeof(*vma), active);
+       return container_of(ref, typeof(struct i915_vma), active);
+}
 
-       i915_vma_put(vma);
+static int __i915_vma_active(struct i915_active *ref)
+{
+       i915_vma_get(active_to_vma(ref));
+       return 0;
+}
+
+static void __i915_vma_retire(struct i915_active *ref)
+{
+       i915_vma_put(active_to_vma(ref));
 }
 
 static struct i915_vma *
@@ -107,7 +116,8 @@ vma_create(struct drm_i915_gem_object *obj,
        vma->size = obj->base.size;
        vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
 
-       i915_active_init(vm->i915, &vma->active, __i915_vma_retire);
+       i915_active_init(vm->i915, &vma->active,
+                        __i915_vma_active, __i915_vma_retire);
        INIT_ACTIVE_REQUEST(&vma->last_fence);
 
        INIT_LIST_HEAD(&vma->closed_link);
@@ -904,11 +914,7 @@ int i915_vma_move_to_active(struct i915_vma *vma,
         * add the active reference first and queue for it to be dropped
         * *last*.
         */
-       if (i915_active_acquire(&vma->active))
-               i915_vma_get(vma);
-
        err = i915_active_ref(&vma->active, rq->fence.context, rq);
-       i915_active_release(&vma->active);
        if (unlikely(err))
                return err;
 
index 98493bcc91f2960128e6fba723da2a143ef15b8b..84fce379c0defa4600d00f5a1adbfdc73512d00a 100644 (file)
@@ -4,6 +4,8 @@
  * Copyright © 2018 Intel Corporation
  */
 
+#include <linux/kref.h>
+
 #include "gem/i915_gem_pm.h"
 
 #include "i915_selftest.h"
 
 struct live_active {
        struct i915_active base;
+       struct kref ref;
        bool retired;
 };
 
+static void __live_get(struct live_active *active)
+{
+       kref_get(&active->ref);
+}
+
 static void __live_free(struct live_active *active)
 {
        i915_active_fini(&active->base);
        kfree(active);
 }
 
+static void __live_release(struct kref *ref)
+{
+       struct live_active *active = container_of(ref, typeof(*active), ref);
+
+       __live_free(active);
+}
+
+static void __live_put(struct live_active *active)
+{
+       kref_put(&active->ref, __live_release);
+}
+
+static int __live_active(struct i915_active *base)
+{
+       struct live_active *active = container_of(base, typeof(*active), base);
+
+       __live_get(active);
+       return 0;
+}
+
 static void __live_retire(struct i915_active *base)
 {
        struct live_active *active = container_of(base, typeof(*active), base);
 
        active->retired = true;
+       __live_put(active);
 }
 
 static struct live_active *__live_alloc(struct drm_i915_private *i915)
@@ -37,7 +66,8 @@ static struct live_active *__live_alloc(struct drm_i915_private *i915)
        if (!active)
                return NULL;
 
-       i915_active_init(i915, &active->base, __live_retire);
+       kref_init(&active->ref);
+       i915_active_init(i915, &active->base, __live_active, __live_retire);
 
        return active;
 }
@@ -62,11 +92,9 @@ __live_active_setup(struct drm_i915_private *i915)
                return ERR_PTR(-ENOMEM);
        }
 
-       if (!i915_active_acquire(&active->base)) {
-               pr_err("First i915_active_acquire should report being idle\n");
-               err = -EINVAL;
+       err = i915_active_acquire(&active->base);
+       if (err)
                goto out;
-       }
 
        for_each_engine(engine, i915, id) {
                struct i915_request *rq;
@@ -97,18 +125,21 @@ __live_active_setup(struct drm_i915_private *i915)
                pr_err("i915_active retired before submission!\n");
                err = -EINVAL;
        }
-       if (active->base.count != count) {
+       if (atomic_read(&active->base.count) != count) {
                pr_err("i915_active not tracking all requests, found %d, expected %d\n",
-                      active->base.count, count);
+                      atomic_read(&active->base.count), count);
                err = -EINVAL;
        }
 
 out:
        i915_sw_fence_commit(submit);
        heap_fence_put(submit);
+       if (err) {
+               __live_put(active);
+               active = ERR_PTR(err);
+       }
 
-       /* XXX leaks live_active on error */
-       return err ? ERR_PTR(err) : active;
+       return active;
 }
 
 static int live_active_wait(void *arg)
@@ -135,7 +166,7 @@ static int live_active_wait(void *arg)
                err = -EINVAL;
        }
 
-       __live_free(active);
+       __live_put(active);
 
        if (igt_flush_test(i915, I915_WAIT_LOCKED))
                err = -EIO;
@@ -174,7 +205,7 @@ static int live_active_retire(void *arg)
                err = -EINVAL;
        }
 
-       __live_free(active);
+       __live_put(active);
 
 err:
        intel_runtime_pm_put(&i915->runtime_pm, wakeref);