Quoting John Einar Reitan (2017-10-03 14:51:00)
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
drivers/dma-buf/sync_file.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 66fb40d0ebdb..ebf5764b868c 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) { 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;
In hindsight, having both the per-fence and global variable be called info was a mistake. Certainly made this diff a little harder to parse than required!
}
static long sync_file_ioctl_fence_info(struct sync_file *sync_file, @@ -408,7 +410,7 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, struct sync_fence_info *fence_info = NULL; struct dma_fence **fences; __u32 size;
int num_fences, ret, i;
int num_fences, ret, i, fences_status = 1; if (copy_from_user(&info, (void __user *)arg, sizeof(info))) return -EFAULT;@@ -424,8 +426,10 @@ 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) {fences_status = dma_fence_is_signaled(sync_file->fence);
Personally I would have set info.status directly rather than add a new fences_status. But that's immaterial to the bug fix,
Reviewed-by: Chris Wilson chris@chris-wilson.co.uk -Chris