minor optimizations

src/graphics/host_gpu/memoryTracker.cpp

@@ -79,7 +79,6 @@ RegionManager* MemoryTracker::GetOrCreateRegion(uint64_t index) {
 
 bool MemoryTracker::IsRegionCpuModified(uint64_t vaddr, uint64_t size) {
 	CheckNotInUploadCallback();
-	std::lock_guard access(m_access_mutex);
 	return Iterate<true>(vaddr, size, [](RegionManager* manager, uint64_t offset, uint64_t bytes) {
 		std::scoped_lock lock(manager->lock);
 		return manager->IsModified<DirtySource::Cpu>(offset, bytes);
@@ -88,7 +87,6 @@ bool MemoryTracker::IsRegionCpuModified(uint64_t vaddr, uint64_t size) {
 
 bool MemoryTracker::IsRegionGpuModified(uint64_t vaddr, uint64_t size) {
 	CheckNotInUploadCallback();
-	std::lock_guard access(m_access_mutex);
 	return Iterate<false>(vaddr, size, [](RegionManager* manager, uint64_t offset, uint64_t bytes) {
 		std::scoped_lock lock(manager->lock);
 		return manager->IsModified<DirtySource::Gpu>(offset, bytes);
@@ -97,7 +95,6 @@ bool MemoryTracker::IsRegionGpuModified(uint64_t vaddr, uint64_t size) {
 
 void MemoryTracker::MarkRegionAsCpuModified(uint64_t vaddr, uint64_t size) {
 	CheckNotInUploadCallback();
-	std::lock_guard access(m_access_mutex);
 	Iterate<true>(vaddr, size, [](RegionManager* manager, uint64_t offset, uint64_t bytes) {
 		std::scoped_lock lock(manager->lock);
 		manager->ChangeState<DirtySource::Cpu, true>(manager->GetCpuAddr() + offset, bytes);
@@ -106,7 +103,6 @@ void MemoryTracker::MarkRegionAsCpuModified(uint64_t vaddr, uint64_t size) {
 
 void MemoryTracker::MarkRegionAsGpuModified(uint64_t vaddr, uint64_t size) {
 	CheckNotInUploadCallback();
-	std::lock_guard access(m_access_mutex);
 	Iterate<true>(vaddr, size, [](RegionManager* manager, uint64_t offset, uint64_t bytes) {
 		std::scoped_lock lock(manager->lock);
 		manager->ChangeState<DirtySource::Gpu, true>(manager->GetCpuAddr() + offset, bytes);
@@ -115,14 +111,13 @@ void MemoryTracker::MarkRegionAsGpuModified(uint64_t vaddr, uint64_t size) {
 
 void MemoryTracker::UnmarkRegionAsGpuModified(uint64_t vaddr, uint64_t size) {
 	CheckNotInUploadCallback();
-	std::lock_guard access(m_access_mutex);
 	Iterate<false>(vaddr, size, [](RegionManager* manager, uint64_t offset, uint64_t bytes) {
 		std::scoped_lock lock(manager->lock);
 		manager->ChangeState<DirtySource::Gpu, false>(manager->GetCpuAddr() + offset, bytes);
 	});
 }
 
-void MemoryTracker::UntrackMemoryLocked(uint64_t vaddr, uint64_t size) {
+void MemoryTracker::UntrackMemoryImpl(uint64_t vaddr, uint64_t size) {
 	std::vector<RegionManager*> managers;
 	managers.reserve((vaddr % TRACKER_REGION_SIZE + size + TRACKER_REGION_SIZE - 1) /
 	                 TRACKER_REGION_SIZE);
@@ -148,8 +143,7 @@ void MemoryTracker::UntrackMemoryLocked(uint64_t vaddr, uint64_t size) {
 
 void MemoryTracker::UntrackMemory(uint64_t vaddr, uint64_t size) {
 	CheckNotInUploadCallback();
-	std::lock_guard access(m_access_mutex);
-	UntrackMemoryLocked(vaddr, size);
+	UntrackMemoryImpl(vaddr, size);
 }
 
 } // namespace Libs::Graphics

src/graphics/host_gpu/memoryTracker.h

@@ -68,7 +68,6 @@ public:
 		static_assert(std::is_nothrow_invocable_v<Preflight&, uint64_t, uint64_t>);
 		static_assert(std::is_nothrow_invocable_v<Func&, uint64_t, uint64_t>);
 		CheckNotInUploadCallback();
-		std::lock_guard             access(m_access_mutex);
 		std::vector<RegionManager*> managers;
 		Iterate<false>(vaddr, size, [&](RegionManager* manager, uint64_t, uint64_t) {
 			managers.push_back(manager);
@@ -109,7 +108,6 @@ public:
 		static_assert(std::is_nothrow_invocable_v<RangeFunc&, uint64_t, uint64_t>);
 		static_assert(std::is_nothrow_invocable_v<UploadFunc&>);
 		CheckNotInUploadCallback();
-		std::unique_lock access(m_access_mutex);
 		Iterate<true>(vaddr, size, [](RegionManager*, uint64_t, uint64_t) {});
 		const auto* previous_upload_owner = std::exchange(s_upload_owner, this);
 		Iterate<false>(vaddr, size, [&](RegionManager* manager, uint64_t offset, uint64_t bytes) {
@@ -173,13 +171,12 @@ private:
 	}
 
 	static void    ValidateRange(uint64_t vaddr, uint64_t size);
-	void           UntrackMemoryLocked(uint64_t vaddr, uint64_t size);
+	void           UntrackMemoryImpl(uint64_t vaddr, uint64_t size);
 	RegionManager* GetOrCreateRegion(uint64_t index);
 
 	std::unique_ptr<std::atomic<RegionManager*>[]> m_regions;
 	std::vector<std::unique_ptr<RegionManager>>    m_region_storage;
 	std::mutex                                     m_region_mutex;
-	std::mutex                                     m_access_mutex;
 	PageManager&                                   m_page_manager;
 };
 

tests/MemoryTrackerTests.cpp

@@ -2,6 +2,8 @@
 #include "graphics/host_gpu/memoryTracker.h"
 #include "graphics/host_gpu/rangeSet.h"
 
+#include <atomic>
+#include <chrono>
 #include <cstdint>
 #include <cstdio>
 #include <cstdlib>
@@ -211,6 +213,40 @@ void TestQueriesDoNotRequireMappedOwnership() {
         "unowned tracker range did not expose its initial CPU-dirty state");
 }
 
+void TestConcurrentRegionPublication() {
+  TrackerHarness harness;
+  auto &tracker = harness.tracker;
+  auto &page_manager = harness.page_manager;
+  const auto page_size = page_manager.GetPageSize();
+  auto *memory = Allocate(page_manager, 1);
+  const auto address = reinterpret_cast<uint64_t>(memory);
+
+  std::binary_semaphore start_first{0};
+  std::binary_semaphore start_second{0};
+  std::atomic_uint32_t cpu_dirty_results{0};
+  std::jthread first([&] {
+    start_first.acquire();
+    if (tracker.IsRegionCpuModified(address, page_size)) {
+      cpu_dirty_results.fetch_add(1, std::memory_order_relaxed);
+    }
+  });
+  std::jthread second([&] {
+    start_second.acquire();
+    if (tracker.IsRegionCpuModified(address, page_size)) {
+      cpu_dirty_results.fetch_add(1, std::memory_order_relaxed);
+    }
+  });
+  start_first.release();
+  start_second.release();
+  first.join();
+  second.join();
+
+  tracker.UntrackMemory(address, page_size);
+  Release(page_manager, memory, page_size);
+  Check(cpu_dirty_results.load(std::memory_order_relaxed) == 2,
+        "concurrent region publication lost initial CPU ownership");
+}
+
 void TestCpuDirtyUpload() {
   TrackerHarness harness;
   auto &tracker = harness.tracker;
@@ -497,6 +533,114 @@ void TestCrossRegionUpload() {
   Release(page_manager, memory, region_size * 2);
 }
 
+void TestUploadDoesNotSerializeDisjointRegion() {
+  constexpr auto region_size = Libs::Graphics::TRACKER_REGION_SIZE;
+  constexpr auto page_size = Libs::Graphics::TRACKER_PAGE_SIZE;
+  TrackerHarness harness;
+  auto &tracker = harness.tracker;
+  auto &page_manager = harness.page_manager;
+  auto *memory = Allocate(page_manager, region_size * 2 / page_size);
+  const auto allocation_base = reinterpret_cast<uint64_t>(memory);
+  const auto second_region =
+      (allocation_base & ~(region_size - 1)) + region_size;
+  Check(second_region + page_size <= allocation_base + region_size * 2,
+        "test allocation does not span two tracker regions");
+
+  // Publish both managers before the concurrent section so this test measures
+  // tracker access serialization rather than manager allocation.
+  Check(tracker.IsRegionCpuModified(allocation_base, page_size) &&
+            tracker.IsRegionCpuModified(second_region, page_size),
+        "disjoint upload setup did not initialize both regions");
+
+  std::binary_semaphore upload_entered{0};
+  std::binary_semaphore finish_upload{0};
+  std::binary_semaphore query_finished{0};
+  std::atomic_bool query_result{false};
+  std::jthread uploader([&] {
+    tracker.ForEachUploadRange(
+        allocation_base, page_size, true, [](uint64_t, uint64_t) noexcept {},
+        [&]() noexcept {
+          upload_entered.release();
+          finish_upload.acquire();
+        });
+  });
+  upload_entered.acquire();
+  std::jthread query([&] {
+    query_result.store(tracker.IsRegionCpuModified(second_region, page_size),
+                       std::memory_order_relaxed);
+    query_finished.release();
+  });
+
+  const bool completed_while_upload_blocked =
+      query_finished.try_acquire_for(std::chrono::seconds(5));
+  finish_upload.release();
+  uploader.join();
+  query.join();
+
+  tracker.UnmarkRegionAsGpuModified(allocation_base, page_size);
+  tracker.MarkRegionAsCpuModified(allocation_base, page_size);
+  tracker.UntrackMemory(allocation_base, region_size * 2);
+  Release(page_manager, memory, region_size * 2);
+  Check(completed_while_upload_blocked &&
+            query_result.load(std::memory_order_relaxed),
+        "upload callback serialized an unrelated tracker region");
+}
+
+void TestDownloadDoesNotSerializeDisjointRegion() {
+  constexpr auto region_size = Libs::Graphics::TRACKER_REGION_SIZE;
+  constexpr auto page_size = Libs::Graphics::TRACKER_PAGE_SIZE;
+  TrackerHarness harness;
+  auto &tracker = harness.tracker;
+  auto &page_manager = harness.page_manager;
+  auto *memory = Allocate(page_manager, region_size * 2 / page_size);
+  const auto allocation_base = reinterpret_cast<uint64_t>(memory);
+  const auto second_region =
+      (allocation_base & ~(region_size - 1)) + region_size;
+  Check(second_region + page_size <= allocation_base + region_size * 2,
+        "test allocation does not span two tracker regions");
+
+  tracker.ForEachUploadRange(
+      allocation_base, page_size, true, [](uint64_t, uint64_t) noexcept {},
+      []() noexcept {});
+  tracker.ForEachUploadRange(
+      second_region, page_size, false, [](uint64_t, uint64_t) noexcept {},
+      []() noexcept {});
+
+  std::binary_semaphore download_entered{0};
+  std::binary_semaphore finish_download{0};
+  std::binary_semaphore mutation_finished{0};
+  std::jthread downloader([&] {
+    tracker.ForEachDownloadRange<false>(allocation_base, page_size,
+                                        [&](uint64_t, uint64_t) noexcept {
+                                          download_entered.release();
+                                          finish_download.acquire();
+                                        });
+  });
+  download_entered.acquire();
+  std::jthread mutation([&] {
+    tracker.MarkRegionAsGpuModified(second_region, page_size);
+    mutation_finished.release();
+  });
+
+  const bool completed_while_download_blocked =
+      mutation_finished.try_acquire_for(std::chrono::seconds(5));
+  finish_download.release();
+  downloader.join();
+  mutation.join();
+
+  const bool both_gpu_owned =
+      tracker.IsRegionGpuModified(allocation_base, page_size) &&
+      tracker.IsRegionGpuModified(second_region, page_size);
+  tracker.UnmarkRegionAsGpuModified(allocation_base, page_size);
+  tracker.UnmarkRegionAsGpuModified(second_region, page_size);
+  tracker.MarkRegionAsCpuModified(allocation_base, page_size);
+  tracker.MarkRegionAsCpuModified(second_region, page_size);
+  tracker.UntrackMemory(allocation_base, region_size * 2);
+  Release(page_manager, memory, region_size * 2);
+  Check(completed_while_download_blocked && both_gpu_owned,
+        "download callback serialized an unrelated tracker region");
+}
+
 void TestGpuUnmarkUsesRegionMask() {
   constexpr auto region_size = Libs::Graphics::TRACKER_REGION_SIZE;
   constexpr auto page_size = Libs::Graphics::TRACKER_PAGE_SIZE;
@@ -683,6 +827,7 @@ int main(int argc, char **argv) {
   }
   TestRangeSet();
   TestQueriesDoNotRequireMappedOwnership();
+  TestConcurrentRegionPublication();
   TestCpuDirtyUpload();
   TestRangeInvalidation();
   TestGpuReacquisitionAfterInvalidation();
@@ -690,6 +835,8 @@ int main(int argc, char **argv) {
   TestExactDirtyIntervalsSharingTrackerPage();
   TestGpuDownloadProtectionMirrors();
   TestCrossRegionUpload();
+  TestUploadDoesNotSerializeDisjointRegion();
+  TestDownloadDoesNotSerializeDisjointRegion();
   TestGpuUnmarkUsesRegionMask();
   TestFullRegionGpuUnmarkBatching();
   TestFatalPaths();