From 2d151ec0feabc125e662e525a69bacfd9d8bd1b0 Mon Sep 17 00:00:00 2001 From: Mateusz Jablonski Date: Tue, 19 Jul 2022 17:13:51 +0000 Subject: [PATCH] Move Drm cleanup logic to separated method Related-To: NEO-6999 Signed-off-by: Mateusz Jablonski --- .../unit_test/linux/drm_null_device_tests.cpp | 2 +- opencl/test/unit_test/linux/drm_wrap.h | 9 +++-- .../test/unit_test/linux/main_linux_dll.cpp | 4 +-- shared/source/os_interface/CMakeLists.txt | 3 +- shared/source/os_interface/linux/drm_neo.cpp | 7 ++-- shared/source/os_interface/linux/drm_neo.h | 1 + shared/source/os_interface/os_interface.cpp | 33 +++++++++++++++++++ shared/source/os_interface/os_interface.h | 21 ++++-------- shared/test/common/libult/linux/drm_mock.cpp | 8 +++++ .../linux/device_command_stream_fixture.cpp | 1 + .../linux/device_command_stream_fixture.h | 1 + .../linux/sys_calls_linux_ult.cpp | 6 ---- .../os_interface/linux/sys_calls_linux_ult.h | 1 - .../linux/drm_query_prelim_tests.cpp | 4 +-- 14 files changed, 69 insertions(+), 32 deletions(-) create mode 100644 shared/source/os_interface/os_interface.cpp diff --git a/opencl/test/unit_test/linux/drm_null_device_tests.cpp b/opencl/test/unit_test/linux/drm_null_device_tests.cpp index 6580c69f01..f5a2283b0d 100644 --- a/opencl/test/unit_test/linux/drm_null_device_tests.cpp +++ b/opencl/test/unit_test/linux/drm_null_device_tests.cpp @@ -38,7 +38,7 @@ class DrmNullDeviceTestsFixture { void TearDown() { // NOLINT(readability-identifier-naming) } - std::unique_ptr drmNullDevice; + std::unique_ptr> drmNullDevice; ExecutionEnvironment executionEnvironment; protected: diff --git a/opencl/test/unit_test/linux/drm_wrap.h b/opencl/test/unit_test/linux/drm_wrap.h index c6b4678ffb..2a1c5f8784 100644 --- a/opencl/test/unit_test/linux/drm_wrap.h +++ b/opencl/test/unit_test/linux/drm_wrap.h @@ -12,6 +12,8 @@ #include "shared/source/os_interface/linux/hw_device_id.h" #include "shared/source/os_interface/os_interface.h" +#include + class DrmWrap : public NEO::Drm { public: using Drm::deviceId; @@ -19,10 +21,13 @@ class DrmWrap : public NEO::Drm { using Drm::queryDeviceIdAndRevision; using Drm::revisionId; using Drm::virtualMemoryIds; - static std::unique_ptr createDrm(RootDeviceEnvironment &rootDeviceEnvironment) { + static std::unique_ptr> createDrm(RootDeviceEnvironment &rootDeviceEnvironment) { auto hwDeviceIds = OSInterface::discoverDevices(rootDeviceEnvironment.executionEnvironment); if (!hwDeviceIds.empty()) { - return std::unique_ptr{static_cast(NEO::Drm::create(std::unique_ptr(hwDeviceIds[0].release()->as()), rootDeviceEnvironment))}; + return std::unique_ptr>{static_cast(NEO::Drm::create(std::unique_ptr(hwDeviceIds[0].release()->as()), rootDeviceEnvironment)), [](Drm *drm) { + drm->cleanup(); + delete drm; + }}; } return nullptr; } diff --git a/opencl/test/unit_test/linux/main_linux_dll.cpp b/opencl/test/unit_test/linux/main_linux_dll.cpp index 3ee54ac3d2..7c18c7f2a5 100644 --- a/opencl/test/unit_test/linux/main_linux_dll.cpp +++ b/opencl/test/unit_test/linux/main_linux_dll.cpp @@ -260,7 +260,7 @@ TEST_F(DrmSimpleTests, givenPrintIoctlTimesWhenCallIoctlThenStatisticsAreGathere auto executionEnvironment = std::make_unique(); executionEnvironment->prepareRootDeviceEnvironments(1); - auto drm = DrmWrap::createDrm(*executionEnvironment->rootDeviceEnvironments[0]).release(); + auto drm = DrmWrap::createDrm(*executionEnvironment->rootDeviceEnvironments[0]); DebugManagerStateRestore restorer; DebugManager.flags.PrintIoctlTimes.set(true); @@ -343,7 +343,7 @@ TEST_F(DrmSimpleTests, givenPrintIoctlTimesWhenCallIoctlThenStatisticsAreGathere ::testing::internal::CaptureStdout(); - delete drm; + drm.reset(); std::string output = ::testing::internal::GetCapturedStdout(); EXPECT_STRNE("", output.c_str()); diff --git a/shared/source/os_interface/CMakeLists.txt b/shared/source/os_interface/CMakeLists.txt index 289b67295e..6f8124dd89 100644 --- a/shared/source/os_interface/CMakeLists.txt +++ b/shared/source/os_interface/CMakeLists.txt @@ -1,5 +1,5 @@ # -# Copyright (C) 2019-2021 Intel Corporation +# Copyright (C) 2019-2022 Intel Corporation # # SPDX-License-Identifier: MIT # @@ -30,6 +30,7 @@ set(NEO_CORE_OS_INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/os_context.cpp ${CMAKE_CURRENT_SOURCE_DIR}/os_context.h ${CMAKE_CURRENT_SOURCE_DIR}/os_environment.h + ${CMAKE_CURRENT_SOURCE_DIR}/os_interface.cpp ${CMAKE_CURRENT_SOURCE_DIR}/os_interface.h ${CMAKE_CURRENT_SOURCE_DIR}/os_library.h ${CMAKE_CURRENT_SOURCE_DIR}/os_memory.cpp diff --git a/shared/source/os_interface/linux/drm_neo.cpp b/shared/source/os_interface/linux/drm_neo.cpp index a6174a58e2..ff355a0a4a 100644 --- a/shared/source/os_interface/linux/drm_neo.cpp +++ b/shared/source/os_interface/linux/drm_neo.cpp @@ -352,7 +352,7 @@ void Drm::destroyDrmContext(uint32_t drmContextId) { void Drm::destroyDrmVirtualMemory(uint32_t drmVmId) { GemVmControl ctl = {}; ctl.vmId = drmVmId; - auto ret = Drm::ioctl(DrmIoctl::GemVmDestroy, &ctl); + auto ret = ioctlHelper->ioctl(DrmIoctl::GemVmDestroy, &ctl); UNRECOVERABLE_IF(ret != 0); } @@ -711,8 +711,11 @@ PhysicalDevicePciBusInfo Drm::getPciBusInfo() const { return pciBusInfo; } -Drm::~Drm() { +void Drm::cleanup() { destroyVirtualMemoryAddressSpace(); +} + +Drm::~Drm() { this->printIoctlStatistics(); } diff --git a/shared/source/os_interface/linux/drm_neo.h b/shared/source/os_interface/linux/drm_neo.h index d67d9f1e5c..9a3d23f7ce 100644 --- a/shared/source/os_interface/linux/drm_neo.h +++ b/shared/source/os_interface/linux/drm_neo.h @@ -251,6 +251,7 @@ class Drm : public DriverModel { bool isVmBindPatIndexProgrammingSupported() const { return vmBindPatIndexProgrammingSupported; } MOCKABLE_VIRTUAL bool getDeviceMemoryMaxClockRateInMhz(uint32_t tileId, uint32_t &clkRate); MOCKABLE_VIRTUAL bool getDeviceMemoryPhysicalSizeInBytes(uint32_t tileId, uint64_t &physicalSize); + void cleanup() override; protected: Drm(std::unique_ptr &&hwDeviceIdIn, RootDeviceEnvironment &rootDeviceEnvironment); diff --git a/shared/source/os_interface/os_interface.cpp b/shared/source/os_interface/os_interface.cpp new file mode 100644 index 0000000000..2aad3fe674 --- /dev/null +++ b/shared/source/os_interface/os_interface.cpp @@ -0,0 +1,33 @@ +/* + * Copyright (C) 2022 Intel Corporation + * + * SPDX-License-Identifier: MIT + * + */ + +#include "shared/source/os_interface/os_interface.h" + +namespace NEO { + +OSInterface::~OSInterface() { + if (this->driverModel) { + this->driverModel->cleanup(); + } +} +DriverModel *OSInterface::getDriverModel() const { + return this->driverModel.get(); +}; + +void OSInterface::setDriverModel(std::unique_ptr driverModel) { + this->driverModel = std::move(driverModel); +}; + +bool OSInterface::are64kbPagesEnabled() { + return osEnabled64kbPages; +} + +static_assert(!std::is_move_constructible_v); +static_assert(!std::is_copy_constructible_v); +static_assert(!std::is_move_assignable_v); +static_assert(!std::is_copy_assignable_v); +} // namespace NEO diff --git a/shared/source/os_interface/os_interface.h b/shared/source/os_interface/os_interface.h index cd24a49ec3..7db722cc89 100644 --- a/shared/source/os_interface/os_interface.h +++ b/shared/source/os_interface/os_interface.h @@ -92,6 +92,8 @@ class DriverModel : public NonCopyableClass { return skipResourceCleanupVar; } + virtual void cleanup() {} + virtual bool isGpuHangDetected(OsContext &osContext) = 0; protected: @@ -101,21 +103,15 @@ class DriverModel : public NonCopyableClass { class OSInterface : public NonCopyableClass { public: - virtual ~OSInterface() = default; - DriverModel *getDriverModel() const { - return driverModel.get(); - }; + virtual ~OSInterface(); + DriverModel *getDriverModel() const; - void setDriverModel(std::unique_ptr driverModel) { - this->driverModel = std::move(driverModel); - }; + void setDriverModel(std::unique_ptr driverModel); MOCKABLE_VIRTUAL bool isDebugAttachAvailable() const; static bool osEnabled64kbPages; static bool osEnableLocalMemory; - static bool are64kbPagesEnabled() { - return osEnabled64kbPages; - } + static bool are64kbPagesEnabled(); static bool newResourceImplicitFlush; static bool gpuIdleImplicitFlush; static bool requiresSupportForWddmTrimNotification; @@ -126,9 +122,4 @@ class OSInterface : public NonCopyableClass { std::unique_ptr driverModel = nullptr; }; -static_assert(!std::is_move_constructible_v); -static_assert(!std::is_copy_constructible_v); -static_assert(!std::is_move_assignable_v); -static_assert(!std::is_copy_assignable_v); - } // namespace NEO diff --git a/shared/test/common/libult/linux/drm_mock.cpp b/shared/test/common/libult/linux/drm_mock.cpp index dc5b50b5f6..0ed7b3768e 100644 --- a/shared/test/common/libult/linux/drm_mock.cpp +++ b/shared/test/common/libult/linux/drm_mock.cpp @@ -117,6 +117,14 @@ int DrmMock::ioctl(DrmIoctl request, void *arg) { return storedRetValForVmCreate; } + if ((request == DrmIoctl::GemVmDestroy) && (arg != nullptr)) { + ioctlCount.gemVmDestroy++; + ioctlCallsCount--; + ioctlCount.total--; + + return 0; + } + if ((request == DrmIoctl::GemContextDestroy) && (arg != nullptr)) { ioctlCount.contextDestroy++; auto destroy = static_cast(arg); diff --git a/shared/test/common/os_interface/linux/device_command_stream_fixture.cpp b/shared/test/common/os_interface/linux/device_command_stream_fixture.cpp index ddedccdd79..72746c1dd8 100644 --- a/shared/test/common/os_interface/linux/device_command_stream_fixture.cpp +++ b/shared/test/common/os_interface/linux/device_command_stream_fixture.cpp @@ -24,6 +24,7 @@ void Ioctls::reset() { gemSetTiling = 0; gemGetTiling = 0; gemVmCreate = 0; + gemVmDestroy = 0; primeFdToHandle = 0; handleToPrimeFd = 0; gemMmapOffset = 0; diff --git a/shared/test/common/os_interface/linux/device_command_stream_fixture.h b/shared/test/common/os_interface/linux/device_command_stream_fixture.h index b9f4cf4846..12d1e66c98 100644 --- a/shared/test/common/os_interface/linux/device_command_stream_fixture.h +++ b/shared/test/common/os_interface/linux/device_command_stream_fixture.h @@ -41,6 +41,7 @@ class Ioctls { std::atomic gemSetTiling; std::atomic gemGetTiling; std::atomic gemVmCreate; + std::atomic gemVmDestroy; std::atomic primeFdToHandle; std::atomic handleToPrimeFd; std::atomic gemMmapOffset; diff --git a/shared/test/common/os_interface/linux/sys_calls_linux_ult.cpp b/shared/test/common/os_interface/linux/sys_calls_linux_ult.cpp index 4468937aa6..ba3d9e7c42 100644 --- a/shared/test/common/os_interface/linux/sys_calls_linux_ult.cpp +++ b/shared/test/common/os_interface/linux/sys_calls_linux_ult.cpp @@ -43,7 +43,6 @@ uint32_t mmapFuncCalled = 0u; uint32_t munmapFuncCalled = 0u; bool isInvalidAILTest = false; const char *drmVersion = "i915"; -uint32_t ioctlVmDestroyCalled = 0u; int (*sysCallsOpen)(const char *pathname, int flags) = nullptr; ssize_t (*sysCallsPread)(int fd, void *buf, size_t count, off_t offset) = nullptr; @@ -92,11 +91,6 @@ int ioctl(int fileDescriptor, unsigned long int request, void *arg) { memcpy_s(pVersion->name, pVersion->nameLen, drmVersion, std::min(pVersion->nameLen, strlen(drmVersion) + 1)); } } - if (request == DRM_IOCTL_I915_GEM_VM_DESTROY) { - ioctlVmDestroyCalled++; - auto control = static_cast(arg); - return (control->vmId > 0) ? 0 : -1; - } if (request == invalidIoctl) { errno = 0; if (setErrno != 0) { diff --git a/shared/test/common/os_interface/linux/sys_calls_linux_ult.h b/shared/test/common/os_interface/linux/sys_calls_linux_ult.h index 20864cd39d..fd36b5bbab 100644 --- a/shared/test/common/os_interface/linux/sys_calls_linux_ult.h +++ b/shared/test/common/os_interface/linux/sys_calls_linux_ult.h @@ -21,6 +21,5 @@ extern int (*sysCallsPoll)(struct pollfd *pollFd, unsigned long int numberOfFds, extern ssize_t (*sysCallsRead)(int fd, void *buf, size_t count); extern const char *drmVersion; -extern uint32_t ioctlVmDestroyCalled; } // namespace SysCalls } // namespace NEO diff --git a/shared/test/unit_test/os_interface/linux/drm_query_prelim_tests.cpp b/shared/test/unit_test/os_interface/linux/drm_query_prelim_tests.cpp index bc87e4e59b..8c6b04a7c7 100644 --- a/shared/test/unit_test/os_interface/linux/drm_query_prelim_tests.cpp +++ b/shared/test/unit_test/os_interface/linux/drm_query_prelim_tests.cpp @@ -324,9 +324,9 @@ TEST(DrmBufferObjectTestPrelim, givenPageFaultSupportedWhenVmBindIsAvailableThen EXPECT_FALSE(drm.receivedGemVmControl.flags & DrmPrelimHelper::getEnablePageFaultVmCreateFlag()); } - VariableBackup ioctlCountBackup(&SysCalls::ioctlVmDestroyCalled, 0u); + drm.ioctlCount.gemVmDestroy = 0; drm.destroyDrmVirtualMemory(vmId); - EXPECT_EQ(1u, SysCalls::ioctlVmDestroyCalled); + EXPECT_EQ(1, drm.ioctlCount.gemVmDestroy.load()); } }