From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Change SYNC_IOC_FILE_INFO behaviour to avoid future API breaks and optimize buffer
Now num_fences can be filled by the caller to inform how many fences it wants to retrieve from the kernel. If the num_fences passed is greater than zero info->sync_fence_info should point to a buffer with enough space to fit all fences.
However if num_fences passed to the kernel is 0, the kernel will reply with number of fences of the sync_file.
Sending first an ioctl with num_fences = 0 can optimize buffer allocation, in a first call with num_fences = 0 userspace will receive the actual number of fences in the num_fences filed.
Then it can allocate a buffer with the correct size on sync_fence_info and call SYNC_IOC_FILE_INFO again, but now with the actual value of num_fences in the sync_file.
Also, info->sync_fence_info was converted to __u64 pointer to prevent 32bit compatibility issues.
An example userspace code for the later would be:
struct sync_file_info *info; int err, size, num_fences;
info = malloc(sizeof(*info));
info.flags = 0; err = ioctl(fd, SYNC_IOC_FILE_INFO, info); num_fences = info->num_fences;
if (num_fences) { info.flags = 0; size = sizeof(struct sync_fence_info) * num_fences; info->num_fences = num_fences; info->sync_fence_info = (uint64_t) calloc(num_fences, sizeof(struct sync_fence_info));
err = ioctl(fd, SYNC_IOC_FILE_INFO, info); }
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com --- v2: fix fence_info memory leak
v3: Comments from Emil Velikov - improve commit message - remove __u64 cast - remove check for output fields in file_info - clean up sync_fill_fence_info()
Comments from Maarten Lankhorst - remove in.num_fences && !in.sync_fence_info check - remove info->len and use only num_fences to calculate size
Comments from Dan Carpenter - fix info->sync_fence_info documentation
v4: remove allocated struct sync_file_info (comment from Maarten) --- drivers/staging/android/sync.c | 70 +++++++++++++++++-------------------- drivers/staging/android/uapi/sync.h | 9 ++--- 2 files changed, 36 insertions(+), 43 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index dc5f382..48ee175 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -479,13 +479,9 @@ err_put_fd: return err; }
-static int sync_fill_fence_info(struct fence *fence, void *data, int size) +static void sync_fill_fence_info(struct fence *fence, + struct sync_fence_info *info) { - struct sync_fence_info *info = data; - - if (size < sizeof(*info)) - return -ENOMEM; - strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), sizeof(info->obj_name)); strlcpy(info->driver_name, fence->ops->get_driver_name(fence), @@ -495,60 +491,60 @@ static int sync_fill_fence_info(struct fence *fence, void *data, int size) else info->status = 0; info->timestamp_ns = ktime_to_ns(fence->timestamp); - - return sizeof(*info); }
static long sync_file_ioctl_fence_info(struct sync_file *sync_file, unsigned long arg) { - struct sync_file_info *info; + struct sync_file_info info; + struct sync_fence_info *fence_info = NULL; __u32 size; - __u32 len = 0; int ret, i;
- if (copy_from_user(&size, (void __user *)arg, sizeof(size))) + if (copy_from_user(&info, (void __user *)arg, sizeof(info))) return -EFAULT;
- if (size < sizeof(struct sync_file_info)) - return -EINVAL; + /* + * Passing num_fences = 0 means that userspace doesn't want to + * retrieve any sync_fence_info. If num_fences = 0 we skip filling + * sync_fence_info and return the actual number of fences on + * info->num_fences. + */ + if (!info.num_fences) + goto no_fences;
- if (size > 4096) - size = 4096; + if (info.num_fences < sync_file->num_fences) + return -EINVAL;
- info = kzalloc(size, GFP_KERNEL); - if (!info) + size = sync_file->num_fences * sizeof(*fence_info); + fence_info = kzalloc(size, GFP_KERNEL); + if (!fence_info) return -ENOMEM;
- strlcpy(info->name, sync_file->name, sizeof(info->name)); - info->status = atomic_read(&sync_file->status); - if (info->status >= 0) - info->status = !info->status; + for (i = 0; i < sync_file->num_fences; ++i) + sync_fill_fence_info(sync_file->cbs[i].fence, &fence_info[i]);
- info->num_fences = sync_file->num_fences; - - len = sizeof(struct sync_file_info); - - for (i = 0; i < sync_file->num_fences; ++i) { - struct fence *fence = sync_file->cbs[i].fence; - - ret = sync_fill_fence_info(fence, (u8 *)info + len, size - len); - - if (ret < 0) - goto out; - - len += ret; + if (copy_to_user((void __user *)info.sync_fence_info, fence_info, + size)) { + ret = -EFAULT; + goto out; }
- info->len = len; +no_fences: + strlcpy(info.name, sync_file->name, sizeof(info.name)); + info.status = atomic_read(&sync_file->status); + if (info.status >= 0) + info.status = !info.status; + + info.num_fences = sync_file->num_fences;
- if (copy_to_user((void __user *)arg, info, len)) + if (copy_to_user((void __user *)arg, &info, sizeof(info))) ret = -EFAULT; else ret = 0;
out: - kfree(info); + kfree(fence_info);
return ret; } diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h index f0b41ce..a122bb5 100644 --- a/drivers/staging/android/uapi/sync.h +++ b/drivers/staging/android/uapi/sync.h @@ -42,21 +42,18 @@ struct sync_fence_info {
/** * struct sync_file_info - data returned from fence info ioctl - * @len: ioctl caller writes the size of the buffer its passing in. - * ioctl returns length of sync_file_info returned to - * userspace including pt_info. * @name: name of fence * @status: status of fence. 1: signaled 0:active <0:error * @num_fences number of fences in the sync_file - * @sync_fence_info: array of sync_fence_info for every fence in the sync_file + * @sync_fence_info: pointer to array of structs sync_fence_info with all + * fences in the sync_file */ struct sync_file_info { - __u32 len; char name[32]; __s32 status; __u32 num_fences;
- __u8 sync_fence_info[0]; + __u64 sync_fence_info; };
#define SYNC_IOC_MAGIC '>'