fdo-mirrors/mesa
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/mesa/mesa.git synced 2026-01-11 01:30:25 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

vulkan/queue: Move shared binary semaphores to temps

Browse source 
If a client creates a semaphore, exports it, and then re-imports it back
into the device, this can trick our semaphore reset logic.  When this
happens, we end up with two different vk_sync structs that have the same
underlying payload so if one is used as the signal and one is used as
the wait of the same submit, we'll end up resetting it because we think
they're different, causing us to lose the signal.

We already have the ability to handle this for the threaded case by
moving the semaphore payload into a new vk_sync which we then destroy
after we're done submitting to the driver.  Use this path for shared
semaphores in the immediate case so we can just wait and signal without
worrying about the reset.

Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/13805
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: mesa-stable
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/37149>
This commit is contained in:
Faith Ekstrand 2025-09-02 17:00:18 -04:00 • committed by Marge Bot
parent e4edf9203b
commit 0f7e0f79ad
1 changed files with 114 additions and 81 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

195
src/vulkan/runtime/vk_queue.c
View file

@ -925,6 +925,100 @@ fail:
return result;
}
static VkResult
vk_queue_submit_move_binary_waits_to_temps(struct vk_device *device,
struct vk_queue_submit *submit,
bool move_non_shared)
{
VkResult result;
if (!submit->_has_binary_permanent_semaphore_wait)
return VK_SUCCESS;
for (uint32_t i = 0; i < submit->wait_count; i++) {
if (submit->waits[i].sync->flags & VK_SYNC_IS_TIMELINE)
continue;
/* From the Vulkan 1.2.194 spec:
*
* "When a batch is submitted to a queue via a queue
* submission, and it includes semaphores to be waited on,
* it defines a memory dependency between prior semaphore
* signal operations and the batch, and defines semaphore
* wait operations.
*
* Such semaphore wait operations set the semaphores
* created with a VkSemaphoreType of
* VK_SEMAPHORE_TYPE_BINARY to the unsignaled state."
*
* For threaded submit, we depend on tracking the unsignaled
* state of binary semaphores to determine when we can safely
* submit. The VK_SYNC_WAIT_PENDING check above as well as the
* one in the sumbit thread depend on all binary semaphores
* being reset when they're not in active use from the point
* of view of the client's CPU timeline. This means we need to
* reset them inside vkQueueSubmit and cannot wait until the
* actual submit which happens later in the thread.
*
* We've already stolen temporary semaphore payloads above as
* part of basic semaphore processing. We steal permanent
* semaphore payloads here by way of vk_sync_move. For shared
* semaphores, this can be a bit expensive (sync file import
* and export) but, for non-shared semaphores, it can be made
* fairly cheap. Also, we only do this semaphore swapping in
* the case where you have real timelines AND the client is
* using timeline semaphores with wait-before-signal (that's
* the only way to get a submit thread) AND mixing those with
* waits on binary semaphores AND said binary semaphore is
* using its permanent payload. In other words, this code
* should basically only ever get executed in CTS tests.
*/
if (submit->_wait_temps[i] != NULL)
continue;
if (!move_non_shared &&
!(submit->waits[i].sync->flags & VK_SYNC_IS_SHARED))
continue;
/* From the Vulkan 1.2.194 spec:
*
* VUID-vkQueueSubmit-pWaitSemaphores-03238
*
* "All elements of the pWaitSemaphores member of all
* elements of pSubmits created with a VkSemaphoreType of
* VK_SEMAPHORE_TYPE_BINARY must reference a semaphore
* signal operation that has been submitted for execution
* and any semaphore signal operations on which it depends
* (if any) must have also been submitted for execution."
*
* Therefore, we can safely do a blocking wait here and it
* won't actually block for long. This ensures that the
* vk_sync_move below will succeed.
*/
result = vk_sync_wait(device, submit->waits[i].sync, 0,
VK_SYNC_WAIT_PENDING, UINT64_MAX);
if (unlikely(result != VK_SUCCESS))
return result;
result = vk_sync_create(device,
submit->waits[i].sync->type,
0 /* flags */,
0 /* initial value */,
&submit->_wait_temps[i]);
if (unlikely(result != VK_SUCCESS))
return result;
result = vk_sync_move(device, submit->_wait_temps[i],
submit->waits[i].sync);
if (unlikely(result != VK_SUCCESS))
return result;
submit->waits[i].sync = submit->_wait_temps[i];
}
return VK_SUCCESS;
}
static VkResult
vk_queue_submit(struct vk_queue *queue,
struct vk_queue_submit *submit)
@ -954,6 +1048,22 @@ vk_queue_submit(struct vk_queue *queue,
switch (queue->submit.mode) {
case VK_QUEUE_SUBMIT_MODE_IMMEDIATE:
/* If threaded submit is possible on this device, we need to ensure that
* binary semaphore payloads get reset so that any other threads can
* properly wait on them for dependency checking. Because we don't
* currently have a submit thread, we can directly reset most binary
* semaphore payloads. However, for shared semaphores the loop below
* doesn't work because we we can't reliably check that a wait was never
* signaled since two different vk_sync structs can refer to the same
* underlying sync primitive. For shared semaphores, we have to fall
* back to moving them to temporaries like we do in the threaded case.
*/
if (vk_device_supports_threaded_submit(device)) {
result = vk_queue_submit_move_binary_waits_to_temps(device, submit, false);
if (unlikely(result != VK_SUCCESS))
goto fail;
}
result = vk_queue_submit_final(queue, submit);
if (unlikely(result != VK_SUCCESS))
goto fail;
@ -964,7 +1074,7 @@ vk_queue_submit(struct vk_queue *queue,
* currently have a submit thread, we can directly reset that binary
* semaphore payloads.
*
* If we the vk_sync is in our signal et, we can consider it to have
* If we the vk_sync is in our signal set, we can consider it to have
* been both reset and signaled by queue_submit_final(). A reset in
* this case would be wrong because it would throw away our signal
* operation. If we don't signal the vk_sync, then we need to reset it.
@ -1001,86 +1111,9 @@ vk_queue_submit(struct vk_queue *queue,
return vk_device_flush(queue->base.device);
case VK_QUEUE_SUBMIT_MODE_THREADED:
if (submit->_has_binary_permanent_semaphore_wait) {
for (uint32_t i = 0; i < submit->wait_count; i++) {
if (submit->waits[i].sync->flags & VK_SYNC_IS_TIMELINE)
continue;
/* From the Vulkan 1.2.194 spec:
*
* "When a batch is submitted to a queue via a queue
* submission, and it includes semaphores to be waited on,
* it defines a memory dependency between prior semaphore
* signal operations and the batch, and defines semaphore
* wait operations.
*
* Such semaphore wait operations set the semaphores
* created with a VkSemaphoreType of
* VK_SEMAPHORE_TYPE_BINARY to the unsignaled state."
*
* For threaded submit, we depend on tracking the unsignaled
* state of binary semaphores to determine when we can safely
* submit. The VK_SYNC_WAIT_PENDING check above as well as the
* one in the sumbit thread depend on all binary semaphores
* being reset when they're not in active use from the point
* of view of the client's CPU timeline. This means we need to
* reset them inside vkQueueSubmit and cannot wait until the
* actual submit which happens later in the thread.
*
* We've already stolen temporary semaphore payloads above as
* part of basic semaphore processing. We steal permanent
* semaphore payloads here by way of vk_sync_move. For shared
* semaphores, this can be a bit expensive (sync file import
* and export) but, for non-shared semaphores, it can be made
* fairly cheap. Also, we only do this semaphore swapping in
* the case where you have real timelines AND the client is
* using timeline semaphores with wait-before-signal (that's
* the only way to get a submit thread) AND mixing those with
* waits on binary semaphores AND said binary semaphore is
* using its permanent payload. In other words, this code
* should basically only ever get executed in CTS tests.
*/
if (submit->_wait_temps[i] != NULL)
continue;
/* From the Vulkan 1.2.194 spec:
*
* VUID-vkQueueSubmit-pWaitSemaphores-03238
*
* "All elements of the pWaitSemaphores member of all
* elements of pSubmits created with a VkSemaphoreType of
* VK_SEMAPHORE_TYPE_BINARY must reference a semaphore
* signal operation that has been submitted for execution
* and any semaphore signal operations on which it depends
* (if any) must have also been submitted for execution."
*
* Therefore, we can safely do a blocking wait here and it
* won't actually block for long. This ensures that the
* vk_sync_move below will succeed.
*/
result = vk_sync_wait(queue->base.device,
submit->waits[i].sync, 0,
VK_SYNC_WAIT_PENDING, UINT64_MAX);
if (unlikely(result != VK_SUCCESS))
goto fail;
result = vk_sync_create(queue->base.device,
submit->waits[i].sync->type,
0 /* flags */,
0 /* initial value */,
&submit->_wait_temps[i]);
if (unlikely(result != VK_SUCCESS))
goto fail;
result = vk_sync_move(queue->base.device,
submit->_wait_temps[i],
submit->waits[i].sync);
if (unlikely(result != VK_SUCCESS))
goto fail;
submit->waits[i].sync = submit->_wait_temps[i];
}
}
result = vk_queue_submit_move_binary_waits_to_temps(device, submit, true);
if (unlikely(result != VK_SUCCESS))
goto fail;
vk_queue_push_submit(queue, submit);