Commit 94dec871 authored by Chris Wilson's avatar Chris Wilson

drm/i915: Reorder context-close to avoid calling i915_vma_close() under RCU

When we close the VMA, we unbind it from the ppgtt and tear down the
page directory pointing at it. That may trigger us to return WC pages
back to the system, requiring conversion back to WB which itself may
sleep. That makes i915_vma_close() unsuitable for use inside the RCU
read lock, which we need to hold to iterate the radixtree.

The fix is quite simple, we can close all the VMA as we close the ppgtt,
we only need to do that instead of closing them during destruction of
the LUT.

v2: Order between closing the LUT and the ppgtt is important; we use the
vma inside the LUT as a means of retrieving the object, and so we must
clear the LUT before freeing the VMA when closing the ppgtt.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103638
Fixes: 547da76b ("drm/i915: Hold rcu_read_lock when iterating over the radixtree (vma idr)")
Fixes: d1b48c1e ("drm/i915: Replace execbuf vma ht with an idr")
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171109085540.32264-1-chris@chris-wilson.co.ukReviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
parent b814b074
...@@ -107,14 +107,9 @@ static void lut_close(struct i915_gem_context *ctx) ...@@ -107,14 +107,9 @@ static void lut_close(struct i915_gem_context *ctx)
rcu_read_lock(); rcu_read_lock();
radix_tree_for_each_slot(slot, &ctx->handles_vma, &iter, 0) { radix_tree_for_each_slot(slot, &ctx->handles_vma, &iter, 0) {
struct i915_vma *vma = rcu_dereference_raw(*slot); struct i915_vma *vma = rcu_dereference_raw(*slot);
struct drm_i915_gem_object *obj = vma->obj;
radix_tree_iter_delete(&ctx->handles_vma, &iter, slot); radix_tree_iter_delete(&ctx->handles_vma, &iter, slot);
__i915_gem_object_release_unless_active(vma->obj);
if (!i915_vma_is_ggtt(vma))
i915_vma_close(vma);
__i915_gem_object_release_unless_active(obj);
} }
rcu_read_unlock(); rcu_read_unlock();
} }
...@@ -200,6 +195,11 @@ static void context_close(struct i915_gem_context *ctx) ...@@ -200,6 +195,11 @@ static void context_close(struct i915_gem_context *ctx)
{ {
i915_gem_context_set_closed(ctx); i915_gem_context_set_closed(ctx);
/*
* The LUT uses the VMA as a backpointer to unref the object,
* so we need to clear the LUT before we close all the VMA (inside
* the ppgtt).
*/
lut_close(ctx); lut_close(ctx);
if (ctx->ppgtt) if (ctx->ppgtt)
i915_ppgtt_close(&ctx->ppgtt->base); i915_ppgtt_close(&ctx->ppgtt->base);
......
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