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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 09/11/2017 08:20, Chris Wilson wrote:
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.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103638
Fixes: 547da76b5777 ("drm/i915: Hold rcu_read_lock when iterating over the radixtree (vma idr)")
Fixes: d1b48c1e7184 ("drm/i915: Replace execbuf vma ht with an idr")
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Matthew Auld <matthew.william.auld@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_gem_context.c | 10 +++-------
  1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 10affb35ac56..0e1c4626870e 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -107,14 +107,9 @@ static void lut_close(struct i915_gem_context *ctx)
  	rcu_read_lock();
  	radix_tree_for_each_slot(slot, &ctx->handles_vma, &iter, 0) {
  		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);
-
-		if (!i915_vma_is_ggtt(vma))
-			i915_vma_close(vma);
-
-		__i915_gem_object_release_unless_active(obj);
+		__i915_gem_object_release_unless_active(vma->obj);
  	}
  	rcu_read_unlock();
  }
@@ -200,10 +195,11 @@ static void context_close(struct i915_gem_context *ctx)
  {
  	i915_gem_context_set_closed(ctx);
- lut_close(ctx);
  	if (ctx->ppgtt)
  		i915_ppgtt_close(&ctx->ppgtt->base);
+ lut_close(ctx);
+
  	ctx->file_priv = ERR_PTR(-EBADF);
  	i915_gem_context_put(ctx);
  }


Looks straightforward enough. Was a bit unsure about how the pre-ppgtt world works but that too looks fine.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux