From b109094e4b522543ccee6ae407617c77f1f13669 Mon Sep 17 00:00:00 2001 From: Dominik Dabek Date: Fri, 29 Mar 2024 11:24:19 +0000 Subject: [PATCH] fix: move ulls controller stop thread Move ulls controller thread stopping to execution environment destructor. This removes the vptr race from ulls controller destructor. Put tests that are actually running the ulls controller into multi thread tests. That way they are compiled with thread sanitizer and removes the variable test time from traditional ULTS. Related-To: NEO-10942 Signed-off-by: Dominik Dabek --- .../mt_tests/direct_submission/CMakeLists.txt | 12 +++ .../direct_submission_controller_tests_mt.cpp | 80 ++++++++++++++++++ .../direct_submission_controller.cpp | 15 ++-- .../direct_submission_controller.h | 1 + .../execution_environment.cpp | 3 + .../direct_submission_controller_tests.cpp | 84 +++++-------------- 6 files changed, 125 insertions(+), 70 deletions(-) create mode 100644 opencl/test/unit_test/mt_tests/direct_submission/CMakeLists.txt create mode 100644 opencl/test/unit_test/mt_tests/direct_submission/direct_submission_controller_tests_mt.cpp diff --git a/opencl/test/unit_test/mt_tests/direct_submission/CMakeLists.txt b/opencl/test/unit_test/mt_tests/direct_submission/CMakeLists.txt new file mode 100644 index 0000000000..13457cb4b8 --- /dev/null +++ b/opencl/test/unit_test/mt_tests/direct_submission/CMakeLists.txt @@ -0,0 +1,12 @@ +# +# Copyright (C) 2024 Intel Corporation +# +# SPDX-License-Identifier: MIT +# + +set(IGDRCL_SRCS_mt_tests_direct_submission + # local files + ${CMAKE_CURRENT_SOURCE_DIR}/CMakeLists.txt + ${CMAKE_CURRENT_SOURCE_DIR}/direct_submission_controller_tests_mt.cpp +) +target_sources(igdrcl_mt_tests PRIVATE ${IGDRCL_SRCS_mt_tests_direct_submission}) diff --git a/opencl/test/unit_test/mt_tests/direct_submission/direct_submission_controller_tests_mt.cpp b/opencl/test/unit_test/mt_tests/direct_submission/direct_submission_controller_tests_mt.cpp new file mode 100644 index 0000000000..9d0ce6f264 --- /dev/null +++ b/opencl/test/unit_test/mt_tests/direct_submission/direct_submission_controller_tests_mt.cpp @@ -0,0 +1,80 @@ +/* + * Copyright (C) 2019-2024 Intel Corporation + * + * SPDX-License-Identifier: MIT + * + */ + +#include "shared/source/os_interface/os_context.h" +#include "shared/source/os_interface/os_thread.h" +#include "shared/test/common/helpers/debug_manager_state_restore.h" +#include "shared/test/common/helpers/engine_descriptor_helper.h" +#include "shared/test/common/mocks/mock_command_stream_receiver.h" +#include "shared/test/common/mocks/mock_execution_environment.h" +#include "shared/test/common/test_macros/test.h" +#include "shared/test/unit_test/direct_submission/direct_submission_controller_mock.h" + +namespace NEO { + +TEST(DirectSubmissionControllerTestsMt, givenDirectSubmissionControllerWhenTimeoutThenDirectSubmissionsAreChecked) { + MockExecutionEnvironment executionEnvironment; + executionEnvironment.prepareRootDeviceEnvironments(1); + executionEnvironment.initializeMemoryManager(); + + DeviceBitfield deviceBitfield(1); + MockCommandStreamReceiver csr(executionEnvironment, 0, deviceBitfield); + std::unique_ptr osContext(OsContext::create(nullptr, 0, 0, + EngineDescriptorHelper::getDefaultDescriptor({aub_stream::ENGINE_CCS, EngineUsage::regular}, + PreemptionMode::ThreadGroup, deviceBitfield))); + csr.setupContext(*osContext.get()); + csr.initializeTagAllocation(); + *csr.tagAddress = 9u; + csr.taskCount.store(9u); + + DirectSubmissionControllerMock controller; + executionEnvironment.directSubmissionController.reset(&controller); + controller.startThread(); + csr.startControllingDirectSubmissions(); + controller.registerDirectSubmission(&csr); + + while (controller.directSubmissions[&csr].taskCount != 9u) { + std::this_thread::yield(); + } + while (!controller.directSubmissions[&csr].isStopped) { + std::this_thread::yield(); + } + { + std::lock_guard lock(controller.directSubmissionsMutex); + EXPECT_NE(controller.directSubmissionControllingThread.get(), nullptr); + EXPECT_TRUE(controller.directSubmissions[&csr].isStopped); + EXPECT_EQ(controller.directSubmissions[&csr].taskCount, 9u); + } + controller.stopThread(); + controller.unregisterDirectSubmission(&csr); + executionEnvironment.directSubmissionController.release(); +} + +TEST(DirectSubmissionControllerTestsMt, givenDirectSubmissionControllerWithStartedControllingWhenShuttingDownThenNoHang) { + DirectSubmissionControllerMock controller; + controller.startThread(); + EXPECT_NE(controller.directSubmissionControllingThread.get(), nullptr); + controller.startControlling(); + + while (!controller.sleepCalled) { + std::this_thread::yield(); + } + controller.stopThread(); +} + +TEST(DirectSubmissionControllerTestsMt, givenDirectSubmissionControllerWithNotStartedControllingWhenShuttingDownThenNoHang) { + DirectSubmissionControllerMock controller; + controller.startThread(); + EXPECT_NE(controller.directSubmissionControllingThread.get(), nullptr); + + while (!controller.sleepCalled) { + std::this_thread::yield(); + } + controller.stopThread(); +} + +} // namespace NEO \ No newline at end of file diff --git a/shared/source/direct_submission/direct_submission_controller.cpp b/shared/source/direct_submission/direct_submission_controller.cpp index ac33d3d733..f2a3913705 100644 --- a/shared/source/direct_submission/direct_submission_controller.cpp +++ b/shared/source/direct_submission/direct_submission_controller.cpp @@ -31,11 +31,7 @@ DirectSubmissionController::DirectSubmissionController() { }; DirectSubmissionController::~DirectSubmissionController() { - keepControlling.store(false); - if (directSubmissionControllingThread) { - directSubmissionControllingThread->join(); - directSubmissionControllingThread.reset(); - } + UNRECOVERABLE_IF(directSubmissionControllingThread); } void DirectSubmissionController::registerDirectSubmission(CommandStreamReceiver *csr) { @@ -88,6 +84,15 @@ void DirectSubmissionController::startThread() { directSubmissionControllingThread = Thread::create(controlDirectSubmissionsState, reinterpret_cast(this)); } +void DirectSubmissionController::stopThread() { + runControlling.store(false); + keepControlling.store(false); + if (directSubmissionControllingThread) { + directSubmissionControllingThread->join(); + directSubmissionControllingThread.reset(); + } +} + void DirectSubmissionController::startControlling() { this->runControlling.store(true); } diff --git a/shared/source/direct_submission/direct_submission_controller.h b/shared/source/direct_submission/direct_submission_controller.h index b62775947d..b1dd5948fa 100644 --- a/shared/source/direct_submission/direct_submission_controller.h +++ b/shared/source/direct_submission/direct_submission_controller.h @@ -45,6 +45,7 @@ class DirectSubmissionController { void startThread(); void startControlling(); + void stopThread(); static bool isSupported(); diff --git a/shared/source/execution_environment/execution_environment.cpp b/shared/source/execution_environment/execution_environment.cpp index 8895ae4e1e..75fe6b239a 100644 --- a/shared/source/execution_environment/execution_environment.cpp +++ b/shared/source/execution_environment/execution_environment.cpp @@ -43,6 +43,9 @@ void ExecutionEnvironment::releaseRootDeviceEnvironmentResources(RootDeviceEnvir } ExecutionEnvironment::~ExecutionEnvironment() { + if (directSubmissionController) { + directSubmissionController->stopThread(); + } if (memoryManager) { memoryManager->commonCleanup(); for (const auto &rootDeviceEnvironment : this->rootDeviceEnvironments) { diff --git a/shared/test/unit_test/direct_submission/direct_submission_controller_tests.cpp b/shared/test/unit_test/direct_submission/direct_submission_controller_tests.cpp index e9bc20cd9e..7971506b68 100644 --- a/shared/test/unit_test/direct_submission/direct_submission_controller_tests.cpp +++ b/shared/test/unit_test/direct_submission/direct_submission_controller_tests.cpp @@ -75,58 +75,6 @@ TEST(DirectSubmissionControllerTests, givenDirectSubmissionControllerWhenRegiste controller.unregisterDirectSubmission(&csr); } -TEST(DirectSubmissionControllerTests, givenDirectSubmissionControllerWhenTimeoutThenDirectSubmissionsAreChecked) { - MockExecutionEnvironment executionEnvironment; - executionEnvironment.prepareRootDeviceEnvironments(1); - executionEnvironment.initializeMemoryManager(); - - DeviceBitfield deviceBitfield(1); - MockCommandStreamReceiver csr(executionEnvironment, 0, deviceBitfield); - std::unique_ptr osContext(OsContext::create(nullptr, 0, 0, - EngineDescriptorHelper::getDefaultDescriptor({aub_stream::ENGINE_CCS, EngineUsage::regular}, - PreemptionMode::ThreadGroup, deviceBitfield))); - csr.setupContext(*osContext.get()); - csr.initializeTagAllocation(); - *csr.tagAddress = 9u; - csr.taskCount.store(9u); - - DirectSubmissionControllerMock controller; - executionEnvironment.directSubmissionController.reset(&controller); - controller.startThread(); - csr.startControllingDirectSubmissions(); - controller.registerDirectSubmission(&csr); - - while (controller.directSubmissions[&csr].taskCount != 9u) { - std::this_thread::yield(); - } - while (!controller.directSubmissions[&csr].isStopped) { - std::this_thread::yield(); - } - { - std::lock_guard lock(controller.directSubmissionsMutex); - EXPECT_NE(controller.directSubmissionControllingThread.get(), nullptr); - EXPECT_TRUE(controller.directSubmissions[&csr].isStopped); - EXPECT_EQ(controller.directSubmissions[&csr].taskCount, 9u); - } - - controller.unregisterDirectSubmission(&csr); - executionEnvironment.directSubmissionController.release(); -} - -TEST(DirectSubmissionControllerTests, givenDirectSubmissionControllerWithStartedControllingWhenShuttingDownThenNoHang) { - DirectSubmissionControllerMock controller; - controller.startThread(); - EXPECT_NE(controller.directSubmissionControllingThread.get(), nullptr); - controller.startControlling(); - - while (!controller.sleepCalled) { - std::this_thread::yield(); - } - controller.keepControlling.store(false); - controller.directSubmissionControllingThread->join(); - controller.directSubmissionControllingThread.reset(); -} - TEST(DirectSubmissionControllerTests, givenDirectSubmissionControllerAndDivisorDisabledWhenIncreaseTimeoutEnabledThenTimeoutIsIncreased) { DebugManagerStateRestore restorer; debugManager.flags.DirectSubmissionControllerMaxTimeout.set(200'000); @@ -224,6 +172,25 @@ void fillTimeoutParamsMap(DirectSubmissionControllerMock &controller) { } } +TEST(DirectSubmissionControllerTests, givenDirectSubmissionControllerAndAdjustOnThrottleAndAcLineStatusDisabledWhenSetTimeoutParamsForPlatformThenTimeoutParamsMapsIsEmpty) { + DebugManagerStateRestore restorer; + debugManager.flags.DirectSubmissionControllerAdjustOnThrottleAndAcLineStatus.set(0); + MockExecutionEnvironment executionEnvironment; + executionEnvironment.prepareRootDeviceEnvironments(1); + executionEnvironment.initializeMemoryManager(); + + DeviceBitfield deviceBitfield(1); + MockCommandStreamReceiver csr(executionEnvironment, 0, deviceBitfield); + std::unique_ptr osContext(OsContext::create(nullptr, 0, 0, + EngineDescriptorHelper::getDefaultDescriptor({aub_stream::ENGINE_CCS, EngineUsage::regular}, + PreemptionMode::ThreadGroup, deviceBitfield))); + csr.setupContext(*osContext.get()); + + DirectSubmissionControllerMock controller; + controller.setTimeoutParamsForPlatform(csr.getProductHelper()); + EXPECT_EQ(0u, controller.timeoutParamsMap.size()); +} + TEST(DirectSubmissionControllerTests, givenDirectSubmissionControllerAndAdjustOnThrottleAndAcLineStatusEnabledWhenThrottleOrAcLineStatusChangesThenTimeoutIsChanged) { DebugManagerStateRestore restorer; debugManager.flags.DirectSubmissionControllerDivisor.set(1); @@ -338,19 +305,6 @@ TEST(DirectSubmissionControllerTests, givenDirectSubmissionControllerAndAdjustOn controller.unregisterDirectSubmission(&csr); } -TEST(DirectSubmissionControllerTests, givenDirectSubmissionControllerWithNotStartedControllingWhenShuttingDownThenNoHang) { - DirectSubmissionControllerMock controller; - controller.startThread(); - EXPECT_NE(controller.directSubmissionControllingThread.get(), nullptr); - - while (!controller.sleepCalled) { - std::this_thread::yield(); - } - controller.keepControlling.store(false); - controller.directSubmissionControllingThread->join(); - controller.directSubmissionControllingThread.reset(); -} - TEST(DirectSubmissionControllerTests, givenDirectSubmissionControllerWhenRegisterCsrsThenTimeoutIsNotAdjusted) { MockExecutionEnvironment executionEnvironment; executionEnvironment.prepareRootDeviceEnvironments(1);