Commit 68d6c840 authored by Chris Wilson's avatar Chris Wilson Committed by Daniel Vetter

drm/i915: Only update the current userptr worker

The userptr worker allows for a slight race condition where upon there
may two or more threads calling get_user_pages for the same object. When
we have the array of pages, then we serialise the update of the object.
However, the worker should only overwrite the obj->userptr.work pointer
if and only if it is the active one. Currently we clear it for a
secondary worker with the effect that we may rarely force a second
lookup.

v2: Rebase and rename a variable to avoid 80cols
v3: Mention v2
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 24dfd073
...@@ -571,25 +571,25 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work) ...@@ -571,25 +571,25 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
struct get_pages_work *work = container_of(_work, typeof(*work), work); struct get_pages_work *work = container_of(_work, typeof(*work), work);
struct drm_i915_gem_object *obj = work->obj; struct drm_i915_gem_object *obj = work->obj;
struct drm_device *dev = obj->base.dev; struct drm_device *dev = obj->base.dev;
const int num_pages = obj->base.size >> PAGE_SHIFT; const int npages = obj->base.size >> PAGE_SHIFT;
struct page **pvec; struct page **pvec;
int pinned, ret; int pinned, ret;
ret = -ENOMEM; ret = -ENOMEM;
pinned = 0; pinned = 0;
pvec = kmalloc(num_pages*sizeof(struct page *), pvec = kmalloc(npages*sizeof(struct page *),
GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY); GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY);
if (pvec == NULL) if (pvec == NULL)
pvec = drm_malloc_ab(num_pages, sizeof(struct page *)); pvec = drm_malloc_ab(npages, sizeof(struct page *));
if (pvec != NULL) { if (pvec != NULL) {
struct mm_struct *mm = obj->userptr.mm->mm; struct mm_struct *mm = obj->userptr.mm->mm;
down_read(&mm->mmap_sem); down_read(&mm->mmap_sem);
while (pinned < num_pages) { while (pinned < npages) {
ret = get_user_pages(work->task, mm, ret = get_user_pages(work->task, mm,
obj->userptr.ptr + pinned * PAGE_SIZE, obj->userptr.ptr + pinned * PAGE_SIZE,
num_pages - pinned, npages - pinned,
!obj->userptr.read_only, 0, !obj->userptr.read_only, 0,
pvec + pinned, NULL); pvec + pinned, NULL);
if (ret < 0) if (ret < 0)
...@@ -601,20 +601,20 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work) ...@@ -601,20 +601,20 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
} }
mutex_lock(&dev->struct_mutex); mutex_lock(&dev->struct_mutex);
if (obj->userptr.work != &work->work) { if (obj->userptr.work == &work->work) {
ret = 0; if (pinned == npages) {
} else if (pinned == num_pages) { ret = __i915_gem_userptr_set_pages(obj, pvec, npages);
ret = __i915_gem_userptr_set_pages(obj, pvec, num_pages); if (ret == 0) {
if (ret == 0) { list_add_tail(&obj->global_list,
list_add_tail(&obj->global_list, &to_i915(dev)->mm.unbound_list); &to_i915(dev)->mm.unbound_list);
obj->get_page.sg = obj->pages->sgl; obj->get_page.sg = obj->pages->sgl;
obj->get_page.last = 0; obj->get_page.last = 0;
pinned = 0;
pinned = 0; }
} }
obj->userptr.work = ERR_PTR(ret);
} }
obj->userptr.work = ERR_PTR(ret);
obj->userptr.workers--; obj->userptr.workers--;
drm_gem_object_unreference(&obj->base); drm_gem_object_unreference(&obj->base);
mutex_unlock(&dev->struct_mutex); mutex_unlock(&dev->struct_mutex);
......
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