| Message ID | 1413048069-1436-1-git-send-email-olvaffe@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Oct 12, 2014 at 1:21 AM, Chia-I Wu <olvaffe@gmail.com> wrote: > 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. > 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. > > Signed-off-by: Chia-I Wu <olvaffe@gmail.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > b/drivers/gpu/drm/i915/i915_gem.c > index ad55b06..3da2d62 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -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: > -- > 2.1.1 > >
On Mon, Oct 13, 2014 at 12:40:48AM +0800, Chia-I Wu wrote: > On Sun, Oct 12, 2014 at 1:21 AM, Chia-I Wu <olvaffe@gmail.com> wrote: > > > 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. > > > 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. 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
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ad55b06..3da2d62 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -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:
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. Signed-off-by: Chia-I Wu <olvaffe@gmail.com> --- drivers/gpu/drm/i915/i915_gem.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)