agc_driver_recorder_tests fails on Windows since #2096: the refresh store check, plus a more frequent device loss #2155

Activity

1. popcoder565

   changed the title [-]agc_driver_recorder_tests fails on Windows since #2096, mostly with VK_ERROR_DEVICE_LOST[/-] [+]agc_driver_recorder_tests fails on Windows since #2096: the refresh store check, plus a more frequent device loss[/+] on Oct 9, 2026
2. rmz-24 commented on Oct 9, 2026

   

   rmz-24

   on Oct 9, 2026

   Contributor

   More actions

   Reproduced on main d70b899: Windows 11, RTX 5060 Ti (NVIDIA 617.42), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release. The refresh store failure happens in 5 of 5 runs. The device loss did not happen in any run on this GPU, so this comment covers only the refresh store.

   What happens

   I added temporary logging to StorageTexture::Refresh and writeBackLayers. The failing refresh is the unwatched pass (VirtualAlloc block):

   refresh unchanged=1 tracked=0 compared=1 keysChanged=0 blockUnits=1 keys=uncompressed uploaded=uncompressed pending=4/4 direct=1
   

   • The import is untracked, so ChangedBlocks fails and 0e1d25a's compareUntracked runs. The guest bytes still match the snapshot taken at upload, so every unit is BlockUnchanged and Refresh returns early through if (unchanged).
   • On the direct path, unchanged pending units keep their results on the GPU (else if (direct) continue;, the same rule as for tracked memory). Nothing is stored at the refresh.
   • Before 0e1d25a, untracked memory could never be proved unchanged. Every refresh treated all units as changed, and since droppable is false without tracking, it stored the pending units. The assertion at Recorder.cpp:1672 was written against that behaviour.

   The results are not lost

   Directly after the refresh, guest memory is still 55 55 55 55 in every unit. After StorageTexture::FlushPending(address, surfaceBytes, nullptr, ...), then recorder.Submit(), device.WaitQueue() and recorder.Sync(), it holds ff 00 00 ff in every unit. The write-back's own comparison saw all blocks unchanged (0 0 0 0), so nothing was dropped as a CPU store. With that flush in place the rest of agc_driver_recorder_tests passes (Recorder read tracking and label tests passed), 8 of 8 runs.

   Question

   Is keeping the results pending in compared (untracked) memory intended? The CPU's reads of that memory aren't trapped, so the results reach it only at the driver's next flush point for that range. Before 0e1d25a that flush point was the refresh. If this is intended, the test could check it like this, keeping the strict assertion for watched memory:

           if (!memoryHolds({255, 0, 0, 255})) {
               Require(!watched && !AgcDriver::GuestMemory::Watched(address, bytes), "the refresh's store of the results did not reach guest memory");
               Require(StorageTexture::FlushPending(address, surfaceBytes, nullptr, "storage refresh test"), "results the refresh kept on the GPU were not pending");
               recorder.Submit();
               device.WaitQueue();
               recorder.Sync();
               Require(memoryHolds({255, 0, 0, 255}), "results the refresh kept on the GPU did not reach guest memory when flushed");
           }

   If it isn't intended, the fix belongs in Refresh: a compared import with pending results would store them, as before. I can open a PR for whichever direction you prefer.

   AI-assisted: yes
3. MarcZX10 commented on Oct 9, 2026

   

   MarcZX10

   on Oct 9, 2026

   Contributor

   More actions

   I checked the existing Refresh contract and agc_driver_dcc_keys_first_write: an unchanged image with pending results must remain resident for the next GPU use, and Refresh() is not the CPU visibility barrier for untracked imports. My attempted production write-back caused agc_driver_dcc_keys_first_write to fail with “an unchanged refresh wrote the pending image back,” so I discarded that approach. PR #2248 updates the recorder regression to assert that the image remains pending, then explicitly FlushPending, wait, and check guest memory. With that test expectation, agc_driver_recorder_tests passes, and agc_driver_dcc_keys_first_write passes (1/1).
