mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-09 03:29:43 -10:00
VULKAN: Route texture unload through video thread when threaded video active
The Vulkan driver is already well-synchronised for most
operations -- queue_lock protects all vkQueueSubmit /
vkQueuePresent / vkQueueWaitIdle calls, and vulkan_load_texture
uploads textures synchronously under that lock so the texture
is fully resident on the GPU by the time the function returns.
But vulkan_unload_texture has a residual race that the existing
TODO comment at line 7307 already acknowledged:
/* TODO: We really want to defer this deletion instead,
* but this will do for now. */
The current sequence is:
1. Lock queue_lock
2. vkQueueWaitIdle (waits for SUBMITTED work)
3. Unlock queue_lock
4. vulkan_destroy_texture (NOT under the lock)
Step 2 only drains commands that have already been submitted
to the queue. It does not account for the video thread
currently RECORDING a command buffer that references the
texture -- that command buffer has not yet been submitted, so
vkQueueWaitIdle doesn't see it.
The sequence that triggers the race:
T0: Video thread is recording commands in vulkan_frame that
reference texture X (uses X->view in a descriptor, or
X->image in a barrier). Not yet submitted.
T1: Main thread enters vulkan_unload_texture for X.
T2: Main thread locks queue_lock, calls vkQueueWaitIdle
(returns immediately -- nothing to wait for).
T3: Main thread unlocks queue_lock.
T4: Main thread calls vulkan_destroy_texture -- X's image,
view, and memory are freed.
T5: Video thread finishes recording, acquires queue_lock,
calls vkQueueSubmit. The command buffer references
freed Vulkan handles -- undefined behaviour, typically
a validation-layer error or GPU hang.
Fix: follow the same pattern just applied to D3D12 (and long
established in gl2/gl3/d3d9cg). When threaded video is active,
dispatch the unload via video_thread_texture_handle so the
destruction runs on the video thread, serialised with command
buffer recording. The video thread can only run one
vulkan_frame or one custom_command at a time, so by the time
the unload wrap function executes, any in-progress recording
has completed and the subsequent vkQueueWaitIdle drains any
submitted work before destruction.
Implementation:
- Split vulkan_unload_texture into an internal function
(vulkan_unload_texture_internal) that does the actual work,
and an outer function that dispatches.
- New vulkan_texture_cmd_t packages the vk_t pointer and
texture handle. (Smaller than d3d12_texture_cmd_t because
Vulkan unload has no filter_type and returns no handle.)
- vulkan_load_texture is NOT changed: unlike D3D12 which
uses deferred upload (dirty flag + per-frame CopyTextureRegion),
Vulkan uploads synchronously inside vulkan_create_texture
under queue_lock, so the texture is fully live before the
function returns. No cross-thread races on the load path.
Cost: texture unload now blocks briefly while the video thread
processes the dispatched command. In practice this is much
less than the existing vkQueueWaitIdle stall -- the video
thread usually completes its current operation in microseconds,
whereas vkQueueWaitIdle can stall for milliseconds waiting for
GPU queue drain. So this patch is approximately a performance
neutral or positive change.
Depends on: 8d55232 (VIDEO/THREAD: barrier + wrapper hardening),
which hardens video_thread_texture_handle to be safe when the
wrapper is inactive.
The TODO comment is retained in the internal function, as the
deferred-deletion pattern it describes would still be a valid
future optimisation -- it would avoid the per-unload
vkQueueWaitIdle stall by piggybacking on the natural frame
fence. This patch eliminates the crash, not the stall.
No behavioural change on non-threaded video (the `threaded ==
false` path calls the internal function directly, same as the
pre-patch code). No behavioural change on HW-context cores
(threaded video is disabled for those by
VIDEO_DRIVER_IS_THREADED_INTERNAL).
This commit is contained in:
1 parent
a2d7b486be
commit
17332fe245
1 file changed
+57
-4
+57
-4
@@ -43,6 +43,9 @@
|
||||
|
||||
#include "../font_driver.h"
|
||||
#include "../video_driver.h"
|
||||
#ifdef HAVE_THREADS
|
||||
#include "../video_thread_wrapper.h"
|
||||
#endif
|
||||
|
||||
#include "../common/vulkan_common.h"
|
||||
|
||||
@@ -7296,11 +7299,21 @@ static uintptr_t vulkan_load_texture(void *video_data, void *data,
|
||||
return (uintptr_t)texture;
|
||||
}
|
||||
|
||||
static void vulkan_unload_texture(void *data,
|
||||
bool threaded, uintptr_t handle)
|
||||
#ifdef HAVE_THREADS
|
||||
typedef struct
|
||||
{
|
||||
vk_t *vk = (vk_t*)data;
|
||||
struct vk_texture *texture = (struct vk_texture*)handle;
|
||||
vk_t *vk;
|
||||
uintptr_t handle;
|
||||
} vulkan_texture_cmd_t;
|
||||
#endif
|
||||
|
||||
/* Inner unload function -- performs the queue wait and texture
|
||||
* destruction. Must run on the same thread that owns the
|
||||
* Vulkan queue submissions (the video thread when threaded
|
||||
* video is active, otherwise the main thread). */
|
||||
static void vulkan_unload_texture_internal(vk_t *vk, uintptr_t handle)
|
||||
{
|
||||
struct vk_texture *texture = (struct vk_texture*)handle;
|
||||
if (!texture || !vk || !vk->context)
|
||||
return;
|
||||
|
||||
@@ -7322,6 +7335,46 @@ static void vulkan_unload_texture(void *data,
|
||||
free(texture);
|
||||
}
|
||||
|
||||
#ifdef HAVE_THREADS
|
||||
/* Wrap function invoked on the video thread via
|
||||
* CMD_CUSTOM_COMMAND. Returns 0 (ignored). */
|
||||
static int vulkan_texture_unload_wrap(void *data)
|
||||
{
|
||||
vulkan_texture_cmd_t *cmd = (vulkan_texture_cmd_t*)data;
|
||||
vulkan_unload_texture_internal(cmd->vk, cmd->handle);
|
||||
return 0;
|
||||
}
|
||||
#endif
|
||||
|
||||
static void vulkan_unload_texture(void *data,
|
||||
bool threaded, uintptr_t handle)
|
||||
{
|
||||
vk_t *vk = (vk_t*)data;
|
||||
if (!handle)
|
||||
return;
|
||||
|
||||
#ifdef HAVE_THREADS
|
||||
/* When threaded video is active, dispatch the Release to
|
||||
* the video thread so it is serialised with command buffer
|
||||
* recording. The queue_lock + vkQueueWaitIdle in the inner
|
||||
* function only waits for submitted work -- it does not
|
||||
* cover command buffers currently being recorded by the
|
||||
* video thread, which may still reference this texture.
|
||||
* Dispatching ensures the video thread completes its
|
||||
* current recording before the texture is destroyed. */
|
||||
if (threaded)
|
||||
{
|
||||
vulkan_texture_cmd_t cmd;
|
||||
cmd.vk = vk;
|
||||
cmd.handle = handle;
|
||||
video_thread_texture_handle(&cmd, vulkan_texture_unload_wrap);
|
||||
return;
|
||||
}
|
||||
#endif
|
||||
|
||||
vulkan_unload_texture_internal(vk, handle);
|
||||
}
|
||||
|
||||
static float vulkan_get_refresh_rate(void *data)
|
||||
{
|
||||
float refresh_rate;
|
||||
|
||||
Reference in new issue
Block a user