On 08/02/2019 13:47, Chris Wilson wrote:
If we allocate while iterating the rbtree of active nodes, we may hit
the shrinker and so retire the i915_active reap the rbtree. Modifying
the rbtree as we iterate is not good behaviour, so acquire the
i915_active first to keep the tree intact whenever we allocate.
Fixes: a42375af0a30 ("drm/i915: Release the active tracker tree upon idling")
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_active.c | 36 +++++++++++++++++++++---------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 215b6ff8aa73..db7bb5bd5add 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -163,17 +163,25 @@ int i915_active_ref(struct i915_active *ref,
struct i915_request *rq)
{
struct i915_active_request *active;
+ int err = 0;
+
+ /* Prevent reaping in case we malloc/wait while building the tree */
+ i915_active_acquire(ref);
active = active_instance(ref, timeline);
- if (IS_ERR(active))
- return PTR_ERR(active);
+ if (IS_ERR(active)) {
+ err = PTR_ERR(active);
+ goto out;
+ }
if (!i915_active_request_isset(active))
ref->count++;
__i915_active_request_set(active, rq);
GEM_BUG_ON(!ref->count);
- return 0;
+out:
+ i915_active_release(ref);
+ return err;
}
bool i915_active_acquire(struct i915_active *ref)
@@ -223,19 +231,25 @@ 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 ret;
+ int err = 0;
- ret = i915_request_await_active_request(rq, &ref->last);
- if (ret)
- return ret;
+ /* await allocates and so we need to avoid hitting the shrinker */
+ if (i915_active_acquire(ref))
+ goto out; /* was idle */
+
+ err = i915_request_await_active_request(rq, &ref->last);
+ if (err)
+ goto out;
rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
- ret = i915_request_await_active_request(rq, &it->base);
- if (ret)
- return ret;
+ err = i915_request_await_active_request(rq, &it->base);
+ if (err)
+ goto out;
}
- return 0;
+out:
+ i915_active_release(ref);
+ return err;
}
#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