4. popcoder565 commented on Oct 9, 2026

   

   popcoder565

   on Oct 9, 2026

   ContributorAuthor

   More actions

   Found the cause of the device loss. It comes from unitShadowTests, not from #2096.

   • On Windows, host imports are compared, so unitShadowTests returns right after it imports its 17 MiB block. Its cleanup removes the registration and releases the block. The import stays until the next lookup, which is the HostImportFor at the start of storageRefreshTests. That retire's vkFreeMemory runs after the pages are gone.

   • I put vkQueueWaitIdle probes around each step. In all 7 device losses out of 20 runs, the queue was healthy just before that vkFreeMemory and lost right after it.

   • It only happens when the block is released shortly after the import. In a local probe that imports a 17 MiB block, releases it, then lets the next lookup retire the import, the device was lost in:

     • 12 of 60 runs when the block was released about 10 µs after the import
     • 4 of 20 at 0.5 ms
     • 1 of 20 at 1 ms
     • 0 of 20 at each of 1.5, 2, 5, 20 and 100 ms
     • 0 of 60 when the import was retired before the release

     That would explain why it's intermittent, and why it may not show up on other GPUs or drivers.

   #2299 makes the cleanup retire the import before it releases the block. With #2248 and #2299, agc_driver_recorder_tests passes 30 of 30 here. With #2299 alone, there was no device loss in 30 runs; all 30 still fail at the refresh store, which #2248 fixes.

   The runtime unmap path uses the same order. GuestAllocationsUnmap_nid_postfix releases the pages before the registry changes, and the import is retired at the next lookup. I haven't checked whether any title releases memory within a millisecond of importing it.

   🤖 Addressed by Claude Code
5. MarcZX10

   added 4 commits that reference this issue on Oct 9, 2026

   test(agc-driver): check explicit flush for untracked refresh

   ...

   This issue will close once commit 8c23628 is merged into the 'main' branch.

   8c23628

   test(agc-driver): check explicit flush for untracked refresh

   ...

   This issue will close once commit 66708cd is merged into the 'main' branch.

   66708cd

   test(agc-driver): check explicit flush for untracked refresh

   ...

   This issue will close once commit 137e808 is merged into the 'main' branch.

   137e808

   test(agc-driver): check explicit flush for untracked refresh

   ...

   This issue will close once commit 97d06df is merged into the 'main' branch.

   97d06df
