applied to experimental drm-intel-collector.
drivers/gpu/drm/i915/i915_gem.c: In function ‘__wait_seqno’:
drivers/gpu/drm/i915/i915_gem.c:1033:20: warning: ‘timeout_jiffies’ may be used
+uninitialized in this function
On Tue, Aug 6, 2013 at 10:03 PM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
> Rather than continue to fix up the timeouts to work around the interface
> impedence in wait_event_*(), open code the combination of
> wait_event[_interruptible][_timeout]. And note the code size reduction,
> and dare say readability?, in doing so..
>
> Food for thought.
> ---
> drivers/gpu/drm/i915/i915_gem.c | 82 ++++++++++++++++++++---------------------
> 1 file changed, 39 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 99ba622..8adbce9 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1088,23 +1088,18 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> bool interruptible, struct timespec *timeout)
> {
> drm_i915_private_t *dev_priv = ring->dev->dev_private;
> - struct timespec before, now, wait_time={1,0};
> - unsigned long timeout_jiffies;
> - long end;
> - bool wait_forever = true;
> + struct timespec before, now;
> + DEFINE_WAIT(__wait);
> + long timeout_jiffies;
> int ret;
>
> if (i915_seqno_passed(ring->get_seqno(ring, true), seqno))
> return 0;
>
> - trace_i915_gem_request_wait_begin(ring, seqno);
> -
> - if (timeout != NULL) {
> - wait_time = *timeout;
> - wait_forever = false;
> - }
> + if (timeout)
> + timeout_jiffies = timespec_to_jiffies_timeout(timeout);
>
> - timeout_jiffies = timespec_to_jiffies_timeout(&wait_time);
> + trace_i915_gem_request_wait_begin(ring, seqno);
>
> if (WARN_ON(!ring->irq_get(ring)))
> return -ENODEV;
> @@ -1112,36 +1107,47 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> /* Record current time in case interrupted by signal, or wedged * */
> getrawmonotonic(&before);
>
> -#define EXIT_COND \
> - (i915_seqno_passed(ring->get_seqno(ring, false), seqno) || \
> - i915_reset_in_progress(&dev_priv->gpu_error) || \
> - reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter))
> - do {
> - if (interruptible)
> - end = wait_event_interruptible_timeout(ring->irq_queue,
> - EXIT_COND,
> - timeout_jiffies);
> - else
> - end = wait_event_timeout(ring->irq_queue, EXIT_COND,
> - timeout_jiffies);
> + for (;;) {
> + prepare_to_wait(&ring->irq_queue, &__wait,
> + interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE);
>
> /* We need to check whether any gpu reset happened in between
> * the caller grabbing the seqno and now ... */
> - if (reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter))
> - end = -EAGAIN;
> + if (reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter)) {
> + /* ... but upgrade the -EGAIN to an -EIO if the gpu
> + * is truely gone. */
> + ret = i915_gem_check_wedge(&dev_priv->gpu_error, interruptible);
> + if (ret == 0)
> + ret = -EAGAIN;
> + break;
> + }
>
> - /* ... but upgrade the -EGAIN to an -EIO if the gpu is truely
> - * gone. */
> - ret = i915_gem_check_wedge(&dev_priv->gpu_error, interruptible);
> - if (ret)
> - end = ret;
> - } while (end == 0 && wait_forever);
> + if (i915_seqno_passed(ring->get_seqno(ring, false), seqno)) {
> + ret = 0;
> + break;
> + }
> +
> + if (interruptible && signal_pending(current)) {
> + ret = -ERESTARTSYS;
> + break;
> + }
> +
> + if (timeout) {
> + if (timeout_jiffies == 0) {
> + ret = -ETIME;
> + break;
> + }
> +
> + timeout_jiffies = schedule_timeout(timeout_jiffies);
> + } else
> + schedule();
> + }
> + finish_wait(&ring->irq_queue, &__wait);
>
> getrawmonotonic(&now);
>
> ring->irq_put(ring);
> trace_i915_gem_request_wait_end(ring, seqno);
> -#undef EXIT_COND
>
> if (timeout) {
> struct timespec sleep_time = timespec_sub(now, before);
> @@ -1150,17 +1156,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
> set_normalized_timespec(timeout, 0, 0);
> }
>
> - switch (end) {
> - case -EIO:
> - case -EAGAIN: /* Wedged */
> - case -ERESTARTSYS: /* Signal */
> - return (int)end;
> - case 0: /* Timeout */
> - return -ETIME;
> - default: /* Completed */
> - WARN_ON(end < 0); /* We're not aware of other errors */
> - return 0;
> - }
> + return ret;
> }
>
> /**
> --
> 1.8.4.rc1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
