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

[Christian König]

Am 26.10.2016 um 10:49 schrieb Chris Wilson:
> 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.

wait_event_timeout() for example, as well as to the fence_wait_timeout() 
and reservation_object_wait_timeout_rcu() functions build on top of this 
one.

Regards,
Christian.

>   
>> 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
>