6. LeandroLP23 commented on Oct 9, 2026

   

   LeandroLP23

   on Oct 9, 2026

   Contributor

   More actions

   AMD data point: on the AMD Windows driver the test stops much earlier than in the NVIDIA runs above, in readTrackingTests, before it reaches storageRefreshTests.

   Radeon 860M (RDNA 3.5, 0x1114), AMD driver 24.30.58.08 (32.0.13058.8001), Windows 11 (26300), WinLibs GCC 15.2.0 r7, Release, main 00159cf. Five of five runs:

   AGC graphics: a signaled batch's read still refuses   (tests/Recorder.cpp:247)
   

   The check right before it calls device.WaitQueue() (vkQueueWaitIdle) and expects the batch's fence to read as signaled, but on this driver the fence usually lags the idle wait. A standalone probe (source below) submits to one queue, calls vkQueueWaitIdle, then vkGetFenceStatus, 2000 times per variant:

   Submit	Fence not ready right after vkQueueWaitIdle	Worst extra vkGetFenceStatus polls until ready
   no command buffer, fence only	1920 / 2000	1415
   command buffer with a barrier, fence	1928 / 2000	6318
   barrier, fence and a timeline semaphore signal (as Recorder::Submit does)	1918 / 2000	369

   So vkQueueWaitIdle returns before the fence reads as signaled on this driver, almost every time. When I polled the overlap query for a short time, that check passed and the test stopped at the next check of the same kind, a signaled batch's write is not settled (line 271), after the same WaitQueue() pattern. Production code only treats such a batch as still in flight a little longer, which is conservative. The test, though, would need to wait on the batch's fence (vkWaitForFences) or poll briefly to get past these checks on AMD, before it can show whether #2248 helps with the refresh store here.

   Probe source (fenceprobe.cpp, g++ -std=c++20 -I3rdparty/Vulkan-Headers/include)

   #define VK_NO_PROTOTYPES
   #include <vulkan/vulkan.h>
   #include <windows.h>
   #include <cstdio>
   #include <cstring>
   #include <vector>
   
   static PFN_vkGetInstanceProcAddr gipa;
   #define IF(name) auto name = reinterpret_cast<PFN_##name>(gipa(instance, #name))
   
   int main(int argc, char** argv) {
       const int iterations = argc > 1 ? std::atoi(argv[1]) : 2000;
       HMODULE lib = LoadLibraryA("vulkan-1.dll");
       gipa = reinterpret_cast<PFN_vkGetInstanceProcAddr>(GetProcAddress(lib, "vkGetInstanceProcAddr"));
       auto createInstance = reinterpret_cast<PFN_vkCreateInstance>(gipa(nullptr, "vkCreateInstance"));
       VkApplicationInfo app{VK_STRUCTURE_TYPE_APPLICATION_INFO};
       app.apiVersion = VK_API_VERSION_1_2;
       VkInstanceCreateInfo ici{VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO};
       ici.pApplicationInfo = &app;
       VkInstance instance;
       if (createInstance(&ici, nullptr, &instance) != VK_SUCCESS) return 2;
       IF(vkEnumeratePhysicalDevices); IF(vkGetPhysicalDeviceProperties2); IF(vkCreateDevice); IF(vkGetDeviceProcAddr);
       IF(vkGetPhysicalDeviceQueueFamilyProperties);
       uint32_t count = 0;
       vkEnumeratePhysicalDevices(instance, &count, nullptr);
       std::vector<VkPhysicalDevice> devices(count);
       vkEnumeratePhysicalDevices(instance, &count, devices.data());
       VkPhysicalDevice physical = devices[0];
       VkPhysicalDeviceDriverProperties driver{VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_DRIVER_PROPERTIES};
       VkPhysicalDeviceProperties2 props{VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROPERTIES_2, &driver};
       vkGetPhysicalDeviceProperties2(physical, &props);
       std::printf("device=%s id=0x%x driver=%s %s\n", props.properties.deviceName, props.properties.deviceID, driver.driverName, driver.driverInfo);
       uint32_t familyCount = 0;
       vkGetPhysicalDeviceQueueFamilyProperties(physical, &familyCount, nullptr);
       std::vector<VkQueueFamilyProperties> families(familyCount);
       vkGetPhysicalDeviceQueueFamilyProperties(physical, &familyCount, families.data());
       uint32_t family = 0;
       while (!(families[family].queueFlags & VK_QUEUE_GRAPHICS_BIT)) ++family;
       float priority = 1.0f;
       VkDeviceQueueCreateInfo qci{VK_STRUCTURE_TYPE_DEVICE_QUEUE_CREATE_INFO};
       qci.queueFamilyIndex = family; qci.queueCount = 1; qci.pQueuePriorities = &priority;
       VkPhysicalDeviceTimelineSemaphoreFeatures timelineFeature{VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_TIMELINE_SEMAPHORE_FEATURES};
       timelineFeature.timelineSemaphore = VK_TRUE;
       VkDeviceCreateInfo dci{VK_STRUCTURE_TYPE_DEVICE_CREATE_INFO, &timelineFeature};
       dci.queueCreateInfoCount = 1; dci.pQueueCreateInfos = &qci;
       VkDevice device;
       if (vkCreateDevice(physical, &dci, nullptr, &device) != VK_SUCCESS) return 3;
   #define DF(name) auto name = reinterpret_cast<PFN_##name>(vkGetDeviceProcAddr(device, #name))
       DF(vkGetDeviceQueue); DF(vkCreateFence); DF(vkResetFences); DF(vkGetFenceStatus); DF(vkQueueSubmit); DF(vkQueueWaitIdle);
       DF(vkCreateCommandPool); DF(vkAllocateCommandBuffers); DF(vkBeginCommandBuffer); DF(vkEndCommandBuffer); DF(vkCmdPipelineBarrier);
       DF(vkCreateSemaphore); DF(vkResetCommandBuffer);
       VkQueue queue;
       vkGetDeviceQueue(device, family, 0, &queue);
       VkCommandPoolCreateInfo pci{VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO};
       pci.flags = VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT; pci.queueFamilyIndex = family;
       VkCommandPool pool; vkCreateCommandPool(device, &pci, nullptr, &pool);
       VkCommandBufferAllocateInfo cai{VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO};
       cai.commandPool = pool; cai.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY; cai.commandBufferCount = 1;
       VkCommandBuffer cmd; vkAllocateCommandBuffers(device, &cai, &cmd);
       VkSemaphoreTypeCreateInfo sti{VK_STRUCTURE_TYPE_SEMAPHORE_TYPE_CREATE_INFO};
       sti.semaphoreType = VK_SEMAPHORE_TYPE_TIMELINE;
       VkSemaphoreCreateInfo sci{VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO, &sti};
       VkSemaphore timeline; vkCreateSemaphore(device, &sci, nullptr, &timeline);
       VkFenceCreateInfo fci{VK_STRUCTURE_TYPE_FENCE_CREATE_INFO};
       VkFence fence; vkCreateFence(device, &fci, nullptr, &fence);
       uint64_t serial = 0;
       const char* names[] = {"no command buffer, fence only", "command buffer with a barrier, fence", "command buffer with a barrier, fence and timeline signal"};
       for (int variant = 0; variant < 3; ++variant) {
           int notReady = 0, maxExtraPolls = 0;
           for (int i = 0; i < iterations; ++i) {
               vkResetFences(device, 1, &fence);
               VkSubmitInfo submit{VK_STRUCTURE_TYPE_SUBMIT_INFO};
               VkTimelineSemaphoreSubmitInfo tsi{VK_STRUCTURE_TYPE_TIMELINE_SEMAPHORE_SUBMIT_INFO};
               if (variant >= 1) {
                   vkResetCommandBuffer(cmd, 0);
                   VkCommandBufferBeginInfo bi{VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO};
                   bi.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT;
                   vkBeginCommandBuffer(cmd, &bi);
                   VkMemoryBarrier mb{VK_STRUCTURE_TYPE_MEMORY_BARRIER};
                   mb.srcAccessMask = VK_ACCESS_SHADER_WRITE_BIT | VK_ACCESS_TRANSFER_WRITE_BIT;
                   mb.dstAccessMask = VK_ACCESS_HOST_READ_BIT;
                   vkCmdPipelineBarrier(cmd, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_HOST_BIT, 0, 1, &mb, 0, nullptr, 0, nullptr);
                   vkEndCommandBuffer(cmd);
                   submit.commandBufferCount = 1; submit.pCommandBuffers = &cmd;
               }
               if (variant == 2) {
                   ++serial;
                   tsi.signalSemaphoreValueCount = 1; tsi.pSignalSemaphoreValues = &serial;
                   submit.pNext = &tsi; submit.signalSemaphoreCount = 1; submit.pSignalSemaphores = &timeline;
               }
               if (vkQueueSubmit(queue, 1, &submit, fence) != VK_SUCCESS) return 4;
               if (vkQueueWaitIdle(queue) != VK_SUCCESS) return 5;
               if (vkGetFenceStatus(device, fence) != VK_SUCCESS) {
                   ++notReady;
                   int polls = 0;
                   while (vkGetFenceStatus(device, fence) != VK_SUCCESS && polls < 10000000) ++polls;
                   if (polls > maxExtraPolls) maxExtraPolls = polls;
               }
           }
           std::printf("%s: %d of %d not ready right after vkQueueWaitIdle, worst %d extra polls\n", names[variant], notReady, iterations, maxExtraPolls);
       }
       return 0;
   }

   AI-assisted: yes (Claude Code).
7. MarcZX10

   added a commit that references this issue on Oct 9, 2026

   test(agc-driver): check explicit flush for untracked refresh

   ...

   This issue will close once commit cf043ca is merged into the 'main' branch.

   cf043ca
8. oneandonlydean

   closed this as completedin cf043caon Oct 10, 2026