[Mesa-dev] [PATCH 3/3] anv: Rework fences

[Chad Versace]

On Thu 03 Nov 2016, Jason Ekstrand wrote:
> 
> 
> On Thu, Nov 3, 2016 at 10:59 AM, Chad Versace <chadversary at chromium.org> wrote:
> 
>     On Wed 02 Nov 2016, Jason Ekstrand wrote:
>     > @@ -1116,6 +1117,11 @@ VkResult anv_QueueSubmit(
>     >        result = anv_device_execbuf(device, &fence->execbuf, &fence_bo);
>     >        if (result != VK_SUCCESS)
>     >           goto out;
>     > +
>     > +      /* Update the fence and wake up any waiters */
>     > +      assert(fence->state == ANV_FENCE_STATE_RESET);
> 
>     I think this assertion is wrong, and I think valid Vulkan usage can hit it.
> 
>     The patch, as-written, only transitions the fence status from
>     ANV_FENCE_STATE_SUBMITTED to ANV_FENCE_STATE_RESET if the user calls
>     vkWaitForFences. But I can find no text in the spec that requires the
>     user to call vkWaitForFences. From my reading of the spec, if the user
>     submits a fence and a batch of work to the queue, and later determines
>     that all work has completed without using vkWaitForFences on that fence,
>     then the fence still enters the signalled state, and therefore the user
>     is allowed to re-submit it. And that would hit the assertion.
> 
> 
> Every VkFence is in one of two states: Signaled and unsignaled.  When created,
> they start off in the unsignaled state.  When you use a fence in VkQueueSubmit,
> it will eventually transition from signaled to unsignaled once the commands in
> that submit operation are completed.  The only way you can transition from
> signaled to unsignaled is with vkResetFences.  From the spec for vkQueueSubmit:
> 
>   • If fence is not VK_NULL_HANDLE, fence must be unsignaled

Ah, that was the critical line of spec text for me.

> So, no, you're not allowed to pass a used-but-reset fence into QueueSubmit.

Did you mean used-but-*not*-reset fence?

Thanks for the correction. I revisited the spec, and, assuming you mean
used-but-*not*-reset, then I agree now. Objections dropped.

>  
> 
>     Please correct me if I'm wrong. I'm no expert (yet) on the Vulkan spec's
>     synchronization sections.