* [PATCH] drm/i915: The timeout for wait_timeout_ioctl is only valid upon success @ 2013-04-09 8:58 Chris Wilson 2013-04-09 9:07 ` Daniel Vetter 0 siblings, 1 reply; 4+ messages in thread From: Chris Wilson @ 2013-04-09 8:58 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky As we only update and sanitize the return timeout value after a successful wait, we should not assert that it is valid for any error returns. Also, for consistency, we should only modify args->timeout_ns upon success. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Ben Widawsky <ben@bwidawsk.net> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 63c05dd..da78cf7 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2377,7 +2377,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) mutex_unlock(&dev->struct_mutex); ret = __wait_seqno(ring, seqno, reset_counter, true, timeout); - if (timeout) { + if (ret == 0 && timeout) { WARN_ON(!timespec_valid(timeout)); args->timeout_ns = timespec_to_ns(timeout); } -- 1.7.10.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: The timeout for wait_timeout_ioctl is only valid upon success 2013-04-09 8:58 [PATCH] drm/i915: The timeout for wait_timeout_ioctl is only valid upon success Chris Wilson @ 2013-04-09 9:07 ` Daniel Vetter 2013-04-09 9:13 ` [PATCH] drm/i915: Always normalize return timeout for wait_timeout_ioctl Chris Wilson 0 siblings, 1 reply; 4+ messages in thread From: Daniel Vetter @ 2013-04-09 9:07 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx, Ben Widawsky On Tue, Apr 09, 2013 at 09:58:15AM +0100, Chris Wilson wrote: > As we only update and sanitize the return timeout value after a > successful wait, we should not assert that it is valid for any error > returns. Also, for consistency, we should only modify args->timeout_ns > upon success. Doesn't that break our -EAGAIN trickery? -Daniel > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Ben Widawsky <ben@bwidawsk.net> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 63c05dd..da78cf7 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2377,7 +2377,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) > mutex_unlock(&dev->struct_mutex); > > ret = __wait_seqno(ring, seqno, reset_counter, true, timeout); > - if (timeout) { > + if (ret == 0 && timeout) { > WARN_ON(!timespec_valid(timeout)); > args->timeout_ns = timespec_to_ns(timeout); > } > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] drm/i915: Always normalize return timeout for wait_timeout_ioctl 2013-04-09 9:07 ` Daniel Vetter @ 2013-04-09 9:13 ` Chris Wilson 2013-04-09 9:26 ` Ville Syrjälä 0 siblings, 1 reply; 4+ messages in thread From: Chris Wilson @ 2013-04-09 9:13 UTC (permalink / raw) To: intel-gfx; +Cc: Ben Widawsky As we recompute the remaining timeout after waiting, there is a potential for that timeout to be less than zero and so need sanitizing. The timeout is always returned to userspace and validated, so we should always perform the sanitation. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Ben Widawsky <ben@bwidawsk.net> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 63c05dd..fcf8492 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1045,6 +1045,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, if (timeout) { struct timespec sleep_time = timespec_sub(now, before); *timeout = timespec_sub(*timeout, sleep_time); + set_normalized_timespec(timeout, 0, 0); } switch (end) { @@ -1053,8 +1054,6 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, case -ERESTARTSYS: /* Signal */ return (int)end; case 0: /* Timeout */ - if (timeout) - set_normalized_timespec(timeout, 0, 0); return -ETIME; default: /* Completed */ WARN_ON(end < 0); /* We're not aware of other errors */ -- 1.7.10.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Always normalize return timeout for wait_timeout_ioctl 2013-04-09 9:13 ` [PATCH] drm/i915: Always normalize return timeout for wait_timeout_ioctl Chris Wilson @ 2013-04-09 9:26 ` Ville Syrjälä 0 siblings, 0 replies; 4+ messages in thread From: Ville Syrjälä @ 2013-04-09 9:26 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx, Ben Widawsky On Tue, Apr 09, 2013 at 10:13:23AM +0100, Chris Wilson wrote: > As we recompute the remaining timeout after waiting, there is a > potential for that timeout to be less than zero and so need sanitizing. > The timeout is always returned to userspace and validated, so we should > always perform the sanitation. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Ben Widawsky <ben@bwidawsk.net> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 63c05dd..fcf8492 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1045,6 +1045,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, > if (timeout) { > struct timespec sleep_time = timespec_sub(now, before); > *timeout = timespec_sub(*timeout, sleep_time); > + set_normalized_timespec(timeout, 0, 0); This will just set timeout = {0,0}, no? And timespec_sub() should already return a normalized timespec, so I guess the warning would just trigger when sleep_time > timeout. So we should maybe just check for that case, and only then set timeout={0,0}? > } > > switch (end) { > @@ -1053,8 +1054,6 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, > case -ERESTARTSYS: /* Signal */ > return (int)end; > case 0: /* Timeout */ > - if (timeout) > - set_normalized_timespec(timeout, 0, 0); > return -ETIME; > default: /* Completed */ > WARN_ON(end < 0); /* We're not aware of other errors */ > -- > 1.7.10.4 -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-04-09 9:26 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-04-09 8:58 [PATCH] drm/i915: The timeout for wait_timeout_ioctl is only valid upon success Chris Wilson 2013-04-09 9:07 ` Daniel Vetter 2013-04-09 9:13 ` [PATCH] drm/i915: Always normalize return timeout for wait_timeout_ioctl Chris Wilson 2013-04-09 9:26 ` Ville Syrjälä
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).