[Intel-gfx] [PATCH 3/3] drm/i915: Never return 0 if request wait succeeds

[Janusz Krzysztofik]

On Wednesday, 16 November 2022 15:42:46 CET Andrzej Hajda wrote:
> On 16.11.2022 12:25, Janusz Krzysztofik wrote:
> > According to the docs of i915_request_wait_timeout(), its return value
> > "may be zero if the request is unfinished after the timeout expires."
> > However, 0 is also returned when the request is found finished right
> > after the timeout has expired.
> > 
> > Since the docs also state: "If the timeout is 0, it will return 1 if the
> > fence is signaled.", return 1 also when the fence is found signaled after
> > non-zero timeout has expired.
> 
> As I understand the patch "drm/i915: Fix i915_request fence wait 
> semantics", and the docs "timeout is 0" means the initial value of 
> timeout argument and this is handled already on the beginning of the 
> function.
> In case initial timeout is greater than zero and then it expires, 
> function should return 0 regardless of fence state. This is what I have 
> understood from reading docs and implementation of 
> dma_fence_default_wait [1], which should be the best source of info 
> about "dma_fence wait semantic".
> 
> As I said already, mixing remaining time and bool in return value of 
> dma_fence_wait* functions is very confusing, but changing it would 
> require major rework of the framework.

OK, let's drop this controversial patch.  The corner case it touches should 
already be handled correctly by intel_gt_retire_requests_timeout(), which this 
series is about, after 1/3 and 2/3 are applied.

Thanks,
Janusz

> 
> [1]: 
> https://elixir.bootlin.com/linux/latest/source/drivers/dma-buf/dma-fence.c#L753
> 
> Regards
> Andrzej
> 
> > 
> > Fixes: 7e2e69ed4678 ("drm/i915: Fix i915_request fence wait semantics")
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> > Cc: stable at vger.kernel.org # v5.17
> > ---
> >   drivers/gpu/drm/i915/i915_request.c | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> > index f949a9495758a..406ddfafbed4d 100644
> > --- a/drivers/gpu/drm/i915/i915_request.c
> > +++ b/drivers/gpu/drm/i915/i915_request.c
> > @@ -2079,6 +2079,8 @@ long i915_request_wait_timeout(struct i915_request *rq,
> >   
> >   		timeout = io_schedule_timeout(timeout);
> >   	}
> > +	if (!timeout)	/* expired but signaled, we shouldn't return 0 */
> > +		timeout = 1;
> >   	__set_current_state(TASK_RUNNING);
> >   
> >   	if (READ_ONCE(wait.tsk))
> 
>