[Intel-gfx] [PATCH 3/3] drm/i915: Never return 0 if request wait succeeds
Janusz Krzysztofik
janusz.krzysztofik at linux.intel.com
Wed Nov 16 15:48:41 UTC 2022
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))
>
>
More information about the dri-devel
mailing list