Commit 393afc2c authored by Chris Wilson's avatar Chris Wilson

drm/i915/userptr: Flush cancellations before mmu-notifier invalidate returns

In order to ensure that all invalidations are completed before the
operation returns to userspace (i.e. before the munmap() syscall returns)
we need to wait upon the outstanding operations.

We are allowed to block inside the invalidate_range_start callback, and
as struct_mutex is the inner lock with mmap_sem we can wait upon the
struct_mutex without provoking lockdep into warning about a deadlock.
However, we don't actually want to wait upon outstanding rendering
whilst holding the struct_mutex if we can help it otherwise we also
block other processes from submitting work to the GPU. So first we do a
wait without the lock and then when we reacquire the lock, we double
check that everything is ready for removing the invalidated pages.

Finally to wait upon the outstanding unpinning tasks, we create a
private workqueue as a means to conveniently wait upon all at once. The
drawback is that this workqueue is per-mm, so any threads concurrently
invalidating objects will wait upon each other. The advantage of using
the workqueue is that we can wait in parallel for completion of
rendering and unpinning of several objects (of particular importance if
the process terminates with a whole mm full of objects).

v2: Apply a cup of tea to the changelog.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94699
Testcase: igt/gem_userptr_blits/sync-unmap-cycles
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1459864801-28606-1-git-send-email-chris@chris-wilson.co.ukReviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
parent ba3150ac
...@@ -49,6 +49,7 @@ struct i915_mmu_notifier { ...@@ -49,6 +49,7 @@ struct i915_mmu_notifier {
struct hlist_node node; struct hlist_node node;
struct mmu_notifier mn; struct mmu_notifier mn;
struct rb_root objects; struct rb_root objects;
struct workqueue_struct *wq;
}; };
struct i915_mmu_object { struct i915_mmu_object {
...@@ -60,6 +61,40 @@ struct i915_mmu_object { ...@@ -60,6 +61,40 @@ struct i915_mmu_object {
bool attached; bool attached;
}; };
static void wait_rendering(struct drm_i915_gem_object *obj)
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_gem_request *requests[I915_NUM_ENGINES];
unsigned reset_counter;
int i, n;
if (!obj->active)
return;
n = 0;
for (i = 0; i < I915_NUM_ENGINES; i++) {
struct drm_i915_gem_request *req;
req = obj->last_read_req[i];
if (req == NULL)
continue;
requests[n++] = i915_gem_request_reference(req);
}
reset_counter = atomic_read(&to_i915(dev)->gpu_error.reset_counter);
mutex_unlock(&dev->struct_mutex);
for (i = 0; i < n; i++)
__i915_wait_request(requests[i], reset_counter, false,
NULL, NULL);
mutex_lock(&dev->struct_mutex);
for (i = 0; i < n; i++)
i915_gem_request_unreference(requests[i]);
}
static void cancel_userptr(struct work_struct *work) static void cancel_userptr(struct work_struct *work)
{ {
struct i915_mmu_object *mo = container_of(work, typeof(*mo), work); struct i915_mmu_object *mo = container_of(work, typeof(*mo), work);
...@@ -75,6 +110,8 @@ static void cancel_userptr(struct work_struct *work) ...@@ -75,6 +110,8 @@ static void cancel_userptr(struct work_struct *work)
struct i915_vma *vma, *tmp; struct i915_vma *vma, *tmp;
bool was_interruptible; bool was_interruptible;
wait_rendering(obj);
was_interruptible = dev_priv->mm.interruptible; was_interruptible = dev_priv->mm.interruptible;
dev_priv->mm.interruptible = false; dev_priv->mm.interruptible = false;
...@@ -140,7 +177,7 @@ static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, ...@@ -140,7 +177,7 @@ static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
*/ */
mo = container_of(it, struct i915_mmu_object, it); mo = container_of(it, struct i915_mmu_object, it);
if (kref_get_unless_zero(&mo->obj->base.refcount)) if (kref_get_unless_zero(&mo->obj->base.refcount))
schedule_work(&mo->work); queue_work(mn->wq, &mo->work);
list_add(&mo->link, &cancelled); list_add(&mo->link, &cancelled);
it = interval_tree_iter_next(it, start, end); it = interval_tree_iter_next(it, start, end);
...@@ -148,6 +185,8 @@ static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, ...@@ -148,6 +185,8 @@ static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
list_for_each_entry(mo, &cancelled, link) list_for_each_entry(mo, &cancelled, link)
del_object(mo); del_object(mo);
spin_unlock(&mn->lock); spin_unlock(&mn->lock);
flush_workqueue(mn->wq);
} }
static const struct mmu_notifier_ops i915_gem_userptr_notifier = { static const struct mmu_notifier_ops i915_gem_userptr_notifier = {
...@@ -167,10 +206,16 @@ i915_mmu_notifier_create(struct mm_struct *mm) ...@@ -167,10 +206,16 @@ i915_mmu_notifier_create(struct mm_struct *mm)
spin_lock_init(&mn->lock); spin_lock_init(&mn->lock);
mn->mn.ops = &i915_gem_userptr_notifier; mn->mn.ops = &i915_gem_userptr_notifier;
mn->objects = RB_ROOT; mn->objects = RB_ROOT;
mn->wq = alloc_workqueue("i915-userptr-release", WQ_UNBOUND, 0);
if (mn->wq == NULL) {
kfree(mn);
return ERR_PTR(-ENOMEM);
}
/* Protected by mmap_sem (write-lock) */ /* Protected by mmap_sem (write-lock) */
ret = __mmu_notifier_register(&mn->mn, mm); ret = __mmu_notifier_register(&mn->mn, mm);
if (ret) { if (ret) {
destroy_workqueue(mn->wq);
kfree(mn); kfree(mn);
return ERR_PTR(ret); return ERR_PTR(ret);
} }
...@@ -256,6 +301,7 @@ i915_mmu_notifier_free(struct i915_mmu_notifier *mn, ...@@ -256,6 +301,7 @@ i915_mmu_notifier_free(struct i915_mmu_notifier *mn,
return; return;
mmu_notifier_unregister(&mn->mn, mm); mmu_notifier_unregister(&mn->mn, mm);
destroy_workqueue(mn->wq);
kfree(mn); kfree(mn);
} }
......
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