From c4e802f01ba2bf39be2e3bff2122412dfabebdab Mon Sep 17 00:00:00 2001 From: Jaroslaw Chodor Date: Fri, 19 Nov 2021 22:58:46 +0100 Subject: [PATCH] WSL - fixing resource cleanup on process shutdown Resolves issues with coexistance of NEO L0 and NEO OCL libraries within a single process running in WSL and using WDDM GPU PV Signed-off-by: Jaroslaw Chodor --- .../os_interface/linux/drm_tests.cpp | 8 +++ .../command_stream_receiver.cpp | 8 +++ .../command_stream/command_stream_receiver.h | 2 + shared/source/device/device.cpp | 9 ++- shared/source/os_interface/os_interface.h | 4 ++ .../os_interface/windows/CMakeLists.txt | 1 + .../os_interface/windows/os_context_win.cpp | 2 +- ...igure_device_address_space_drm_or_wddm.cpp | 2 +- .../skip_resource_cleanup_drm_or_wddm.cpp | 23 +++++++ .../wddm/skip_resource_cleanup_wddm.cpp | 16 +++++ .../source/os_interface/windows/wddm/wddm.h | 1 + ..._device_address_space_drm_or_wddm_test.cpp | 65 ++++++++++++------- .../os_interface/windows/wddm_tests.cpp | 5 ++ 13 files changed, 120 insertions(+), 26 deletions(-) create mode 100644 shared/source/os_interface/windows/wddm/skip_resource_cleanup_drm_or_wddm.cpp create mode 100644 shared/source/os_interface/windows/wddm/skip_resource_cleanup_wddm.cpp diff --git a/opencl/test/unit_test/os_interface/linux/drm_tests.cpp b/opencl/test/unit_test/os_interface/linux/drm_tests.cpp index 70e4bc2c95..1b821c1345 100644 --- a/opencl/test/unit_test/os_interface/linux/drm_tests.cpp +++ b/opencl/test/unit_test/os_interface/linux/drm_tests.cpp @@ -875,3 +875,11 @@ TEST(DrmQueryTest, GivenRpsMaxFreqFileDoesntExistWhenFrequencyIsQueriedThenFallb EXPECT_EQ(expectedMaxFrequency, maxFrequency); } + +TEST(DrmTest, whenCheckedIfResourcesCleanupCanBeSkippedThenReturnsFalse) { + auto executionEnvironment = std::make_unique(); + executionEnvironment->prepareRootDeviceEnvironments(1); + DrmMock *pDrm = new DrmMock(*executionEnvironment->rootDeviceEnvironments[0]); + EXPECT_FALSE(pDrm->skipResourceCleanup()); + delete pDrm; +} diff --git a/shared/source/command_stream/command_stream_receiver.cpp b/shared/source/command_stream/command_stream_receiver.cpp index 4a5a4b8881..1b3baf7cc6 100644 --- a/shared/source/command_stream/command_stream_receiver.cpp +++ b/shared/source/command_stream/command_stream_receiver.cpp @@ -211,7 +211,15 @@ bool CommandStreamReceiver::isRcs() const { return this->osContext->getEngineType() == aub_stream::ENGINE_RCS; } +bool CommandStreamReceiver::skipResourceCleanup() const { + return this->getOSInterface() && this->getOSInterface()->getDriverModel() && this->getOSInterface()->getDriverModel()->skipResourceCleanup(); +} + void CommandStreamReceiver::cleanupResources() { + if (this->skipResourceCleanup()) { + return; + } + waitForTaskCountAndCleanAllocationList(this->latestFlushedTaskCount, TEMPORARY_ALLOCATION); waitForTaskCountAndCleanAllocationList(this->latestFlushedTaskCount, REUSABLE_ALLOCATION); diff --git a/shared/source/command_stream/command_stream_receiver.h b/shared/source/command_stream/command_stream_receiver.h index ae8cd2b49f..200c41f38e 100644 --- a/shared/source/command_stream/command_stream_receiver.h +++ b/shared/source/command_stream/command_stream_receiver.h @@ -288,6 +288,8 @@ class CommandStreamReceiver { return activePartitions; } + bool skipResourceCleanup() const; + std::unique_ptr pageTableManager; protected: diff --git a/shared/source/device/device.cpp b/shared/source/device/device.cpp index 2e935b098d..558c0f8162 100644 --- a/shared/source/device/device.cpp +++ b/shared/source/device/device.cpp @@ -36,10 +36,15 @@ Device::Device(ExecutionEnvironment *executionEnvironment) } Device::~Device() { - getMemoryManager()->freeGraphicsMemory(rtMemoryBackedBuffer); - rtMemoryBackedBuffer = nullptr; + if (false == commandStreamReceivers.empty()) { + if (commandStreamReceivers[0]->skipResourceCleanup()) { + return; + } + } DEBUG_BREAK_IF(nullptr == executionEnvironment->memoryManager.get()); + getMemoryManager()->freeGraphicsMemory(rtMemoryBackedBuffer); + rtMemoryBackedBuffer = nullptr; if (performanceCounters) { performanceCounters->shutdown(); diff --git a/shared/source/os_interface/os_interface.h b/shared/source/os_interface/os_interface.h index 45bbd820f9..1582ca0245 100644 --- a/shared/source/os_interface/os_interface.h +++ b/shared/source/os_interface/os_interface.h @@ -83,6 +83,10 @@ class DriverModel : public NonCopyableClass { return std::numeric_limits::max(); } + virtual bool skipResourceCleanup() const { + return false; + } + protected: DriverModelType driverModelType; }; diff --git a/shared/source/os_interface/windows/CMakeLists.txt b/shared/source/os_interface/windows/CMakeLists.txt index eee3a0ae8e..3d56eac74d 100644 --- a/shared/source/os_interface/windows/CMakeLists.txt +++ b/shared/source/os_interface/windows/CMakeLists.txt @@ -87,6 +87,7 @@ set(NEO_CORE_OS_INTERFACE_WDDM ${CMAKE_CURRENT_SOURCE_DIR}/wddm/set_gmm_input_args_${DRIVER_MODEL}.cpp ${CMAKE_CURRENT_SOURCE_DIR}/wddm/max_mem_alloc_size_${DRIVER_MODEL}.cpp ${CMAKE_CURRENT_SOURCE_DIR}/wddm/helper_${DRIVER_MODEL}.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/wddm/skip_resource_cleanup_${DRIVER_MODEL}.cpp ${CMAKE_CURRENT_SOURCE_DIR}/wddm/wddm.cpp ${CMAKE_CURRENT_SOURCE_DIR}/wddm/wddm.h ${CMAKE_CURRENT_SOURCE_DIR}/wddm/wddm_defs.h diff --git a/shared/source/os_interface/windows/os_context_win.cpp b/shared/source/os_interface/windows/os_context_win.cpp index c8a24c64af..29f4de42d0 100644 --- a/shared/source/os_interface/windows/os_context_win.cpp +++ b/shared/source/os_interface/windows/os_context_win.cpp @@ -39,7 +39,7 @@ void OsContextWin::initializeContext() { }; OsContextWin::~OsContextWin() { - if (contextInitialized) { + if (contextInitialized && (false == this->wddm.skipResourceCleanup())) { wddm.getWddmInterface()->destroyHwQueue(hardwareQueue.handle); wddm.getWddmInterface()->destroyMonitorFence(residencyController.getMonitoredFence()); wddm.destroyContext(wddmContextHandle); diff --git a/shared/source/os_interface/windows/wddm/configure_device_address_space_drm_or_wddm.cpp b/shared/source/os_interface/windows/wddm/configure_device_address_space_drm_or_wddm.cpp index 6dfb3176ed..0ca7136af5 100644 --- a/shared/source/os_interface/windows/wddm/configure_device_address_space_drm_or_wddm.cpp +++ b/shared/source/os_interface/windows/wddm/configure_device_address_space_drm_or_wddm.cpp @@ -159,7 +159,7 @@ bool ensureGpuAddressRangeIsReserved(uint64_t address, size_t size, D3DKMT_HANDL rangeDesc.MaximumAddress = alignUp(address + size, MemoryConstants::pageSize64k); rangeDesc.Size = MemoryConstants::pageSize64k; status = gdi.reserveGpuVirtualAddress(&rangeDesc); - if (status != STATUS_SUCCESS) { + if (status == STATUS_SUCCESS) { DEBUG_BREAK_IF(true); return false; } diff --git a/shared/source/os_interface/windows/wddm/skip_resource_cleanup_drm_or_wddm.cpp b/shared/source/os_interface/windows/wddm/skip_resource_cleanup_drm_or_wddm.cpp new file mode 100644 index 0000000000..4483acbc6a --- /dev/null +++ b/shared/source/os_interface/windows/wddm/skip_resource_cleanup_drm_or_wddm.cpp @@ -0,0 +1,23 @@ +/* + * Copyright (C) 2021 Intel Corporation + * + * SPDX-License-Identifier: MIT + * + */ + +#include "shared/source/os_interface/windows/gdi_interface.h" +#include "shared/source/os_interface/windows/wddm/wddm.h" + +namespace NEO { + +bool Wddm::skipResourceCleanup() const { + D3DKMT_GETDEVICESTATE deviceState = {}; + deviceState.hDevice = device; + deviceState.StateType = D3DKMT_DEVICESTATE_PRESENT; + + NTSTATUS status = STATUS_SUCCESS; + status = getGdi()->getDeviceState(&deviceState); + return status != STATUS_SUCCESS; +} + +} // namespace NEO diff --git a/shared/source/os_interface/windows/wddm/skip_resource_cleanup_wddm.cpp b/shared/source/os_interface/windows/wddm/skip_resource_cleanup_wddm.cpp new file mode 100644 index 0000000000..701e35df18 --- /dev/null +++ b/shared/source/os_interface/windows/wddm/skip_resource_cleanup_wddm.cpp @@ -0,0 +1,16 @@ +/* + * Copyright (C) 2021 Intel Corporation + * + * SPDX-License-Identifier: MIT + * + */ + +#include "shared/source/os_interface/windows/wddm/wddm.h" + +namespace NEO { + +bool Wddm::skipResourceCleanup() const { + return false; +} + +} // namespace NEO diff --git a/shared/source/os_interface/windows/wddm/wddm.h b/shared/source/os_interface/windows/wddm/wddm.h index fe519638eb..abd0dc9c5e 100644 --- a/shared/source/os_interface/windows/wddm/wddm.h +++ b/shared/source/os_interface/windows/wddm/wddm.h @@ -189,6 +189,7 @@ class Wddm : public DriverModel { PhysicalDevicePciBusInfo getPciBusInfo() const override; size_t getMaxMemAllocSize() const override; + bool skipResourceCleanup() const override; static std::vector> discoverDevices(ExecutionEnvironment &executionEnvironment); diff --git a/shared/test/unit_test/os_interface/wddm_linux/configure_device_address_space_drm_or_wddm_test.cpp b/shared/test/unit_test/os_interface/wddm_linux/configure_device_address_space_drm_or_wddm_test.cpp index 251da24490..14d52e6dc5 100644 --- a/shared/test/unit_test/os_interface/wddm_linux/configure_device_address_space_drm_or_wddm_test.cpp +++ b/shared/test/unit_test/os_interface/wddm_linux/configure_device_address_space_drm_or_wddm_test.cpp @@ -118,6 +118,9 @@ struct GdiMockConfig { D3DKMT_LOCK2 receivedLock2Args = {}; D3DKMT_DESTROYALLOCATION2 receivedDestroyAllocation2Args = {}; uint32_t mockAllocationHandle = 7U; + + D3DKMT_GETDEVICESTATE receivedGetDeviceStateArgs = {}; + GdiMockCallbackWithReturn getDeviceStateClb = {}; } gdiMockConfig; NTSTATUS __stdcall reserveDeviceAddressSpaceMock(D3DDDI_RESERVEGPUVIRTUALADDRESS *arg) { @@ -185,6 +188,15 @@ NTSTATUS __stdcall escapeMock(const D3DKMT_ESCAPE *arg) { return gdiMockConfig.escapeClb.returnValue; } +NTSTATUS __stdcall getDeviceStateMock(D3DKMT_GETDEVICESTATE *arg) { + gdiMockConfig.receivedGetDeviceStateArgs = *arg; + gdiMockConfig.getDeviceStateClb.callCount += 1; + if (gdiMockConfig.getDeviceStateClb.callback) { + gdiMockConfig.getDeviceStateClb.callback(); + } + return gdiMockConfig.getDeviceStateClb.returnValue; +} + struct WddmLinuxTest : public ::testing::Test { void SetUp() override { mockRootDeviceEnvironment = std::make_unique(mockExecEnv); @@ -248,9 +260,16 @@ TEST_F(WddmLinuxConfigureDeviceAddressSpaceTest, givenPreReservedSvmAddressSpace } this->osEnvironment->gdi->reserveGpuVirtualAddress = reserveDeviceAddressSpaceMock; - gdiMockConfig.reserveGpuVaClb.returnValue = -2; - gdiMockConfig.reserveGpuVaClb.callback = []() { gdiMockConfig.reserveGpuVaClb.returnValue += 1; }; + auto cantReserveWholeGpuVAButCanReservePortion = []() { + gdiMockConfig.reserveGpuVaClb.returnValue = (gdiMockConfig.reserveGpuVaClb.callCount == 1) ? -1 : 0; + }; + gdiMockConfig.reserveGpuVaClb.callback = cantReserveWholeGpuVAButCanReservePortion; bool success = this->wddm->configureDeviceAddressSpace(); + EXPECT_FALSE(success); + + auto cantReserveWholeGpuVAAndCantReservePortion = []() { gdiMockConfig.reserveGpuVaClb.returnValue = -1; }; + gdiMockConfig.reserveGpuVaClb.callback = cantReserveWholeGpuVAAndCantReservePortion; + success = this->wddm->configureDeviceAddressSpace(); EXPECT_TRUE(success); auto svmSize = this->getMaxSvmSize(); @@ -261,19 +280,6 @@ TEST_F(WddmLinuxConfigureDeviceAddressSpaceTest, givenPreReservedSvmAddressSpace EXPECT_EQ(wddm->getAdapter(), gdiMockConfig.receivedReserveGpuVaArgs.hAdapter); } -TEST_F(WddmLinuxConfigureDeviceAddressSpaceTest, givenSvmAddressSpaceWhenCouldNotReserveGpuVAForSvmThenFail) { - if (NEO::hardwareInfoTable[productFamily]->capabilityTable.gpuAddressSpace < MemoryConstants::max64BitAppAddress) { - GTEST_SKIP(); - } - - osEnvironment->gdi->reserveGpuVirtualAddress = reserveDeviceAddressSpaceMock; - this->osEnvironment->gdi->reserveGpuVirtualAddress = reserveDeviceAddressSpaceMock; - - gdiMockConfig.reserveGpuVaClb.returnValue = -1; - bool success = this->wddm->configureDeviceAddressSpace(); - EXPECT_FALSE(success); -} - TEST_F(WddmLinuxConfigureDeviceAddressSpaceTest, givenNonSvmAddressSpaceThenReserveGpuVAForUSMIsNotCalled) { if (NEO::hardwareInfoTable[productFamily]->capabilityTable.gpuAddressSpace >= MemoryConstants::max64BitAppAddress) { GTEST_SKIP(); @@ -531,10 +537,13 @@ TEST_F(WddmLinuxConfigureReduced48bitDeviceAddressSpaceTest, givenTwoSvmAddressS } this->wddm->featureTable->ftrCCSRing = 1; - gdiMockConfig.reserveGpuVaClb.returnValue = -1; osEnvironment->gdi->escape = escapeMock; osEnvironment->gdi->reserveGpuVirtualAddress = reserveDeviceAddressSpaceMock; + auto cantReserveWholeGpuVAButCanReservePortion = []() { + gdiMockConfig.reserveGpuVaClb.returnValue = (gdiMockConfig.reserveGpuVaClb.callCount == 1) ? -1 : 0; + }; + gdiMockConfig.reserveGpuVaClb.callback = cantReserveWholeGpuVAButCanReservePortion; bool success = this->wddm->configureDeviceAddressSpace(); EXPECT_FALSE(success); EXPECT_EQ(1U, this->wddm->validAddressRangeReservations.size()); @@ -549,14 +558,12 @@ TEST_F(WddmLinuxConfigureReduced48bitDeviceAddressSpaceTest, givenTwoSvmAddressS } this->wddm->featureTable->ftrCCSRing = 1; - gdiMockConfig.reserveGpuVaClb.callback = []() { - if (gdiMockConfig.reserveGpuVaClb.callCount > 1) { - gdiMockConfig.reserveGpuVaClb.returnValue = -1; - }; - }; - osEnvironment->gdi->escape = escapeMock; osEnvironment->gdi->reserveGpuVirtualAddress = reserveDeviceAddressSpaceMock; + auto cantReserveWholeGpuVAOfSecondButCanReservePortionOfSecont = []() { + gdiMockConfig.reserveGpuVaClb.returnValue = (gdiMockConfig.reserveGpuVaClb.callCount == 2) ? -1 : 0; + }; + gdiMockConfig.reserveGpuVaClb.callback = cantReserveWholeGpuVAOfSecondButCanReservePortionOfSecont; bool success = this->wddm->configureDeviceAddressSpace(); EXPECT_FALSE(success); EXPECT_EQ(1U, this->wddm->validAddressRangeReservations.size()); @@ -605,6 +612,20 @@ TEST_F(WddmLinuxTest, givenRequestFor32bitAllocationWithoutPreexistingHostPtrWhe memoryManager.freeGraphicsMemoryImpl(alloc); } +TEST_F(WddmLinuxTest, whenCheckedIfResourcesCleanupCanBeSkippedAndDeviceIsAliveThenReturnsFalse) { + osEnvironment->gdi->getDeviceState = getDeviceStateMock; + gdiMockConfig.getDeviceStateClb.returnValue = STATUS_SUCCESS; + EXPECT_FALSE(this->wddm->skipResourceCleanup()); + EXPECT_EQ(1, gdiMockConfig.getDeviceStateClb.callCount); +} + +TEST_F(WddmLinuxTest, whenCheckedIfResourcesCleanupCanBeSkippedAndDeviceIsLostThenReturnsTrue) { + osEnvironment->gdi->getDeviceState = getDeviceStateMock; + gdiMockConfig.getDeviceStateClb.returnValue = -1; + EXPECT_TRUE(this->wddm->skipResourceCleanup()); + EXPECT_EQ(1, gdiMockConfig.getDeviceStateClb.callCount); +} + class MockOsTimeLinux : public NEO::OSTimeLinux { public: MockOsTimeLinux(NEO::OSInterface *osInterface, std::unique_ptr deviceTime) : NEO::OSTimeLinux(osInterface, std::move(deviceTime)) {} diff --git a/shared/test/unit_test/os_interface/windows/wddm_tests.cpp b/shared/test/unit_test/os_interface/windows/wddm_tests.cpp index fff9e2dd10..1adddfd37f 100644 --- a/shared/test/unit_test/os_interface/windows/wddm_tests.cpp +++ b/shared/test/unit_test/os_interface/windows/wddm_tests.cpp @@ -70,4 +70,9 @@ TEST_F(WddmTests, givenWddmWhenPassesIncorrectHandleToVerifyNTHandleThenReturnFa EXPECT_FALSE(wddm->verifyNTHandle(handle)); } +TEST_F(WddmTests, whenCheckedIfResourcesCleanupCanBeSkippedThenReturnsFalse) { + init(); + EXPECT_FALSE(wddm->skipResourceCleanup()); +} + } // namespace NEO