| Message ID | 1458307660-31986-3-git-send-email-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Greg, Any comments on this? Thanks, Gustavo 2016-03-18 Gustavo Padovan <gustavo@padovan.org>: > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > Change SYNC_IOC_FILE_INFO (former SYNC_IOC_FENCE_INFO) behaviour to avoid > future API breaks and optimize buffer allocation. > > 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. > > info->sync_fence_info was converted to __u64 pointer to prevent 32bit > compatibility issues. And a flags member was added. > > 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); > } > > Finally the IOCTLs numbers were changed to avoid any potential old > userspace running the old API to get weird errors. Changing the opcodes > will make them fail right away. This is just a precaution, there no > upstream users of these interfaces yet and the only user is Android, but > we don't expect anyone trying to run android userspace and all it > dependencies on top of upstream kernels. > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Acked-by: Greg Hackmann <ghackmann@google.com> > Acked-by: Rob Clark <robdclark@gmail.com> > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > --- > 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) > > v5: merge all commits that were changing the ABI > > v6: fix -Wint-to-pointer-cast error on info.sync_fence_info > --- > drivers/staging/android/sync.c | 76 ++++++++++++++++++++----------------- > drivers/staging/android/uapi/sync.h | 36 +++++++++++++----- > 2 files changed, 67 insertions(+), 45 deletions(-) > > diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c > index 3a8f210..f9c6094 100644 > --- a/drivers/staging/android/sync.c > +++ b/drivers/staging/android/sync.c > @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, > goto err_put_fd; > } > > + if (data.flags || data.pad) { > + err = -EINVAL; > + goto err_put_fd; > + } > + > fence2 = sync_file_fdget(data.fd2); > if (!fence2) { > err = -ENOENT; > @@ -479,13 +484,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,58 +496,63 @@ 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)) > + if (info.flags || info.pad) > return -EINVAL; > > - if (size > 4096) > - size = 4096; > - > - info = kzalloc(size, GFP_KERNEL); > - if (!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; > - > - len = sizeof(struct sync_file_info); > + /* > + * 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; > > - for (i = 0; i < sync_file->num_fences; ++i) { > - struct fence *fence = sync_file->cbs[i].fence; > + if (info.num_fences < sync_file->num_fences) > + return -EINVAL; > > - ret = sync_fill_fence_info(fence, (u8 *)info + len, size - len); > + size = sync_file->num_fences * sizeof(*fence_info); > + fence_info = kzalloc(size, GFP_KERNEL); > + if (!fence_info) > + return -ENOMEM; > > - if (ret < 0) > - goto out; > + for (i = 0; i < sync_file->num_fences; ++i) > + sync_fill_fence_info(sync_file->cbs[i].fence, &fence_info[i]); > > - len += ret; > + if (copy_to_user(u64_to_user_ptr(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; > } > @@ -560,7 +566,7 @@ static long sync_file_ioctl(struct file *file, unsigned int cmd, > case SYNC_IOC_MERGE: > return sync_file_ioctl_merge(sync_file, arg); > > - case SYNC_IOC_FENCE_INFO: > + case SYNC_IOC_FILE_INFO: > return sync_file_ioctl_fence_info(sync_file, arg); > > default: > diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h > index 4467c76..fbadb8a 100644 > --- a/drivers/staging/android/uapi/sync.h > +++ b/drivers/staging/android/uapi/sync.h > @@ -16,14 +16,18 @@ > > /** > * struct sync_merge_data - data passed to merge ioctl > - * @fd2: file descriptor of second fence > * @name: name of new fence > + * @fd2: file descriptor of second fence > * @fence: returns the fd of the new fence to userspace > + * @flags: merge_data flags > + * @pad: padding for 64-bit alignment, should always be zero > */ > struct sync_merge_data { > - __s32 fd2; > char name[32]; > + __s32 fd2; > __s32 fence; > + __u32 flags; > + __u32 pad; > }; > > /** > @@ -31,42 +35,54 @@ struct sync_merge_data { > * @obj_name: name of parent sync_timeline > * @driver_name: name of driver implementing the parent > * @status: status of the fence 0:active 1:signaled <0:error > + * @flags: fence_info flags > * @timestamp_ns: timestamp of status change in nanoseconds > */ > struct sync_fence_info { > char obj_name[32]; > char driver_name[32]; > __s32 status; > + __u32 flags; > __u64 timestamp_ns; > }; > > /** > * 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 > - * @sync_fence_info: array of sync_fence_info for every fence in the sync_file > + * @flags: sync_file_info flags > + * @num_fences number of fences in the sync_file > + * @pad: padding for 64-bit alignment, should always be zero > + * @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 flags; > + __u32 num_fences; > + __u32 pad; > > - __u8 sync_fence_info[0]; > + __u64 sync_fence_info; > }; > > #define SYNC_IOC_MAGIC '>' > > /** > + * Opcodes 0, 1 and 2 were burned during a API change to avoid users of the > + * old API to get weird errors when trying to handling sync_files. The API > + * change happened during the de-stage of the Sync Framework when there was > + * no upstream users available. > + */ > + > +/** > * DOC: SYNC_IOC_MERGE - merge two fences > * > * Takes a struct sync_merge_data. Creates a new fence containing copies of > * the sync_pts in both the calling fd and sync_merge_data.fd2. Returns the > * new fence's fd in sync_merge_data.fence > */ > -#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 1, struct sync_merge_data) > +#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 3, struct sync_merge_data) > > /** > * DOC: SYNC_IOC_FENCE_INFO - get detailed information on a fence > @@ -79,6 +95,6 @@ struct sync_file_info { > * pt_info is a buffer containing sync_pt_infos for every sync_pt in the fence. > * To iterate over the sync_pt_infos, use the sync_pt_info.len field. > */ > -#define SYNC_IOC_FENCE_INFO _IOWR(SYNC_IOC_MAGIC, 2, struct sync_file_info) > +#define SYNC_IOC_FILE_INFO _IOWR(SYNC_IOC_MAGIC, 4, struct sync_file_info) > > #endif /* _UAPI_LINUX_SYNC_H */ > -- > 2.5.0 >
Hi Greg, Any comment on this? Thanks, Gustavo 2016-03-18 Gustavo Padovan <gustavo@padovan.org>: > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > Change SYNC_IOC_FILE_INFO (former SYNC_IOC_FENCE_INFO) behaviour to avoid > future API breaks and optimize buffer allocation. > > 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. > > info->sync_fence_info was converted to __u64 pointer to prevent 32bit > compatibility issues. And a flags member was added. > > 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); > } > > Finally the IOCTLs numbers were changed to avoid any potential old > userspace running the old API to get weird errors. Changing the opcodes > will make them fail right away. This is just a precaution, there no > upstream users of these interfaces yet and the only user is Android, but > we don't expect anyone trying to run android userspace and all it > dependencies on top of upstream kernels. > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Acked-by: Greg Hackmann <ghackmann@google.com> > Acked-by: Rob Clark <robdclark@gmail.com> > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > --- > 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) > > v5: merge all commits that were changing the ABI > > v6: fix -Wint-to-pointer-cast error on info.sync_fence_info > --- > drivers/staging/android/sync.c | 76 ++++++++++++++++++++----------------- > drivers/staging/android/uapi/sync.h | 36 +++++++++++++----- > 2 files changed, 67 insertions(+), 45 deletions(-) > > diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c > index 3a8f210..f9c6094 100644 > --- a/drivers/staging/android/sync.c > +++ b/drivers/staging/android/sync.c > @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, > goto err_put_fd; > } > > + if (data.flags || data.pad) { > + err = -EINVAL; > + goto err_put_fd; > + } > + > fence2 = sync_file_fdget(data.fd2); > if (!fence2) { > err = -ENOENT; > @@ -479,13 +484,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,58 +496,63 @@ 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)) > + if (info.flags || info.pad) > return -EINVAL; > > - if (size > 4096) > - size = 4096; > - > - info = kzalloc(size, GFP_KERNEL); > - if (!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; > - > - len = sizeof(struct sync_file_info); > + /* > + * 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; > > - for (i = 0; i < sync_file->num_fences; ++i) { > - struct fence *fence = sync_file->cbs[i].fence; > + if (info.num_fences < sync_file->num_fences) > + return -EINVAL; > > - ret = sync_fill_fence_info(fence, (u8 *)info + len, size - len); > + size = sync_file->num_fences * sizeof(*fence_info); > + fence_info = kzalloc(size, GFP_KERNEL); > + if (!fence_info) > + return -ENOMEM; > > - if (ret < 0) > - goto out; > + for (i = 0; i < sync_file->num_fences; ++i) > + sync_fill_fence_info(sync_file->cbs[i].fence, &fence_info[i]); > > - len += ret; > + if (copy_to_user(u64_to_user_ptr(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; > } > @@ -560,7 +566,7 @@ static long sync_file_ioctl(struct file *file, unsigned int cmd, > case SYNC_IOC_MERGE: > return sync_file_ioctl_merge(sync_file, arg); > > - case SYNC_IOC_FENCE_INFO: > + case SYNC_IOC_FILE_INFO: > return sync_file_ioctl_fence_info(sync_file, arg); > > default: > diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h > index 4467c76..fbadb8a 100644 > --- a/drivers/staging/android/uapi/sync.h > +++ b/drivers/staging/android/uapi/sync.h > @@ -16,14 +16,18 @@ > > /** > * struct sync_merge_data - data passed to merge ioctl > - * @fd2: file descriptor of second fence > * @name: name of new fence > + * @fd2: file descriptor of second fence > * @fence: returns the fd of the new fence to userspace > + * @flags: merge_data flags > + * @pad: padding for 64-bit alignment, should always be zero > */ > struct sync_merge_data { > - __s32 fd2; > char name[32]; > + __s32 fd2; > __s32 fence; > + __u32 flags; > + __u32 pad; > }; > > /** > @@ -31,42 +35,54 @@ struct sync_merge_data { > * @obj_name: name of parent sync_timeline > * @driver_name: name of driver implementing the parent > * @status: status of the fence 0:active 1:signaled <0:error > + * @flags: fence_info flags > * @timestamp_ns: timestamp of status change in nanoseconds > */ > struct sync_fence_info { > char obj_name[32]; > char driver_name[32]; > __s32 status; > + __u32 flags; > __u64 timestamp_ns; > }; > > /** > * 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 > - * @sync_fence_info: array of sync_fence_info for every fence in the sync_file > + * @flags: sync_file_info flags > + * @num_fences number of fences in the sync_file > + * @pad: padding for 64-bit alignment, should always be zero > + * @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 flags; > + __u32 num_fences; > + __u32 pad; > > - __u8 sync_fence_info[0]; > + __u64 sync_fence_info; > }; > > #define SYNC_IOC_MAGIC '>' > > /** > + * Opcodes 0, 1 and 2 were burned during a API change to avoid users of the > + * old API to get weird errors when trying to handling sync_files. The API > + * change happened during the de-stage of the Sync Framework when there was > + * no upstream users available. > + */ > + > +/** > * DOC: SYNC_IOC_MERGE - merge two fences > * > * Takes a struct sync_merge_data. Creates a new fence containing copies of > * the sync_pts in both the calling fd and sync_merge_data.fd2. Returns the > * new fence's fd in sync_merge_data.fence > */ > -#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 1, struct sync_merge_data) > +#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 3, struct sync_merge_data) > > /** > * DOC: SYNC_IOC_FENCE_INFO - get detailed information on a fence > @@ -79,6 +95,6 @@ struct sync_file_info { > * pt_info is a buffer containing sync_pt_infos for every sync_pt in the fence. > * To iterate over the sync_pt_infos, use the sync_pt_info.len field. > */ > -#define SYNC_IOC_FENCE_INFO _IOWR(SYNC_IOC_MAGIC, 2, struct sync_file_info) > +#define SYNC_IOC_FILE_INFO _IOWR(SYNC_IOC_MAGIC, 4, struct sync_file_info) > > #endif /* _UAPI_LINUX_SYNC_H */ > -- > 2.5.0 >
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 3a8f210..f9c6094 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, goto err_put_fd; } + if (data.flags || data.pad) { + err = -EINVAL; + goto err_put_fd; + } + fence2 = sync_file_fdget(data.fd2); if (!fence2) { err = -ENOENT; @@ -479,13 +484,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,58 +496,63 @@ 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)) + if (info.flags || info.pad) return -EINVAL; - if (size > 4096) - size = 4096; - - info = kzalloc(size, GFP_KERNEL); - if (!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; - - len = sizeof(struct sync_file_info); + /* + * 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; - for (i = 0; i < sync_file->num_fences; ++i) { - struct fence *fence = sync_file->cbs[i].fence; + if (info.num_fences < sync_file->num_fences) + return -EINVAL; - ret = sync_fill_fence_info(fence, (u8 *)info + len, size - len); + size = sync_file->num_fences * sizeof(*fence_info); + fence_info = kzalloc(size, GFP_KERNEL); + if (!fence_info) + return -ENOMEM; - if (ret < 0) - goto out; + for (i = 0; i < sync_file->num_fences; ++i) + sync_fill_fence_info(sync_file->cbs[i].fence, &fence_info[i]); - len += ret; + if (copy_to_user(u64_to_user_ptr(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; } @@ -560,7 +566,7 @@ static long sync_file_ioctl(struct file *file, unsigned int cmd, case SYNC_IOC_MERGE: return sync_file_ioctl_merge(sync_file, arg); - case SYNC_IOC_FENCE_INFO: + case SYNC_IOC_FILE_INFO: return sync_file_ioctl_fence_info(sync_file, arg); default: diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h index 4467c76..fbadb8a 100644 --- a/drivers/staging/android/uapi/sync.h +++ b/drivers/staging/android/uapi/sync.h @@ -16,14 +16,18 @@ /** * struct sync_merge_data - data passed to merge ioctl - * @fd2: file descriptor of second fence * @name: name of new fence + * @fd2: file descriptor of second fence * @fence: returns the fd of the new fence to userspace + * @flags: merge_data flags + * @pad: padding for 64-bit alignment, should always be zero */ struct sync_merge_data { - __s32 fd2; char name[32]; + __s32 fd2; __s32 fence; + __u32 flags; + __u32 pad; }; /** @@ -31,42 +35,54 @@ struct sync_merge_data { * @obj_name: name of parent sync_timeline * @driver_name: name of driver implementing the parent * @status: status of the fence 0:active 1:signaled <0:error + * @flags: fence_info flags * @timestamp_ns: timestamp of status change in nanoseconds */ struct sync_fence_info { char obj_name[32]; char driver_name[32]; __s32 status; + __u32 flags; __u64 timestamp_ns; }; /** * 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 - * @sync_fence_info: array of sync_fence_info for every fence in the sync_file + * @flags: sync_file_info flags + * @num_fences number of fences in the sync_file + * @pad: padding for 64-bit alignment, should always be zero + * @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 flags; + __u32 num_fences; + __u32 pad; - __u8 sync_fence_info[0]; + __u64 sync_fence_info; }; #define SYNC_IOC_MAGIC '>' /** + * Opcodes 0, 1 and 2 were burned during a API change to avoid users of the + * old API to get weird errors when trying to handling sync_files. The API + * change happened during the de-stage of the Sync Framework when there was + * no upstream users available. + */ + +/** * DOC: SYNC_IOC_MERGE - merge two fences * * Takes a struct sync_merge_data. Creates a new fence containing copies of * the sync_pts in both the calling fd and sync_merge_data.fd2. Returns the * new fence's fd in sync_merge_data.fence */ -#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 1, struct sync_merge_data) +#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 3, struct sync_merge_data) /** * DOC: SYNC_IOC_FENCE_INFO - get detailed information on a fence @@ -79,6 +95,6 @@ struct sync_file_info { * pt_info is a buffer containing sync_pt_infos for every sync_pt in the fence. * To iterate over the sync_pt_infos, use the sync_pt_info.len field. */ -#define SYNC_IOC_FENCE_INFO _IOWR(SYNC_IOC_MAGIC, 2, struct sync_file_info) +#define SYNC_IOC_FILE_INFO _IOWR(SYNC_IOC_MAGIC, 4, struct sync_file_info) #endif /* _UAPI_LINUX_SYNC_H */