On 06/30/2015 05:55 PM, Chris Wilson wrote:
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.
Secondary worker can fire only if invalidate clears the current one, no?
(if (obj->userptr.work == NULL && ...))
It then "cancels" the worker so that the st_set_pages path is avoided.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem_userptr.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 7a5242cd5ea5..cb367d9f7909 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -581,17 +581,17 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
}
mutex_lock(&dev->struct_mutex);
- if (obj->userptr.work != &work->work) {
- ret = 0;
- } else if (pinned == num_pages) {
- ret = st_set_pages(&obj->pages, pvec, num_pages);
- if (ret == 0) {
- list_add_tail(&obj->global_list, &to_i915(dev)->mm.unbound_list);
- pinned = 0;
+ if (obj->userptr.work == &work->work) {
+ if (pinned == num_pages) {
+ ret = st_set_pages(&obj->pages, pvec, num_pages);
+ if (ret == 0) {
+ list_add_tail(&obj->global_list, &to_i915(dev)->mm.unbound_list);
+ pinned = 0;
+ }
}
+ obj->userptr.work = ERR_PTR(ret);
}
- obj->userptr.work = ERR_PTR(ret);
obj->userptr.workers--;
drm_gem_object_unreference(&obj->base);
mutex_unlock(&dev->struct_mutex);
Previously the canceled worker would allow another worker to be created
in case it failed (obj->userptr.work != &work->work; ret = 0;) and now
it still does since obj->userptr.work remains at NULL from cancellation.
Both seem wrong, am I missing the change?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
