Repository navigation
Conversation
…rror allocation When /dev/udmabuf exists but cannot import guest memory (e.g. kernel/driver rejection with EINVAL, EBADFD, or EPERM), the driver would continuously fail imports on every pool allocation and spam warnings without disabling udmabuf. In addition, pools that failed mirror creation under transient VRAM pressure remained permanently disabled from mirroring. - Implement disable_dmabuf_imports() to cleanly close udmabuf handles and fall back to host-pointer imports when guest imports are rejected before any succeeded. - Add mirror_retry_epoch tracking to retry device mirror buffer creation once VRAM pressure subsides instead of permanently refusing the pool.
Joey-Magnus
left a comment
There was a problem hiding this comment.
I ran the suite merged into main and it passes, but:
mirror_makenow callsmirrors_give_back(n)(vk_memory.cpp:3060), which is reached fromgpu_buffer_bindmid-recording (:2752).mirror_freethen rewrites the page table and bumpspools_genwhile the recording is still open. That used to happen only at the submission boundary.- With VRAM genuinely full, the retry at
mirror_epoch + 120(:3063) repeats free-and-retry every 120 submissions, freeing idle mirrors for an allocation that keeps failing. Low-VRAM cards churn. - Imports are switched off for the session on the first failed import before any success (
:204), so one transientVK_ERROR_OUT_OF_DEVICE_MEMORYdisables dma-buf imports for good. The log was already capped at 8 lines.
I'd split out or drop the retry, and attach a run from the game.
ghost
left a comment
There was a problem hiding this comment.
Thanks. The udmabuf half makes sense to me: EBADFD/EINVAL from udmabuf on a memfd it won't take are permanent, and so is "no memory type", so giving up on the first one is fine there. Two things I'd still change, plus I agree with @Joey-Magnus's review:
- His third point stands:
vkImportMemoryFdKHR/bind failing withVK_ERROR_OUT_OF_DEVICE_MEMORYis not proof the driver can't import, so I'd only disable on the ioctl errnos and the missing memory type, and keep the import-failure path as it was. - His first point (
mirrors_give_backreached fromgpu_buffer_bindmid-recording, page table rewritten andpools_genbumped outside the submission boundary) is a real behaviour change and not mentioned in the description. More below on why the immediate retry rarely helps anyway, which makes it easy to just drop. - The 120-submission retry of
MIRROR_REFUSEDpools on its own is reasonable, but I'd split it from the udmabuf fix into its own PR with a run from the game on a low-VRAM card, as Joey asked.
Nit: the description says the failures meant "continuous ioctl failure logging"; the log was already capped at 8 lines, so this is about the wasted ioctls/imports, not log spam.
CI has no checks reported on this branch yet.
| if (!create_device_buffer(n, kUsage, p->mirror, p->mirror_alloc)) { | ||
| p->mstate = Pool::MIRROR_REFUSED; | ||
| return false; | ||
| mirrors_give_back(n); |
There was a problem hiding this comment.
We only get here after the budget check above passed, so inside mirrors_give_back the mirror_bytes + want_bytes > mirror_budget() clause is false: it frees only retired mirrors and ones idle past kLongIdle, which memory_begin_submission already does every 64 submissions. So the second create_device_buffer is usually the same allocation against the same VRAM, and the cost is the mid-recording mirror_free Joey pointed out. I'd drop the give-back-and-retry here and keep just the mirror_retry_epoch part (in its own PR).
| void disable_dmabuf_imports(const char* why) { | ||
| if (dmabuf_memfd >= 0) logf("vk: disabling udmabuf pool imports (%s); falling back to host-pointer imports", why); | ||
| dmabuf_memfd = -1; | ||
| if (dmabuf_dev >= 0) close(dmabuf_dev), dmabuf_dev = -1; |
There was a problem hiding this comment.
Minor: dma_buf_memory can run on the background import thread (bg_main calls import_pool without pools_mu) as well as on the recording thread, so closing dmabuf_dev here can race the other thread's ioctl on the same fd (or a reused fd number). The background imports are opt-in (RX_VK_BG_IMPORT, RX_VK_ASYNC_IMPORT, prefetch), so not a default-path problem; setting dmabuf_memfd = -1 and leaving the fd open would avoid it.
|
I tested this PR in a real play session. It fixes the stuck refused mirrors I reported in #2412 (#2412 (comment)). Setup: RTX 3070 Laptop 8 GB, NVIDIA 620.70.06, CachyOS (Linux). I merged this PR into main at 57ed3aa ( Timeline
At Lake Hylia, VRAM ran out the same way as in my earlier report. The log showed Same scene size, four sessions (same PC and settings)
(fps: mean of the Lowering the texture budget to 2560 did not leave room for the mirrors. VRAM still filled up to 6.4-6.7 GB, textures were missing or blurry in some areas, and fps was worse than with 3072. With this PR and the budget back at 3072, the session at up to 1800 draws a frame matched my best earlier one, The lower fps in Hyrule Field comes from the size of the scene, not from memory. At 1500-2200 draws a frame, both the replay ( On NVIDIA the udmabuf part of the PR doesn't come into play: the log has no |
|
Tested this on Windows with an 8 GB card and it fixes the slowdown for me. Setup: RTX 5060 Ti 8 GB, driver 610.88, Core Ultra 7 265F, Windows 11, 1080p. Built on main at 497060f with this PR merged on top. My build also has a few other local changes (#1377 and some switches), but those were identical in both runs below. The only difference between them is this PR. Same route both times: Graveyard, Kakariko, Hyrule Field, then into the Market from the field, then back out. Without the PR, the Market entry pushed VRAM over the limit and two pools (160 MB and 240 MB) ended up in state 3 for the rest of the session. The GPU read them over PCIe the whole time and the game never recovered, even after leaving. With the PR they got their mirrors back.
The Market itself still drops to around 22 fps, but that one looks CPU-bound (#995), not this. (I used Claude to help read the logs; the playing and testing were mine.) |
|
Thanks @Joey-Magnus and @hckrman101 for the feedback, and @YakuzaFanA for the benchmark run on Windows! Pushed an update addressing all points:
Tested locally on Linux (6 GB hardware):
|
Joey-Magnus
left a comment
There was a problem hiding this comment.
The update addresses my first round: no mirrors_give_back mid-recording, the retry is budget-gated and frees nothing, and an out-of-device-memory import no longer turns dma-buf imports off (gpu/vk/vk_memory.cpp:213).
I merged it into current main and ran tests/run_all.py. One failure is this PR's: tests/test_direct_buffer_writes.py cuts gpu_buffer_bind out, and the retry block (:3042) uses mirror_bytes and mirror_budget(), which tests/direct_buffer_writes_test.cpp doesn't declare, so it no longer compiles. Please add the two stubs. The other CI reds are main's from early Nov 12.
Still open:
- hckrman101's point on
:163:disable_dmabuf_importsclosesdmabuf_devwhile the background import thread (RX_VK_BG_IMPORT,RX_VK_ASYNC_IMPORT) can be inioctlon it. Settingdmabuf_memfd = -1and leaving the fd open avoids that. - The description still describes the give-back retry and log spam.
- felipefl142's and YakuzaFanA's runs were on 2fa7431; a short re-run on 993f7f0 would confirm the budget-gated retry still recovers.
|
I played the updated head (993f7f0) for about 2.5 h, merged into main 51da046 ( Route: Desert Colossus → Spirit Temple → Death Mountain Crater → Market → Ganon's Castle → Ganondorf → the collapsing castle → Ganon → credits. The three changes:
Separate from this PR, for reference: what is left is the budget itself, not the retry. In the collapsing castle the 2048 MB mirror budget was full, and a 208–272 MB pool stayed in state 1 (WANTED). The GPU read it over PCIe at up to 1.9 GB per 600 submits (~30–33 fps, readbacks waiting 2–6 ms each instead of ~0.1 ms). I then tried CI: merged into current main, the only check that fails is The other red checks in this PR's CI run pass once current main is merged in. |
Problem
When
/dev/udmabufexists on the host system but fails to import guest memory (e.g. kernel/driver rejection withEINVAL,EBADFD, orEPERM, or when the driver reports no valid memory type), the Vulkan backend continuously attempts imports on every buffer pool allocation. This causes continuous ioctl failure logging and wasted kernel transitions instead of cleanly falling back to host-pointer imports (VK_EXT_external_memory_host).In addition, pools that failed device mirror creation under transient VRAM pressure remained permanently marked as
MIRROR_REFUSED. If memory was later freed or other mirrors were reclaimed, these pools would never be retried, leaving hot buffers in slower system memory permanently.Solution
disable_dmabuf_imports(const char* why): if udmabuf creation or driver import is rejected before any successful import has occurred, close the udmabuf handle and permanently fallback to host-pointer imports for subsequent allocations.mirror_make, if device buffer creation fails due to VRAM pressure, triggermirrors_give_back()and retry once.mirror_retry_epoch = mirror_epoch + 120rather than permanently refusing mirrors for the pool. Ingpu_buffer_bind, allow pools to exitMIRROR_REFUSEDonce the retry epoch is reached.Impact
ioctl(..., kUdmabufCreate)syscalls and log spam on Linux configurations where/dev/udmabufis present but unsupported or restricted.tests/run_all.py(359 passed, 0 failed).