From: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx> 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 <rmk+kernel@xxxxxxxxxxxxxxxx> --- drivers/staging/etnaviv/etnaviv_gem_submit.c | 98 ++++++++++++++++------------ 1 file changed, 57 insertions(+), 41 deletions(-) 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; } -- 2.5.1 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel