<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Nov 3, 2016 at 2:53 AM, Chris Wilson <span dir="ltr"><<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, Nov 02, 2016 at 05:15:52PM -0700, Jason Ekstrand wrote:<br>
> @@ -1562,22 +1580,98 @@ VkResult anv_WaitForFences(<br>
>      * best we can do is to clamp the timeout to INT64_MAX.  This limits the<br>
>      * maximum timeout from 584 years to 292 years - likely not a big deal.<br>
>      */<br>
> -   if (timeout > INT64_MAX)<br>
> -      timeout = INT64_MAX;<br>
> -<br>
> -   int64_t t = timeout;<br>
> +   int64_t timeout = MIN2(_timeout, INT64_MAX);<br>
<br>
</span><span class="">> +      if (pending_fences && !signaled_fences) {<br>
> +         /* If we've hit this then someone decided to vkWaitForFences before<br>
> +          * they've actually submitted any of them to a queue.  This is a<br>
> +          * fairly pessimal case, so it's ok to lock here and use a standard<br>
> +          * pthreads condition variable.<br>
> +          */<br>
> +         pthread_mutex_lock(&device-><wbr>mutex);<br>
> +<br>
> +         /* It's possible that some of the fences have changed state since the<br>
> +          * last time we checked.  Now that we have the lock, check for<br>
> +          * pending fences again and don't wait if it's changed.<br>
> +          */<br>
> +         uint32_t now_pending_fences = 0;<br>
> +         for (uint32_t i = 0; i < fenceCount; i++) {<br>
> +            ANV_FROM_HANDLE(anv_fence, fence, pFences[i]);<br>
> +            if (fence->state == ANV_FENCE_STATE_RESET)<br>
> +               now_pending_fences++;<br>
> +         }<br>
> +         assert(now_pending_fences <= pending_fences);<br>
> +<br>
> +         bool timeout = false;<br>
<br>
</span>Shadowed timeout...<span class=""><br></span></blockquote><div><br></div><div>Drp...<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> +         if (now_pending_fences == pending_fences) {<br>
> +            struct timeval before;<br>
> +            gettimeofday(&before, NULL);<br>
> +<br>
> +            struct timespec ts = {<br>
> +               .tv_nsec = timeout % 1000000000,<br>
> +               .tv_sec = timeout / 1000000000,<br>
> +            };<br>
> +            pthread_cond_timedwait(&<wbr>device->queue_submit, &device->mutex, &ts);<br>
> +<br>
> +            struct timeval after;<br>
> +            gettimeofday(&after, NULL);<br>
> +            uint64_t time_elapsed =<br>
> +               ((uint64_t)after.tv_sec * 1000000000 + after.tv_usec * 1000) -<br>
> +               ((uint64_t)before.tv_sec * 1000000000 + before.tv_usec * 1000);<br>
> +<br>
> +            if (time_elapsed > timeout) {<br>
> +               pthread_mutex_unlock(&device-><wbr>mutex);<br>
> +               return VK_TIMEOUT;<br>
> +            }<br>
> +<br>
> +            timeout -= time_elapsed;<br>
<br>
</span>Adjusts the bool and not the int64_t nanosecond counter.<br>
<br>
Looks ok, the only question is whether that should be a queue mutex<br>
rather than a device mutex for submitting. (Are fences local to a<br>
queue? Is the shared relocation state local to a queue or global...)<br></blockquote><div><br></div><div>Fences get submitted on a queue so you could claim that a queue mutex is better.  However, this case should never happen in practice but only really in CTS tests (why would you wait on a fence you haven't submitted?) so using a more global mutex isn't a huge deal.  Also, we only have one queue per device so it works out the same.<br><br></div><div>Incidentally, I realized as I was working on some WSI stuff that I did my timedwait all wrong here.  The time value you provide to pthread_cond_timedwait is supposed to be an absolute time rather than a delta relative to the call.  I'll get that sorted and send a v2.<br><br></div><div>--Jason <br></div></div></div></div>