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 <sultan@kerneltoast.com>
Signed-off-by: Ruchit <ruchitmarathe@gmail.com>

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(sync_file, fence2);
-	fence3 = sync_file_merge(data.name, 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;
+	size_t len, offset;
-	int num_fences, ret, i;
+	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);
+	offset = offsetof(struct sync_fence_info, status);
-	fence_info = kzalloc(size, GFP_KERNEL);
-	if (!fence_info)
-		return -ENOMEM;
-
 	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;
-	}
+		dest = info.sync_fence_info + i * sizeof(*finfo) + offset;
-
+		if (copy_to_user(u64_to_user_ptr(dest), &fence_info,
-	if (copy_to_user(u64_to_user_ptr(info.sync_fence_info), fence_info,
+				 sizeof(fence_info)))
-			 size)) {
+			return -EFAULT;
-		ret = -EFAULT;
-		goto out;
 	}
 
 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.status, len))
-	if (copy_to_user((void __user *)arg, &info, sizeof(info)))
+		return -EFAULT;
-		ret = -EFAULT;
+	return 0;
-	else
-		ret = 0;
-
-out:
-	kfree(fence_info);
-
-	return ret;
 }
 
 static long sync_file_ioctl(struct file *file, unsigned int cmd,

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 */