[33/48] staging: etnaviv: copy submit command and bos in one go

Commit Message

Lucas Stach Sept. 25, 2015, 11:57 a.m. UTC

From: Russell King <[e-mail]>

On GPU submission, copy the submit command and bo structures in one go
and outside of dev->struct_mutex.  There are two reasons:

- This avoids the overhead from calling and checking copy_from_user()
  multiple times.
- Holding dev->struct_mutex over a page fault should be avoided as this
  will block all users of the GPU while page IO is being performed.

Signed-off-by: Russell King <[e-mail]>
---
 drivers/staging/etnaviv/etnaviv_gem_submit.c | 98 ++++++++++++++++------------
 1 file changed, 57 insertions(+), 41 deletions(-)

Patch

diff --git a/drivers/staging/etnaviv/etnaviv_gem_submit.c b/drivers/staging/etnaviv/etnaviv_gem_submit.c
index e2c94f476810..f94b6e8fb4fb 100644
--- a/drivers/staging/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/staging/etnaviv/etnaviv_gem_submit.c
@@ -55,41 +55,34 @@  static struct etnaviv_gem_submit *submit_create(struct drm_device *dev,
 }
 
 static int submit_lookup_objects(struct etnaviv_gem_submit *submit,
-		struct drm_etnaviv_gem_submit *args, struct drm_file *file)
+	struct drm_file *file, struct drm_etnaviv_gem_submit_bo *submit_bos,
+	unsigned nr_bos)
 {
+	struct drm_etnaviv_gem_submit_bo *bo;
 	unsigned i;
 	int ret = 0;
 
 	spin_lock(&file->table_lock);
 
-	for (i = 0; i < args->nr_bos; i++) {
-		struct drm_etnaviv_gem_submit_bo submit_bo;
+	for (i = 0, bo = submit_bos; i < nr_bos; i++, bo++) {
 		struct drm_gem_object *obj;
 		struct etnaviv_gem_object *etnaviv_obj;
-		void __user *userptr =
-			to_user_ptr(args->bos + (i * sizeof(submit_bo)));
-
-		ret = copy_from_user(&submit_bo, userptr, sizeof(submit_bo));
-		if (ret) {
-			ret = -EFAULT;
-			goto out_unlock;
-		}
 
-		if (submit_bo.flags & BO_INVALID_FLAGS) {
-			DRM_ERROR("invalid flags: %x\n", submit_bo.flags);
+		if (bo->flags & BO_INVALID_FLAGS) {
+			DRM_ERROR("invalid flags: %x\n", bo->flags);
 			ret = -EINVAL;
 			goto out_unlock;
 		}
 
-		submit->bos[i].flags = submit_bo.flags;
+		submit->bos[i].flags = bo->flags;
 
 		/* normally use drm_gem_object_lookup(), but for bulk lookup
 		 * all under single table_lock just hit object_idr directly:
 		 */
-		obj = idr_find(&file->object_idr, submit_bo.handle);
+		obj = idr_find(&file->object_idr, bo->handle);
 		if (!obj) {
 			DRM_ERROR("invalid handle %u at index %u\n",
-				  submit_bo.handle, i);
+				  bo->handle, i);
 			ret = -EINVAL;
 			goto out_unlock;
 		}
@@ -98,7 +91,7 @@  static int submit_lookup_objects(struct etnaviv_gem_submit *submit,
 
 		if (!list_empty(&etnaviv_obj->submit_entry)) {
 			DRM_ERROR("handle %u at index %u already on submit list\n",
-				  submit_bo.handle, i);
+				  bo->handle, i);
 			ret = -EINVAL;
 			goto out_unlock;
 		}
@@ -298,6 +291,8 @@  int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 	struct etnaviv_drm_private *priv = dev->dev_private;
 	struct drm_etnaviv_gem_submit *args = data;
 	struct etnaviv_file_private *ctx = file->driver_priv;
+	struct drm_etnaviv_gem_submit_cmd *cmds;
+	struct drm_etnaviv_gem_submit_bo *bos;
 	struct etnaviv_gem_submit *submit;
 	struct etnaviv_gpu *gpu;
 	unsigned i;
@@ -314,6 +309,29 @@  int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 		return -EINVAL;
 
 	/*
+	 * Copy the command submission and bo array to kernel space in
+	 * one go, and do this outside of the dev->struct_mutex lock.
+	 */
+	cmds = drm_malloc_ab(args->nr_cmds, sizeof(*cmds));
+	bos = drm_malloc_ab(args->nr_bos, sizeof(*bos));
+	if (!cmds || !bos)
+		return -ENOMEM;
+
+	ret = copy_from_user(cmds, to_user_ptr(args->cmds),
+			     args->nr_cmds * sizeof(*cmds));
+	if (ret) {
+		ret = -EFAULT;
+		goto err_submit_cmds;
+	}
+
+	ret = copy_from_user(bos, to_user_ptr(args->bos),
+			     args->nr_bos * sizeof(*bos));
+	if (ret) {
+		ret = -EFAULT;
+		goto err_submit_cmds;
+	}
+
+	/*
 	 * Avoid big circular locking dependency loops:
 	 * - reading debugfs results in mmap_sem depending on i_mutex_key#3
 	 *   (iterate_dir -> filldir64)
@@ -332,7 +350,7 @@  int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 	 */
 	ret = etnaviv_gpu_pm_get_sync(gpu);
 	if (ret < 0)
-		return ret;
+		goto err_submit_cmds;
 
 	mutex_lock(&dev->struct_mutex);
 
@@ -343,7 +361,7 @@  int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 	}
 	submit->exec_state = args->exec_state;
 
-	ret = submit_lookup_objects(submit, args, file);
+	ret = submit_lookup_objects(submit, file, bos, args->nr_bos);
 	if (ret)
 		goto out;
 
@@ -352,19 +370,11 @@  int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 		goto out;
 
 	for (i = 0; i < args->nr_cmds; i++) {
-		struct drm_etnaviv_gem_submit_cmd submit_cmd;
-		void __user *userptr =
-			to_user_ptr(args->cmds + (i * sizeof(submit_cmd)));
+		struct drm_etnaviv_gem_submit_cmd *submit_cmd = cmds + i;
 		struct etnaviv_gem_object *etnaviv_obj;
 		unsigned max_size;
 
-		ret = copy_from_user(&submit_cmd, userptr, sizeof(submit_cmd));
-		if (ret) {
-			ret = -EFAULT;
-			goto out;
-		}
-
-		ret = submit_bo(submit, submit_cmd.submit_idx, &etnaviv_obj,
+		ret = submit_bo(submit, submit_cmd->submit_idx, &etnaviv_obj,
 				NULL);
 		if (ret)
 			goto out;
@@ -375,16 +385,16 @@  int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 			goto out;
 		}
 
-		if (submit_cmd.size % 4) {
+		if (submit_cmd->size % 4) {
 			DRM_ERROR("non-aligned cmdstream buffer size: %u\n",
-					submit_cmd.size);
+					submit_cmd->size);
 			ret = -EINVAL;
 			goto out;
 		}
 
-		if (submit_cmd.submit_offset % 8) {
+		if (submit_cmd->submit_offset % 8) {
 			DRM_ERROR("non-aligned cmdstream buffer size: %u\n",
-					submit_cmd.size);
+					submit_cmd->size);
 			ret = -EINVAL;
 			goto out;
 		}
@@ -395,17 +405,17 @@  int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 		 */
 		max_size = etnaviv_obj->base.size - 8;
 
-		if (submit_cmd.size > max_size ||
-		    submit_cmd.submit_offset > max_size - submit_cmd.size) {
+		if (submit_cmd->size > max_size ||
+		    submit_cmd->submit_offset > max_size - submit_cmd->size) {
 			DRM_ERROR("invalid cmdstream size: %u\n",
-				  submit_cmd.size);
+				  submit_cmd->size);
 			ret = -EINVAL;
 			goto out;
 		}
 
-		submit->cmd[i].type = submit_cmd.type;
-		submit->cmd[i].offset = submit_cmd.submit_offset / 4;
-		submit->cmd[i].size = submit_cmd.size / 4;
+		submit->cmd[i].type = submit_cmd->type;
+		submit->cmd[i].offset = submit_cmd->submit_offset / 4;
+		submit->cmd[i].size = submit_cmd->size / 4;
 		submit->cmd[i].obj = etnaviv_obj;
 
 		if (!etnaviv_cmd_validate_one(gpu, etnaviv_obj,
@@ -416,8 +426,8 @@  int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 		}
 
 		ret = submit_reloc(submit, etnaviv_obj,
-				   submit_cmd.submit_offset,
-				   submit_cmd.nr_relocs, submit_cmd.relocs);
+				   submit_cmd->submit_offset,
+				   submit_cmd->nr_relocs, submit_cmd->relocs);
 		if (ret)
 			goto out;
 	}
@@ -443,5 +453,11 @@  out:
 	if (ret == -EAGAIN)
 		flush_workqueue(priv->wq);
 
+ err_submit_cmds:
+	if (bos)
+		drm_free_large(bos);
+	if (cmds)
+		drm_free_large(cmds);
+
 	return ret;
 }