From c6cbfc79fd56b3cf9d5bcbe284aed0c466999273 Mon Sep 17 00:00:00 2001 From: Mateusz Jablonski Date: Tue, 10 Oct 2023 09:55:08 +0000 Subject: [PATCH] refactor: add wrapper for handling file descriptor of SysmanHwDeviceIdDrm Related-To: NEO-9038 Signed-off-by: Mateusz Jablonski --- .../engine/linux/sysman_os_engine_imp.cpp | 9 ++++--- .../linux/sysman_os_engine_imp_prelim.cpp | 11 ++++---- .../linux/sysman_os_global_operations_imp.cpp | 8 ++---- .../source/linux/sysman_hw_device_id_linux.h | 22 +++++++++++++--- .../sysman/source/linux/zes_os_sysman_imp.cpp | 16 +++++------- .../sysman/source/linux/zes_os_sysman_imp.h | 2 +- .../memory/linux/sysman_os_memory_imp_dg1.cpp | 10 +++++--- .../linux/sysman_os_memory_imp_prelim.cpp | 19 ++++++++------ .../linux/sysman_os_scheduler_imp_prelim.cpp | 10 +++++--- .../shared/linux/sysman_kmd_interface.cpp | 9 ++++--- .../linux/test_sysman_hw_device_id.cpp | 25 +++++++++++-------- 11 files changed, 82 insertions(+), 59 deletions(-) diff --git a/level_zero/sysman/source/engine/linux/sysman_os_engine_imp.cpp b/level_zero/sysman/source/engine/linux/sysman_os_engine_imp.cpp index 16bad36807..af3303755c 100644 --- a/level_zero/sysman/source/engine/linux/sysman_os_engine_imp.cpp +++ b/level_zero/sysman/source/engine/linux/sysman_os_engine_imp.cpp @@ -37,11 +37,12 @@ static const std::multimap engineToI915Map = { ze_result_t OsEngine::getNumEngineTypeAndInstances(std::set> &engineGroupInstance, OsSysman *pOsSysman) { LinuxSysmanImp *pLinuxSysmanImp = static_cast(pOsSysman); NEO::Drm *pDrm = pLinuxSysmanImp->getDrm(); - auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceId(); - hwDeviceId->openFileDescriptor(); - auto status = pDrm->sysmanQueryEngineInfo(); - hwDeviceId->closeFileDescriptor(); + bool status = false; + { + auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceIdInstance(); + status = pDrm->sysmanQueryEngineInfo(); + } if (status == false) { NEO::printDebugString(NEO::DebugManager.flags.PrintDebugMessages.get(), stderr, "Error@ %s():sysmanQueryEngineInfo is returning false and error:0x%x \n", __FUNCTION__, ZE_RESULT_ERROR_UNSUPPORTED_FEATURE); diff --git a/level_zero/sysman/source/engine/linux/sysman_os_engine_imp_prelim.cpp b/level_zero/sysman/source/engine/linux/sysman_os_engine_imp_prelim.cpp index ba5da88ab2..d7d986c9e2 100644 --- a/level_zero/sysman/source/engine/linux/sysman_os_engine_imp_prelim.cpp +++ b/level_zero/sysman/source/engine/linux/sysman_os_engine_imp_prelim.cpp @@ -37,11 +37,12 @@ zes_engine_group_t LinuxEngineImp::getGroupFromEngineType(zes_engine_group_t typ ze_result_t OsEngine::getNumEngineTypeAndInstances(std::set> &engineGroupInstance, OsSysman *pOsSysman) { LinuxSysmanImp *pLinuxSysmanImp = static_cast(pOsSysman); NEO::Drm *pDrm = pLinuxSysmanImp->getDrm(); - auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceId(); - hwDeviceId->openFileDescriptor(); - auto status = pDrm->sysmanQueryEngineInfo(); - hwDeviceId->closeFileDescriptor(); + bool status = false; + { + auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceIdInstance(); + status = pDrm->sysmanQueryEngineInfo(); + } if (status == false) { NEO::printDebugString(NEO::DebugManager.flags.PrintDebugMessages.get(), stderr, "Error@ %s():sysmanQueryEngineInfo is returning false and error message:0x%x \n", __FUNCTION__, ZE_RESULT_ERROR_UNSUPPORTED_FEATURE); @@ -113,4 +114,4 @@ std::unique_ptr OsEngine::create(OsSysman *pOsSysman, zes_engine_group } } // namespace Sysman -} // namespace L0 \ No newline at end of file +} // namespace L0 diff --git a/level_zero/sysman/source/global_operations/linux/sysman_os_global_operations_imp.cpp b/level_zero/sysman/source/global_operations/linux/sysman_os_global_operations_imp.cpp index d837257502..c94fd9f5f1 100644 --- a/level_zero/sysman/source/global_operations/linux/sysman_os_global_operations_imp.cpp +++ b/level_zero/sysman/source/global_operations/linux/sysman_os_global_operations_imp.cpp @@ -259,10 +259,8 @@ bool LinuxGlobalOperationsImp::getUuid(std::arraygetSubDeviceCount(); if (NEO::DebugManager.flags.EnableChipsetUniqueUUID.get() != 0) { if (gfxCoreHelper.isChipsetUniqueUUIDSupported()) { - auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceId(); - hwDeviceId->openFileDescriptor(); + auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceIdInstance(); this->uuid.isValid = productHelper.getUuid(driverModel, subDeviceCount, 0u, this->uuid.id); - hwDeviceId->closeFileDescriptor(); } } @@ -726,8 +724,7 @@ ze_result_t LinuxGlobalOperationsImp::scanProcessesState(std::vectorgetSysmanHwDeviceId(); - hwDeviceId->openFileDescriptor(); + auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceIdInstance(); auto pDrm = pLinuxSysmanImp->getDrm(); // Device is said to be in wedged if context creation returns EIO. auto ret = pDrm->getIoctlHelper()->ioctl(NEO::DrmIoctl::GemContextCreateExt, &gcc); @@ -735,7 +732,6 @@ void LinuxGlobalOperationsImp::getWedgedStatus(zes_device_state_t *pState) { pDrm->destroyDrmContext(gcc.contextId); return; } - hwDeviceId->closeFileDescriptor(); if (pDrm->getErrno() == EIO) { pState->reset |= ZES_RESET_REASON_FLAG_WEDGED; diff --git a/level_zero/sysman/source/linux/sysman_hw_device_id_linux.h b/level_zero/sysman/source/linux/sysman_hw_device_id_linux.h index 8f1328a29d..a3604955c2 100644 --- a/level_zero/sysman/source/linux/sysman_hw_device_id_linux.h +++ b/level_zero/sysman/source/linux/sysman_hw_device_id_linux.h @@ -13,17 +13,33 @@ namespace L0 { namespace Sysman { class SysmanHwDeviceIdDrm : public NEO::HwDeviceIdDrm { - public: using NEO::HwDeviceIdDrm::HwDeviceIdDrm; + class SingleInstance { + public: + SingleInstance(SysmanHwDeviceIdDrm &input) : instance(input), fileDescriptor(input.openFileDescriptor()) { + UNRECOVERABLE_IF(fileDescriptor < 0); + } + ~SingleInstance() { instance.closeFileDescriptor(); } + int getFileDescriptor() const { return fileDescriptor; } + + private: + SysmanHwDeviceIdDrm &instance; + const int fileDescriptor; + }; SysmanHwDeviceIdDrm() = delete; + + SingleInstance getSingleInstance() { + return SingleInstance(*this); + } + + private: MOCKABLE_VIRTUAL int openFileDescriptor(); MOCKABLE_VIRTUAL int closeFileDescriptor(); - private: std::mutex fdMutex{}; uint32_t fdRefCounter = 0; }; } // namespace Sysman -} // namespace L0 \ No newline at end of file +} // namespace L0 diff --git a/level_zero/sysman/source/linux/zes_os_sysman_imp.cpp b/level_zero/sysman/source/linux/zes_os_sysman_imp.cpp index 840685770e..dc11be67df 100644 --- a/level_zero/sysman/source/linux/zes_os_sysman_imp.cpp +++ b/level_zero/sysman/source/linux/zes_os_sysman_imp.cpp @@ -45,9 +45,8 @@ ze_result_t LinuxSysmanImp::init() { if (osInterface.getDriverModel()->getDriverModelType() != NEO::DriverModelType::DRM) { return ZE_RESULT_ERROR_UNSUPPORTED_FEATURE; } - auto sysmanHwDeviceId = getSysmanHwDeviceId(); - sysmanHwDeviceId->openFileDescriptor(); - int myDeviceFd = sysmanHwDeviceId->getFileDescriptor(); + auto sysmanHwDeviceId = getSysmanHwDeviceIdInstance(); + int myDeviceFd = sysmanHwDeviceId.getFileDescriptor(); std::string myDeviceName; result = pProcfsAccess->getFileName(pProcfsAccess->myProcessId(), myDeviceFd, myDeviceName); if (ZE_RESULT_SUCCESS != result) { @@ -71,8 +70,6 @@ ze_result_t LinuxSysmanImp::init() { pSysmanProductHelper = SysmanProductHelper::create(getProductFamily()); osInterface.getDriverModel()->as()->cleanup(); - // Close Drm handles - sysmanHwDeviceId->closeFileDescriptor(); pPmuInterface = PmuInterface::create(this); return createPmtHandles(); } @@ -81,8 +78,9 @@ std::string &LinuxSysmanImp::getPciRootPath() { return rootPath; } -SysmanHwDeviceIdDrm *LinuxSysmanImp::getSysmanHwDeviceId() { - return static_cast(getDrm()->getHwDeviceId().get()); +SysmanHwDeviceIdDrm::SingleInstance LinuxSysmanImp::getSysmanHwDeviceIdInstance() { + UNRECOVERABLE_IF(!getDrm() || !getDrm()->getHwDeviceId()); + return static_cast(getDrm()->getHwDeviceId().get())->getSingleInstance(); } NEO::Drm *LinuxSysmanImp::getDrm() { @@ -523,16 +521,14 @@ ze_result_t LinuxSysmanImp::osColdReset() { uint32_t LinuxSysmanImp::getMemoryType() { if (memType == unknownMemoryType) { NEO::Drm *pDrm = getDrm(); - auto hwDeviceId = getSysmanHwDeviceId(); + auto hwDeviceIdInstance = getSysmanHwDeviceIdInstance(); - hwDeviceId->openFileDescriptor(); if (pDrm->querySystemInfo()) { auto memSystemInfo = getDrm()->getSystemInfo(); if (memSystemInfo != nullptr) { memType = memSystemInfo->getMemoryType(); } } - hwDeviceId->closeFileDescriptor(); } return memType; } diff --git a/level_zero/sysman/source/linux/zes_os_sysman_imp.h b/level_zero/sysman/source/linux/zes_os_sysman_imp.h index 064034bfd5..16d4ab12bc 100644 --- a/level_zero/sysman/source/linux/zes_os_sysman_imp.h +++ b/level_zero/sysman/source/linux/zes_os_sysman_imp.h @@ -54,7 +54,7 @@ class LinuxSysmanImp : public OsSysman, NEO::NonCopyableOrMovableClass { static std::string getPciRootPortDirectoryPath(std::string realPciPath); PlatformMonitoringTech *getPlatformMonitoringTechAccess(uint32_t subDeviceId); PRODUCT_FAMILY getProductFamily() const { return pParentSysmanDeviceImp->getProductFamily(); } - SysmanHwDeviceIdDrm *getSysmanHwDeviceId(); + SysmanHwDeviceIdDrm::SingleInstance getSysmanHwDeviceIdInstance(); NEO::Drm *getDrm(); void releasePmtObject(); MOCKABLE_VIRTUAL void releaseSysmanDeviceResources(); diff --git a/level_zero/sysman/source/memory/linux/sysman_os_memory_imp_dg1.cpp b/level_zero/sysman/source/memory/linux/sysman_os_memory_imp_dg1.cpp index a8d9af08bb..73e144aca5 100644 --- a/level_zero/sysman/source/memory/linux/sysman_os_memory_imp_dg1.cpp +++ b/level_zero/sysman/source/memory/linux/sysman_os_memory_imp_dg1.cpp @@ -44,10 +44,12 @@ ze_result_t LinuxMemoryImp::getBandwidth(zes_mem_bandwidth_t *pBandwidth) { ze_result_t LinuxMemoryImp::getState(zes_mem_state_t *pState) { std::vector deviceRegions; - auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceId(); - hwDeviceId->openFileDescriptor(); - auto status = pDrm->queryMemoryInfo(); - hwDeviceId->closeFileDescriptor(); + + bool status = false; + { + auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceIdInstance(); + status = pDrm->queryMemoryInfo(); + } if (status == false) { return ZE_RESULT_ERROR_UNSUPPORTED_FEATURE; diff --git a/level_zero/sysman/source/memory/linux/sysman_os_memory_imp_prelim.cpp b/level_zero/sysman/source/memory/linux/sysman_os_memory_imp_prelim.cpp index 3c72b4106b..5c6c2e0e25 100644 --- a/level_zero/sysman/source/memory/linux/sysman_os_memory_imp_prelim.cpp +++ b/level_zero/sysman/source/memory/linux/sysman_os_memory_imp_prelim.cpp @@ -45,10 +45,12 @@ bool LinuxMemoryImp::isMemoryModuleSupported() { ze_result_t LinuxMemoryImp::getProperties(zes_mem_properties_t *pProperties) { pProperties->type = ZES_MEM_TYPE_DDR; pProperties->numChannels = -1; - auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceId(); - hwDeviceId->openFileDescriptor(); - auto status = pDrm->querySystemInfo(); - hwDeviceId->closeFileDescriptor(); + + bool status = false; + { + auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceIdInstance(); + status = pDrm->querySystemInfo(); + } if (status) { auto memSystemInfo = pDrm->getSystemInfo(); @@ -342,10 +344,11 @@ ze_result_t LinuxMemoryImp::getState(zes_mem_state_t *pState) { pFwInterface->fwGetMemoryHealthIndicator(&pState->health); } - auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceId(); - hwDeviceId->openFileDescriptor(); - auto memoryInfo = pDrm->getIoctlHelper()->createMemoryInfo(); - hwDeviceId->closeFileDescriptor(); + std::unique_ptr memoryInfo; + { + auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceIdInstance(); + memoryInfo = pDrm->getIoctlHelper()->createMemoryInfo(); + } auto region = memoryInfo->getMemoryRegion(MemoryBanks::getBankForLocalMemory(subdeviceId)); diff --git a/level_zero/sysman/source/scheduler/linux/sysman_os_scheduler_imp_prelim.cpp b/level_zero/sysman/source/scheduler/linux/sysman_os_scheduler_imp_prelim.cpp index 59e317dc50..12ff66c34e 100644 --- a/level_zero/sysman/source/scheduler/linux/sysman_os_scheduler_imp_prelim.cpp +++ b/level_zero/sysman/source/scheduler/linux/sysman_os_scheduler_imp_prelim.cpp @@ -481,11 +481,13 @@ ze_result_t LinuxSchedulerImp::disableComputeUnitDebugMode(ze_bool_t *pNeedReloa static ze_result_t getNumEngineTypeAndInstancesForSubDevices(std::map> &mapOfEngines, LinuxSysmanImp *pLinuxSysmanImp, uint32_t subdeviceId) { - auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceId(); auto pDrm = pLinuxSysmanImp->getDrm(); - hwDeviceId->openFileDescriptor(); - auto engineInfo = pDrm->getEngineInfo(); - hwDeviceId->closeFileDescriptor(); + NEO::EngineInfo *engineInfo = nullptr; + { + auto hwDeviceId = pLinuxSysmanImp->getSysmanHwDeviceIdInstance(); + engineInfo = pDrm->getEngineInfo(); + } + if (engineInfo == nullptr) { return ZE_RESULT_ERROR_UNSUPPORTED_FEATURE; } diff --git a/level_zero/sysman/source/shared/linux/sysman_kmd_interface.cpp b/level_zero/sysman/source/shared/linux/sysman_kmd_interface.cpp index e0daaed5ff..45a06a329a 100644 --- a/level_zero/sysman/source/shared/linux/sysman_kmd_interface.cpp +++ b/level_zero/sysman/source/shared/linux/sysman_kmd_interface.cpp @@ -275,10 +275,11 @@ static ze_result_t getNumEngineTypeAndInstancesForSubDevices(std::map(pDrm->getHwDeviceId().get()); - hwDeviceId->openFileDescriptor(); - auto engineInfo = pDrm->getEngineInfo(); - hwDeviceId->closeFileDescriptor(); + NEO::EngineInfo *engineInfo = nullptr; + { + auto hwDeviceId = static_cast(pDrm->getHwDeviceId().get())->getSingleInstance(); + engineInfo = pDrm->getEngineInfo(); + } if (engineInfo == nullptr) { return ZE_RESULT_ERROR_UNSUPPORTED_FEATURE; } diff --git a/level_zero/sysman/test/unit_tests/sources/linux/test_sysman_hw_device_id.cpp b/level_zero/sysman/test/unit_tests/sources/linux/test_sysman_hw_device_id.cpp index 9ddbe7811e..7168f6060a 100644 --- a/level_zero/sysman/test/unit_tests/sources/linux/test_sysman_hw_device_id.cpp +++ b/level_zero/sysman/test/unit_tests/sources/linux/test_sysman_hw_device_id.cpp @@ -30,9 +30,10 @@ TEST(sysmanHwDeviceIdTest, whenOpenFileDescriptorIsCalledMultipleTimesThenOpenIs }); openCallCount = 0; - hwDeviceId.openFileDescriptor(); + + auto instance1 = hwDeviceId.getSingleInstance(); EXPECT_EQ(openCallCount, 1u); - hwDeviceId.openFileDescriptor(); + auto instance2 = hwDeviceId.getSingleInstance(); EXPECT_EQ(openCallCount, 1u); } @@ -44,14 +45,18 @@ TEST(sysmanHwDeviceIdTest, whenOpenFileDescriptorIsCalledMultipleTimesThenCloseI }); const auto referenceCloseCount = NEO::SysCalls::closeFuncCalled; - hwDeviceId.openFileDescriptor(); - hwDeviceId.openFileDescriptor(); - hwDeviceId.openFileDescriptor(); - hwDeviceId.closeFileDescriptor(); - EXPECT_EQ(NEO::SysCalls::closeFuncCalled, referenceCloseCount); - hwDeviceId.closeFileDescriptor(); - EXPECT_EQ(NEO::SysCalls::closeFuncCalled, referenceCloseCount); - hwDeviceId.closeFileDescriptor(); + { + auto instance1 = hwDeviceId.getSingleInstance(); + EXPECT_EQ(NEO::SysCalls::closeFuncCalled, referenceCloseCount); + { + auto instance2 = hwDeviceId.getSingleInstance(); + { + auto instance3 = hwDeviceId.getSingleInstance(); + } + EXPECT_EQ(NEO::SysCalls::closeFuncCalled, referenceCloseCount); + } + EXPECT_EQ(NEO::SysCalls::closeFuncCalled, referenceCloseCount); + } EXPECT_EQ(NEO::SysCalls::closeFuncCalled, referenceCloseCount + 1u); }