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@xxxxxxxxxxxxxxx> > > Cc: stable@xxxxxxxxxxxxxxx # 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)) > >
