[Intel-gfx] [PATCH 190/190] drm/i915: Do a nonblocking wait first in pread/pwrite

[Chris Wilson]

If we try and read or write to an active request, we first must wait
upon the GPU completing that request. Let's do that without holding the
mutex (and so allow someone else to access the GPU whilst we wait). Upn
completion, we will reacquire the mutex and only then start the
operation (i.e. we do not rely on state from before dropping the mutex).

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 208 +++++++++++++++++++++-------------------
 1 file changed, 110 insertions(+), 98 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7be5a8fb9180..e51118473df6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -434,6 +434,104 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
 			       args->size, &args->handle);
 }
 
+/**
+ * Ensures that all rendering to the object has completed and the object is
+ * safe to unbind from the GTT or access from the CPU.
+ */
+int
+i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
+			       bool readonly)
+{
+	int ret, i;
+
+	if (!i915_gem_object_is_active(obj))
+		return 0;
+
+	if (readonly) {
+		ret = i915_wait_request(obj->last_write.request);
+		if (ret)
+			return ret;
+	} else {
+		for (i = 0; i < I915_NUM_RINGS; i++) {
+			ret = i915_wait_request(obj->last_read[i].request);
+			if (ret)
+				return ret;
+		}
+		GEM_BUG_ON(i915_gem_object_is_active(obj));
+	}
+
+	return 0;
+}
+
+/* A nonblocking variant of the above wait. This is a highly dangerous routine
+ * as the object state may change during this call.
+ */
+static __must_check int
+i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
+					    struct intel_rps_client *rps,
+					    bool readonly)
+{
+	struct drm_device *dev = obj->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_request *requests[I915_NUM_RINGS];
+	int ret, i, n = 0;
+
+	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
+	BUG_ON(!dev_priv->mm.interruptible);
+
+	if (!i915_gem_object_is_active(obj))
+		return 0;
+
+	if (readonly) {
+		struct drm_i915_gem_request *req;
+
+		req = obj->last_write.request;
+		if (req == NULL)
+			return 0;
+
+		if (i915_gem_request_completed(req))
+			i915_gem_request_retire_upto(req);
+		else
+			requests[n++] = i915_gem_request_get(req);
+	} else {
+		for (i = 0; i < I915_NUM_RINGS; i++) {
+			struct drm_i915_gem_request *req;
+
+			req = obj->last_read[i].request;
+			if (req == NULL)
+				continue;
+
+			if (i915_gem_request_completed(req))
+				i915_gem_request_retire_upto(req);
+			else
+				requests[n++] = i915_gem_request_get(req);
+		}
+	}
+
+	if (n == 0)
+		return 0;
+
+	mutex_unlock(&dev->struct_mutex);
+	ret = 0;
+	for (i = 0; ret == 0 && i < n; i++)
+		ret = __i915_wait_request(requests[i], true, NULL, rps);
+	mutex_lock(&dev->struct_mutex);
+
+	for (i = 0; i < n; i++) {
+		if (ret == 0)
+			i915_gem_request_retire_upto(requests[i]);
+		i915_gem_request_put(requests[i]);
+	}
+
+	return ret;
+}
+
+static struct intel_rps_client *to_rps_client(struct drm_file *file)
+{
+	struct drm_i915_file_private *fpriv = file->driver_priv;
+	return &fpriv->rps;
+}
+
 static inline int
 __copy_to_user_swizzled(char __user *cpu_vaddr,
 			const char *gpu_vaddr, int gpu_offset,
@@ -805,6 +903,12 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
 		goto out;
 	}
 
+	ret = i915_gem_object_wait_rendering__nonblocking(obj,
+							  to_rps_client(file),
+							  true);
+	if (ret)
+		goto out;
+
 	trace_i915_gem_object_pread(obj, args->offset, args->size);
 
 	ret = i915_gem_shmem_pread(dev, obj, args, file);
@@ -1136,6 +1240,12 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 		goto out;
 	}
 
+	ret = i915_gem_object_wait_rendering__nonblocking(obj,
+							  to_rps_client(file),
+							  false);
+	if (ret)
+		goto out;
+
 	trace_i915_gem_object_pwrite(obj, args->offset, args->size);
 
 	ret = -EFAULT;
@@ -1172,104 +1282,6 @@ put_rpm:
 }
 
 /**
- * Ensures that all rendering to the object has completed and the object is
- * safe to unbind from the GTT or access from the CPU.
- */
-int
-i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
-			       bool readonly)
-{
-	int ret, i;
-
-	if (!i915_gem_object_is_active(obj))
-		return 0;
-
-	if (readonly) {
-		ret = i915_wait_request(obj->last_write.request);
-		if (ret)
-			return ret;
-	} else {
-		for (i = 0; i < I915_NUM_RINGS; i++) {
-			ret = i915_wait_request(obj->last_read[i].request);
-			if (ret)
-				return ret;
-		}
-		GEM_BUG_ON(i915_gem_object_is_active(obj));
-	}
-
-	return 0;
-}
-
-/* A nonblocking variant of the above wait. This is a highly dangerous routine
- * as the object state may change during this call.
- */
-static __must_check int
-i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
-					    struct intel_rps_client *rps,
-					    bool readonly)
-{
-	struct drm_device *dev = obj->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_i915_gem_request *requests[I915_NUM_RINGS];
-	int ret, i, n = 0;
-
-	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
-	BUG_ON(!dev_priv->mm.interruptible);
-
-	if (!i915_gem_object_is_active(obj))
-		return 0;
-
-	if (readonly) {
-		struct drm_i915_gem_request *req;
-
-		req = obj->last_write.request;
-		if (req == NULL)
-			return 0;
-
-		if (i915_gem_request_completed(req))
-			i915_gem_request_retire_upto(req);
-		else
-			requests[n++] = i915_gem_request_get(req);
-	} else {
-		for (i = 0; i < I915_NUM_RINGS; i++) {
-			struct drm_i915_gem_request *req;
-
-			req = obj->last_read[i].request;
-			if (req == NULL)
-				continue;
-
-			if (i915_gem_request_completed(req))
-				i915_gem_request_retire_upto(req);
-			else
-				requests[n++] = i915_gem_request_get(req);
-		}
-	}
-
-	if (n == 0)
-		return 0;
-
-	mutex_unlock(&dev->struct_mutex);
-	ret = 0;
-	for (i = 0; ret == 0 && i < n; i++)
-		ret = __i915_wait_request(requests[i], true, NULL, rps);
-	mutex_lock(&dev->struct_mutex);
-
-	for (i = 0; i < n; i++) {
-		if (ret == 0)
-			i915_gem_request_retire_upto(requests[i]);
-		i915_gem_request_put(requests[i]);
-	}
-
-	return ret;
-}
-
-static struct intel_rps_client *to_rps_client(struct drm_file *file)
-{
-	struct drm_i915_file_private *fpriv = file->driver_priv;
-	return &fpriv->rps;
-}
-
-/**
  * Called when user space prepares to use an object with the CPU, either
  * through the mmap ioctl's mapping or a GTT mapping.
  */
-- 
2.7.0.rc3