[PATCH 1/4] dma-buf/fence: make timeout handling in fence_default_wait consistent

Chris Wilson chris at chris-wilson.co.uk
Wed Oct 26 08:49:54 UTC 2016


On Tue, Oct 25, 2016 at 02:25:08PM +0200, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
> 
> Kernel functions taking a timeout usually return 1 on success even
> when they get a zero timeout.

Which? The canonical example of schedule_timeout() doesn't behave like
this.
 
> Signen-off-by: Christian König <christian.koenig at amd.com>
> Reviewed-by: Chunming Zhou <david1.zhou at amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> ---
>  drivers/dma-buf/fence.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
> index 4d51f9e..fb915ab 100644
> --- a/drivers/dma-buf/fence.c
> +++ b/drivers/dma-buf/fence.c
> @@ -335,18 +335,20 @@ fence_default_wait_cb(struct fence *fence, struct fence_cb *cb)
>   * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
>   *
>   * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or the
> - * remaining timeout in jiffies on success.
> + * remaining timeout in jiffies on success. If timeout is zero the value one is
> + * returned if the fence is already signaled for consistency with other
> + * functions taking a jiffies timeout.
>   */
>  signed long
>  fence_default_wait(struct fence *fence, bool intr, signed long timeout)
>  {
>  	struct default_wait_cb cb;
>  	unsigned long flags;
> -	signed long ret = timeout;
> +	signed long ret = timeout ? timeout : 1;
>  	bool was_set;

This is horrible.

reservation_object_wait_timeout_rcu(), we pass it a timeout of zero to
do a safe probe on all attached fences.

The first fence happens to be signaled, so now we set the timeout to 1
jiffie. Any subsequent incomplete fence will then wait for that jiffie
before reporting 0.

When we report to userspace, we convert the timeout into an error,
usually EBUSY or ETIME. Applying that same convention here makes this
all much cleaner...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


More information about the dri-devel mailing list