From 083edd1a3ea4566e7df0161776bdbe189e8cd124 Mon Sep 17 00:00:00 2001 From: Vinod Tipparaju Date: Mon, 17 Aug 2020 17:33:26 +0530 Subject: [PATCH] Thread safety violation fixes - Events & USM Change-Id: I57de59f204d9fd4407be768d0b14bf579dae967d Signed-off-by: Vinod Tipparaju --- level_zero/core/source/cmdqueue/cmdqueue.cpp | 2 +- .../core/source/cmdqueue/cmdqueue_imp.h | 2 +- level_zero/core/source/event/event.cpp | 28 ---------------- .../unit_tests/sources/event/test_event.cpp | 33 ------------------- .../memory_manager/unified_memory_manager.cpp | 3 +- 5 files changed, 4 insertions(+), 64 deletions(-) diff --git a/level_zero/core/source/cmdqueue/cmdqueue.cpp b/level_zero/core/source/cmdqueue/cmdqueue.cpp index 977af92cfc..98de020574 100644 --- a/level_zero/core/source/cmdqueue/cmdqueue.cpp +++ b/level_zero/core/source/cmdqueue/cmdqueue.cpp @@ -67,7 +67,7 @@ ze_result_t CommandQueueImp::synchronize(uint64_t timeout) { ze_result_t CommandQueueImp::synchronizeByPollingForTaskCount(uint64_t timeout) { UNRECOVERABLE_IF(csr == nullptr); - auto taskCountToWait = this->taskCount; + auto taskCountToWait = getTaskCount(); bool enableTimeout = true; int64_t timeoutMicroseconds = static_cast(timeout); if (timeout == std::numeric_limits::max()) { diff --git a/level_zero/core/source/cmdqueue/cmdqueue_imp.h b/level_zero/core/source/cmdqueue/cmdqueue_imp.h index 8f1f578963..a55e5e69e8 100644 --- a/level_zero/core/source/cmdqueue/cmdqueue_imp.h +++ b/level_zero/core/source/cmdqueue/cmdqueue_imp.h @@ -91,7 +91,7 @@ struct CommandQueueImp : public CommandQueue { NEO::CommandStreamReceiver *csr = nullptr; const ze_command_queue_desc_t desc; NEO::LinearStream *commandStream = nullptr; - uint32_t taskCount = 0; + std::atomic taskCount{0}; std::vector printfFunctionContainer; bool gsbaInit = false; bool frontEndInit = false; diff --git a/level_zero/core/source/event/event.cpp b/level_zero/core/source/event/event.cpp index f9219a0cdc..d5fc50553d 100644 --- a/level_zero/core/source/event/event.cpp +++ b/level_zero/core/source/event/event.cpp @@ -70,7 +70,6 @@ struct EventImp : public Event { protected: ze_result_t hostEventSetValue(uint32_t eventValue); ze_result_t hostEventSetValueTimestamps(uint32_t eventVal); - void makeAllocationResident(); }; struct EventPoolImp : public EventPool { @@ -162,33 +161,10 @@ NEO::GraphicsAllocation &Event::getAllocation() { } ze_result_t Event::destroy() { - auto eventImp = static_cast(this); - auto deviceImp = static_cast(eventImp->device); - - NEO::MemoryOperationsHandler *memoryOperationsIface = - deviceImp->neoDevice->getRootDeviceEnvironment().memoryOperationsInterface.get(); - - if (memoryOperationsIface) { - memoryOperationsIface->evict(deviceImp->getNEODevice(), - eventImp->eventPool->getAllocation()); - } - delete this; return ZE_RESULT_SUCCESS; } -void EventImp::makeAllocationResident() { - auto deviceImp = static_cast(this->device); - NEO::MemoryOperationsHandler *memoryOperationsIface = - deviceImp->neoDevice->getRootDeviceEnvironment().memoryOperationsInterface.get(); - - if (memoryOperationsIface) { - auto alloc = &(this->eventPool->getAllocation()); - memoryOperationsIface->makeResident(deviceImp->neoDevice, - ArrayRef(&alloc, 1)); - } -} - ze_result_t EventImp::hostEventSetValueTimestamps(uint32_t eventVal) { auto baseAddr = reinterpret_cast(hostAddress); @@ -207,8 +183,6 @@ ze_result_t EventImp::hostEventSetValueTimestamps(uint32_t eventVal) { eventTsSetFunc(baseAddr + offsetof(KernelTimestampEvent, contextEnd)); eventTsSetFunc(baseAddr + offsetof(KernelTimestampEvent, globalEnd)); - makeAllocationResident(); - return ZE_RESULT_SUCCESS; } @@ -221,8 +195,6 @@ ze_result_t EventImp::hostEventSetValue(uint32_t eventVal) { UNRECOVERABLE_IF(hostAddr == nullptr); memcpy_s(static_cast(hostAddr), sizeof(uint32_t), static_cast(&eventVal), sizeof(uint32_t)); - makeAllocationResident(); - if (!this->signalScope) { NEO::CpuIntrinsics::clFlush(hostAddr); } diff --git a/level_zero/core/test/unit_tests/sources/event/test_event.cpp b/level_zero/core/test/unit_tests/sources/event/test_event.cpp index 79e48adf6a..8103cc9640 100644 --- a/level_zero/core/test/unit_tests/sources/event/test_event.cpp +++ b/level_zero/core/test/unit_tests/sources/event/test_event.cpp @@ -164,39 +164,6 @@ struct EventCreateAllocationResidencyTest : public ::testing::Test { L0::Device *device = nullptr; }; -TEST_F(EventCreateAllocationResidencyTest, - givenEventCreateAndEventDestroyCallsThenMakeResidentAndEvictAreCalled) { - ze_event_pool_desc_t eventPoolDesc = { - ZE_STRUCTURE_TYPE_EVENT_POOL_DESC, - nullptr, - ZE_EVENT_POOL_FLAG_HOST_VISIBLE, - 1}; - - auto deviceHandle = device->toHandle(); - auto eventPool = EventPool::create(driverHandle.get(), 1, &deviceHandle, &eventPoolDesc); - ASSERT_NE(nullptr, eventPool); - - const ze_event_desc_t eventDesc = { - ZE_STRUCTURE_TYPE_EVENT_DESC, - nullptr, - 0, - ZE_EVENT_SCOPE_FLAG_DEVICE, - ZE_EVENT_SCOPE_FLAG_DEVICE}; - - EXPECT_CALL(*mockMemoryOperationsHandler, makeResident).Times(1); - auto event = L0::Event::create(eventPool, &eventDesc, device); - ASSERT_NE(nullptr, event); - - NEO::MemoryOperationsHandler *memoryOperationsIface = - neoDevice->getRootDeviceEnvironment().memoryOperationsInterface.get(); - EXPECT_NE(nullptr, memoryOperationsIface); - - EXPECT_CALL(*mockMemoryOperationsHandler, evict).Times(1); - event->destroy(); - - eventPool->destroy(); -} - class TimestampEventCreate : public Test { public: void SetUp() override { diff --git a/shared/source/memory_manager/unified_memory_manager.cpp b/shared/source/memory_manager/unified_memory_manager.cpp index f7161ed9fa..d0e864cb9b 100644 --- a/shared/source/memory_manager/unified_memory_manager.cpp +++ b/shared/source/memory_manager/unified_memory_manager.cpp @@ -101,7 +101,6 @@ void *SVMAllocsManager::createSVMAlloc(uint32_t rootDeviceIndex, size_t size, co if (size == 0) return nullptr; - std::unique_lock lock(mtx); if (!memoryManager->isLocalMemorySupported(rootDeviceIndex)) { return createZeroCopySvmAllocation(rootDeviceIndex, size, svmProperties, deviceBitfield); } else { @@ -283,6 +282,7 @@ void *SVMAllocsManager::createZeroCopySvmAllocation(uint32_t rootDeviceIndex, si allocData.gpuAllocations.addAllocation(allocation); allocData.size = size; + std::unique_lock lock(mtx); this->SVMAllocs.insert(allocData); return allocation->getUnderlyingBuffer(); } @@ -328,6 +328,7 @@ void *SVMAllocsManager::createUnifiedAllocationWithDeviceStorage(uint32_t rootDe allocData.device = unifiedMemoryProperties.device; allocData.size = size; + std::unique_lock lock(mtx); this->SVMAllocs.insert(allocData); return svmPtr; }