From f8253647709b3bc63ea3f4abfc1fe419b67dea63 Mon Sep 17 00:00:00 2001 From: Lukasz Jobczyk Date: Wed, 14 Oct 2020 09:39:02 +0200 Subject: [PATCH] Unprotect memory after migration to CPU Change-Id: I280296422ec30583752a0b62c7c1c8777aa84c32 Signed-off-by: Lukasz Jobczyk --- .../cpu_page_fault_manager.cpp | 7 +-- .../cpu_page_fault_manager.h | 2 - .../linux/cpu_page_fault_manager_linux.cpp | 45 +----------------- .../linux/cpu_page_fault_manager_linux.h | 3 -- .../cpu_page_fault_manager_windows.cpp | 2 - .../windows/cpu_page_fault_manager_windows.h | 1 - .../cpu_page_fault_manager_linux_tests.cpp | 47 ------------------- .../mock_cpu_page_fault_manager.h | 1 - 8 files changed, 2 insertions(+), 106 deletions(-) diff --git a/shared/source/page_fault_manager/cpu_page_fault_manager.cpp b/shared/source/page_fault_manager/cpu_page_fault_manager.cpp index dce20ee39a..10b52a3a8e 100644 --- a/shared/source/page_fault_manager/cpu_page_fault_manager.cpp +++ b/shared/source/page_fault_manager/cpu_page_fault_manager.cpp @@ -76,13 +76,12 @@ bool PageFaultManager::verifyPageFault(void *ptr) { auto allocPtr = alloc.first; auto &pageFaultData = alloc.second; if (ptr >= allocPtr && ptr < ptrOffset(allocPtr, pageFaultData.size)) { - this->broadcastWaitSignal(); - this->allowCPUMemoryAccess(allocPtr, pageFaultData.size); this->setAubWritable(true, allocPtr, pageFaultData.unifiedMemoryManager); if (pageFaultData.domain == AllocationDomain::Gpu) { this->transferToCpu(allocPtr, pageFaultData.size, pageFaultData.cmdQ); } pageFaultData.domain = AllocationDomain::Cpu; + this->allowCPUMemoryAccess(allocPtr, pageFaultData.size); return true; } } @@ -95,8 +94,4 @@ void PageFaultManager::setAubWritable(bool writable, void *ptr, SVMAllocsManager gpuAlloc->setAubWritable(writable, GraphicsAllocation::allBanks); } -void PageFaultManager::waitForCopy() { - std::unique_lock lock{mtx}; -} - } // namespace NEO diff --git a/shared/source/page_fault_manager/cpu_page_fault_manager.h b/shared/source/page_fault_manager/cpu_page_fault_manager.h index 4140584dcc..10ae12b114 100644 --- a/shared/source/page_fault_manager/cpu_page_fault_manager.h +++ b/shared/source/page_fault_manager/cpu_page_fault_manager.h @@ -49,8 +49,6 @@ class PageFaultManager : public NonCopyableOrMovableClass { virtual void protectCPUMemoryAccess(void *ptr, size_t size) = 0; virtual void evictMemoryAfterImplCopy(GraphicsAllocation *allocation, Device *device) = 0; - virtual void broadcastWaitSignal() = 0; - MOCKABLE_VIRTUAL void waitForCopy(); MOCKABLE_VIRTUAL bool verifyPageFault(void *ptr); MOCKABLE_VIRTUAL void transferToCpu(void *ptr, size_t size, void *cmdQ); diff --git a/shared/source/page_fault_manager/linux/cpu_page_fault_manager_linux.cpp b/shared/source/page_fault_manager/linux/cpu_page_fault_manager_linux.cpp index 2bab71ffe7..7b142b9bbd 100644 --- a/shared/source/page_fault_manager/linux/cpu_page_fault_manager_linux.cpp +++ b/shared/source/page_fault_manager/linux/cpu_page_fault_manager_linux.cpp @@ -12,11 +12,7 @@ #include "shared/source/helpers/debug_helpers.h" #include "shared/source/memory_manager/memory_operations_handler.h" -#include #include -#include -#include -#include namespace NEO { std::unique_ptr PageFaultManager::create() { @@ -27,9 +23,7 @@ std::function PageFaultManager PageFaultManagerLinux::PageFaultManagerLinux() { pageFaultHandler = [&](int signal, siginfo_t *info, void *context) { - if (signal == SIGUSR1) { - this->waitForCopy(); - } else if (!this->verifyPageFault(info->si_addr)) { + if (!this->verifyPageFault(info->si_addr)) { callPreviousHandler(signal, info, context); } }; @@ -41,9 +35,6 @@ PageFaultManagerLinux::PageFaultManagerLinux() { auto retVal = sigaction(SIGSEGV, &pageFaultManagerHandler, &previousPageFaultHandler); UNRECOVERABLE_IF(retVal != 0); - retVal = sigaction(SIGUSR1, &pageFaultManagerHandler, &previousUserSignalHandler); - UNRECOVERABLE_IF(retVal != 0); - this->evictMemoryAfterCopy = DebugManager.flags.EnableDirectSubmission.get() && DebugManager.flags.USMEvictAfterMigration.get(); } @@ -52,9 +43,6 @@ PageFaultManagerLinux::~PageFaultManagerLinux() { if (!previousHandlerRestored) { auto retVal = sigaction(SIGSEGV, &previousPageFaultHandler, nullptr); UNRECOVERABLE_IF(retVal != 0); - - retVal = sigaction(SIGUSR1, &previousUserSignalHandler, nullptr); - UNRECOVERABLE_IF(retVal != 0); } } @@ -88,37 +76,6 @@ void PageFaultManagerLinux::callPreviousHandler(int signal, siginfo_t *info, voi } } -/* This function is a WA for USM issue in multithreaded environment - While handling page fault, before copy starts, user signal (SIGUSR1) - is broadcasted to ensure that every thread received signal and is - stucked on PageFaultHandler's mutex before copy from GPU to CPU proceeds. */ -void PageFaultManagerLinux::broadcastWaitSignal() { - auto selfThreadId = syscall(__NR_gettid); - - auto procDir = opendir("/proc/self/task"); - UNRECOVERABLE_IF(!procDir); - - struct dirent *dirEntry; - while ((dirEntry = readdir(procDir)) != NULL) { - if (dirEntry->d_name[0] == '.') { - continue; - } - - int threadId = atoi(dirEntry->d_name); - if (threadId == selfThreadId) { - continue; - } - - sendSignalToThread(threadId); - } - - closedir(procDir); -} - -void PageFaultManagerLinux::sendSignalToThread(int threadId) { - syscall(SYS_tkill, threadId, SIGUSR1); -} - void PageFaultManagerLinux::evictMemoryAfterImplCopy(GraphicsAllocation *allocation, Device *device) { if (evictMemoryAfterCopy) { device->getRootDeviceEnvironment().memoryOperationsInterface->evict(device, *allocation); diff --git a/shared/source/page_fault_manager/linux/cpu_page_fault_manager_linux.h b/shared/source/page_fault_manager/linux/cpu_page_fault_manager_linux.h index 77c522e14f..1ec5fcbf20 100644 --- a/shared/source/page_fault_manager/linux/cpu_page_fault_manager_linux.h +++ b/shared/source/page_fault_manager/linux/cpu_page_fault_manager_linux.h @@ -25,8 +25,6 @@ class PageFaultManagerLinux : public PageFaultManager { void protectCPUMemoryAccess(void *ptr, size_t size) override; void evictMemoryAfterImplCopy(GraphicsAllocation *allocation, Device *device) override; - void broadcastWaitSignal() override; - MOCKABLE_VIRTUAL void sendSignalToThread(int threadId); void callPreviousHandler(int signal, siginfo_t *info, void *context); bool previousHandlerRestored = false; @@ -34,7 +32,6 @@ class PageFaultManagerLinux : public PageFaultManager { static std::function pageFaultHandler; struct sigaction previousPageFaultHandler = {}; - struct sigaction previousUserSignalHandler = {}; bool evictMemoryAfterCopy = false; }; diff --git a/shared/source/page_fault_manager/windows/cpu_page_fault_manager_windows.cpp b/shared/source/page_fault_manager/windows/cpu_page_fault_manager_windows.cpp index 601f020cf9..d91a5a929e 100644 --- a/shared/source/page_fault_manager/windows/cpu_page_fault_manager_windows.cpp +++ b/shared/source/page_fault_manager/windows/cpu_page_fault_manager_windows.cpp @@ -53,6 +53,4 @@ void PageFaultManagerWindows::protectCPUMemoryAccess(void *ptr, size_t size) { void PageFaultManagerWindows::evictMemoryAfterImplCopy(GraphicsAllocation *allocation, Device *device) {} -void PageFaultManagerWindows::broadcastWaitSignal() {} - } // namespace NEO diff --git a/shared/source/page_fault_manager/windows/cpu_page_fault_manager_windows.h b/shared/source/page_fault_manager/windows/cpu_page_fault_manager_windows.h index 1db51e89da..64efd06c29 100644 --- a/shared/source/page_fault_manager/windows/cpu_page_fault_manager_windows.h +++ b/shared/source/page_fault_manager/windows/cpu_page_fault_manager_windows.h @@ -26,7 +26,6 @@ class PageFaultManagerWindows : public PageFaultManager { void protectCPUMemoryAccess(void *ptr, size_t size) override; void evictMemoryAfterImplCopy(GraphicsAllocation *allocation, Device *device) override; - void broadcastWaitSignal() override; static std::function pageFaultHandler; PVOID previousHandler; diff --git a/shared/test/unit_test/page_fault_manager/linux/cpu_page_fault_manager_linux_tests.cpp b/shared/test/unit_test/page_fault_manager/linux/cpu_page_fault_manager_linux_tests.cpp index 07e893a8e9..4059af006a 100644 --- a/shared/test/unit_test/page_fault_manager/linux/cpu_page_fault_manager_linux_tests.cpp +++ b/shared/test/unit_test/page_fault_manager/linux/cpu_page_fault_manager_linux_tests.cpp @@ -18,59 +18,12 @@ #include #include -#include -#include -#include using namespace NEO; using PageFaultManagerLinuxTest = PageFaultManagerConfigFixture; using MockPageFaultManagerLinux = MockPageFaultManagerHandlerInvoke; -struct UserSignalMockPageFaultManagerLinux : public PageFaultManagerLinux { - using PageFaultManager::verifyPageFault; - - UserSignalMockPageFaultManagerLinux() { - ownThread = std::thread([&]() { - while (!waitForCopyCalled) { - if (ownThreadId == -1) { - ownThreadId = static_cast(syscall(__NR_gettid)); - } - } - }); - while (ownThreadId == -1) { - } - } - - void allowCPUMemoryAccess(void *ptr, size_t size) override {} - void transferToCpu(void *ptr, size_t size, void *cmdQ) override {} - void setAubWritable(bool writable, void *ptr, SVMAllocsManager *unifiedMemoryManager) override {} - - void sendSignalToThread(int threadId) override { - PageFaultManagerLinux::sendSignalToThread(ownThreadId); - } - - void waitForCopy() override { - PageFaultManagerLinux::waitForCopy(); - waitForCopyCalled = true; - } - - std::thread ownThread; - int ownThreadId = -1; - bool waitForCopyCalled = false; -}; - -TEST_F(PageFaultManagerLinuxTest, whenVeryfyingPageFaultThenUserSignalIsSentToOtherThreads) { - auto pageFaultManager = std::make_unique(); - - auto alloc = reinterpret_cast(0x1); - pageFaultManager->insertAllocation(alloc, 10, nullptr, nullptr, {}); - pageFaultManager->verifyPageFault(alloc); - pageFaultManager->ownThread.join(); - - EXPECT_TRUE(pageFaultManager->waitForCopyCalled); -} - TEST_F(PageFaultManagerLinuxTest, whenPageFaultIsRaisedThenHandlerIsInvoked) { auto pageFaultManager = std::make_unique(); EXPECT_FALSE(pageFaultManager->handlerInvoked); diff --git a/shared/test/unit_test/page_fault_manager/mock_cpu_page_fault_manager.h b/shared/test/unit_test/page_fault_manager/mock_cpu_page_fault_manager.h index 401da6641e..a5862d38fb 100644 --- a/shared/test/unit_test/page_fault_manager/mock_cpu_page_fault_manager.h +++ b/shared/test/unit_test/page_fault_manager/mock_cpu_page_fault_manager.h @@ -49,7 +49,6 @@ class MockPageFaultManager : public PageFaultManager { void baseGpuTransfer(void *ptr, void *cmdQ) { PageFaultManager::transferToGpu(ptr, cmdQ); } - void broadcastWaitSignal() override {} void evictMemoryAfterImplCopy(GraphicsAllocation *allocation, Device *device) override {} int allowMemoryAccessCalled = 0;