* [PATCH v2] drm/i915: Always normalize return timeout for wait_timeout_ioctl
@ 2013-04-26 13:09 ville.syrjala
2013-04-26 13:22 ` [PATCH v3] " ville.syrjala
0 siblings, 1 reply; 4+ messages in thread
From: ville.syrjala @ 2013-04-26 13:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
From: Chris Wilson <chris@chris-wilson.co.uk>
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.
v2 [vsyrjala]: Only normalize the timespec if it's invalid
Cc: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6be940e..a469298 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1045,6 +1045,8 @@ 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);
+ if (!timespec_valid(timeout))
+ set_normalized_timespec(timeout, 0, 0);
}
switch (end) {
@@ -1053,8 +1055,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.8.1.5
_______________________________________________
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* [PATCH v3] drm/i915: Always normalize return timeout for wait_timeout_ioctl
2013-04-26 13:09 [PATCH v2] drm/i915: Always normalize return timeout for wait_timeout_ioctl ville.syrjala
@ 2013-04-26 13:22 ` ville.syrjala
2013-04-27 19:51 ` Ben Widawsky
0 siblings, 1 reply; 4+ messages in thread
From: ville.syrjala @ 2013-04-26 13:22 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
From: Chris Wilson <chris@chris-wilson.co.uk>
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.
v2 [vsyrjala]: Only normalize the timespec if it's invalid
v3: Add a comment to clarify the situation and remove the now
useless WARN_ON() (ickle)
Cc: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6be940e..6165535 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1045,6 +1045,8 @@ 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);
+ if (!timespec_valid(timeout)) /* i.e. negative time remains */
+ set_normalized_timespec(timeout, 0, 0);
}
switch (end) {
@@ -1053,8 +1055,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 */
@@ -2377,10 +2377,8 @@ 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) {
- WARN_ON(!timespec_valid(timeout));
+ if (timeout)
args->timeout_ns = timespec_to_ns(timeout);
- }
return ret;
out:
--
1.8.1.5
_______________________________________________
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 v3] drm/i915: Always normalize return timeout for wait_timeout_ioctl
2013-04-26 13:22 ` [PATCH v3] " ville.syrjala
@ 2013-04-27 19:51 ` Ben Widawsky
2013-04-30 8:50 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Ben Widawsky @ 2013-04-27 19:51 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Fri, Apr 26, 2013 at 04:22:46PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> 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.
>
> v2 [vsyrjala]: Only normalize the timespec if it's invalid
> v3: Add a comment to clarify the situation and remove the now
> useless WARN_ON() (ickle)
>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
This does an excellent job of removing the warnings I'd sometimes see.
Unfortunately I've confirmed I can still reproduce the hang, and now
with no errors to leave any clues. I can't promise, but I'm 75%ish
certain I'd not always see a WARN before too (in fact I think that was
the case when Chris wrote the original patch)
The frequency with which I see the hangs seem to be increasing as well.
Does nobody else see this? I will have to take a shot at getting more
info.
[snip]
--
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] drm/i915: Always normalize return timeout for wait_timeout_ioctl
2013-04-27 19:51 ` Ben Widawsky
@ 2013-04-30 8:50 ` Daniel Vetter
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2013-04-30 8:50 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Sat, Apr 27, 2013 at 12:51:33PM -0700, Ben Widawsky wrote:
> On Fri, Apr 26, 2013 at 04:22:46PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > 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.
> >
> > v2 [vsyrjala]: Only normalize the timespec if it's invalid
> > v3: Add a comment to clarify the situation and remove the now
> > useless WARN_ON() (ickle)
> >
> > Cc: Ben Widawsky <ben@bwidawsk.net>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> This does an excellent job of removing the warnings I'd sometimes see.
>
> Unfortunately I've confirmed I can still reproduce the hang, and now
> with no errors to leave any clues. I can't promise, but I'm 75%ish
> certain I'd not always see a WARN before too (in fact I think that was
> the case when Chris wrote the original patch)
>
> The frequency with which I see the hangs seem to be increasing as well.
> Does nobody else see this? I will have to take a shot at getting more
> info.
Picked up for -fixes, thanks for the patch.
-Daniel
--
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
end of thread, other threads:[~2013-04-30 8:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-26 13:09 [PATCH v2] drm/i915: Always normalize return timeout for wait_timeout_ioctl ville.syrjala
2013-04-26 13:22 ` [PATCH v3] " ville.syrjala
2013-04-27 19:51 ` Ben Widawsky
2013-04-30 8:50 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox