[Mesa-dev] [PATCH] i965: Fix comment about DRM_IOCTL_I915_GEM_WAIT.

[Daniel Vetter]

On Wed, Jul 15, 2015 at 12:20:15PM -0700, Kristian Høgsberg wrote:
> On Wed, Jul 15, 2015 at 10:22 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> > From: Chris Wilson <chris at chris-wilson.co.uk>
> >
> > The kernel actually waits forever when supplied a timeout value < 0,
> > rather than returning immediately.  See i915_gem_wait_ioctl() in
> > i915_gem.c's call to __i915_wait_request().
> >
> > (split by Ken from a large patch authored by Chris Wilson)
> >
> > Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/mesa/drivers/dri/i965/intel_syncobj.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_syncobj.c b/src/mesa/drivers/dri/i965/intel_syncobj.c
> > index c44c4be..c2f4fa9 100644
> > --- a/src/mesa/drivers/dri/i965/intel_syncobj.c
> > +++ b/src/mesa/drivers/dri/i965/intel_syncobj.c
> > @@ -105,9 +105,9 @@ brw_fence_client_wait(struct brw_context *brw, struct brw_fence *fence,
> >     assert(fence->batch_bo);
> >
> >     /* DRM_IOCTL_I915_GEM_WAIT uses a signed 64 bit timeout and returns
> > -    * immediately for timeouts <= 0.  The best we can do is to clamp the
> > -    * timeout to INT64_MAX.  This limits the maximum timeout from 584 years to
> > -    * 292 years - likely not a big deal.
> > +    * immediately for timeout == 0, and indefinitely if timeout is negative.
> > +    * The best we can do is to clamp the timeout to INT64_MAX.  This limits
> > +    * the maximum timeout from 584 years to 292 years - likely not a big deal.
> 
> No, there are kernel versions in the wild that has this bug. The
> comment after the patch says:
> 
>  /* DRM_IOCTL_I915_GEM_WAIT uses a signed 64 bit timeout and returns
>  * immediately for timeout == 0, and indefinitely if timeout is negative.
>  * The best we can do is to clamp the timeout to INT64_MAX.  This limits
>  * the maximum timeout from 584 years to 292 years - likely not a big deal.
> 
> which doesn't make sense. If we feel like we need to point out that
> we've fixed the bug, that's fine, but keep the part about how some
> kernels are broken so it's clear why the workaround is needed.

That fixed got cc: stable'ed. If you're still running broken kernels you
don't care enough about anything that I don't think this would matter
either. It was just that it took us a few releases to spot this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch