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@xxxxxxx>
> Signed-off-by: John Einar Reitan <john.reitan@xxxxxxx>
> Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
> Cc: Gustavo Padovan <gustavo@xxxxxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> ---
> 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@xxxxxxxxxxxxxxxxxx>
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
