From fe1d3ee5cb9476f401ce6c371d2bae73dfdd2a94 Mon Sep 17 00:00:00 2001 From: Sultan Alsawaf Date: Wed, 6 Jan 2021 21:00:31 -0800 Subject: [PATCH] dma-buf/sync_file: Speed up ioctl by omitting debug names A lot of CPU time is wasted on allocating, populating, and copying debug names back and forth with userspace when they're not actually needed. We can't just remove the name buffers from the various sync data structures though because we must preserve ABI compatibility with userspace, but instead we can just pretend the name fields of the user-shared structs aren't there. This massively reduces the sizes of memory allocated for these data structures and the amount of data passed between userspace, as well as eliminates a kzalloc() entirely from sync_file_ioctl_fence_info(), thus improving graphics performance. Signed-off-by: Sultan Alsawaf Signed-off-by: Ruchit --- drivers/dma-buf/sync_file.c | 99 ++++++++++++------------------------- include/linux/sync_file.h | 9 ---- 2 files changed, 31 insertions(+), 77 deletions(-) diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index bf65e634590b..3bbce058bd4b 100755 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -124,36 +124,6 @@ struct dma_fence *sync_file_get_fence(int fd) } EXPORT_SYMBOL(sync_file_get_fence); -/** - * sync_file_get_name - get the name of the sync_file - * @sync_file: sync_file to get the fence from - * @buf: destination buffer to copy sync_file name into - * @len: available size of destination buffer. - * - * Each sync_file may have a name assigned either by the user (when merging - * sync_files together) or created from the fence it contains. In the latter - * case construction of the name is deferred until use, and so requires - * sync_file_get_name(). - * - * Returns: a string representing the name. - */ -char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len) -{ - if (sync_file->user_name[0]) { - strlcpy(buf, sync_file->user_name, len); - } else { - struct dma_fence *fence = sync_file->fence; - - snprintf(buf, len, "%s-%s%llu-%d", - fence->ops->get_driver_name(fence), - fence->ops->get_timeline_name(fence), - fence->context, - fence->seqno); - } - - return buf; -} - static int sync_file_set_fence(struct sync_file *sync_file, struct dma_fence **fences, int num_fences) { @@ -216,7 +186,7 @@ static void add_fence(struct dma_fence **fences, * @a and @b. @a and @b remain valid, independent sync_file. Returns the * new merged sync_file or NULL in case of error. */ -static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, +static struct sync_file *sync_file_merge(struct sync_file *a, struct sync_file *b) { struct sync_file *sync_file; @@ -291,7 +261,6 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, goto err; } - strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name)); return sync_file; err: @@ -335,11 +304,14 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, int err; struct sync_file *fence2, *fence3; struct sync_merge_data data; + size_t len; if (fd < 0) return fd; - if (copy_from_user(&data, (void __user *)arg, sizeof(data))) { + arg += offsetof(typeof(data), fd2); + len = sizeof(data) - offsetof(typeof(data), fd2); + if (copy_from_user(&data.fd2, (void __user *)arg, len)) { err = -EFAULT; goto err_put_fd; } @@ -355,15 +327,14 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, goto err_put_fd; } - data.name[sizeof(data.name) - 1] = '\0'; - fence3 = sync_file_merge(data.name, sync_file, fence2); + fence3 = sync_file_merge(sync_file, fence2); if (!fence3) { err = -ENOMEM; goto err_put_fence2; } data.fence = fd; - if (copy_to_user((void __user *)arg, &data, sizeof(data))) { + if (copy_to_user((void __user *)arg, &data.fd2, len)) { err = -EFAULT; goto err_put_fence3; } @@ -386,11 +357,6 @@ err_put_fd: static int sync_fill_fence_info(struct dma_fence *fence, struct sync_fence_info *info) { - strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), - sizeof(info->obj_name)); - strlcpy(info->driver_name, fence->ops->get_driver_name(fence), - sizeof(info->driver_name)); - info->status = dma_fence_get_status(fence); while (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) && !test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags)) @@ -407,12 +373,13 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, unsigned long arg) { struct sync_file_info info; - struct sync_fence_info *fence_info = NULL; struct dma_fence **fences; - __u32 size; - int num_fences, ret, i; + size_t len, offset; + int num_fences, i; - if (copy_from_user(&info, (void __user *)arg, sizeof(info))) + arg += offsetof(typeof(info), status); + len = sizeof(info) - offsetof(typeof(info), status); + if (copy_from_user(&info.status, (void __user *)arg, len)) return -EFAULT; if (info.flags || info.pad) @@ -436,35 +403,31 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, if (info.num_fences < num_fences) return -EINVAL; - size = num_fences * sizeof(*fence_info); - fence_info = kzalloc(size, GFP_KERNEL); - if (!fence_info) - return -ENOMEM; - + offset = offsetof(struct sync_fence_info, status); for (i = 0; i < num_fences; i++) { - int status = sync_fill_fence_info(fences[i], &fence_info[i]); + struct { + __s32 status; + __u32 flags; + __u64 timestamp_ns; + } fence_info; + struct sync_fence_info *finfo = (void *)&fence_info - offset; + int status = sync_fill_fence_info(fences[i], finfo); + u64 dest; + + /* Don't leak kernel memory to userspace via finfo->flags */ + finfo->flags = 0; info.status = info.status <= 0 ? info.status : status; - } - - if (copy_to_user(u64_to_user_ptr(info.sync_fence_info), fence_info, - size)) { - ret = -EFAULT; - goto out; + dest = info.sync_fence_info + i * sizeof(*finfo) + offset; + if (copy_to_user(u64_to_user_ptr(dest), &fence_info, + sizeof(fence_info))) + return -EFAULT; } no_fences: - sync_file_get_name(sync_file, info.name, sizeof(info.name)); info.num_fences = num_fences; - - if (copy_to_user((void __user *)arg, &info, sizeof(info))) - ret = -EFAULT; - else - ret = 0; - -out: - kfree(fence_info); - - return ret; + if (copy_to_user((void __user *)arg, &info.status, len)) + return -EFAULT; + return 0; } static long sync_file_ioctl(struct file *file, unsigned int cmd, diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h index 0ad87c434ae6..b5063948cdd0 100755 --- a/include/linux/sync_file.h +++ b/include/linux/sync_file.h @@ -30,14 +30,6 @@ */ struct sync_file { struct file *file; - /** - * @user_name: - * - * Name of the sync file provided by userspace, for merged fences. - * Otherwise generated through driver callbacks (in which case the - * entire array is 0). - */ - char user_name[32]; #ifdef CONFIG_DEBUG_FS struct list_head sync_file_list; #endif @@ -53,6 +45,5 @@ struct sync_file { struct sync_file *sync_file_create(struct dma_fence *fence); struct dma_fence *sync_file_get_fence(int fd); -char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len); #endif /* _LINUX_SYNC_H */