On 2017年10月09日 21:49, John Einar Reitan wrote:
sync_file_ioctl_fence_info has a race between filling the status of the underlying fences and the overall status of the sync_file. If fence transitions in the time frame between its sync_fill_fence_info and the later dma_fence_is_signaled for the sync_file, the returned information is inconsistent showing non-signaled underlying fences but an overall signaled state.
This patch changes sync_file_ioctl_fence_info to track what has been encoded and using that as the overall sync_file status.
Tested-by: Vamsidhar Reddy Gaddam vamsidhar.gaddam@arm.com Signed-off-by: John Einar Reitan john.reitan@arm.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Gustavo Padovan gustavo@padovan.org Cc: dri-devel@lists.freedesktop.org
Changes since v1 (thanks Chris Wilson):
Replaced an unneeded local variable by writing directly to the struct
drivers/dma-buf/sync_file.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 66fb40d0ebdb..03830634e141 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -383,7 +383,7 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, return err; }
-static void sync_fill_fence_info(struct dma_fence *fence, +static int sync_fill_fence_info(struct dma_fence *fence, struct sync_fence_info *info)
Out of curious, why name to sync_fill_xxx not sync_file_xxx?
Regards, David Zhou
{ strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), @@ -399,6 +399,8 @@ static void sync_fill_fence_info(struct dma_fence *fence, test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags) ? ktime_to_ns(fence->timestamp) : ktime_set(0, 0);
return info->status; }
static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
@@ -424,8 +426,12 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, * sync_fence_info and return the actual number of fences on * info->num_fences. */
- if (!info.num_fences)
if (!info.num_fences) {
info.status = dma_fence_is_signaled(sync_file->fence);goto no_fences;
} else {
info.status = 1;}
if (info.num_fences < num_fences) return -EINVAL;
@@ -435,8 +441,10 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, if (!fence_info) return -ENOMEM;
- for (i = 0; i < num_fences; i++)
sync_fill_fence_info(fences[i], &fence_info[i]);
for (i = 0; i < num_fences; i++) {
int status = sync_fill_fence_info(fences[i], &fence_info[i]);info.status = info.status <= 0 ? info.status : status;}
if (copy_to_user(u64_to_user_ptr(info.sync_fence_info), fence_info, size)) {
@@ -446,7 +454,6 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
no_fences: sync_file_get_name(sync_file, info.name, sizeof(info.name));
info.status = dma_fence_is_signaled(sync_file->fence); info.num_fences = num_fences;
if (copy_to_user((void __user *)arg, &info, sizeof(info)))