diff --git a/src/graphics/host_gpu/regionManager.h b/src/graphics/host_gpu/regionManager.h index 375210ac9..4c95cc2c3 100644 --- a/src/graphics/host_gpu/regionManager.h +++ b/src/graphics/host_gpu/regionManager.h @@ -114,6 +114,9 @@ public: void ForEachModifiedRange(uint64_t vaddr, uint64_t size, Func&& func) { const auto [start, end] = GetPageRange(vaddr, size); auto& bits = GetBits(); + if (bits.FirstRangeFrom(start).first >= end) { + return; + } RegionBits mask(bits, start, end); if constexpr (clear) { bits.UnsetRange(start, end); diff --git a/tests/MemoryTrackerTests.cpp b/tests/MemoryTrackerTests.cpp index 007995ab4..98b6611e1 100644 --- a/tests/MemoryTrackerTests.cpp +++ b/tests/MemoryTrackerTests.cpp @@ -402,6 +402,100 @@ void TestCpuDirtyUpload() { Release(memory); } +void TestCleanUploadPreservesOwnership() { + constexpr auto page_size = Libs::Graphics::TRACKER_PAGE_SIZE; + constexpr auto region_size = Libs::Graphics::TRACKER_REGION_SIZE; + TrackerHarness harness; + auto &tracker = harness.tracker; + auto *memory = AllocateFixedGuestRange(region_size * 2, region_size); + const auto address = reinterpret_cast(memory); + tracker.ForEachUploadRange(address, region_size * 2, false, + [](uint64_t, uint64_t) noexcept {}, + []() noexcept {}); + + for (const auto start : {address + 63 * page_size, + address + region_size - page_size}) { + const auto before = start - page_size; + const auto after = start + 2 * page_size; + tracker.MarkRegionAsCpuModified(before, page_size); + tracker.MarkRegionAsCpuModified(after, page_size); + uint32_t ranges = 0; + uint32_t completions = 0; + ResetProtectionLog(); + tracker.ForEachUploadRange( + start, 2 * page_size, false, + [&](uint64_t, uint64_t) noexcept { ranges++; }, + [&]() noexcept { completions++; }); + Check(ranges == 0 && completions == 1 && g_protection_calls == 0 && + !tracker.IsRegionCpuModified(start, 2 * page_size), + "clean read-only upload changed dirty state or protection"); + + tracker.ForEachUploadRange( + start, 2 * page_size, true, + [&](uint64_t, uint64_t) noexcept { ranges++; }, + [&]() noexcept { completions++; }); + Check(ranges == 0 && completions == 2 && + tracker.IsRegionGpuModified(start, page_size) && + tracker.IsRegionGpuModified(start + page_size, page_size) && + Protection(reinterpret_cast(start)) == PAGE_NOACCESS && + Protection(reinterpret_cast(start + page_size)) == + PAGE_NOACCESS && + tracker.IsRegionCpuModified(before, page_size) && + tracker.IsRegionCpuModified(after, page_size) && + !tracker.IsRegionGpuModified(before, page_size) && + !tracker.IsRegionGpuModified(after, page_size) && + IsWritable(reinterpret_cast(before)) && + IsWritable(reinterpret_cast(after)), + "clean written upload lost GPU ownership or changed dirty neighbors"); + tracker.UnmarkRegionAsGpuModified(start, 2 * page_size); + } + tracker.UntrackMemory(address, region_size * 2); + Release(memory); +} + +void BenchmarkCleanUploads() { + constexpr uint64_t size = 256ull * 1024 * 1024; + TrackerHarness harness; + auto &tracker = harness.tracker; + auto *memory = AllocateFixedGuestRange(size, 0); + const auto address = reinterpret_cast(memory); + tracker.ForEachUploadRange(address, size, false, + [](uint64_t, uint64_t) noexcept {}, + []() noexcept {}); + std::puts("chunk_bytes,sweeps,calls,elapsed_ns,ns_per_sweep,ns_per_call"); + for (const uint64_t chunk : {size, Libs::Graphics::TRACKER_REGION_SIZE, + uint64_t{64 * 1024}}) { + uint64_t ranges = 0; + uint64_t completions = 0; + uint64_t sweeps = 0; + ResetProtectionLog(); + const auto start = std::chrono::steady_clock::now(); + std::chrono::nanoseconds elapsed{}; + do { + for (uint64_t offset = 0; offset < size; offset += chunk) { + tracker.ForEachUploadRange( + address + offset, chunk, false, + [&](uint64_t, uint64_t) noexcept { ranges++; }, + [&]() noexcept { completions++; }); + } + sweeps++; + elapsed = std::chrono::steady_clock::now() - start; + } while (elapsed < std::chrono::milliseconds(250)); + Check(ranges == 0 && completions == sweeps * (size / chunk) && + g_protection_calls == 0, + "clean upload benchmark changed ranges, completion, or protection"); + std::printf("%llu,%llu,%llu,%lld,%.2f,%.2f\n", + static_cast(chunk), + static_cast(sweeps), + static_cast(completions), + static_cast(elapsed.count()), + static_cast(elapsed.count()) / sweeps, + static_cast(elapsed.count()) / completions); + } + tracker.UntrackMemory(address, size); + Release(memory); +} + void TestRangeInvalidation() { TrackerHarness harness; auto &tracker = harness.tracker; @@ -1027,11 +1121,16 @@ int main(int argc, char **argv) { if (argc == 3 && std::strcmp(argv[1], "--death") == 0) { RunDeathCase(argv[2]); } + if (argc == 2 && std::strcmp(argv[1], "--benchmark-clean-upload") == 0) { + BenchmarkCleanUploads(); + return 0; + } TestGuestRange(); TestRangeSet(); TestQueriesDoNotRequireMappedOwnership(); TestConcurrentRegionPublication(); TestCpuDirtyUpload(); + TestCleanUploadPreservesOwnership(); TestRangeInvalidation(); TestGpuReacquisitionAfterInvalidation(); TestGpuDirtyBits();