Commit fc158405 authored by Chris Wilson's avatar Chris Wilson

drm/i915: Integrate i915_sw_fence with debugobjects

Add the tracking required to enable debugobjects for fences to improve
error detection in BAT. The debugobject interface lets us track the
lifetime and phases of the fences even while being embedded into larger
structs, i.e. to check they are not used after they have been released.

v2: Don't populate the stubs, debugobjects checks for a NULL pointer and
treats it equivalently.
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20161125131718.20978-4-chris@chris-wilson.co.uk
parent 48bc2a4a
...@@ -22,6 +22,7 @@ config DRM_I915_DEBUG ...@@ -22,6 +22,7 @@ config DRM_I915_DEBUG
select X86_MSR # used by igt/pm_rpm select X86_MSR # used by igt/pm_rpm
select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks) select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks)
select DRM_DEBUG_MM if DRM=y select DRM_DEBUG_MM if DRM=y
select DRM_I915_SW_FENCE_DEBUG_OBJECTS if DRM_I915=y
default n default n
help help
Choose this option to turn on extra driver debugging that may affect Choose this option to turn on extra driver debugging that may affect
...@@ -43,3 +44,15 @@ config DRM_I915_DEBUG_GEM ...@@ -43,3 +44,15 @@ config DRM_I915_DEBUG_GEM
If in doubt, say "N". If in doubt, say "N".
config DRM_I915_SW_FENCE_DEBUG_OBJECTS
bool "Enable additional driver debugging for fence objects"
depends on DRM_I915=y
select DEBUG_OBJECTS
default n
help
Choose this option to turn on extra driver debugging that may affect
performance but will catch some internal issues.
Recommended for driver developers only.
If in doubt, say "N".
...@@ -62,6 +62,15 @@ static void i915_fence_release(struct dma_fence *fence) ...@@ -62,6 +62,15 @@ static void i915_fence_release(struct dma_fence *fence)
{ {
struct drm_i915_gem_request *req = to_request(fence); struct drm_i915_gem_request *req = to_request(fence);
/* The request is put onto a RCU freelist (i.e. the address
* is immediately reused), mark the fences as being freed now.
* Otherwise the debugobjects for the fences are only marked as
* freed when the slab cache itself is freed, and so we would get
* caught trying to reuse dead objects.
*/
i915_sw_fence_fini(&req->submit);
i915_sw_fence_fini(&req->execute);
kmem_cache_free(req->i915->requests, req); kmem_cache_free(req->i915->requests, req);
} }
......
...@@ -17,6 +17,92 @@ ...@@ -17,6 +17,92 @@
static DEFINE_SPINLOCK(i915_sw_fence_lock); static DEFINE_SPINLOCK(i915_sw_fence_lock);
enum {
DEBUG_FENCE_IDLE = 0,
DEBUG_FENCE_NOTIFY,
};
#ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
static void *i915_sw_fence_debug_hint(void *addr)
{
return (void *)(((struct i915_sw_fence *)addr)->flags & I915_SW_FENCE_MASK);
}
static struct debug_obj_descr i915_sw_fence_debug_descr = {
.name = "i915_sw_fence",
.debug_hint = i915_sw_fence_debug_hint,
};
static inline void debug_fence_init(struct i915_sw_fence *fence)
{
debug_object_init(fence, &i915_sw_fence_debug_descr);
}
static inline void debug_fence_activate(struct i915_sw_fence *fence)
{
debug_object_activate(fence, &i915_sw_fence_debug_descr);
}
static inline void debug_fence_set_state(struct i915_sw_fence *fence,
int old, int new)
{
debug_object_active_state(fence, &i915_sw_fence_debug_descr, old, new);
}
static inline void debug_fence_deactivate(struct i915_sw_fence *fence)
{
debug_object_deactivate(fence, &i915_sw_fence_debug_descr);
}
static inline void debug_fence_destroy(struct i915_sw_fence *fence)
{
debug_object_destroy(fence, &i915_sw_fence_debug_descr);
}
static inline void debug_fence_free(struct i915_sw_fence *fence)
{
debug_object_free(fence, &i915_sw_fence_debug_descr);
}
static inline void debug_fence_assert(struct i915_sw_fence *fence)
{
debug_object_assert_init(fence, &i915_sw_fence_debug_descr);
}
#else
static inline void debug_fence_init(struct i915_sw_fence *fence)
{
}
static inline void debug_fence_activate(struct i915_sw_fence *fence)
{
}
static inline void debug_fence_set_state(struct i915_sw_fence *fence,
int old, int new)
{
}
static inline void debug_fence_deactivate(struct i915_sw_fence *fence)
{
}
static inline void debug_fence_destroy(struct i915_sw_fence *fence)
{
}
static inline void debug_fence_free(struct i915_sw_fence *fence)
{
}
static inline void debug_fence_assert(struct i915_sw_fence *fence)
{
}
#endif
static int __i915_sw_fence_notify(struct i915_sw_fence *fence, static int __i915_sw_fence_notify(struct i915_sw_fence *fence,
enum i915_sw_fence_notify state) enum i915_sw_fence_notify state)
{ {
...@@ -26,25 +112,37 @@ static int __i915_sw_fence_notify(struct i915_sw_fence *fence, ...@@ -26,25 +112,37 @@ static int __i915_sw_fence_notify(struct i915_sw_fence *fence,
return fn(fence, state); return fn(fence, state);
} }
static void i915_sw_fence_free(struct kref *kref) #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
void i915_sw_fence_fini(struct i915_sw_fence *fence)
{
debug_fence_free(fence);
}
#endif
static void i915_sw_fence_release(struct kref *kref)
{ {
struct i915_sw_fence *fence = container_of(kref, typeof(*fence), kref); struct i915_sw_fence *fence = container_of(kref, typeof(*fence), kref);
WARN_ON(atomic_read(&fence->pending) > 0); WARN_ON(atomic_read(&fence->pending) > 0);
debug_fence_destroy(fence);
if (fence->flags & I915_SW_FENCE_MASK) if (fence->flags & I915_SW_FENCE_MASK) {
__i915_sw_fence_notify(fence, FENCE_FREE); __i915_sw_fence_notify(fence, FENCE_FREE);
else } else {
i915_sw_fence_fini(fence);
kfree(fence); kfree(fence);
}
} }
static void i915_sw_fence_put(struct i915_sw_fence *fence) static void i915_sw_fence_put(struct i915_sw_fence *fence)
{ {
kref_put(&fence->kref, i915_sw_fence_free); 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) static struct i915_sw_fence *i915_sw_fence_get(struct i915_sw_fence *fence)
{ {
debug_fence_assert(fence);
kref_get(&fence->kref); kref_get(&fence->kref);
return fence; return fence;
} }
...@@ -56,6 +154,7 @@ static void __i915_sw_fence_wake_up_all(struct i915_sw_fence *fence, ...@@ -56,6 +154,7 @@ static void __i915_sw_fence_wake_up_all(struct i915_sw_fence *fence,
wait_queue_t *pos, *next; wait_queue_t *pos, *next;
unsigned long flags; unsigned long flags;
debug_fence_deactivate(fence);
atomic_set_release(&fence->pending, -1); /* 0 -> -1 [done] */ atomic_set_release(&fence->pending, -1); /* 0 -> -1 [done] */
/* /*
...@@ -88,23 +187,33 @@ static void __i915_sw_fence_wake_up_all(struct i915_sw_fence *fence, ...@@ -88,23 +187,33 @@ static void __i915_sw_fence_wake_up_all(struct i915_sw_fence *fence,
} while (1); } while (1);
} }
spin_unlock_irqrestore(&x->lock, flags); spin_unlock_irqrestore(&x->lock, flags);
debug_fence_assert(fence);
} }
static void __i915_sw_fence_complete(struct i915_sw_fence *fence, static void __i915_sw_fence_complete(struct i915_sw_fence *fence,
struct list_head *continuation) struct list_head *continuation)
{ {
debug_fence_assert(fence);
if (!atomic_dec_and_test(&fence->pending)) if (!atomic_dec_and_test(&fence->pending))
return; return;
debug_fence_set_state(fence, DEBUG_FENCE_IDLE, DEBUG_FENCE_NOTIFY);
if (fence->flags & I915_SW_FENCE_MASK && if (fence->flags & I915_SW_FENCE_MASK &&
__i915_sw_fence_notify(fence, FENCE_COMPLETE) != NOTIFY_DONE) __i915_sw_fence_notify(fence, FENCE_COMPLETE) != NOTIFY_DONE)
return; return;
debug_fence_set_state(fence, DEBUG_FENCE_NOTIFY, DEBUG_FENCE_IDLE);
__i915_sw_fence_wake_up_all(fence, continuation); __i915_sw_fence_wake_up_all(fence, continuation);
} }
static void i915_sw_fence_complete(struct i915_sw_fence *fence) static void i915_sw_fence_complete(struct i915_sw_fence *fence)
{ {
debug_fence_assert(fence);
if (WARN_ON(i915_sw_fence_done(fence))) if (WARN_ON(i915_sw_fence_done(fence)))
return; return;
...@@ -113,6 +222,7 @@ static void i915_sw_fence_complete(struct i915_sw_fence *fence) ...@@ -113,6 +222,7 @@ static void i915_sw_fence_complete(struct i915_sw_fence *fence)
static void i915_sw_fence_await(struct i915_sw_fence *fence) static void i915_sw_fence_await(struct i915_sw_fence *fence)
{ {
debug_fence_assert(fence);
WARN_ON(atomic_inc_return(&fence->pending) <= 1); WARN_ON(atomic_inc_return(&fence->pending) <= 1);
} }
...@@ -123,18 +233,26 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence, ...@@ -123,18 +233,26 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence,
{ {
BUG_ON((unsigned long)fn & ~I915_SW_FENCE_MASK); BUG_ON((unsigned long)fn & ~I915_SW_FENCE_MASK);
debug_fence_init(fence);
__init_waitqueue_head(&fence->wait, name, key); __init_waitqueue_head(&fence->wait, name, key);
kref_init(&fence->kref); kref_init(&fence->kref);
atomic_set(&fence->pending, 1); atomic_set(&fence->pending, 1);
fence->flags = (unsigned long)fn; fence->flags = (unsigned long)fn;
} }
void i915_sw_fence_commit(struct i915_sw_fence *fence) static void __i915_sw_fence_commit(struct i915_sw_fence *fence)
{ {
i915_sw_fence_complete(fence); i915_sw_fence_complete(fence);
i915_sw_fence_put(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);
}
static int i915_sw_fence_wake(wait_queue_t *wq, unsigned mode, int flags, void *key) static int i915_sw_fence_wake(wait_queue_t *wq, unsigned mode, int flags, void *key)
{ {
list_del(&wq->task_list); list_del(&wq->task_list);
...@@ -206,9 +324,13 @@ static int __i915_sw_fence_await_sw_fence(struct i915_sw_fence *fence, ...@@ -206,9 +324,13 @@ static int __i915_sw_fence_await_sw_fence(struct i915_sw_fence *fence,
unsigned long flags; unsigned long flags;
int pending; int pending;
debug_fence_assert(fence);
if (i915_sw_fence_done(signaler)) if (i915_sw_fence_done(signaler))
return 0; return 0;
debug_fence_assert(signaler);
/* The dependency graph must be acyclic. */ /* The dependency graph must be acyclic. */
if (unlikely(i915_sw_fence_check_if_after(fence, signaler))) if (unlikely(i915_sw_fence_check_if_after(fence, signaler)))
return -EINVAL; return -EINVAL;
...@@ -279,7 +401,7 @@ static void timer_i915_sw_fence_wake(unsigned long data) ...@@ -279,7 +401,7 @@ static void timer_i915_sw_fence_wake(unsigned long data)
dma_fence_put(cb->dma); dma_fence_put(cb->dma);
cb->dma = NULL; cb->dma = NULL;
i915_sw_fence_commit(cb->fence); __i915_sw_fence_commit(cb->fence);
cb->timer.function = NULL; cb->timer.function = NULL;
} }
...@@ -290,7 +412,7 @@ static void dma_i915_sw_fence_wake(struct dma_fence *dma, ...@@ -290,7 +412,7 @@ static void dma_i915_sw_fence_wake(struct dma_fence *dma,
del_timer_sync(&cb->timer); del_timer_sync(&cb->timer);
if (cb->timer.function) if (cb->timer.function)
i915_sw_fence_commit(cb->fence); __i915_sw_fence_commit(cb->fence);
dma_fence_put(cb->dma); dma_fence_put(cb->dma);
kfree(cb); kfree(cb);
...@@ -304,6 +426,8 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence, ...@@ -304,6 +426,8 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence,
struct i915_sw_dma_fence_cb *cb; struct i915_sw_dma_fence_cb *cb;
int ret; int ret;
debug_fence_assert(fence);
if (dma_fence_is_signaled(dma)) if (dma_fence_is_signaled(dma))
return 0; return 0;
...@@ -349,6 +473,8 @@ int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, ...@@ -349,6 +473,8 @@ int i915_sw_fence_await_reservation(struct i915_sw_fence *fence,
struct dma_fence *excl; struct dma_fence *excl;
int ret = 0, pending; int ret = 0, pending;
debug_fence_assert(fence);
if (write) { if (write) {
struct dma_fence **shared; struct dma_fence **shared;
unsigned int count, i; unsigned int count, i;
......
...@@ -56,6 +56,12 @@ do { \ ...@@ -56,6 +56,12 @@ do { \
__i915_sw_fence_init((fence), (fn), NULL, NULL) __i915_sw_fence_init((fence), (fn), NULL, NULL)
#endif #endif
#ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS
void i915_sw_fence_fini(struct i915_sw_fence *fence);
#else
static inline void i915_sw_fence_fini(struct i915_sw_fence *fence) {}
#endif
void i915_sw_fence_commit(struct i915_sw_fence *fence); void i915_sw_fence_commit(struct i915_sw_fence *fence);
int i915_sw_fence_await_sw_fence(struct i915_sw_fence *fence, int i915_sw_fence_await_sw_fence(struct i915_sw_fence *fence,
......
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