[vulkan] Fix hang bug in CTS test.
Bug: 42086544
Change-Id: If2585bec444b4161f05f32b016d25ba4d719ca3e
Reviewed-on: https://fuchsia-review.googlesource.com/c/third_party/mesa/+/1044103
Reviewed-by: Craig Stout <cstout@google.com>
Commit-Queue: John Rosasco <rosasco@google.com>
diff --git a/src/gallium/frontends/lavapipe/lvp_device.c b/src/gallium/frontends/lavapipe/lvp_device.c
index 40d3218..498cf4e 100644
--- a/src/gallium/frontends/lavapipe/lvp_device.c
+++ b/src/gallium/frontends/lavapipe/lvp_device.c
@@ -280,11 +280,12 @@
device->sync_timeline_type = vk_sync_timeline_get_type(&lvp_pipe_sync_type);
uint32_t st = 0;
#if VK_USE_PLATFORM_FUCHSIA
+ // TODO(rosasco): add emulated sync type ?
device->sync_types[st++] = &lvp_pipe_sync_fuchsia_type;
#else
device->sync_types[st++] = &lvp_pipe_sync_type;
-#endif
device->sync_types[st++] = &device->sync_timeline_type.sync;
+#endif
device->sync_types[st] = NULL;
assert(st < MAX_SYNC_TYPES);
device->vk.supported_sync_types = device->sync_types;
diff --git a/src/gallium/frontends/lavapipe/lvp_pipe_sync_fuchsia.c b/src/gallium/frontends/lavapipe/lvp_pipe_sync_fuchsia.c
index 317a51b..e6acaf3 100644
--- a/src/gallium/frontends/lavapipe/lvp_pipe_sync_fuchsia.c
+++ b/src/gallium/frontends/lavapipe/lvp_pipe_sync_fuchsia.c
@@ -34,33 +34,33 @@
#include "vk_log.h"
#include "vulkan/vulkan_core.h"
-#define ZX_STATUS_VK_RTN(status, device, message) \
- switch (status) { \
- case ZX_OK: \
- return VK_SUCCESS; \
- case ZX_ERR_BAD_HANDLE: \
- return vk_errorf(device, VK_ERROR_INVALID_EXTERNAL_HANDLE, \
- "%s - Bad Zircon handle: %m", message); \
- case ZX_ERR_TIMED_OUT: \
- return VK_TIMEOUT; \
- default: \
- return vk_errorf(device, VK_ERROR_UNKNOWN, \
- "%s - Unknown Zircon error: %m", message); \
- }
+#define ZX_STATUS_VK_RTN(status, device, handle, message) \
+ switch (status) { \
+ case ZX_OK: \
+ return VK_SUCCESS; \
+ case ZX_ERR_BAD_HANDLE: \
+ return vk_errorf(device, VK_ERROR_INVALID_EXTERNAL_HANDLE, \
+ "%s - Bad Zircon handle 0x%0x: %m", message, handle); \
+ case ZX_ERR_TIMED_OUT: \
+ return VK_TIMEOUT; \
+ default: \
+ return vk_errorf(device, VK_ERROR_UNKNOWN, \
+ "%s - Unknown Zircon error: %m", message); \
+ }
/* No-op when status == ZX_OK. */
-#define ZX_STATUS_VK_RTN_ERR(status, device, message) \
- switch (status) { \
- case ZX_OK: \
- break; \
- case ZX_ERR_BAD_HANDLE: \
- return vk_errorf(device, VK_ERROR_INVALID_EXTERNAL_HANDLE, \
- "%s - Bad Zircon handle: %m", message); \
- case ZX_ERR_TIMED_OUT: \
- return VK_TIMEOUT; \
- default: \
- return vk_errorf(device, VK_ERROR_UNKNOWN, \
- "%s - Unknown Zircon error: %m", message); \
+#define ZX_STATUS_VK_RTN_ERR(status, device, handle, message) \
+ switch (status) { \
+ case ZX_OK: \
+ break; \
+ case ZX_ERR_BAD_HANDLE: \
+ return vk_errorf(device, VK_ERROR_INVALID_EXTERNAL_HANDLE, \
+ "%s - Bad Zircon handl 0x%0x: %m", message, handle); \
+ case ZX_ERR_TIMED_OUT: \
+ return VK_TIMEOUT; \
+ default: \
+ return vk_errorf(device, VK_ERROR_UNKNOWN, \
+ "%s - Unknown Zircon error: %m", message); \
}
static bool is_signaled(zx_handle_t handle) {
@@ -68,16 +68,12 @@
return (status == ZX_OK);
}
-static void lvp_pipe_sync_fuchsia_validate(
- ASSERTED struct lvp_pipe_sync_fuchsia *sync) {
- if (is_signaled(sync->signaled)) assert(sync->fence == NULL);
-}
-
static VkResult lvp_pipe_sync_fuchsia_init(UNUSED struct vk_device *vk_device,
struct vk_sync *vk_sync,
uint64_t initial_value) {
struct lvp_pipe_sync_fuchsia *sync =
vk_sync_as_lvp_pipe_sync_fuchsia(vk_sync);
+ mtx_init(&sync->lock, mtx_plain);
zx_handle_t handle = (zx_handle_t)initial_value;
@@ -89,14 +85,14 @@
if (sync->signaled != ZX_HANDLE_INVALID) {
if (zx_handle_close(sync->signaled) != ZX_OK) {
return vk_errorf(vk_device, VK_ERROR_INVALID_EXTERNAL_HANDLE,
- "Init (close): %m");
+ "Init (close) bad handle 0x%x: %m", sync->signaled);
}
}
/* Duplicate and store |handle| into |sync->signaled|. */
zx_status_t status =
zx_handle_replace(handle, ZX_RIGHT_SAME_RIGHTS, &sync->signaled);
- ZX_STATUS_VK_RTN_ERR(status, vk_device, "Init (replace)");
+ ZX_STATUS_VK_RTN_ERR(status, vk_device, handle, "Init (replace)");
} else {
zx_status_t status = zx_event_create(0, &sync->signaled);
if (status != ZX_OK) {
@@ -107,7 +103,7 @@
if (initial_value == 1) {
zx_status_t status =
zx_object_signal(sync->signaled, 0u, ZX_EVENT_SIGNALED);
- ZX_STATUS_VK_RTN(status, vk_device, "Init (signal)");
+ ZX_STATUS_VK_RTN(status, vk_device, sync->signaled, "Init (signal)");
}
}
sync->fence = NULL;
@@ -121,42 +117,38 @@
struct lvp_pipe_sync_fuchsia *sync =
vk_sync_as_lvp_pipe_sync_fuchsia(vk_sync);
- lvp_pipe_sync_fuchsia_validate(sync);
+ mtx_lock(&sync->lock);
if (sync->fence)
device->pscreen->fence_reference(device->pscreen, &sync->fence, NULL);
if (sync->signaled == ZX_HANDLE_INVALID) {
+ mtx_unlock(&sync->lock);
vk_logi(VK_LOG_OBJS(device), "Finish - invalid handle\n");
return;
}
zx_status_t status = zx_handle_close(sync->signaled);
- switch (status) {
- case ZX_OK:
- break;
- case ZX_ERR_BAD_HANDLE:
- vk_loge(VK_LOG_OBJS(device),
- "Finish - attempt to close bad Zircon event handle.\n");
- return;
- default:
- vk_loge(VK_LOG_OBJS(device), "Finish - Unknown error.\n");
- return;
+ if (status == ZX_OK) {
+ sync->signaled = ZX_HANDLE_INVALID;
+ mtx_unlock(&sync->lock);
+ mtx_destroy(&sync->lock);
+ return;
}
- sync->signaled = ZX_HANDLE_INVALID;
+ if (status == ZX_ERR_BAD_HANDLE) {
+ vk_loge(VK_LOG_OBJS(device), "Finish - attempt to close bad Zircon event handle: %u.\n",
+ sync->signaled);
+ } else {
+ vk_loge(VK_LOG_OBJS(device), "Finish - Unknown error.\n");
+ }
}
void lvp_pipe_sync_fuchsia_signal_with_fence(struct lvp_device *device,
struct lvp_pipe_sync_fuchsia *sync,
struct pipe_fence_handle *fence) {
- lvp_pipe_sync_fuchsia_validate(sync);
- if (fence == NULL) {
- if (zx_object_signal(sync->signaled, 0u, ZX_EVENT_SIGNALED) != ZX_OK) {
- mesa_loge("Signal with fence (== NULL) failed.");
- }
- } else {
- if (zx_object_signal(sync->signaled, ZX_EVENT_SIGNALED, 0u) != ZX_OK) {
- mesa_loge("Signal with fence (!= NULL) failed.");
- }
- }
+ mtx_lock(&sync->lock);
device->pscreen->fence_reference(device->pscreen, &sync->fence, fence);
+ if (zx_object_signal(sync->signaled, 0u, ZX_EVENT_SIGNALED) != ZX_OK) {
+ mesa_loge("Signal handle 0x%x with fence (== NULL) failed.", sync->signaled);
+ }
+ mtx_unlock(&sync->lock);
}
static VkResult lvp_pipe_sync_fuchsia_signal(struct vk_device *vk_device,
@@ -166,14 +158,13 @@
struct lvp_pipe_sync_fuchsia *sync =
vk_sync_as_lvp_pipe_sync_fuchsia(vk_sync);
- lvp_pipe_sync_fuchsia_validate(sync);
- zx_status_t status = zx_object_signal(sync->signaled, 0u, ZX_EVENT_SIGNALED);
- if (status == ZX_OK) {
- if (sync->fence)
- device->pscreen->fence_reference(device->pscreen, &sync->fence, NULL);
- return VK_SUCCESS;
+ mtx_lock(&sync->lock);
+ if (sync->fence) {
+ device->pscreen->fence_reference(device->pscreen, &sync->fence, NULL);
}
- ZX_STATUS_VK_RTN(status, vk_device, "Signal (signal)");
+ zx_status_t status = zx_object_signal(sync->signaled, 0u, ZX_EVENT_SIGNALED);
+ mtx_unlock(&sync->lock);
+ ZX_STATUS_VK_RTN(status, vk_device, sync->signaled, "Signal (signal)");
}
static VkResult lvp_pipe_sync_fuchsia_reset(struct vk_device *vk_device,
@@ -182,21 +173,27 @@
struct lvp_pipe_sync_fuchsia *sync =
vk_sync_as_lvp_pipe_sync_fuchsia(vk_sync);
- lvp_pipe_sync_fuchsia_validate(sync);
+ mtx_lock(&sync->lock);
if (sync->signaled == ZX_HANDLE_INVALID) {
+ mtx_unlock(&sync->lock);
return vk_errorf(
device, VK_ERROR_INVALID_EXTERNAL_HANDLE,
- "Reset - Attempt to reset invalid Zircon event handle: %m");
+ "Reset - Attempt to reset invalid handle: %m");
}
- zx_status_t status = zx_object_signal(sync->signaled, ZX_EVENT_SIGNALED, 0u);
- if (status == ZX_OK) {
- if (sync->fence)
- device->pscreen->fence_reference(device->pscreen, &sync->fence, NULL);
+ if (sync->fence) {
+ device->pscreen->fence_reference(device->pscreen, &sync->fence, NULL);
}
- ZX_STATUS_VK_RTN(status, vk_device, "Reset (signal)");
+ zx_status_t status = zx_object_signal(sync->signaled, ZX_EVENT_SIGNALED, 0u);
+ mtx_unlock(&sync->lock);
+ ZX_STATUS_VK_RTN(status, vk_device, sync->signaled, "Reset (signal)");
}
+#define DUAL_LOCK(a, b) mtx_lock(a); mtx_lock(b);
+#define DUAL_UNLOCK(a, b) mtx_unlock(a); mtx_unlock(b);
+
+/* This matches the lvp_pipe_sync move implementation. Unlike typical move semantics, */
+/* it's expected that vk_src->signaled will be used after this move. */
static VkResult lvp_pipe_sync_fuchsia_move(struct vk_device *vk_device,
struct vk_sync *vk_dst,
struct vk_sync *vk_src) {
@@ -204,23 +201,39 @@
struct lvp_pipe_sync_fuchsia *dst = vk_sync_as_lvp_pipe_sync_fuchsia(vk_dst);
struct lvp_pipe_sync_fuchsia *src = vk_sync_as_lvp_pipe_sync_fuchsia(vk_src);
+ DUAL_LOCK(&src->lock, &dst->lock);
if (src->signaled == ZX_HANDLE_INVALID) {
+ DUAL_UNLOCK(&src->lock, &dst->lock);
return vk_errorf(&device->vk, VK_ERROR_INVALID_EXTERNAL_HANDLE,
"Move - Unable to move invalid handle: %m");
}
- zx_handle_t handle_out = ZX_HANDLE_INVALID;
- zx_status_t status =
- zx_handle_replace(src->signaled, ZX_RIGHT_SAME_RIGHTS, &handle_out);
- ZX_STATUS_VK_RTN_ERR(status, &device->vk, "Move (replace)");
+ struct pipe_fence_handle *fence = src->fence;
+ src->fence = NULL;
+ if (dst->fence) {
+ device->pscreen->fence_reference(device->pscreen, &dst->fence, NULL);
+ }
+ dst->fence = fence;
- src->signaled = ZX_HANDLE_INVALID;
- *dst = *src;
- dst->signaled = handle_out;
+ zx_status_t status = ZX_ERR_BAD_STATE;
+ if(is_signaled(src->signaled)) {
+ status = zx_object_signal(dst->signaled, 0u, ZX_EVENT_SIGNALED);
+ } else {
+ status = zx_object_signal(dst->signaled, ZX_EVENT_SIGNALED, 0u);
+ }
+ if(status != ZX_OK) {
+ DUAL_UNLOCK(&src->lock, &dst->lock);
+ return vk_errorf(&device->vk, VK_ERROR_UNKNOWN,
+ "Move - unable to signal handle 0x%x: %m", dst->signaled);
+ }
+ status = zx_object_signal(src->signaled, ZX_EVENT_SIGNALED, 0u);
+ DUAL_UNLOCK(&src->lock, &dst->lock);
return VK_SUCCESS;
}
+/* The submit thread is force enabled even when requesting "immediate". A client's wait may */
+/* may begin before the submit thread calls signal_with_fence. */
static VkResult lvp_pipe_sync_fuchsia_wait(struct vk_device *vk_device,
struct vk_sync *vk_sync,
uint64_t wait_value,
@@ -229,26 +242,28 @@
struct lvp_device *device = container_of(vk_device, struct lvp_device, vk);
struct lvp_pipe_sync_fuchsia *sync =
vk_sync_as_lvp_pipe_sync_fuchsia(vk_sync);
-
assert(!(wait_flags & VK_SYNC_WAIT_ANY));
-
- lvp_pipe_sync_fuchsia_validate(sync);
-
zx_status_t status = ZX_OK;
zx_time_t now_ns = zx_clock_get_monotonic();
abs_timeout_ns =
(abs_timeout_ns > ZX_TIME_INFINITE) ? ZX_TIME_INFINITE : abs_timeout_ns;
+
+ mtx_lock(&sync->lock);
while (!is_signaled(sync->signaled) && !sync->fence) {
+ mtx_unlock(&sync->lock);
if (now_ns >= abs_timeout_ns) return VK_TIMEOUT;
status = zx_object_wait_one(sync->signaled, ZX_EVENT_SIGNALED,
(zx_time_t)abs_timeout_ns, NULL);
- ZX_STATUS_VK_RTN_ERR(status, device, "Wait (wait one)");
- lvp_pipe_sync_fuchsia_validate(sync);
+ ZX_STATUS_VK_RTN_ERR(status, device, sync->signaled, "Wait (wait one)");
+ mtx_lock(&sync->lock);
now_ns = zx_clock_get_monotonic();
}
- if (is_signaled(sync->signaled) || (wait_flags & VK_SYNC_WAIT_PENDING))
+ if ((!sync->fence && is_signaled(sync->signaled)) ||
+ (wait_flags & VK_SYNC_WAIT_PENDING)) {
+ mtx_unlock(&sync->lock);
return VK_SUCCESS;
+ }
/* Grab a reference before we drop the lock */
struct pipe_fence_handle *fence = NULL;
@@ -261,49 +276,58 @@
device->pscreen->fence_reference(device->pscreen, &fence, NULL);
- lvp_pipe_sync_fuchsia_validate(sync);
-
- if (!complete) return VK_TIMEOUT;
+ if (!complete) {
+ mtx_unlock(&sync->lock);
+ return VK_TIMEOUT;
+ }
if (sync->fence == fence) {
device->pscreen->fence_reference(device->pscreen, &sync->fence, NULL);
status = zx_object_signal(sync->signaled, 0u, ZX_EVENT_SIGNALED);
- ZX_STATUS_VK_RTN_ERR(status, device, "Wait (signal)");
+ mtx_unlock(&sync->lock);
+ ZX_STATUS_VK_RTN_ERR(status, device, sync->signaled, "Wait (signal)");
}
+ mtx_unlock(&sync->lock);
return VK_SUCCESS;
}
static VkResult lvp_pipe_sync_fuchsia_import_zircon_handle(
- struct vk_device *device, struct vk_sync *sync, uint32_t handle) {
- struct lvp_pipe_sync_fuchsia *sobj = vk_sync_as_lvp_pipe_sync_fuchsia(sync);
+ struct vk_device *device, struct vk_sync *vk_sync, uint32_t handle) {
+ struct lvp_pipe_sync_fuchsia *sync = vk_sync_as_lvp_pipe_sync_fuchsia(vk_sync);
zx_handle_t semaphore = ZX_HANDLE_INVALID;
-
zx_status_t status =
zx_handle_replace(handle, ZX_RIGHT_SAME_RIGHTS, &semaphore);
- ZX_STATUS_VK_RTN_ERR(status, device, "Import (replace)");
- status = zx_handle_close(sobj->signaled);
+ ZX_STATUS_VK_RTN_ERR(status, device, handle, "Import (replace)");
+
+ mtx_lock(&sync->lock);
+ status = zx_handle_close(sync->signaled);
if (status == ZX_OK) {
- sobj->signaled = semaphore;
+ sync->signaled = semaphore;
+ mtx_unlock(&sync->lock);
return VK_SUCCESS;
}
zx_handle_close(semaphore);
- ZX_STATUS_VK_RTN(status, device, "Import (close - signaled)");
+ mtx_unlock(&sync->lock);
+ ZX_STATUS_VK_RTN(status, device, semaphore, "Import (close - signaled)");
}
static VkResult lvp_pipe_sync_fuchsia_export_zircon_handle(
- struct vk_device *device, struct vk_sync *sync, uint32_t *handle_out) {
- struct lvp_pipe_sync_fuchsia *sobj = vk_sync_as_lvp_pipe_sync_fuchsia(sync);
+ struct vk_device *device, struct vk_sync *vk_sync, uint32_t *handle_out) {
+ struct lvp_pipe_sync_fuchsia *sync = vk_sync_as_lvp_pipe_sync_fuchsia(vk_sync);
- if (sobj->signaled == ZX_HANDLE_INVALID) {
+ mtx_lock(&sync->lock);
+ if (sync->signaled == ZX_HANDLE_INVALID) {
+ mtx_unlock(&sync->lock);
return vk_errorf(
device, VK_ERROR_INVALID_EXTERNAL_HANDLE,
- "Export - Attempt to export invalid Zircon event handle: %m");
+ "Export - Attempt to export invalid handle: %m");
}
zx_status_t status =
- zx_handle_duplicate(sobj->signaled, ZX_RIGHT_SAME_RIGHTS, handle_out);
- ZX_STATUS_VK_RTN(status, device, "Export (duplicate)");
+ zx_handle_duplicate(sync->signaled, ZX_RIGHT_SAME_RIGHTS, handle_out);
+ mtx_unlock(&sync->lock);
+ ZX_STATUS_VK_RTN(status, device, sync->signaled, "Export (duplicate)");
}
const struct vk_sync_type lvp_pipe_sync_fuchsia_type = {
diff --git a/src/gallium/frontends/lavapipe/lvp_private.h b/src/gallium/frontends/lavapipe/lvp_private.h
index 88e08af..7ed6ab5 100644
--- a/src/gallium/frontends/lavapipe/lvp_private.h
+++ b/src/gallium/frontends/lavapipe/lvp_private.h
@@ -227,6 +227,7 @@
* |signaled| field below. */
zx_handle_t signaled;
+ mtx_t lock;
struct pipe_fence_handle *fence;
};
