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

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

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.

Regards,
Lucas

>  	}
>  	dma_resv_iter_end(&cursor);
>