From e453d2c04f8d28c71365c4ed2b45ad06a63668db Mon Sep 17 00:00:00 2001 From: Andrzej Swierczynski Date: Mon, 24 Feb 2020 12:46:03 +0100 Subject: [PATCH] Change method of access to pinBB vectors Related-To: NEO-4319 Change-Id: I70695361c368a7769b6dbb7db57597f188226133 Signed-off-by: Andrzej Swierczynski --- .../mocks/linux/mock_drm_memory_manager.cpp | 6 +-- .../linux/drm_command_stream_mm_tests.cpp | 8 ++-- .../linux/drm_memory_manager_tests.cpp | 44 +++++++++---------- .../os_interface/linux/drm_memory_manager.cpp | 11 +++-- 4 files changed, 33 insertions(+), 36 deletions(-) diff --git a/opencl/test/unit_test/mocks/linux/mock_drm_memory_manager.cpp b/opencl/test/unit_test/mocks/linux/mock_drm_memory_manager.cpp index 72f0771233..343504f429 100644 --- a/opencl/test/unit_test/mocks/linux/mock_drm_memory_manager.cpp +++ b/opencl/test/unit_test/mocks/linux/mock_drm_memory_manager.cpp @@ -46,10 +46,10 @@ TestedDrmMemoryManager::TestedDrmMemoryManager(bool enableLocalMemory, } void TestedDrmMemoryManager::injectPinBB(BufferObject *newPinBB) { - BufferObject *currentPinBB = pinBBs.at(newPinBB->peekRootDeviceIndex()); - pinBBs.at(newPinBB->peekRootDeviceIndex()) = nullptr; + BufferObject *currentPinBB = pinBBs[newPinBB->peekRootDeviceIndex()]; + pinBBs[newPinBB->peekRootDeviceIndex()] = nullptr; DrmMemoryManager::unreference(currentPinBB, true); - pinBBs.at(newPinBB->peekRootDeviceIndex()) = newPinBB; + pinBBs[newPinBB->peekRootDeviceIndex()] = newPinBB; } DrmGemCloseWorker *TestedDrmMemoryManager::getgemCloseWorker() { return this->gemCloseWorker.get(); } diff --git a/opencl/test/unit_test/os_interface/linux/drm_command_stream_mm_tests.cpp b/opencl/test/unit_test/os_interface/linux/drm_command_stream_mm_tests.cpp index 3fb818489e..c01fedd3b8 100644 --- a/opencl/test/unit_test/os_interface/linux/drm_command_stream_mm_tests.cpp +++ b/opencl/test/unit_test/os_interface/linux/drm_command_stream_mm_tests.cpp @@ -39,7 +39,7 @@ HWTEST_F(DrmCommandStreamMMTest, MMwithPinBB) { auto memoryManager = new TestedDrmMemoryManager(false, true, false, executionEnvironment); executionEnvironment.memoryManager.reset(memoryManager); ASSERT_NE(nullptr, memoryManager); - EXPECT_NE(nullptr, memoryManager->pinBBs.at(0)); + EXPECT_NE(nullptr, memoryManager->pinBBs[0]); } HWTEST_F(DrmCommandStreamMMTest, givenForcePinDisabledWhenMemoryManagerIsCreatedThenPinBBIsCreated) { @@ -58,12 +58,10 @@ HWTEST_F(DrmCommandStreamMMTest, givenForcePinDisabledWhenMemoryManagerIsCreated executionEnvironment.memoryManager.reset(memoryManager); ASSERT_NE(nullptr, memoryManager); - EXPECT_NE(nullptr, memoryManager->pinBBs.at(0)); + EXPECT_NE(nullptr, memoryManager->pinBBs[0]); } HWTEST_F(DrmCommandStreamMMTest, givenExecutionEnvironmentWithMoreThanOneRootDeviceEnvWhenCreatingDrmMemoryManagerThenCreateAsManyPinBBs) { - DebugManagerStateRestore dbgRestorer; - MockExecutionEnvironment executionEnvironment; executionEnvironment.prepareRootDeviceEnvironments(2); executionEnvironment.setHwInfo(*platformDevices); @@ -79,6 +77,6 @@ HWTEST_F(DrmCommandStreamMMTest, givenExecutionEnvironmentWithMoreThanOneRootDev executionEnvironment.memoryManager.reset(memoryManager); ASSERT_NE(nullptr, memoryManager); for (uint32_t rootDeviceIndex = 0; rootDeviceIndex < executionEnvironment.rootDeviceEnvironments.size(); rootDeviceIndex++) { - EXPECT_NE(nullptr, memoryManager->pinBBs.at(rootDeviceIndex)); + EXPECT_NE(nullptr, memoryManager->pinBBs[rootDeviceIndex]); } } diff --git a/opencl/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp b/opencl/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp index 0fb193c910..61b9e39999 100644 --- a/opencl/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp +++ b/opencl/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp @@ -103,7 +103,7 @@ TEST_F(DrmMemoryManagerTest, GivenGraphicsAllocationWhenAddAndRemoveAllocationTo TEST_F(DrmMemoryManagerWithExplicitExpectationsTest, givenforcePinAllowedWhenMemoryManagerIsCreatedThenPinBbIsCreated) { auto memoryManager = std::make_unique(false, true, false, *executionEnvironment); - EXPECT_NE(nullptr, memoryManager->pinBBs.at(0)); + EXPECT_NE(nullptr, memoryManager->pinBBs[0]); } TEST_F(DrmMemoryManagerTest, pinBBisCreated) { @@ -111,7 +111,7 @@ TEST_F(DrmMemoryManagerTest, pinBBisCreated) { mock->ioctl_expected.gemClose = 1; auto memoryManager = std::make_unique(false, true, false, *executionEnvironment); - EXPECT_NE(nullptr, memoryManager->pinBBs.at(0)); + EXPECT_NE(nullptr, memoryManager->pinBBs[0]); } TEST_F(DrmMemoryManagerTest, givenNotAllowedForcePinWhenMemoryManagerIsCreatedThenPinBBIsNotCreated) { @@ -119,14 +119,14 @@ TEST_F(DrmMemoryManagerTest, givenNotAllowedForcePinWhenMemoryManagerIsCreatedTh false, false, *executionEnvironment)); - EXPECT_EQ(nullptr, memoryManager->pinBBs.at(0)); + EXPECT_EQ(nullptr, memoryManager->pinBBs[0]); } TEST_F(DrmMemoryManagerTest, pinBBnotCreatedWhenIoctlFailed) { mock->ioctl_expected.gemUserptr = 1; mock->ioctl_res = -1; auto memoryManager = new (std::nothrow) TestedDrmMemoryManager(false, true, false, *executionEnvironment); - EXPECT_EQ(nullptr, memoryManager->pinBBs.at(0)); + EXPECT_EQ(nullptr, memoryManager->pinBBs[0]); mock->ioctl_res = 0; delete memoryManager; } @@ -142,7 +142,7 @@ TEST_F(DrmMemoryManagerTest, pinAfterAllocateWhenAskedAndAllowedAndBigAllocation for (auto engine : memoryManager->registeredEngines) { engine.osContext->incRefInternal(); } - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); auto alloc = static_cast(memoryManager->allocateGraphicsMemoryWithProperties(createAllocationProperties(10 * MemoryConstants::megaByte, true))); ASSERT_NE(nullptr, alloc); @@ -176,7 +176,7 @@ TEST_F(DrmMemoryManagerTest, givenDrmContextIdWhenAllocationIsCreatedThenPinWith engine.osContext->incRefInternal(); } auto drmContextId = memoryManager->getDefaultDrmContextId(); - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); EXPECT_NE(0u, drmContextId); auto alloc = memoryManager->allocateGraphicsMemoryWithProperties(createAllocationProperties(memoryManager->pinThreshold, true)); @@ -191,7 +191,7 @@ TEST_F(DrmMemoryManagerTest, doNotPinAfterAllocateWhenAskedAndAllowedButSmallAll mock->ioctl_expected.gemClose = 2; auto memoryManager = std::make_unique(false, true, false, *executionEnvironment); - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); // one page is too small for early pinning auto alloc = static_cast(memoryManager->allocateGraphicsMemoryWithProperties(createAllocationProperties(MemoryConstants::pageSize, true))); @@ -211,7 +211,7 @@ TEST_F(DrmMemoryManagerTest, doNotPinAfterAllocateWhenNotAskedButAllowed) { for (auto engine : memoryManager->registeredEngines) { engine.osContext->incRefInternal(); } - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); auto alloc = static_cast(memoryManager->allocateGraphicsMemoryWithProperties(createAllocationProperties(MemoryConstants::pageSize, false))); ASSERT_NE(nullptr, alloc); @@ -250,7 +250,7 @@ TEST_F(DrmMemoryManagerTest, pinAfterAllocateWhenAskedAndAllowedAndBigAllocation for (auto engine : memoryManager->registeredEngines) { engine.osContext->incRefInternal(); } - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); allocationData.size = 10 * MB; allocationData.hostPtr = ::alignedMalloc(allocationData.size, 4096); @@ -273,7 +273,7 @@ TEST_F(DrmMemoryManagerTest, givenSmallAllocationHostPtrAllocationWhenForcePinIs for (auto engine : memoryManager->registeredEngines) { engine.osContext->incRefInternal(); } - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); // one page is too small for early pinning allocationData.size = 4 * 1024; @@ -294,7 +294,7 @@ TEST_F(DrmMemoryManagerTest, doNotPinAfterAllocateWhenNotAskedButAllowedHostPtr) mock->ioctl_expected.gemClose = 2; auto memoryManager = std::make_unique(false, true, false, *executionEnvironment); - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); allocationData.size = 4 * 1024; allocationData.hostPtr = ::alignedMalloc(allocationData.size, 4096); @@ -2566,7 +2566,7 @@ TEST_F(DrmMemoryManagerBasic, givenEnabledHostMemoryValidationWhenMemoryManagerI true, executionEnvironment)); ASSERT_NE(nullptr, memoryManager.get()); - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); } TEST_F(DrmMemoryManagerBasic, givenEnabledHostMemoryValidationAndForcePinWhenMemoryManagerIsCreatedThenPinBBIsCreated) { @@ -2575,7 +2575,7 @@ TEST_F(DrmMemoryManagerBasic, givenEnabledHostMemoryValidationAndForcePinWhenMem true, executionEnvironment)); ASSERT_NE(nullptr, memoryManager.get()); - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); } TEST_F(DrmMemoryManagerBasic, givenMemoryManagerWhenAllocateGraphicsMemoryIsCalledThenMemoryPoolIsSystem4KBPages) { @@ -2678,7 +2678,7 @@ TEST_F(DrmMemoryManagerWithExplicitExpectationsTest, givenDisabledForcePinAndEna engine.osContext->incRefInternal(); } ASSERT_NE(nullptr, memoryManager.get()); - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); mock->reset(); mock->ioctl_expected.gemUserptr = 1; @@ -2722,7 +2722,7 @@ TEST_F(DrmMemoryManagerWithExplicitExpectationsTest, givenDisabledForcePinAndEna engine.osContext->incRefInternal(); } ASSERT_NE(nullptr, memoryManager.get()); - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); PinBufferObject *pinBB = new PinBufferObject(this->mock); memoryManager->injectPinBB(pinBB); @@ -2774,7 +2774,7 @@ TEST_F(DrmMemoryManagerWithExplicitExpectationsTest, givenValidateHostPtrMemoryE for (auto engine : memoryManager->registeredEngines) { engine.osContext->incRefInternal(); } - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); size_t size = 10 * MB; void *ptr = ::alignedMalloc(size, 4096); @@ -2818,7 +2818,7 @@ TEST_F(DrmMemoryManagerTest, givenForcePinAndHostMemoryValidationEnabledWhenSmal for (auto engine : memoryManager->registeredEngines) { engine.osContext->incRefInternal(); } - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); // one page is too small for early pinning but pinning is used for host memory validation allocationData.size = 4 * 1024; @@ -2841,7 +2841,7 @@ TEST_F(DrmMemoryManagerTest, givenForcePinAllowedAndNoPinBBInMemoryManagerWhenAl for (auto engine : memoryManager->registeredEngines) { engine.osContext->incRefInternal(); } - EXPECT_EQ(nullptr, memoryManager->pinBBs.at(0)); + EXPECT_EQ(nullptr, memoryManager->pinBBs[0]); mock->ioctl_res = 0; auto allocation = memoryManager->allocateGraphicsMemoryWithProperties(createAllocationProperties(MemoryConstants::pageSize, true)); @@ -2990,7 +2990,7 @@ TEST_F(DrmMemoryManagerWithExplicitExpectationsTest, givenEnabledValidateHostMem engine.osContext->incRefInternal(); } ASSERT_NE(nullptr, memoryManager.get()); - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); mock->reset(); @@ -3040,7 +3040,7 @@ TEST_F(DrmMemoryManagerWithExplicitExpectationsTest, givenEnabledValidateHostMem for (auto engine : memoryManager->registeredEngines) { engine.osContext->incRefInternal(); } - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); mock->reset(); @@ -3089,7 +3089,7 @@ TEST_F(DrmMemoryManagerWithExplicitExpectationsTest, givenEnabledValidateHostMem for (auto engine : memoryManager->registeredEngines) { engine.osContext->incRefInternal(); } - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); mock->reset(); mock->ioctl_expected.gemUserptr = 1; @@ -3115,7 +3115,7 @@ TEST_F(DrmMemoryManagerWithExplicitExpectationsTest, givenEnabledValidateHostMem TEST_F(DrmMemoryManagerWithExplicitExpectationsTest, givenDrmMemoryManagerWhenCleanOsHandlesDeletesHandleDataThenOsHandleStorageAndResidencyIsSetToNullptr) { std::unique_ptr memoryManager(new TestedDrmMemoryManager(false, false, true, *executionEnvironment)); - ASSERT_NE(nullptr, memoryManager->pinBBs.at(0)); + ASSERT_NE(nullptr, memoryManager->pinBBs[0]); OsHandleStorage handleStorage; handleStorage.fragmentStorageData[0].osHandleStorage = new OsHandle(); diff --git a/shared/source/os_interface/linux/drm_memory_manager.cpp b/shared/source/os_interface/linux/drm_memory_manager.cpp index 48958848b1..9542c22420 100644 --- a/shared/source/os_interface/linux/drm_memory_manager.cpp +++ b/shared/source/os_interface/linux/drm_memory_manager.cpp @@ -47,11 +47,11 @@ DrmMemoryManager::DrmMemoryManager(gemCloseWorkerMode mode, for (uint32_t rootDeviceIndex = 0; rootDeviceIndex < gfxPartitions.size(); ++rootDeviceIndex) { if (forcePinEnabled || validateHostPtrMemory) { memoryForPinBBs.push_back(alignedMallocWrapper(MemoryConstants::pageSize, MemoryConstants::pageSize)); - DEBUG_BREAK_IF(memoryForPinBBs.at(rootDeviceIndex) == nullptr); - pinBBs.push_back(allocUserptr(reinterpret_cast(memoryForPinBBs.at(rootDeviceIndex)), MemoryConstants::pageSize, 0, rootDeviceIndex)); - if (!pinBBs.at(rootDeviceIndex)) { - alignedFreeWrapper(memoryForPinBBs.at(rootDeviceIndex)); - memoryForPinBBs.at(rootDeviceIndex) = nullptr; + DEBUG_BREAK_IF(memoryForPinBBs[rootDeviceIndex] == nullptr); + pinBBs.push_back(allocUserptr(reinterpret_cast(memoryForPinBBs[rootDeviceIndex]), MemoryConstants::pageSize, 0, rootDeviceIndex)); + if (!pinBBs[rootDeviceIndex]) { + alignedFreeWrapper(memoryForPinBBs[rootDeviceIndex]); + memoryForPinBBs[rootDeviceIndex] = nullptr; DEBUG_BREAK_IF(true); UNRECOVERABLE_IF(validateHostPtrMemory); } @@ -67,7 +67,6 @@ DrmMemoryManager::~DrmMemoryManager() { MemoryManager::alignedFreeWrapper(memoryForPinBB); } } - memoryForPinBBs.clear(); } void DrmMemoryManager::commonCleanup() {