[PATCH] dma-buf: fix timeout handling in dma_resv_wait_timeout

[Christian König]

Am 28.01.25 um 11:48 schrieb Lucas Stach:
> Hi Christian,
>
> Am Dienstag, dem 28.01.2025 um 11:08 +0100 schrieb Christian König:
>> Even the kerneldoc says that with a zero timeout the function should not
>> wait for anything, but still return 1 to indicate that the fences are
>> signaled now.
>>
>> Unfortunately that isn't what was implemented, instead of only returning
>> 1 we also waited for at least one jiffies.
>>
>> Fix that by adjusting the handling to what the function is actually
>> documented to do.
>>
>> Reported-by: Marek Olšák <marek.olsak at amd.com>
>> Reported-by: Lucas Stach <l.stach at pengutronix.de>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> Cc: <stable at vger.kernel.org>
>> ---
>>   drivers/dma-buf/dma-resv.c | 11 ++++++-----
>>   1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
>> index 5f8d010516f0..ae92d9d2394d 100644
>> --- a/drivers/dma-buf/dma-resv.c
>> +++ b/drivers/dma-buf/dma-resv.c
>> @@ -684,11 +684,12 @@ long dma_resv_wait_timeout(struct dma_resv *obj, enum dma_resv_usage usage,
>>   	dma_resv_iter_begin(&cursor, obj, usage);
>>   	dma_resv_for_each_fence_unlocked(&cursor, fence) {
>>   
>> -		ret = dma_fence_wait_timeout(fence, intr, ret);
>> -		if (ret <= 0) {
>> -			dma_resv_iter_end(&cursor);
>> -			return ret;
>> -		}
>> +		ret = dma_fence_wait_timeout(fence, intr, timeout);
>> +		if (ret <= 0)
>> +			break;
>> +
>> +		/* Even for zero timeout the return value is 1 */
>> +		timeout = min(timeout, ret);
> This min construct and the comment confused me a bit at first. I think
> it would be easier to read as
>
> /* With a zero timeout dma_fence_wait_timeout makes up a
>   * positive return value for already signaled fences.
>   */
> if (timeout)
> 	timeout = ret;

Good point, going to change that.

>
> Also please change the initialization of ret on top of the function to
> ret = 1 so it has the right value when no fences are present. Now that
> you use the timeout variable for the fence wait, there is no point in
> initializing ret to the timeout.

When no fences are present returning 1 would be incorrect if the timeout 
value was non zero.

Only when the timeout value is zero we should return 1 to indicate that 
there is nothing to wait for.

Regards,
Christian.

>
> Regards,
> Lucas
>
>>   	}
>>   	dma_resv_iter_end(&cursor);
>>