Re: [Intel-gfx] [PATCH v2] drm/i915: Handle full s64 precision for wait-ioctl

[Chris Wilson]

Quoting Joonas Lahtinen (2017-08-14 11:42:05)
> On Fri, 2017-08-11 at 11:57 +0100, Chris Wilson wrote:
> > The wait-ioctl is optionally supplied a timeout with nanosecond
> > precision in a s64 field. We use nsecs_to_jiffies64() to convert that
> > into the jiffies consumed by the scheduler, but internally
> > nsecs_to_jiffies64() does not guard against overflow (as it's purpose is
> > for use by the scheduler and not drivers!). So we must guard against the
> > overflow ourselves, and in the process note that we may then return
> > much earlier than the timeout selected by the user, so don't report
> > ETIME unless we do hit the timeout. (Woe betold us though if the user
> > waits for a year (32bit) and the request is still not complete!)
> > 
> > v2: Refine overflow detection (to not include an overffow itself)
> > 
> > Reported-by: Jason Ekstrand 
> > Signed-off-by: Chris Wilson 
> > Cc: Joonas Lahtinen 
> > Cc: Daniel Vetter 
> 
> Reviewed-by: Joonas Lahtinen 

And pushed, thanks for the review.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: Handle full s64 precision for wait-ioctl

[Joonas Lahtinen]

On Fri, 2017-08-11 at 11:57 +0100, Chris Wilson wrote:
> The wait-ioctl is optionally supplied a timeout with nanosecond
> precision in a s64 field. We use nsecs_to_jiffies64() to convert that
> into the jiffies consumed by the scheduler, but internally
> nsecs_to_jiffies64() does not guard against overflow (as it's purpose is
> for use by the scheduler and not drivers!). So we must guard against the
> overflow ourselves, and in the process note that we may then return
> much earlier than the timeout selected by the user, so don't report
> ETIME unless we do hit the timeout. (Woe betold us though if the user
> waits for a year (32bit) and the request is still not complete!)
> 
> v2: Refine overflow detection (to not include an overffow itself)
> 
> Reported-by: Jason Ekstrand 
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 
> Cc: Daniel Vetter 

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2] drm/i915: Handle full s64 precision for wait-ioctl

[Chris Wilson]

The wait-ioctl is optionally supplied a timeout with nanosecond
precision in a s64 field. We use nsecs_to_jiffies64() to convert that
into the jiffies consumed by the scheduler, but internally
nsecs_to_jiffies64() does not guard against overflow (as it's purpose is
for use by the scheduler and not drivers!). So we must guard against the
overflow ourselves, and in the process note that we may then return
much earlier than the timeout selected by the user, so don't report
ETIME unless we do hit the timeout. (Woe betold us though if the user
waits for a year (32bit) and the request is still not complete!)

v2: Refine overflow detection (to not include an overffow itself)

Reported-by: Jason Ekstrand 
Signed-off-by: Chris Wilson 
Cc: Joonas Lahtinen 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_drv.h | 5 +
 drivers/gpu/drm/i915/i915_gem.c | 6 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 907603cba447..4f9f7b6ac276 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -4171,6 +4171,11 @@ static inline unsigned long 
msecs_to_jiffies_timeout(const unsigned int m)
 
 static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
 {
+   /* nsecs_to_jiffies64() does not guard against overflow */
+   if (NSEC_PER_SEC % HZ &&
+   div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
+   return MAX_JIFFY_OFFSET;
+
 return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 000a764ee8d9..dbda7d078245 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3284,7 +3284,7 @@ static unsigned long to_wait_timeout(s64 timeout_ns)
  *  -ERESTARTSYS: signal interrupted the wait
  *  -ENONENT: object doesn't exist
  * Also possible, but rare:
- *  -EAGAIN: GPU wedged
+ *  -EAGAIN: incomplete, restart syscall
  *  -ENOMEM: damn
  *  -ENODEV: Internal IRQ fail
  *  -E?: The add request failed
@@ -3332,6 +3332,10 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, 
struct drm_file *file)
 */
if (ret == -ETIME && !nsecs_to_jiffies(args->timeout_ns))
args->timeout_ns = 0;
+
+   /* Asked to wait beyond the jiffie/scheduler precision? */
+   if (ret == -ETIME && args->timeout_ns)
+   ret = -EAGAIN;
}
 
i915_gem_object_put(obj);
-- 
2.13.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx