[PATCH 2/4] drm/i915: Don't silently drop i915_active_fence errors
Thomas Hellström
thomas.hellstrom at linux.intel.com
Sat Jun 19 18:00:30 UTC 2021
From: Thomas Hellström <thomas.hellstrom at intel.com>
Introduce a possibility to keep i915_active_fence errors even when
the current dma_fence is signaled. We do this by exchanging the
fence pointer with an error pointer.
Also use these error-propagating struct 915_active_fences for the
struct i915_active's exclusive fence only, since error pointers are
also used by struct i915_active to identify idle_barriers.
Finally, use to make sure we propagate vma binding errors so that
a binding task will either wait for the binding fence or see an
error pointer in __i915_request_bind. The -EAGAIN error code is not
propagated to the next bind.
Signed-off-by: Thomas Hellström <thomas.hellstrom at intel.com>
---
drivers/gpu/drm/i915/i915_active.c | 75 ++++++++++++------------------
drivers/gpu/drm/i915/i915_active.h | 27 +++++++++--
drivers/gpu/drm/i915/i915_vma.c | 12 ++++-
3 files changed, 65 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index b1aa1c482c32..46a62e11476d 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -140,7 +140,7 @@ __active_retire(struct i915_active *ref)
if (!atomic_dec_and_lock_irqsave(&ref->count, &ref->tree_lock, flags))
return;
- GEM_BUG_ON(rcu_access_pointer(ref->excl.fence));
+ GEM_BUG_ON(!IS_ERR_OR_NULL(rcu_access_pointer(ref->excl.fence)));
debug_active_deactivate(ref);
/* Even if we have not used the cache, we may still have a barrier */
@@ -220,6 +220,16 @@ active_fence_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
return cmpxchg(__active_fence_slot(active), fence, NULL) == fence;
}
+static inline bool
+active_fence_cb_err(struct dma_fence *fence, struct dma_fence_cb *cb)
+{
+ struct i915_active_fence *active =
+ container_of(cb, typeof(*active), cb);
+
+ return cmpxchg(__active_fence_slot(active), fence,
+ ERR_PTR(fence->error)) == fence;
+}
+
static void
node_retire(struct dma_fence *fence, struct dma_fence_cb *cb)
{
@@ -230,7 +240,7 @@ node_retire(struct dma_fence *fence, struct dma_fence_cb *cb)
static void
excl_retire(struct dma_fence *fence, struct dma_fence_cb *cb)
{
- if (active_fence_cb(fence, cb))
+ if (active_fence_cb_err(fence, cb))
active_retire(container_of(cb, struct i915_active, excl.cb));
}
@@ -465,14 +475,9 @@ __i915_active_set_fence(struct i915_active *ref,
{
struct dma_fence *prev;
- if (replace_barrier(ref, active)) {
- RCU_INIT_POINTER(active->fence, fence);
- return NULL;
- }
-
rcu_read_lock();
prev = __i915_active_fence_set(active, fence);
- if (prev)
+ if (!IS_ERR_OR_NULL(prev))
prev = dma_fence_get_rcu(prev);
else
__i915_active_acquire(ref);
@@ -481,29 +486,6 @@ __i915_active_set_fence(struct i915_active *ref,
return prev;
}
-static struct i915_active_fence *
-__active_fence(struct i915_active *ref, u64 idx)
-{
- struct active_node *it;
-
- it = __active_lookup(ref, idx);
- if (unlikely(!it)) { /* Contention with parallel tree builders! */
- spin_lock_irq(&ref->tree_lock);
- it = __active_lookup(ref, idx);
- spin_unlock_irq(&ref->tree_lock);
- }
- GEM_BUG_ON(!it); /* slot must be preallocated */
-
- return &it->base;
-}
-
-struct dma_fence *
-__i915_active_ref(struct i915_active *ref, u64 idx, struct dma_fence *fence)
-{
- /* Only valid while active, see i915_active_acquire_for_context() */
- return __i915_active_set_fence(ref, __active_fence(ref, idx), fence);
-}
-
struct dma_fence *
i915_active_set_exclusive(struct i915_active *ref, struct dma_fence *f)
{
@@ -576,15 +558,16 @@ void i915_active_release(struct i915_active *ref)
active_retire(ref);
}
-static void enable_signaling(struct i915_active_fence *active)
+static void enable_signaling(struct i915_active_fence *active,
+ const bool check_barrier)
{
struct dma_fence *fence;
- if (unlikely(is_barrier(active)))
+ if (check_barrier && unlikely(is_barrier(active)))
return;
fence = i915_active_fence_get(active);
- if (!fence)
+ if (IS_ERR_OR_NULL(fence))
return;
dma_fence_enable_sw_signaling(fence);
@@ -611,13 +594,13 @@ static int flush_lazy_signals(struct i915_active *ref)
struct active_node *it, *n;
int err = 0;
- enable_signaling(&ref->excl);
+ enable_signaling(&ref->excl, false);
rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
err = flush_barrier(it); /* unconnected idle barrier? */
if (err)
break;
- enable_signaling(&it->base);
+ enable_signaling(&it->base, true);
}
return err;
@@ -650,16 +633,17 @@ int __i915_active_wait(struct i915_active *ref, int state)
}
static int __await_active(struct i915_active_fence *active,
+ const bool check_barrier,
int (*fn)(void *arg, struct dma_fence *fence),
void *arg)
{
struct dma_fence *fence;
- if (is_barrier(active)) /* XXX flush the barrier? */
+ if (check_barrier && is_barrier(active)) /* XXX flush the barrier? */
return 0;
fence = i915_active_fence_get(active);
- if (fence) {
+ if (!IS_ERR_OR_NULL(fence)) {
int err;
err = fn(arg, fence);
@@ -668,7 +652,7 @@ static int __await_active(struct i915_active_fence *active,
return err;
}
- return 0;
+ return PTR_ERR_OR_ZERO(fence);
}
struct wait_barrier {
@@ -725,7 +709,7 @@ static int await_active(struct i915_active *ref,
if (flags & I915_ACTIVE_AWAIT_EXCL &&
rcu_access_pointer(ref->excl.fence)) {
- err = __await_active(&ref->excl, fn, arg);
+ err = __await_active(&ref->excl, false, fn, arg);
if (err)
goto out;
}
@@ -734,7 +718,7 @@ static int await_active(struct i915_active *ref,
struct active_node *it, *n;
rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
- err = __await_active(&it->base, fn, arg);
+ err = __await_active(&it->base, true, fn, arg);
if (err)
goto out;
}
@@ -1082,7 +1066,7 @@ __i915_active_fence_set(struct i915_active_fence *active,
*/
spin_lock_irqsave(fence->lock, flags);
prev = xchg(__active_fence_slot(active), fence);
- if (prev) {
+ if (!IS_ERR_OR_NULL(prev)) {
GEM_BUG_ON(prev == fence);
spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING);
__list_del_entry(&active->cb.node);
@@ -1103,12 +1087,15 @@ int i915_active_fence_set(struct i915_active_fence *active,
/* Must maintain timeline ordering wrt previous active requests */
rcu_read_lock();
fence = __i915_active_fence_set(active, &rq->fence);
- if (fence) /* but the previous fence may not belong to that timeline! */
+ if (!IS_ERR_OR_NULL(fence))
+ /* but the previous fence may not belong to that timeline! */
fence = dma_fence_get_rcu(fence);
rcu_read_unlock();
- if (fence) {
+ if (!IS_ERR_OR_NULL(fence)) {
err = i915_request_await_dma_fence(rq, fence);
dma_fence_put(fence);
+ } else if (IS_ERR(fence)) {
+ err = PTR_ERR(fence);
}
return err;
diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
index d0feda68b874..ce36f74f2d13 100644
--- a/drivers/gpu/drm/i915/i915_active.h
+++ b/drivers/gpu/drm/i915/i915_active.h
@@ -103,7 +103,26 @@ i915_active_fence_get(struct i915_active_fence *active)
struct dma_fence *fence;
rcu_read_lock();
- fence = dma_fence_get_rcu_safe(&active->fence);
+ /*
+ * See dma_fence_get_rcu_safe, which lacks the
+ * IS_ERR_OR_NULL() test.
+ */
+ do {
+ fence = rcu_dereference(active->fence);
+ if (IS_ERR_OR_NULL(fence))
+ break;
+
+ if (!dma_fence_get_rcu(fence))
+ continue;
+
+ if (fence != rcu_access_pointer(active->fence)) {
+ dma_fence_put(fence);
+ continue;
+ }
+
+ fence = rcu_pointer_handoff(fence);
+ break;
+ } while (1);
rcu_read_unlock();
return fence;
@@ -181,7 +200,7 @@ i915_active_set_exclusive(struct i915_active *ref, struct dma_fence *f);
static inline bool i915_active_has_exclusive(struct i915_active *ref)
{
- return rcu_access_pointer(ref->excl.fence);
+ return !IS_ERR_OR_NULL(rcu_access_pointer(ref->excl.fence));
}
int __i915_active_wait(struct i915_active *ref, int state);
@@ -239,9 +258,11 @@ static inline int __i915_request_await_exclusive(struct i915_request *rq,
int err = 0;
fence = i915_active_fence_get(&active->excl);
- if (fence) {
+ if (!IS_ERR_OR_NULL(fence)) {
err = i915_request_await_dma_fence(rq, fence);
dma_fence_put(fence);
+ } else if (IS_ERR(fence)) {
+ err = PTR_ERR(fence);
}
return err;
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 0f227f28b280..63565057d830 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -407,6 +407,7 @@ int i915_vma_bind(struct i915_vma *vma,
trace_i915_vma_bind(vma, bind_flags);
if (work && bind_flags & vma->vm->bind_async_flags) {
struct dma_fence *prev;
+ int ret = 0;
work->vma = vma;
work->cache_level = cache_level;
@@ -422,14 +423,21 @@ int i915_vma_bind(struct i915_vma *vma,
* execution and not content or object's backing store lifetime.
*/
prev = i915_active_set_exclusive(&vma->active, &work->base.dma);
- if (prev) {
+ if (!IS_ERR_OR_NULL(prev)) {
__i915_sw_fence_await_dma_fence(&work->base.chain,
prev,
&work->cb);
dma_fence_put(prev);
+ } else if (IS_ERR(prev)) {
+ ret = PTR_ERR(prev);
+ if (ret == -EAGAIN)
+ ret = 0;
}
- work->base.dma.error = 0; /* enable the queue_work() */
+ /* Propagate previous error or enable the queue_work() */
+ work->base.dma.error = ret;
+ if (ret)
+ return ret;
if (vma->obj) {
__i915_gem_object_pin_pages(vma->obj);
--
2.31.1
More information about the Intel-gfx-trybot
mailing list