From dd8f97ab9e77dde25c323feaff97cfc8f19207fa Mon Sep 17 00:00:00 2001 From: gdkchan Date: Sun, 5 Jun 2022 15:12:42 -0300 Subject: [PATCH] Remove freed memory range from tree on memory block disposal (#3347) * Remove freed memory range from tree on memory block disposal * PR feedback --- Ryujinx.Memory/MemoryBlock.cs | 6 +- Ryujinx.Memory/MemoryManagement.cs | 12 ++-- Ryujinx.Memory/MemoryManagementWindows.cs | 10 +-- .../WindowsShared/PlaceholderManager.cs | 69 ++++++++++++++++--- 4 files changed, 76 insertions(+), 21 deletions(-) diff --git a/Ryujinx.Memory/MemoryBlock.cs b/Ryujinx.Memory/MemoryBlock.cs index c6b85b58..b68a1000 100644 --- a/Ryujinx.Memory/MemoryBlock.cs +++ b/Ryujinx.Memory/MemoryBlock.cs @@ -19,6 +19,8 @@ namespace Ryujinx.Memory private ConcurrentDictionary _viewStorages; private int _viewCount; + internal bool ForceWindows4KBView => _forceWindows4KBView; + /// /// Pointer to the memory block data. /// @@ -145,7 +147,7 @@ namespace Ryujinx.Memory srcBlock.IncrementViewCount(); } - MemoryManagement.MapView(srcBlock._sharedMemory, srcOffset, GetPointerInternal(dstOffset, size), size, _forceWindows4KBView); + MemoryManagement.MapView(srcBlock._sharedMemory, srcOffset, GetPointerInternal(dstOffset, size), size, this); } /// @@ -156,7 +158,7 @@ namespace Ryujinx.Memory /// Size of the range to be unmapped public void UnmapView(MemoryBlock srcBlock, ulong offset, ulong size) { - MemoryManagement.UnmapView(srcBlock._sharedMemory, GetPointerInternal(offset, size), size, _forceWindows4KBView); + MemoryManagement.UnmapView(srcBlock._sharedMemory, GetPointerInternal(offset, size), size, this); } /// diff --git a/Ryujinx.Memory/MemoryManagement.cs b/Ryujinx.Memory/MemoryManagement.cs index 3b8a9664..77a8a1ef 100644 --- a/Ryujinx.Memory/MemoryManagement.cs +++ b/Ryujinx.Memory/MemoryManagement.cs @@ -68,17 +68,17 @@ namespace Ryujinx.Memory } } - public static void MapView(IntPtr sharedMemory, ulong srcOffset, IntPtr address, ulong size, bool force4KBMap) + public static void MapView(IntPtr sharedMemory, ulong srcOffset, IntPtr address, ulong size, MemoryBlock owner) { if (OperatingSystem.IsWindows()) { - if (force4KBMap) + if (owner.ForceWindows4KBView) { MemoryManagementWindows.MapView4KB(sharedMemory, srcOffset, address, (IntPtr)size); } else { - MemoryManagementWindows.MapView(sharedMemory, srcOffset, address, (IntPtr)size); + MemoryManagementWindows.MapView(sharedMemory, srcOffset, address, (IntPtr)size, owner); } } else if (OperatingSystem.IsLinux() || OperatingSystem.IsMacOS()) @@ -91,17 +91,17 @@ namespace Ryujinx.Memory } } - public static void UnmapView(IntPtr sharedMemory, IntPtr address, ulong size, bool force4KBMap) + public static void UnmapView(IntPtr sharedMemory, IntPtr address, ulong size, MemoryBlock owner) { if (OperatingSystem.IsWindows()) { - if (force4KBMap) + if (owner.ForceWindows4KBView) { MemoryManagementWindows.UnmapView4KB(address, (IntPtr)size); } else { - MemoryManagementWindows.UnmapView(sharedMemory, address, (IntPtr)size); + MemoryManagementWindows.UnmapView(sharedMemory, address, (IntPtr)size, owner); } } else if (OperatingSystem.IsLinux() || OperatingSystem.IsMacOS()) diff --git a/Ryujinx.Memory/MemoryManagementWindows.cs b/Ryujinx.Memory/MemoryManagementWindows.cs index 3d51b64f..6b4e7fbe 100644 --- a/Ryujinx.Memory/MemoryManagementWindows.cs +++ b/Ryujinx.Memory/MemoryManagementWindows.cs @@ -68,9 +68,9 @@ namespace Ryujinx.Memory return WindowsApi.VirtualFree(location, size, AllocationType.Decommit); } - public static void MapView(IntPtr sharedMemory, ulong srcOffset, IntPtr location, IntPtr size) + public static void MapView(IntPtr sharedMemory, ulong srcOffset, IntPtr location, IntPtr size, MemoryBlock owner) { - _placeholders.MapView(sharedMemory, srcOffset, location, size); + _placeholders.MapView(sharedMemory, srcOffset, location, size, owner); } public static void MapView4KB(IntPtr sharedMemory, ulong srcOffset, IntPtr location, IntPtr size) @@ -106,9 +106,9 @@ namespace Ryujinx.Memory } } - public static void UnmapView(IntPtr sharedMemory, IntPtr location, IntPtr size) + public static void UnmapView(IntPtr sharedMemory, IntPtr location, IntPtr size, MemoryBlock owner) { - _placeholders.UnmapView(sharedMemory, location, size); + _placeholders.UnmapView(sharedMemory, location, size, owner); } public static void UnmapView4KB(IntPtr location, IntPtr size) @@ -154,7 +154,7 @@ namespace Ryujinx.Memory } else { - _placeholders.UnmapView(IntPtr.Zero, address, size); + _placeholders.UnreserveRange((ulong)address, (ulong)size); } return WindowsApi.VirtualFree(address, IntPtr.Zero, AllocationType.Release); diff --git a/Ryujinx.Memory/WindowsShared/PlaceholderManager.cs b/Ryujinx.Memory/WindowsShared/PlaceholderManager.cs index d465f341..1b425d66 100644 --- a/Ryujinx.Memory/WindowsShared/PlaceholderManager.cs +++ b/Ryujinx.Memory/WindowsShared/PlaceholderManager.cs @@ -44,6 +44,50 @@ namespace Ryujinx.Memory.WindowsShared } } + /// + /// Unreserves a range of memory that has been previously reserved with . + /// + /// Start address of the region to unreserve + /// Size in bytes of the region to unreserve + /// Thrown when the Windows API returns an error unreserving the memory + public void UnreserveRange(ulong address, ulong size) + { + ulong endAddress = address + size; + + var overlaps = Array.Empty>(); + int count; + + lock (_mappings) + { + count = _mappings.Get(address, endAddress, ref overlaps); + + for (int index = 0; index < count; index++) + { + var overlap = overlaps[index]; + + if (IsMapped(overlap.Value)) + { + if (!WindowsApi.UnmapViewOfFile2(WindowsApi.CurrentProcessHandle, (IntPtr)overlap.Start, 2)) + { + throw new WindowsApiException("UnmapViewOfFile2"); + } + } + + _mappings.Remove(overlap); + } + } + + if (count > 1) + { + CheckFreeResult(WindowsApi.VirtualFree( + (IntPtr)address, + (IntPtr)size, + AllocationType.Release | AllocationType.CoalescePlaceholders)); + } + + RemoveProtection(address, size); + } + /// /// Maps a shared memory view on a previously reserved memory region. /// @@ -51,13 +95,14 @@ namespace Ryujinx.Memory.WindowsShared /// Offset in the shared memory to map /// Address to map the view into /// Size of the view in bytes - public void MapView(IntPtr sharedMemory, ulong srcOffset, IntPtr location, IntPtr size) + /// Memory block that owns the mapping + public void MapView(IntPtr sharedMemory, ulong srcOffset, IntPtr location, IntPtr size, MemoryBlock owner) { _partialUnmapLock.AcquireReaderLock(Timeout.Infinite); try { - UnmapViewInternal(sharedMemory, location, size); + UnmapViewInternal(sharedMemory, location, size, owner); MapViewInternal(sharedMemory, srcOffset, location, size); } finally @@ -173,13 +218,14 @@ namespace Ryujinx.Memory.WindowsShared /// Shared memory that the view being unmapped belongs to /// Address to unmap /// Size of the region to unmap in bytes - public void UnmapView(IntPtr sharedMemory, IntPtr location, IntPtr size) + /// Memory block that owns the mapping + public void UnmapView(IntPtr sharedMemory, IntPtr location, IntPtr size, MemoryBlock owner) { _partialUnmapLock.AcquireReaderLock(Timeout.Infinite); try { - UnmapViewInternal(sharedMemory, location, size); + UnmapViewInternal(sharedMemory, location, size, owner); } finally { @@ -197,8 +243,9 @@ namespace Ryujinx.Memory.WindowsShared /// Shared memory that the view being unmapped belongs to /// Address to unmap /// Size of the region to unmap in bytes + /// Memory block that owns the mapping /// Thrown when the Windows API returns an error unmapping or remapping the memory - private void UnmapViewInternal(IntPtr sharedMemory, IntPtr location, IntPtr size) + private void UnmapViewInternal(IntPtr sharedMemory, IntPtr location, IntPtr size, MemoryBlock owner) { ulong startAddress = (ulong)location; ulong unmapSize = (ulong)size; @@ -272,7 +319,7 @@ namespace Ryujinx.Memory.WindowsShared } } - CoalesceForUnmap(startAddress, unmapSize); + CoalesceForUnmap(startAddress, unmapSize, owner); RemoveProtection(startAddress, unmapSize); } @@ -281,15 +328,21 @@ namespace Ryujinx.Memory.WindowsShared /// /// Address of the region that was unmapped /// Size of the region that was unmapped in bytes - private void CoalesceForUnmap(ulong address, ulong size) + /// Memory block that owns the mapping + private void CoalesceForUnmap(ulong address, ulong size, MemoryBlock owner) { ulong endAddress = address + size; + ulong blockAddress = (ulong)owner.Pointer; + ulong blockEnd = blockAddress + owner.Size; var overlaps = Array.Empty>(); int unmappedCount = 0; lock (_mappings) { - int count = _mappings.Get(address - MinimumPageSize, endAddress + MinimumPageSize, ref overlaps); + int count = _mappings.Get( + Math.Max(address - MinimumPageSize, blockAddress), + Math.Min(endAddress + MinimumPageSize, blockEnd), ref overlaps); + if (count < 2) { // Nothing to coalesce if we only have 1 or no overlaps.