[Intel-gfx] [PATCH 3/3] drm/i915: Use a task to cancel the userptr on invalidate_range

[Chris Wilson]

On Wed, Jul 01, 2015 at 01:56:30PM +0100, Tvrtko Ursulin wrote:
> >@@ -562,12 +524,20 @@ __i915_gem_userptr_set_active(struct drm_i915_gem_object *obj,
> >  	 */
> >  #if defined(CONFIG_MMU_NOTIFIER)
> >  	if (obj->userptr.mmu_object == NULL)
> >-		return;
> >+		return 0;
> >
> >  	spin_lock(&obj->userptr.mmu_object->mn->lock);
> >-	obj->userptr.mmu_object->active = value;
> >+	/* In order to serialise get_pages with an outstanding
> >+	 * cancel_userptr, we must drop the struct_mutex and try again.
> >+	 */
> >+	if (!value || !work_pending(&obj->userptr.mmu_object->work))
> >+		obj->userptr.mmu_object->active = value;
> >+	else
> >+		ret = -EAGAIN;
> >  	spin_unlock(&obj->userptr.mmu_object->mn->lock);
> >  #endif
> >+
> >+	return ret;
> >  }
> 
> I think it would be a lot more efficient if we dropped the mutex
> here and waited for the worker to complete in kernel, rather than
> letting userspace hammer it (the mutex). Especially having
> experienced one worker-structmutex starvation yesterday.

Yes, it would be much easier and simpler. It also goes bang very quickly
- as we cannot drop the mutex here in the middle of an execbuffer as
that quickly corrupts the state believed reserved by the execbuffer.

If you look at other patches in my tree, we can hide the starvation
issue in the kernel - but we still have no way to solve the priority
inversion problem of using a worker for a high priority task, except to
move them to the high priority queue. Worth it? Not sure. But flushing a
single queue is easier than doing semaphore completion.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre