feature: unregister CSR client on Event host synchronize

BartoszDunajski · Compute-Runtime-Automation · commit e64450a84aab · 2023-06-29T16:12:09.000+02:00
Related-To: NEO-7458 Source: aea5f43 Signed-off-by: Dunajski, Bartosz <bartosz.dunajski@intel.com>
diff --git a/level_zero/core/source/cmdlist/cmdlist_hw_immediate.inl b/level_zero/core/source/cmdlist/cmdlist_hw_immediate.inl
@@ -722,6 +722,8 @@ ze_result_t CommandListCoreFamilyImmediate<gfxCoreFamily>::hostSynchronize(uint6
 template <GFXCORE_FAMILY gfxCoreFamily>
 ze_result_t CommandListCoreFamilyImmediate<gfxCoreFamily>::flushImmediate(ze_result_t inputRet, bool performMigration, bool hasStallingCmds,
                                                                           bool hasRelaxedOrderingDependencies, ze_event_handle_t hSignalEvent) {
+    auto signalEvent = Event::fromHandle(hSignalEvent);
+
     if (inputRet == ZE_RESULT_SUCCESS) {
         if (isInOrderExecutionEnabled()) {
             auto node = this->timestampPacketContainer->peekNodes()[0];
@@ -730,14 +732,15 @@ ze_result_t CommandListCoreFamilyImmediate<gfxCoreFamily>::flushImmediate(ze_res
         }
 
         if (this->isFlushTaskSubmissionEnabled) {
+            if (signalEvent && (NEO::DebugManager.flags.TrackNumCsrClientsOnSyncPoints.get() != 0)) {
+                signalEvent->setLatestUsedCmdQueue(this->cmdQImmediate);
+            }
             inputRet = executeCommandListImmediateWithFlushTask(performMigration, hasStallingCmds, hasRelaxedOrderingDependencies);
         } else {
             inputRet = executeCommandListImmediate(performMigration);
         }
     }
 
-    auto signalEvent = Event::fromHandle(hSignalEvent);
-
     if (signalEvent) {
         signalEvent->setCsr(this->csr);
 
diff --git a/level_zero/core/source/cmdqueue/cmdqueue.cpp b/level_zero/core/source/cmdqueue/cmdqueue.cpp
@@ -53,9 +53,7 @@ CommandQueueImp::CommandQueueImp(Device *device, NEO::CommandStreamReceiver *csr
 }
 
 ze_result_t CommandQueueImp::destroy() {
-    if (this->clientId != CommandQueue::clientNotRegistered) {
-        this->csr->unregisterClient();
-    }
+    unregisterCsrClient();
 
     if (commandStream.getCpuBase() != nullptr) {
         commandStream.replaceGraphicsAllocation(nullptr);
@@ -251,6 +249,13 @@ CommandQueue *CommandQueue::create(uint32_t productFamily, Device *device, NEO::
     return commandQueue;
 }
 
+void CommandQueueImp::unregisterCsrClient() {
+    if (getClientId() != CommandQueue::clientNotRegistered) {
+        this->csr->unregisterClient();
+        setClientId(CommandQueue::clientNotRegistered);
+    }
+}
+
 ze_command_queue_mode_t CommandQueueImp::getSynchronousMode() const {
     return desc.mode;
 }
diff --git a/level_zero/core/source/cmdqueue/cmdqueue.h b/level_zero/core/source/cmdqueue/cmdqueue.h
@@ -60,8 +60,9 @@ struct CommandQueue : _ze_command_queue_handle_t {
 
     bool peekIsCopyOnlyCommandQueue() const { return this->isCopyOnlyCommandQueue; }
 
-    uint32_t getClientId() { return this->clientId; }
+    uint32_t getClientId() const { return this->clientId; }
     void setClientId(uint32_t value) { this->clientId = value; }
+    virtual void unregisterCsrClient() = 0;
 
     static constexpr uint32_t clientNotRegistered = std::numeric_limits<uint32_t>::max();
 
diff --git a/level_zero/core/source/cmdqueue/cmdqueue_imp.h b/level_zero/core/source/cmdqueue/cmdqueue_imp.h
@@ -91,6 +91,7 @@ struct CommandQueueImp : public CommandQueue {
     void makeResidentAndMigrate(bool performMigration, const NEO::ResidencyContainer &residencyContainer) override;
     void printKernelsPrintfOutput(bool hangDetected);
     void checkAssert();
+    void unregisterCsrClient() override;
 
   protected:
     MOCKABLE_VIRTUAL NEO::SubmissionStatus submitBatchBuffer(size_t offset, NEO::ResidencyContainer &residencyContainer, void *endingCmdPtr,
diff --git a/level_zero/core/source/event/event.cpp b/level_zero/core/source/event/event.cpp
@@ -389,6 +389,7 @@ void Event::setIsCompleted() {
     if (this->isCompleted.load() == STATE_CLEARED) {
         this->isCompleted = STATE_SIGNALED;
     }
+    unsetCmdQueue(true);
 }
 
 void Event::enableInOrderExecMode(const NEO::TimestampPacketContainer &inOrderSyncNodes) {
@@ -401,4 +402,15 @@ void Event::enableInOrderExecMode(const NEO::TimestampPacketContainer &inOrderSy
     inOrderTimestampPacket->assignAndIncrementNodesRefCounts(inOrderSyncNodes);
 }
 
+void Event::setLatestUsedCmdQueue(CommandQueue *newCmdQ) {
+    this->latestUsedCmdQueue = newCmdQ;
+}
+
+void Event::unsetCmdQueue(bool unregisterClient) {
+    if (latestUsedCmdQueue && unregisterClient) {
+        latestUsedCmdQueue->unregisterCsrClient();
+    }
+    latestUsedCmdQueue = nullptr;
+}
+
 } // namespace L0
diff --git a/level_zero/core/source/event/event.h b/level_zero/core/source/event/event.h
@@ -36,6 +36,7 @@ struct EventPool;
 struct MetricStreamer;
 struct ContextImp;
 struct Context;
+struct CommandQueue;
 struct DriverHandle;
 struct DriverHandleImp;
 struct Device;
@@ -205,10 +206,13 @@ struct Event : _ze_event_handle_t {
         this->metricStreamer = metricStreamer;
     }
     void enableInOrderExecMode(const NEO::TimestampPacketContainer &inOrderSyncNodes);
+    void setLatestUsedCmdQueue(CommandQueue *newCmdQ);
 
   protected:
     Event(EventPool *eventPool, int index, Device *device) : device(device), eventPool(eventPool), index(index) {}
 
+    void unsetCmdQueue(bool unregisterClient);
+
     uint64_t globalStartTS = 1;
     uint64_t globalEndTS = 1;
     uint64_t contextStartTS = 1;
@@ -237,6 +241,7 @@ struct Event : _ze_event_handle_t {
     EventPool *eventPool = nullptr;
     Kernel *kernelWithPrintf = nullptr;
     std::unique_ptr<NEO::TimestampPacketContainer> inOrderTimestampPacket;
+    CommandQueue *latestUsedCmdQueue = nullptr;
 
     uint32_t maxKernelCount = 0;
     uint32_t kernelCount = 1u;
diff --git a/level_zero/core/source/event/event_impl.inl b/level_zero/core/source/event/event_impl.inl
@@ -161,6 +161,7 @@ void EventImp<TagSizeT>::handleSuccessfulHostSynchronization() {
         }
     }
     this->setIsCompleted();
+    unsetCmdQueue(true);
     for (auto &csr : csrs) {
         csr->getInternalAllocationStorage()->cleanAllocationList(csr->peekTaskCount(), NEO::AllocationUsage::TEMPORARY_ALLOCATION);
     }
@@ -396,6 +397,7 @@ ze_result_t EventImp<TagSizeT>::reset() {
         inOrderExecEvent = false;
         inOrderTimestampPacket->releaseNodes();
     }
+    unsetCmdQueue(false);
     this->resetCompletionStatus();
     this->resetDeviceCompletionData(false);
     this->l3FlushAppliedOnKernel.reset();
diff --git a/level_zero/core/test/unit_tests/sources/cmdlist/test_cmdlist_1.cpp b/level_zero/core/test/unit_tests/sources/cmdlist/test_cmdlist_1.cpp
@@ -10,6 +10,7 @@
 #include "shared/source/indirect_heap/indirect_heap.h"
 #include "shared/source/memory_manager/internal_allocation_storage.h"
 #include "shared/test/common/cmd_parse/gen_cmd_parse.h"
+#include "shared/test/common/helpers/debug_manager_state_restore.h"
 #include "shared/test/common/helpers/relaxed_ordering_commands_helper.h"
 #include "shared/test/common/helpers/unit_test_helper.h"
 #include "shared/test/common/libult/ult_command_stream_receiver.h"
@@ -1108,6 +1109,113 @@ HWTEST2_F(CommandListCreate, whenDispatchingThenPassNumCsrClients, IsAtLeastXeHp
     EXPECT_EQ(ultCsr->latestFlushedBatchBuffer.numCsrClients, ultCsr->getNumClients());
 }
 
+HWTEST_F(CommandListCreate, givenSignalEventWhenCallingSynchronizeThenUnregisterClient) {
+    ze_command_queue_desc_t desc = {};
+    desc.mode = ZE_COMMAND_QUEUE_MODE_SYNCHRONOUS;
+    ze_result_t returnValue;
+    std::unique_ptr<L0::CommandList> commandList(CommandList::createImmediate(productFamily, device, &desc, false, NEO::EngineGroupType::RenderCompute, returnValue));
+    ASSERT_NE(nullptr, commandList);
+    auto whiteBoxCmdList = static_cast<CommandList *>(commandList.get());
+
+    Mock<::L0::Kernel> kernel;
+    ze_group_count_t groupCount{1, 1, 1};
+    CmdListKernelLaunchParams launchParams = {};
+
+    auto ultCsr = static_cast<NEO::UltCommandStreamReceiver<FamilyType> *>(whiteBoxCmdList->csr);
+
+    ze_event_pool_desc_t eventPoolDesc = {};
+    eventPoolDesc.count = 3;
+
+    ze_event_desc_t eventDesc = {};
+
+    ze_event_handle_t event1 = nullptr;
+    ze_event_handle_t event2 = nullptr;
+    ze_event_handle_t event3 = nullptr;
+
+    std::unique_ptr<L0::EventPool> eventPool(EventPool::create(driverHandle.get(), context, 0, nullptr, &eventPoolDesc, returnValue));
+
+    ASSERT_EQ(ZE_RESULT_SUCCESS, eventPool->createEvent(&eventDesc, &event1));
+    ASSERT_EQ(ZE_RESULT_SUCCESS, eventPool->createEvent(&eventDesc, &event2));
+    ASSERT_EQ(ZE_RESULT_SUCCESS, eventPool->createEvent(&eventDesc, &event3));
+
+    EXPECT_EQ(ultCsr->getNumClients(), 0u);
+
+    {
+        commandList->appendLaunchKernel(kernel.toHandle(), &groupCount, event1, 0, nullptr, launchParams, false);
+        EXPECT_EQ(ultCsr->getNumClients(), 1u);
+
+        Event::fromHandle(event1)->setIsCompleted();
+
+        zeEventHostSynchronize(event1, std::numeric_limits<uint64_t>::max());
+        EXPECT_EQ(ultCsr->getNumClients(), 0u);
+    }
+
+    {
+        commandList->appendLaunchKernel(kernel.toHandle(), &groupCount, event2, 0, nullptr, launchParams, false);
+        EXPECT_EQ(ultCsr->getNumClients(), 1u);
+
+        *reinterpret_cast<uint32_t *>(Event::fromHandle(event2)->getHostAddress()) = static_cast<uint32_t>(Event::STATE_SIGNALED);
+
+        zeEventHostSynchronize(event2, std::numeric_limits<uint64_t>::max());
+        EXPECT_EQ(ultCsr->getNumClients(), 0u);
+    }
+
+    {
+        commandList->appendLaunchKernel(kernel.toHandle(), &groupCount, event3, 0, nullptr, launchParams, false);
+        EXPECT_EQ(ultCsr->getNumClients(), 1u);
+
+        zeEventHostReset(event3);
+
+        zeEventHostSynchronize(event3, 1);
+        EXPECT_EQ(ultCsr->getNumClients(), 1u);
+    }
+
+    zeEventDestroy(event1);
+    zeEventDestroy(event2);
+    zeEventDestroy(event3);
+}
+
+HWTEST_F(CommandListCreate, givenDebugFlagSetWhenCallingSynchronizeThenDontUnregister) {
+    DebugManagerStateRestore restore;
+    DebugManager.flags.TrackNumCsrClientsOnSyncPoints.set(0);
+
+    ze_command_queue_desc_t desc = {};
+    desc.mode = ZE_COMMAND_QUEUE_MODE_SYNCHRONOUS;
+    ze_result_t returnValue;
+    std::unique_ptr<L0::CommandList> commandList(CommandList::createImmediate(productFamily, device, &desc, false, NEO::EngineGroupType::RenderCompute, returnValue));
+    ASSERT_NE(nullptr, commandList);
+    auto whiteBoxCmdList = static_cast<CommandList *>(commandList.get());
+
+    Mock<::L0::Kernel> kernel;
+    ze_group_count_t groupCount{1, 1, 1};
+    CmdListKernelLaunchParams launchParams = {};
+
+    auto ultCsr = static_cast<NEO::UltCommandStreamReceiver<FamilyType> *>(whiteBoxCmdList->csr);
+
+    ze_event_pool_desc_t eventPoolDesc = {};
+    eventPoolDesc.count = 1;
+
+    ze_event_desc_t eventDesc = {};
+
+    ze_event_handle_t event = nullptr;
+
+    std::unique_ptr<L0::EventPool> eventPool(EventPool::create(driverHandle.get(), context, 0, nullptr, &eventPoolDesc, returnValue));
+
+    ASSERT_EQ(ZE_RESULT_SUCCESS, eventPool->createEvent(&eventDesc, &event));
+
+    EXPECT_EQ(ultCsr->getNumClients(), 0u);
+    commandList->appendLaunchKernel(kernel.toHandle(), &groupCount, event, 0, nullptr, launchParams, false);
+    EXPECT_EQ(ultCsr->getNumClients(), 1u);
+
+    Event::fromHandle(event)->setIsCompleted();
+
+    zeEventHostSynchronize(event, std::numeric_limits<uint64_t>::max());
+
+    EXPECT_EQ(ultCsr->getNumClients(), 1u);
+
+    zeEventDestroy(event);
+}
+
 HWTEST2_F(CommandListCreate, givenDirectSubmissionAndImmCmdListWhenDispatchingThenPassRelaxedOrderingDependenciesInfo, IsAtLeastXeHpcCore) {
     DebugManagerStateRestore restore;
     DebugManager.flags.DirectSubmissionRelaxedOrdering.set(1);
diff --git a/shared/source/debug_settings/debug_variables_base.inl b/shared/source/debug_settings/debug_variables_base.inl
@@ -241,6 +241,7 @@ DECLARE_DEBUG_VARIABLE(int64_t, OverrideEventSynchronizeTimeout, -1, "-1: defaul
 DECLARE_DEBUG_VARIABLE(int32_t, ForceTlbFlush, -1, "-1: default,  0: Tlb flush disabled, 1: Tlb Flush enabled")
 DECLARE_DEBUG_VARIABLE(int32_t, DebugSetMemoryDiagnosticsDelay, -1, "-1: default, >=0: delay time in minutes necessary for completion of Memory diagnostics")
 DECLARE_DEBUG_VARIABLE(int32_t, EnableDeviceStateVerification, -1, "-1: default, 0: disable, 1: enable check of device state before submit on Windows")
+DECLARE_DEBUG_VARIABLE(int32_t, TrackNumCsrClientsOnSyncPoints, -1, "-1: default, 0: Disabled, 1: If set, synchronization points like zeEventHostSynchronize will unregister CmdQ from CSR clients")
 
 /*LOGGING FLAGS*/
 DECLARE_DEBUG_VARIABLE(int32_t, PrintDriverDiagnostics, -1, "prints driver diagnostics messages to standard output, value corresponds to hint level")
diff --git a/shared/test/common/test_files/igdrcl.config b/shared/test/common/test_files/igdrcl.config
@@ -528,4 +528,5 @@ OverrideHwIpVersion = -1
 PrintGlobalTimestampInNs = 0
 EnableDeviceStateVerification = -1
 VfBarResourceAllocationWa = 1
+TrackNumCsrClientsOnSyncPoints = -1
 # Please don't edit below this line

Original file line number	Diff line number	Diff line change
`@@ -161,6 +161,7 @@ void EventImp<TagSizeT>::handleSuccessfulHostSynchronization() {`
`161`	`161`	`}`
`162`	`162`	`}`
`163`	`163`	`this->setIsCompleted();`
	`164`	`+ unsetCmdQueue(true);`
`164`	`165`	`for (auto &csr : csrs) {`
`165`	`166`	`csr->getInternalAllocationStorage()->cleanAllocationList(csr->peekTaskCount(), NEO::AllocationUsage::TEMPORARY_ALLOCATION);`
`166`	`167`	`}`
`@@ -396,6 +397,7 @@ ze_result_t EventImp<TagSizeT>::reset() {`
`396`	`397`	`inOrderExecEvent = false;`
`397`	`398`	`inOrderTimestampPacket->releaseNodes();`
`398`	`399`	`}`
	`400`	`+ unsetCmdQueue(false);`
`399`	`401`	`this->resetCompletionStatus();`
`400`	`402`	`this->resetDeviceCompletionData(false);`
`401`	`403`	`this->l3FlushAppliedOnKernel.reset();`