Skip to content

Commit

Permalink
dma-buf/sync_file: Speed up ioctl by omitting debug names
Browse files Browse the repository at this point in the history 
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 <[email protected]>
[Kazuki: Port to v5.18]
Signed-off-by: Kazuki Hashimoto <[email protected]>
  • Loading branch information
@kerneltoast @Kaz205
kerneltoast authored and Kaz205 committed Aug 6, 2022
1 parent fcd6b4a commit 4501dbf
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 79 deletions.
101 changes: 31 additions & 70 deletions drivers/dma-buf/sync_file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,36 +116,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-%lld",
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)
{
Expand Down Expand Up @@ -200,7 +170,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 dma_fence *a_fence, *b_fence, **fences;
Expand Down Expand Up @@ -270,7 +240,6 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
if (sync_file_set_fence(sync_file, fences, index) < 0)
goto err_put_fences;

strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name));
return sync_file;

err_put_fences:
Expand Down Expand Up @@ -318,11 +287,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;
}
Expand All @@ -338,15 +310,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;
}
Expand All @@ -369,11 +340,6 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file,
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))
Expand All @@ -389,15 +355,15 @@ static int sync_fill_fence_info(struct dma_fence *fence,
static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
unsigned long arg)
{
struct sync_fence_info *fence_info = NULL;
struct dma_fence_unwrap iter;
struct sync_file_info info;
unsigned int num_fences;
struct dma_fence *fence;
int ret;
__u32 size;
size_t len, offset;

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)
Expand All @@ -423,38 +389,33 @@ 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;

num_fences = 0;
offset = offsetof(struct sync_fence_info, status);
dma_fence_unwrap_for_each(fence, &iter, sync_file->fence) {
int status;

status = sync_fill_fence_info(fence, &fence_info[num_fences++]);
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(fence, 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 + num_fences * sizeof(*finfo) + offset;
if (copy_to_user(u64_to_user_ptr(dest), &fence_info,
sizeof(fence_info)))
return -EFAULT;
num_fences++;
}

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,
Expand Down
9 changes: 0 additions & 9 deletions include/linux/sync_file.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,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
Expand All @@ -57,6 +49,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 */

0 comments on commit 4501dbf

Please sign in to comment.