Commit d0024911 authored by Chris Wilson's avatar Chris Wilson

drm/i915: Differentiate between aliasing-ppgtt and ggtt pinning

Userptr causes lockdep to complain when we are using the aliasing-ppgtt
(and ggtt, but for that it is rightfully so to complain about) in that
when we revoke the userptr we take a mutex which we also use to revoke
the mmaps. However, we only revoke mmaps for GGTT bindings and we never
allow userptr to create a GGTT binding so the warning should be false
and is simply caused by our conflation of the aliasing-ppgtt with the
ggtt. So lets try treating the binding into the aliasing-ppgtt as a
separate lockclass from the ggtt. The downside is that we are
deliberately suppressing lockdep;s ability to warn us of cycles.

Closes: https://gitlab.freedesktop.org/drm/intel/issues/478Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarAndi Shyti <andi.shyti@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200326142727.31962-1-chris@chris-wilson.co.uk
parent 0c1abaa7
...@@ -913,11 +913,30 @@ int i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags) ...@@ -913,11 +913,30 @@ int i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
if (flags & PIN_GLOBAL) if (flags & PIN_GLOBAL)
wakeref = intel_runtime_pm_get(&vma->vm->i915->runtime_pm); wakeref = intel_runtime_pm_get(&vma->vm->i915->runtime_pm);
/* No more allocations allowed once we hold vm->mutex */ /*
err = mutex_lock_interruptible(&vma->vm->mutex); * Differentiate between user/kernel vma inside the aliasing-ppgtt.
*
* We conflate the Global GTT with the user's vma when using the
* aliasing-ppgtt, but it is still vitally important to try and
* keep the use cases distinct. For example, userptr objects are
* not allowed inside the Global GTT as that will cause lock
* inversions when we have to evict them the mmu_notifier callbacks -
* but they are allowed to be part of the user ppGTT which can never
* be mapped. As such we try to give the distinct users of the same
* mutex, distinct lockclasses [equivalent to how we keep i915_ggtt
* and i915_ppgtt separate].
*
* NB this may cause us to mask real lock inversions -- while the
* code is safe today, lockdep may not be able to spot future
* transgressions.
*/
err = mutex_lock_interruptible_nested(&vma->vm->mutex,
!(flags & PIN_GLOBAL));
if (err) if (err)
goto err_fence; goto err_fence;
/* No more allocations allowed now we hold vm->mutex */
if (unlikely(i915_vma_is_closed(vma))) { if (unlikely(i915_vma_is_closed(vma))) {
err = -ENOENT; err = -ENOENT;
goto err_unlock; goto err_unlock;
...@@ -1320,7 +1339,7 @@ int i915_vma_unbind(struct i915_vma *vma) ...@@ -1320,7 +1339,7 @@ int i915_vma_unbind(struct i915_vma *vma)
if (err) if (err)
goto out_rpm; goto out_rpm;
err = mutex_lock_interruptible(&vm->mutex); err = mutex_lock_interruptible_nested(&vma->vm->mutex, !wakeref);
if (err) if (err)
goto out_rpm; goto out_rpm;
......
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