Re: [Intel-gfx] [PATCH 2/2] drm: Fix syncobj handing of schedule() returning 0
Quoting Daniel Vetter (2018-09-21 10:15:41) > On Tue, Sep 18, 2018 at 11:07:20AM +0100, Chris Wilson wrote: > > After schedule() returns 0, we must do one last check of COND to > > determine the reason for the wakeup with 0 jiffies remaining before > > reporting the timeout -- otherwise we may lose the signal due to > > scheduler delays. > > Ah classic! > > Reviewed-by: Daniel Vetter Indeed, thanks for the reviews, pushed to drm-misc-next. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm: Fix syncobj handing of schedule() returning 0
On Tue, Sep 18, 2018 at 11:07:20AM +0100, Chris Wilson wrote:
> After schedule() returns 0, we must do one last check of COND to
> determine the reason for the wakeup with 0 jiffies remaining before
> reporting the timeout -- otherwise we may lose the signal due to
> scheduler delays.
Ah classic!
Reviewed-by: Daniel Vetter
> References: https://bugs.freedesktop.org/show_bug.cgi?id=106690
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/drm_syncobj.c | 41 +--
> 1 file changed, 15 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index e254f97fed7d..5bcb3ef9b256 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -672,7 +672,6 @@ static signed long drm_syncobj_array_wait_timeout(struct
> drm_syncobj **syncobjs,
> {
> struct syncobj_wait_entry *entries;
> struct dma_fence *fence;
> - signed long ret;
> uint32_t signaled_count, i;
>
> entries = kcalloc(count, sizeof(*entries), GFP_KERNEL);
> @@ -692,7 +691,7 @@ static signed long drm_syncobj_array_wait_timeout(struct
> drm_syncobj **syncobjs,
> if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
> continue;
> } else {
> - ret = -EINVAL;
> + timeout = -EINVAL;
> goto cleanup_entries;
> }
> }
> @@ -704,12 +703,6 @@ static signed long drm_syncobj_array_wait_timeout(struct
> drm_syncobj **syncobjs,
> }
> }
>
> - /* Initialize ret to the max of timeout and 1. That way, the
> - * default return value indicates a successful wait and not a
> - * timeout.
> - */
> - ret = max_t(signed long, timeout, 1);
> -
> if (signaled_count == count ||
> (signaled_count > 0 &&
>!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL)))
> @@ -760,18 +753,17 @@ static signed long
> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
> goto done_waiting;
>
> if (timeout == 0) {
> - /* If we are doing a 0 timeout wait and we got
> - * here, then we just timed out.
> - */
> - ret = 0;
> + timeout = -ETIME;
> goto done_waiting;
> }
>
> - ret = schedule_timeout(ret);
> + if (signal_pending(current)) {
> + timeout = -ERESTARTSYS;
> + goto done_waiting;
> + }
>
> - if (ret > 0 && signal_pending(current))
> - ret = -ERESTARTSYS;
> - } while (ret > 0);
> + timeout = schedule_timeout(timeout);
> + } while (1);
>
> done_waiting:
> __set_current_state(TASK_RUNNING);
> @@ -788,7 +780,7 @@ static signed long drm_syncobj_array_wait_timeout(struct
> drm_syncobj **syncobjs,
> }
> kfree(entries);
>
> - return ret;
> + return timeout;
> }
>
> /**
> @@ -829,19 +821,16 @@ static int drm_syncobj_array_wait(struct drm_device
> *dev,
> struct drm_syncobj **syncobjs)
> {
> signed long timeout = drm_timeout_abs_to_jiffies(wait->timeout_nsec);
> - signed long ret = 0;
> uint32_t first = ~0;
>
> - ret = drm_syncobj_array_wait_timeout(syncobjs,
> - wait->count_handles,
> - wait->flags,
> - timeout, );
> - if (ret < 0)
> - return ret;
> + timeout = drm_syncobj_array_wait_timeout(syncobjs,
> + wait->count_handles,
> + wait->flags,
> + timeout, );
> + if (timeout < 0)
> + return timeout;
>
> wait->first_signaled = first;
> - if (ret == 0)
> - return -ETIME;
> return 0;
> }
>
> --
> 2.19.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm: Fix syncobj handing of schedule() returning 0
After schedule() returns 0, we must do one last check of COND to
determine the reason for the wakeup with 0 jiffies remaining before
reporting the timeout -- otherwise we may lose the signal due to
scheduler delays.
References: https://bugs.freedesktop.org/show_bug.cgi?id=106690
Signed-off-by: Chris Wilson
Reviewed-by: Tvrtko Ursulin
---
drivers/gpu/drm/drm_syncobj.c | 41 +--
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index e254f97fed7d..5bcb3ef9b256 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -672,7 +672,6 @@ static signed long drm_syncobj_array_wait_timeout(struct
drm_syncobj **syncobjs,
{
struct syncobj_wait_entry *entries;
struct dma_fence *fence;
- signed long ret;
uint32_t signaled_count, i;
entries = kcalloc(count, sizeof(*entries), GFP_KERNEL);
@@ -692,7 +691,7 @@ static signed long drm_syncobj_array_wait_timeout(struct
drm_syncobj **syncobjs,
if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
continue;
} else {
- ret = -EINVAL;
+ timeout = -EINVAL;
goto cleanup_entries;
}
}
@@ -704,12 +703,6 @@ static signed long drm_syncobj_array_wait_timeout(struct
drm_syncobj **syncobjs,
}
}
- /* Initialize ret to the max of timeout and 1. That way, the
-* default return value indicates a successful wait and not a
-* timeout.
-*/
- ret = max_t(signed long, timeout, 1);
-
if (signaled_count == count ||
(signaled_count > 0 &&
!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL)))
@@ -760,18 +753,17 @@ static signed long drm_syncobj_array_wait_timeout(struct
drm_syncobj **syncobjs,
goto done_waiting;
if (timeout == 0) {
- /* If we are doing a 0 timeout wait and we got
-* here, then we just timed out.
-*/
- ret = 0;
+ timeout = -ETIME;
goto done_waiting;
}
- ret = schedule_timeout(ret);
+ if (signal_pending(current)) {
+ timeout = -ERESTARTSYS;
+ goto done_waiting;
+ }
- if (ret > 0 && signal_pending(current))
- ret = -ERESTARTSYS;
- } while (ret > 0);
+ timeout = schedule_timeout(timeout);
+ } while (1);
done_waiting:
__set_current_state(TASK_RUNNING);
@@ -788,7 +780,7 @@ static signed long drm_syncobj_array_wait_timeout(struct
drm_syncobj **syncobjs,
}
kfree(entries);
- return ret;
+ return timeout;
}
/**
@@ -829,19 +821,16 @@ static int drm_syncobj_array_wait(struct drm_device *dev,
struct drm_syncobj **syncobjs)
{
signed long timeout = drm_timeout_abs_to_jiffies(wait->timeout_nsec);
- signed long ret = 0;
uint32_t first = ~0;
- ret = drm_syncobj_array_wait_timeout(syncobjs,
-wait->count_handles,
-wait->flags,
-timeout, );
- if (ret < 0)
- return ret;
+ timeout = drm_syncobj_array_wait_timeout(syncobjs,
+wait->count_handles,
+wait->flags,
+timeout, );
+ if (timeout < 0)
+ return timeout;
wait->first_signaled = first;
- if (ret == 0)
- return -ETIME;
return 0;
}
--
2.19.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm: Fix syncobj handing of schedule() returning 0
After schedule() returns 0, we must do one last check of COND to
determine the reason for the wakeup with 0 jiffies remaining before
reporting the timeout -- otherwise we may lose the signal due to
scheduler delays.
References: https://bugs.freedesktop.org/show_bug.cgi?id=106690
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/drm_syncobj.c | 41 +--
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index e254f97fed7d..5bcb3ef9b256 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -672,7 +672,6 @@ static signed long drm_syncobj_array_wait_timeout(struct
drm_syncobj **syncobjs,
{
struct syncobj_wait_entry *entries;
struct dma_fence *fence;
- signed long ret;
uint32_t signaled_count, i;
entries = kcalloc(count, sizeof(*entries), GFP_KERNEL);
@@ -692,7 +691,7 @@ static signed long drm_syncobj_array_wait_timeout(struct
drm_syncobj **syncobjs,
if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
continue;
} else {
- ret = -EINVAL;
+ timeout = -EINVAL;
goto cleanup_entries;
}
}
@@ -704,12 +703,6 @@ static signed long drm_syncobj_array_wait_timeout(struct
drm_syncobj **syncobjs,
}
}
- /* Initialize ret to the max of timeout and 1. That way, the
-* default return value indicates a successful wait and not a
-* timeout.
-*/
- ret = max_t(signed long, timeout, 1);
-
if (signaled_count == count ||
(signaled_count > 0 &&
!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL)))
@@ -760,18 +753,17 @@ static signed long drm_syncobj_array_wait_timeout(struct
drm_syncobj **syncobjs,
goto done_waiting;
if (timeout == 0) {
- /* If we are doing a 0 timeout wait and we got
-* here, then we just timed out.
-*/
- ret = 0;
+ timeout = -ETIME;
goto done_waiting;
}
- ret = schedule_timeout(ret);
+ if (signal_pending(current)) {
+ timeout = -ERESTARTSYS;
+ goto done_waiting;
+ }
- if (ret > 0 && signal_pending(current))
- ret = -ERESTARTSYS;
- } while (ret > 0);
+ timeout = schedule_timeout(timeout);
+ } while (1);
done_waiting:
__set_current_state(TASK_RUNNING);
@@ -788,7 +780,7 @@ static signed long drm_syncobj_array_wait_timeout(struct
drm_syncobj **syncobjs,
}
kfree(entries);
- return ret;
+ return timeout;
}
/**
@@ -829,19 +821,16 @@ static int drm_syncobj_array_wait(struct drm_device *dev,
struct drm_syncobj **syncobjs)
{
signed long timeout = drm_timeout_abs_to_jiffies(wait->timeout_nsec);
- signed long ret = 0;
uint32_t first = ~0;
- ret = drm_syncobj_array_wait_timeout(syncobjs,
-wait->count_handles,
-wait->flags,
-timeout, );
- if (ret < 0)
- return ret;
+ timeout = drm_syncobj_array_wait_timeout(syncobjs,
+wait->count_handles,
+wait->flags,
+timeout, );
+ if (timeout < 0)
+ return timeout;
wait->first_signaled = first;
- if (ret == 0)
- return -ETIME;
return 0;
}
--
2.19.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
