drm/i915: fix wait ioctl with negative timeout
Subsystems (2)
PATCH 1/1 drm/i915: fix wait ioctl with negative timeout
When timeout_ns is negative, it really means to wait indefinitely instead of returning immediately. But since userspace can no longer rely on that, I am not sure if there is any point fixing it.
--- drivers/gpu/drm/i915/i915_gem.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/i915_gem.c +4 -3
@@ -2787,9 +2787,9 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) goto out;/* Do this after OLR check to make sure we make forward progress polling -* on this IOCTL with a timeout <=0 (like busy ioctl) +* on this IOCTL with a timeout == 0 (like busy ioctl) */ -if (args->timeout_ns <=0 ) { +if (args->timeout_ns ==0 ) { ret = -ETIME ; goto out; }
@@ -2798,7 +2798,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) reset_counter =atomic_read (&dev_priv->gpu_error.reset_counter);mutex_unlock (&dev->struct_mutex); -return __wait_seqno (ring, seqno, reset_counter, true, &args->timeout_ns, +return __wait_seqno (ring, seqno, reset_counter, true, + (args->timeout_ns >0 ) ? &args->timeout_ns :NULL , file->driver_priv); out:
Signature · 1 line
2.1.1
Quoted 6 lines · Chia-I Wu
When
timeout_nsis negative, it really means to wait indefinitely instead of
returning immediately. But since userspace can no longer rely on that, I am
not sure if there is any point fixing it.
Note that userspace may use GL_TIMEOUT_IGNORED for timeout_ns to wait indefinitely. The macro is defined to
#define GL_TIMEOUT_IGNORED 0xFFFFFFFFFFFFFFFFull
Prior to 3.17, the kernel would behave as expected. But on 3.17, it would return immediately with -ETIME if the bo is busy.
Quoted 42 lines
Signed-off-by Chia-I Wu <[email protected]>--- drivers/gpu/drm/i915/i915_gem.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)drivers/gpu/drm/i915/i915_gem.c +0
-0@@ -2787,9 +2787,9 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) goto out;/* Do this after OLR check to make sure we make forward progress polling
drivers/gpu/drm/i915/i915_gem.c +4
-3-* on this IOCTL with a timeout <=0 (like busy ioctl) +* on this IOCTL with a timeout == 0 (like busy ioctl) */ -if (args->timeout_ns <=0 ) { +if (args->timeout_ns ==0 ) { ret = -ETIME ; goto out; }@@ -2798,7 +2798,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) reset_counter =atomic_read (&dev_priv->gpu_error.reset_counter);mutex_unlock (&dev->struct_mutex); - return __wait_seqno(ring, seqno, reset_counter, true, &args->timeout_ns, + return __wait_seqno(ring, seqno, reset_counter, true, + (args->timeout_ns > 0) ? &args->timeout_ns : NULL,file->driver_priv);out:
--
2.1.1
Signature · 1 line
[email protected]
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 8 lines · Chia-I Wu
Quoted 6 lines · Chia-I Wu
When
timeout_nsis negative, it really means to wait indefinitely instead of
returning immediately. But since userspace can no longer rely on that, I am
not sure if there is any point fixing it.
Quoted 7 lines
Note that userspace may use
GL_TIMEOUT_IGNOREDfortimeout_nsto wait indefinitely. The macro is defined to#define
GL_TIMEOUT_IGNORED0xFFFFFFFFFFFFFFFFullPrior to 3.17, the kernel would behave as expected. But on 3.17, it would return immediately with
-ETIMEif the bo is busy.
That sounds like a regression, for which we need a testcase in igt and a special-case in our wait ioctl to make sure that we have an infinite timeout for this input value. Can you please take care of both?
I guess we could also restore the bevahiour for negative timeouts, just for the sake of it. Would again need a testcase though. Also note that I've recently refactored the wait ioctl testcase, so adding new subtests should be a lot easier now.
Thanks, Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch