From a861f9a0b26a6f904b0c7954d90beb96af3ba046 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Wed, 7 Sep 2022 02:21:14 +0000 Subject: [PATCH] zink: fix zink_create_fence_fd to properly import This change fixes below: 1. Dup the fence fd, otherwise, since external semaphore import takes the ownership of the fd, non-Vulkan part touches the fd leading to undefined behavior. This can be hit on implementations that defer the processing of the passed fd. 2. Use VK_SEMAPHORE_IMPORT_TEMPORARY_BIT for importing since that's required for SYNC_FD handle type because of its copy transference. Meanwhile, doing temporary import for opaque fd is fine in this path. Fixes: 32597e116d7 ("zink: implement GL semaphores") Signed-off-by: Yiwei Zhang Reviewed-By: Mike Blumenkrantz Part-of: (cherry picked from commit c1b827d6a25527ae61dd0c102c5cc3f6e42dc90a) --- .pick_status.json | 2 +- src/gallium/drivers/zink/zink_fence.c | 63 +++++++++++++++++---------- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index f90b7fcd77e..e2f77718b4a 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -3037,7 +3037,7 @@ "description": "zink: fix zink_create_fence_fd to properly import", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "32597e116d7317127ef8a7caf8dc75b50f48b8e1" }, diff --git a/src/gallium/drivers/zink/zink_fence.c b/src/gallium/drivers/zink/zink_fence.c index 91f1499ea00..425136f6e10 100644 --- a/src/gallium/drivers/zink/zink_fence.c +++ b/src/gallium/drivers/zink/zink_fence.c @@ -28,6 +28,7 @@ #include "zink_resource.h" #include "zink_screen.h" +#include "util/os_file.h" #include "util/set.h" #include "util/u_memory.h" @@ -226,43 +227,57 @@ void zink_create_fence_fd(struct pipe_context *pctx, struct pipe_fence_handle **pfence, int fd, enum pipe_fd_type type) { struct zink_screen *screen = zink_screen(pctx->screen); - VkResult ret = VK_ERROR_UNKNOWN; - VkSemaphoreCreateInfo sci = { - VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO, - NULL, - 0 - }; + VkResult result; + + assert(fd >= 0); + struct zink_tc_fence *mfence = zink_create_tc_fence(); - VkExternalSemaphoreHandleTypeFlagBits flags[] = { + if (!mfence) + goto fail_tc_fence_create; + + const VkSemaphoreCreateInfo sci = { + .sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO, + }; + result = VKSCR(CreateSemaphore)(screen->dev, &sci, NULL, &mfence->sem); + if (result != VK_SUCCESS) { + mesa_loge("ZINK: vkCreateSemaphore failed (%s)", vk_Result_to_str(result)); + goto fail_sem_create; + } + + int dup_fd = os_dupfd_cloexec(fd); + if (dup_fd < 0) + goto fail_fd_dup; + + static const VkExternalSemaphoreHandleTypeFlagBits flags[] = { [PIPE_FD_TYPE_NATIVE_SYNC] = VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT, [PIPE_FD_TYPE_SYNCOBJ] = VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_OPAQUE_FD_BIT, }; - VkImportSemaphoreFdInfoKHR sdi = {0}; assert(type < ARRAY_SIZE(flags)); - *pfence = NULL; - - VkResult result = VKSCR(CreateSemaphore)(screen->dev, &sci, NULL, &mfence->sem); - if (result != VK_SUCCESS) { - mesa_loge("ZINK: vkCreateSemaphore failed (%s)", vk_Result_to_str(result)); - FREE(mfence); - return; + const VkImportSemaphoreFdInfoKHR sdi = { + .sType = VK_STRUCTURE_TYPE_IMPORT_SEMAPHORE_FD_INFO_KHR, + .semaphore = mfence->sem, + .flags = VK_SEMAPHORE_IMPORT_TEMPORARY_BIT, + .handleType = flags[type], + .fd = dup_fd, + }; + result = VKSCR(ImportSemaphoreFdKHR)(screen->dev, &sdi); + if (!zink_screen_handle_vkresult(screen, result)) { + mesa_loge("ZINK: vkImportSemaphoreFdKHR failed (%s)", vk_Result_to_str(result)); + goto fail_sem_import; } - sdi.sType = VK_STRUCTURE_TYPE_IMPORT_SEMAPHORE_FD_INFO_KHR; - sdi.semaphore = mfence->sem; - sdi.handleType = flags[type]; - sdi.fd = fd; - ret = VKSCR(ImportSemaphoreFdKHR)(screen->dev, &sdi); - - if (!zink_screen_handle_vkresult(screen, ret)) - goto fail; *pfence = (struct pipe_fence_handle *)mfence; return; -fail: +fail_sem_import: + close(dup_fd); +fail_fd_dup: VKSCR(DestroySemaphore)(screen->dev, mfence->sem, NULL); +fail_sem_create: FREE(mfence); +fail_tc_fence_create: + *pfence = NULL; } #ifdef _WIN32