drm/i915: Remove kref from i915_sw_fence
authorChris Wilson <chris@chris-wilson.co.uk>
Wed, 17 May 2017 12:09:56 +0000 (13:09 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Wed, 17 May 2017 12:38:01 +0000 (13:38 +0100)
My original intention was for i915_sw_fence to be the base class and
provide the reference count for the container. This was from starting
with a design to handle async_work. In practice, for i915 we embed
fences into structs which have their own independent reference counting,
making the i915_sw_fence.kref duplicitous. If we remove the kref, we
remove the i915_sw_fence's ability to free itself and its independence,
it can only exist within a container and must be supplied with a
callback to handle its release.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170517121007.27224-1-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/i915_sw_fence.c
drivers/gpu/drm/i915/i915_sw_fence.h

index a277f8e..a0a690d 100644 (file)
@@ -120,34 +120,6 @@ void i915_sw_fence_fini(struct i915_sw_fence *fence)
 }
 #endif
 
-static void i915_sw_fence_release(struct kref *kref)
-{
-       struct i915_sw_fence *fence = container_of(kref, typeof(*fence), kref);
-
-       WARN_ON(atomic_read(&fence->pending) > 0);
-       debug_fence_destroy(fence);
-
-       if (fence->flags & I915_SW_FENCE_MASK) {
-               __i915_sw_fence_notify(fence, FENCE_FREE);
-       } else {
-               i915_sw_fence_fini(fence);
-               kfree(fence);
-       }
-}
-
-static void i915_sw_fence_put(struct i915_sw_fence *fence)
-{
-       debug_fence_assert(fence);
-       kref_put(&fence->kref, i915_sw_fence_release);
-}
-
-static struct i915_sw_fence *i915_sw_fence_get(struct i915_sw_fence *fence)
-{
-       debug_fence_assert(fence);
-       kref_get(&fence->kref);
-       return fence;
-}
-
 static void __i915_sw_fence_wake_up_all(struct i915_sw_fence *fence,
                                        struct list_head *continuation)
 {
@@ -202,13 +174,15 @@ static void __i915_sw_fence_complete(struct i915_sw_fence *fence,
 
        debug_fence_set_state(fence, DEBUG_FENCE_IDLE, DEBUG_FENCE_NOTIFY);
 
-       if (fence->flags & I915_SW_FENCE_MASK &&
-           __i915_sw_fence_notify(fence, FENCE_COMPLETE) != NOTIFY_DONE)
+       if (__i915_sw_fence_notify(fence, FENCE_COMPLETE) != NOTIFY_DONE)
                return;
 
        debug_fence_set_state(fence, DEBUG_FENCE_NOTIFY, DEBUG_FENCE_IDLE);
 
        __i915_sw_fence_wake_up_all(fence, continuation);
+
+       debug_fence_destroy(fence);
+       __i915_sw_fence_notify(fence, FENCE_FREE);
 }
 
 static void i915_sw_fence_complete(struct i915_sw_fence *fence)
@@ -232,33 +206,26 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence,
                          const char *name,
                          struct lock_class_key *key)
 {
-       BUG_ON((unsigned long)fn & ~I915_SW_FENCE_MASK);
+       BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK);
 
        debug_fence_init(fence);
 
        __init_waitqueue_head(&fence->wait, name, key);
-       kref_init(&fence->kref);
        atomic_set(&fence->pending, 1);
        fence->flags = (unsigned long)fn;
 }
 
-static void __i915_sw_fence_commit(struct i915_sw_fence *fence)
-{
-       i915_sw_fence_complete(fence);
-       i915_sw_fence_put(fence);
-}
-
 void i915_sw_fence_commit(struct i915_sw_fence *fence)
 {
        debug_fence_activate(fence);
-       __i915_sw_fence_commit(fence);
+       i915_sw_fence_complete(fence);
 }
 
 static int i915_sw_fence_wake(wait_queue_t *wq, unsigned mode, int flags, void *key)
 {
        list_del(&wq->task_list);
        __i915_sw_fence_complete(wq->private, key);
-       i915_sw_fence_put(wq->private);
+
        if (wq->flags & I915_SW_FENCE_FLAG_ALLOC)
                kfree(wq);
        return 0;
@@ -353,7 +320,7 @@ static int __i915_sw_fence_await_sw_fence(struct i915_sw_fence *fence,
        INIT_LIST_HEAD(&wq->task_list);
        wq->flags = pending;
        wq->func = i915_sw_fence_wake;
-       wq->private = i915_sw_fence_get(fence);
+       wq->private = fence;
 
        i915_sw_fence_await(fence);
 
@@ -402,7 +369,7 @@ static void timer_i915_sw_fence_wake(unsigned long data)
        dma_fence_put(cb->dma);
        cb->dma = NULL;
 
-       __i915_sw_fence_commit(cb->fence);
+       i915_sw_fence_complete(cb->fence);
        cb->timer.function = NULL;
 }
 
@@ -413,7 +380,7 @@ static void dma_i915_sw_fence_wake(struct dma_fence *dma,
 
        del_timer_sync(&cb->timer);
        if (cb->timer.function)
-               __i915_sw_fence_commit(cb->fence);
+               i915_sw_fence_complete(cb->fence);
        dma_fence_put(cb->dma);
 
        kfree(cb);
@@ -440,7 +407,7 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,
                return dma_fence_wait(dma, false);
        }
 
-       cb->fence = i915_sw_fence_get(fence);
+       cb->fence = fence;
        i915_sw_fence_await(fence);
 
        cb->dma = NULL;
index d31cefb..1d3b605 100644 (file)
@@ -23,7 +23,6 @@ struct reservation_object;
 struct i915_sw_fence {
        wait_queue_head_t wait;
        unsigned long flags;
-       struct kref kref;
        atomic_t pending;
 };