Skip to content

fix(vk): check the cooperative matrix probe's mapping, fence and timeout - #2742

Open
juliotec wants to merge 1 commit into
NotRexed:mainfrom
juliotec:fix/vk-coopmat-probe
Open

juliotec wants to merge 1 commit into
NotRexed:mainfrom
juliotec:fix/vk-coopmat-probe

Conversation

@juliotec

Copy link
Copy Markdown

run_shader, the startup probe for cooperative matrices, wrote through the
mapped pointer without checking vkMapMemory, so a failed mapping crashed at
startup. And when the probe's dispatch was submitted but did not finish within
the 5 s wait (a slow or hung driver), it destroyed the pipeline, descriptor pool,
command pool, buffer and memory the GPU could still be using.

What changed (gpu/vk/vk_coopmat.cpp, run_shader):

  • a failed vkMapMemory frees the buffer and memory and fails the probe
  • a submitted dispatch that did not complete leaves its resources alive (a
    one-time leak at startup) instead of destroying them under the GPU
  • a fence that could not be created skips the submit and fails the probe, with
    everything freed
    The probe's result and every successful path are unchanged.

How tested: release build; Zelda OoT on Radeon RX 9070 XT (Windows,
Adrenalin).

Assisted-by: GitHub Copilot

run_shader, the startup probe for cooperative matrices, wrote through the
mapped pointer without checking vkMapMemory, so a failed mapping crashed at
startup. And when the probe's dispatch was submitted but did not finish within
the 5 s wait (a slow or hung driver), it destroyed the pipeline, descriptor pool,
command pool, buffer and memory the GPU could still be using.

What changed (gpu/vk/vk_coopmat.cpp, run_shader):
- a failed vkMapMemory frees the buffer and memory and fails the probe
- a submitted dispatch that did not complete leaves its resources alive (a
  one-time leak at startup) instead of destroying them under the GPU
- a fence that could not be created skips the submit and fails the probe, with
  everything freed
The probe's result and every successful path are unchanged.

How tested: release build; Zelda OoT on Radeon RX 9070 XT (Windows,
Adrenalin).

Assisted-by: GitHub Copilot

@Joey-Magnus Joey-Magnus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI never ran here. I merged this with the other four of your Vulkan fixes onto current main, compiled vk_coopmat.cpp with clang++ using the back end's flags, and ran tests/run_all.py with the Vulkan headers. No failure comes from this file.

The changes are right. A failed vkMapMemory now frees the buffer and memory instead of memcpy into null. A missing fence skips the submit, and everything is freed because nothing was queued. A submitted dispatch that times out leaves its pool, pipeline, buffer and memory alive rather than destroying them while the GPU may still use them (gpu/vk/vk_coopmat.cpp:215). Destroying the shader module there is fine after pipeline creation. On a device that really hung, later submits queue behind it anyway, so the one-time leak is the right trade.

#1975 (cooperative matrices for the AA networks) changes run_shader's signature. It merges cleanly with this one, but whoever lands second should check that the new early return still frees what #1975 adds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants