[Intel-gfx] [PATCH 2/2] drm: Fix syncobj handing of schedule() returning 0

[Daniel Vetter]

On Tue, Sep 18, 2018 at 11:07:20AM +0100, Chris Wilson wrote:
> After schedule() returns 0, we must do one last check of COND to
> determine the reason for the wakeup with 0 jiffies remaining before
> reporting the timeout -- otherwise we may lose the signal due to
> scheduler delays.

Ah classic!

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> References: https://bugs.freedesktop.org/show_bug.cgi?id=106690
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/drm_syncobj.c | 41 +++++++++++++----------------------
>  1 file changed, 15 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index e254f97fed7d..5bcb3ef9b256 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -672,7 +672,6 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>  {
>  	struct syncobj_wait_entry *entries;
>  	struct dma_fence *fence;
> -	signed long ret;
>  	uint32_t signaled_count, i;
>  
>  	entries = kcalloc(count, sizeof(*entries), GFP_KERNEL);
> @@ -692,7 +691,7 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>  			if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
>  				continue;
>  			} else {
> -				ret = -EINVAL;
> +				timeout = -EINVAL;
>  				goto cleanup_entries;
>  			}
>  		}
> @@ -704,12 +703,6 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>  		}
>  	}
>  
> -	/* Initialize ret to the max of timeout and 1.  That way, the
> -	 * default return value indicates a successful wait and not a
> -	 * timeout.
> -	 */
> -	ret = max_t(signed long, timeout, 1);
> -
>  	if (signaled_count == count ||
>  	    (signaled_count > 0 &&
>  	     !(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL)))
> @@ -760,18 +753,17 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>  			goto done_waiting;
>  
>  		if (timeout == 0) {
> -			/* If we are doing a 0 timeout wait and we got
> -			 * here, then we just timed out.
> -			 */
> -			ret = 0;
> +			timeout = -ETIME;
>  			goto done_waiting;
>  		}
>  
> -		ret = schedule_timeout(ret);
> +		if (signal_pending(current)) {
> +			timeout = -ERESTARTSYS;
> +			goto done_waiting;
> +		}
>  
> -		if (ret > 0 && signal_pending(current))
> -			ret = -ERESTARTSYS;
> -	} while (ret > 0);
> +		timeout = schedule_timeout(timeout);
> +	} while (1);
>  
>  done_waiting:
>  	__set_current_state(TASK_RUNNING);
> @@ -788,7 +780,7 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>  	}
>  	kfree(entries);
>  
> -	return ret;
> +	return timeout;
>  }
>  
>  /**
> @@ -829,19 +821,16 @@ static int drm_syncobj_array_wait(struct drm_device *dev,
>  				  struct drm_syncobj **syncobjs)
>  {
>  	signed long timeout = drm_timeout_abs_to_jiffies(wait->timeout_nsec);
> -	signed long ret = 0;
>  	uint32_t first = ~0;
>  
> -	ret = drm_syncobj_array_wait_timeout(syncobjs,
> -					     wait->count_handles,
> -					     wait->flags,
> -					     timeout, &first);
> -	if (ret < 0)
> -		return ret;
> +	timeout = drm_syncobj_array_wait_timeout(syncobjs,
> +						 wait->count_handles,
> +						 wait->flags,
> +						 timeout, &first);
> +	if (timeout < 0)
> +		return timeout;
>  
>  	wait->first_signaled = first;
> -	if (ret == 0)
> -		return -ETIME;
>  	return 0;
>  }
>  
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch