Open
Conversation
locke-lunarg
added
approved-to-run-ci
Comment on lines
+1330
to
+1336
| #if defined(WIN32) | ||
| for (auto& entry : memory_info_) | ||
| { | ||
| auto& info = entry.second; | ||
| ResetWriteWatch(info.aligned_address, info.mapped_range + info.aligned_offset); | ||
| } | ||
| #endif |
Contributor
There was a problem hiding this comment.
Could this introduce a race condition in processing modified pages? For example:
PageGuardManager::ProcessMemoryEntriesis called. Memory A is found to be modified and is processed.ProcessMemoryEntriescontinues to processing the remaining modified pages.- After memory A is processed, but before
ProcessMemoryEntriescallsResetAllWriteWatch();(it could be processing other modified memory pages), the application completes another write to memory A. ResetAllWriteWatch();is called, clearing the new write watch flag on memory A, causing the application's latest write to be skipped at the next call toProcessMemoryEntries.
locke-lunarg
force-pushed
the
locke-fix-tight-aglin
branch
2 times, most recently
from
4750a87 to
7df7643
Compare
When it uses DX12 tight alignment, the alignment could be 8B~256B. It's much smaller than a page of memory, like 4KB. One page could have two resources, so if it reset the WriteWatch, the another resource on the same page will miss its update.
This reverts commit 7df7643.
locke-lunarg
force-pushed
the
locke-fix-tight-aglin
branch
2 times, most recently
from
558327b to
3129b5d
Compare
locke-lunarg
force-pushed
the
locke-fix-tight-aglin
branch
from
3129b5d to
fd0628c
Compare
Contributor
Author
|
@davidd-lunarg Upload a new solution. It updates the memory info on the same page in |
locke-lunarg
added
the
approved-to-run-ci
MarkY-LunarG
approved these changes
Apr 2, 2026
Contributor
MarkY-LunarG
left a comment
MarkY-LunarG
left a comment
There was a problem hiding this comment.
Good catch. I think this is a bug that has been bugging us for a while. So good thing you ran into it in this scenario.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closed: https://github.com/LunarG/Projects/issues/1125
Ref: https://microsoft.github.io/DirectX-Specs/d3d/D3D12TightPlacedResourceAlignment.html
Regularly, the alignment of the resource should be
D3D12_DEFAULT_RESOURCE_PLACEMENT_ALIGNMENT(64KB). But when it uses tight alignment, it becomes 8B ~ 256B. It's smaller than a page size(system_page_size_: 4KB). In this case, one page could have two resources. ForWriteWatchcase, inLoadActiveWriteStates, if it resetsWriteWatchonGetWriteWatch, another resource on the same page will miss its update. AlthoughGetWriteWatchcould set precise address, it operates on page by page. It shouldn't have resetWriteWatchuntil all memory infos get updated.There are the other two solutions that could fix it.
Force
ID3D12Device_Wrapper::GetResourceAllocationInfoto return alignment to multiple of a page size, so a page won't have two resources. But we shouldn't change the original value.GFXRECON_PAGE_GUARD_EXTERNAL_MEMORYset false could disableWriteWatch. It will use shadow memory, instead ofWriteWatch. But shadow memory consumes huge memory. AndPageGuardManager::HandleGuardPageViolationinterrupts the process to update memory info. It could impact the performance.