fix: do not wait with locked csr

Related-To: NEO-16532, HSD-18043732674 Signed-off-by: Maciej Plewka <maciej.plewka@intel.com>

Author: MaciejPlewka

level_zero/core/source/cmdqueue/cmdqueue_hw.h

@@ -91,6 +91,7 @@ struct CommandQueueHw : public CommandQueueImp {
  NEO::ScratchSpaceController *scratchSpaceController = nullptr;
  NEO::GraphicsAllocation *globalStatelessAllocation = nullptr;
  std::unique_lock<std::mutex> *outerLockForIndirect = nullptr;
+ std::unique_lock<NEO::CommandStreamReceiver::MutexType> *lockCSR = nullptr;
 
  NEO::PreemptionMode preemptionMode{};
  NEO::PreemptionMode statePreemption{};
@@ -219,8 +220,10 @@ struct CommandQueueHw : public CommandQueueImp {
  inline void updateTaskCountAndPostSync(bool isDispatchTaskCountPostSyncRequired,
  uint32_t numCommandLists,
  ze_command_list_handle_t *commandListHandles);
- inline ze_result_t waitForCommandQueueCompletionAndCleanHeapContainer();
- inline ze_result_t handleSubmissionAndCompletionResults(NEO::SubmissionStatus submitRet, ze_result_t completionRet);
+ ze_result_t waitForCommandQueueCompletion(CommandListExecutionContext &ctx);
+ inline ze_result_t handleNonParentImmediateStream(ze_fence_handle_t hFence, CommandListExecutionContext &ctx, uint32_t numCommandLists,
+ ze_command_list_handle_t *phCommandLists, NEO::LinearStream *streamForDispatch, bool isFenceRequired);
+ inline ze_result_t handleSubmission(NEO::SubmissionStatus submitRet);
  inline size_t estimatePipelineSelectCmdSizeForMultipleCommandLists(NEO::StreamProperties &csrState,
  const NEO::StreamProperties &cmdListRequired,
  const NEO::StreamProperties &cmdListFinal,

level_zero/core/source/cmdqueue/cmdqueue_hw.inl

@@ -128,7 +128,7 @@ ze_result_t CommandQueueHw<gfxCoreFamily>::executeCommandLists(
  !this->commandQueueDebugCmdsProgrammed &&
  device->getL0Debugger();
  ctx.lockScratchController = lockScratchController;
-
+ ctx.lockCSR = &lockCSR;
  if (this->isCopyOnlyCommandQueue) {
  ret = this->executeCommandListsCopyOnly(ctx, numCommandLists, phCommandLists, hFence, parentImmediateCommandlistLinearStream);
  } else if (this->heaplessStateInitEnabled) {
@@ -230,18 +230,13 @@ ze_result_t CommandQueueHw<gfxCoreFamily>::executeCommandListsRegularHeapless(
  this->dispatchPatchPreambleEnding(ctx);
 
  if (!ctx.containsParentImmediateStream) {
- this->assignCsrTaskCountToFenceIfAvailable(hFence);
- this->dispatchTaskCountPostSyncRegular(ctx.isDispatchTaskCountPostSyncRequired, *streamForDispatch);
-
- auto submitResult = this->prepareAndSubmitBatchBuffer(ctx, *streamForDispatch);
- this->updateTaskCountAndPostSync(ctx.isDispatchTaskCountPostSyncRequired, numCommandLists, commandListHandles);
-
- this->csr->makeSurfacePackNonResident(this->csr->getResidencyAllocations(), false);
-
- auto completionResult = this->waitForCommandQueueCompletionAndCleanHeapContainer();
- retVal = this->handleSubmissionAndCompletionResults(submitResult, completionResult);
- this->csr->getResidencyAllocations().clear();
+ retVal = handleNonParentImmediateStream(hFence, ctx, numCommandLists, commandListHandles, streamForDispatch, false);
+ if (retVal != ZE_RESULT_SUCCESS) {
+ return retVal;
+ }
+ retVal = this->waitForCommandQueueCompletion(ctx);
  }
+
  return retVal;
 }
 
@@ -458,21 +453,17 @@ ze_result_t CommandQueueHw<gfxCoreFamily>::executeCommandListsRegular(
  this->csr->setPreemptionMode(ctx.statePreemption);
 
  if (!ctx.containsParentImmediateStream) {
- this->assignCsrTaskCountToFenceIfAvailable(hFence);
- this->dispatchTaskCountPostSyncRegular(ctx.isDispatchTaskCountPostSyncRequired, *streamForDispatch);
-
- auto submitResult = this->prepareAndSubmitBatchBuffer(ctx, *streamForDispatch);
- this->updateTaskCountAndPostSync(ctx.isDispatchTaskCountPostSyncRequired, numCommandLists, commandListHandles);
-
- this->csr->makeSurfacePackNonResident(this->csr->getResidencyAllocations(), false);
-
- auto completionResult = this->waitForCommandQueueCompletionAndCleanHeapContainer();
- retVal = this->handleSubmissionAndCompletionResults(submitResult, completionResult);
- this->csr->getResidencyAllocations().clear();
+ retVal = handleNonParentImmediateStream(hFence, ctx, numCommandLists, commandListHandles, streamForDispatch, false);
  }
 
  this->stateChanges.clear();
  this->currentStateChangeIndex = 0;
+ if (retVal != ZE_RESULT_SUCCESS) {
+ return retVal;
+ }
+ if (!ctx.containsParentImmediateStream) {
+ return this->waitForCommandQueueCompletion(ctx);
+ }
  return retVal;
 }
 
@@ -543,19 +534,12 @@ ze_result_t CommandQueueHw<gfxCoreFamily>::executeCommandListsCopyOnly(
  this->makeCsrTagAllocationResident();
 
  if (!ctx.containsParentImmediateStream) {
- this->assignCsrTaskCountToFenceIfAvailable(hFence);
- this->dispatchTaskCountPostSyncByMiFlushDw(ctx.isDispatchTaskCountPostSyncRequired, fenceRequired, *streamForDispatch);
-
- auto submitResult = this->prepareAndSubmitBatchBuffer(ctx, *streamForDispatch);
- this->updateTaskCountAndPostSync(ctx.isDispatchTaskCountPostSyncRequired, numCommandLists, phCommandLists);
-
- this->csr->makeSurfacePackNonResident(this->csr->getResidencyAllocations(), false);
-
- auto completionResult = this->waitForCommandQueueCompletionAndCleanHeapContainer();
- retVal = this->handleSubmissionAndCompletionResults(submitResult, completionResult);
- this->csr->getResidencyAllocations().clear();
+ retVal = handleNonParentImmediateStream(hFence, ctx, numCommandLists, phCommandLists, streamForDispatch, fenceRequired);
+ if (retVal != ZE_RESULT_SUCCESS) {
+ return retVal;
+ }
+ retVal = this->waitForCommandQueueCompletion(ctx);
  }
-
  return retVal;
 }
 
@@ -1689,39 +1673,52 @@ void CommandQueueHw<gfxCoreFamily>::updateTaskCountAndPostSync(bool isDispatchTa
 }
 
 template <GFXCORE_FAMILY gfxCoreFamily>
-ze_result_t CommandQueueHw<gfxCoreFamily>::waitForCommandQueueCompletionAndCleanHeapContainer() {
-
+ze_result_t CommandQueueHw<gfxCoreFamily>::waitForCommandQueueCompletion(CommandListExecutionContext &ctx) {
  ze_result_t ret = ZE_RESULT_SUCCESS;
-
  if (this->isSynchronousMode()) {
+ ctx.lockCSR->unlock();
  if (const auto syncRet = this->synchronize(std::numeric_limits<uint64_t>::max()); syncRet == ZE_RESULT_ERROR_DEVICE_LOST) {
  ret = syncRet;
  }
  }
- this->heapContainer.clear();
 
  return ret;
 }
 
 template <GFXCORE_FAMILY gfxCoreFamily>
-ze_result_t CommandQueueHw<gfxCoreFamily>::handleSubmissionAndCompletionResults(
- NEO::SubmissionStatus submitRet,
- ze_result_t completionRet) {
+ze_result_t CommandQueueHw<gfxCoreFamily>::handleNonParentImmediateStream(ze_fence_handle_t hFence, CommandListExecutionContext &ctx, uint32_t numCommandLists,
+ ze_command_list_handle_t *phCommandLists, NEO::LinearStream *streamForDispatch, bool isFenceRequired) {
+ this->assignCsrTaskCountToFenceIfAvailable(hFence);
+ if (!this->isCopyOnlyCommandQueue) {
+ this->dispatchTaskCountPostSyncRegular(ctx.isDispatchTaskCountPostSyncRequired, *streamForDispatch);
+ } else {
+ this->dispatchTaskCountPostSyncByMiFlushDw(ctx.isDispatchTaskCountPostSyncRequired, isFenceRequired, *streamForDispatch);
+ }
+ auto submitResult = this->prepareAndSubmitBatchBuffer(ctx, *streamForDispatch);
+ this->updateTaskCountAndPostSync(ctx.isDispatchTaskCountPostSyncRequired, numCommandLists, phCommandLists);
 
- if ((submitRet != NEO::SubmissionStatus::success) || (completionRet == ZE_RESULT_ERROR_DEVICE_LOST)) {
+ this->csr->makeSurfacePackNonResident(this->csr->getResidencyAllocations(), false);
+
+ auto retVal = this->handleSubmission(submitResult);
+ this->csr->getResidencyAllocations().clear();
+ this->heapContainer.clear();
+ return retVal;
+}
+
+template <GFXCORE_FAMILY gfxCoreFamily>
+ze_result_t CommandQueueHw<gfxCoreFamily>::handleSubmission(
+ NEO::SubmissionStatus submitRet) {
+ if (submitRet != NEO::SubmissionStatus::success) {
  for (auto &gfx : this->csr->getResidencyAllocations()) {
  if (this->csr->peekLatestFlushedTaskCount() == 0) {
  gfx->releaseUsageInOsContext(this->csr->getOsContext().getContextId());
  } else {
  gfx->updateTaskCount(this->csr->peekLatestFlushedTaskCount(), this->csr->getOsContext().getContextId());
  }
  }
- if (completionRet != ZE_RESULT_ERROR_DEVICE_LOST) {
- completionRet = getErrorCodeForSubmissionStatus(submitRet);
- }
+ return getErrorCodeForSubmissionStatus(submitRet);
  }
-
- return completionRet;
+ return ZE_RESULT_SUCCESS;
 }
 
 template <GFXCORE_FAMILY gfxCoreFamily>

level_zero/core/test/unit_tests/mocks/mock_cmdqueue.h

@@ -99,6 +99,7 @@ struct MockCommandQueueHw : public L0::CommandQueueHw<gfxCoreFamily> {
  using BaseClass::prepareAndSubmitBatchBuffer;
  using BaseClass::printfKernelContainer;
  using BaseClass::startingCmdBuffer;
+ using BaseClass::waitForCommandQueueCompletion;
  using L0::CommandQueue::activeSubDevices;
  using L0::CommandQueue::cmdListHeapAddressModel;
  using L0::CommandQueue::dispatchCmdListBatchBufferAsPrimary;

level_zero/core/test/unit_tests/sources/cmdqueue/test_cmdqueue_2.cpp

@@ -1232,5 +1232,19 @@ HWTEST_F(HostFunctionsCmdPatchTests, givenHostFunctionPatchCommandsWhenPatchComm
  commandQueue->csr = oldCsr;
 }
 
+HWTEST_F(CommandQueueSynchronizeTest, givenCmdQueueWhenCallWaitForCommandQueueCompletionThenSynchronizeWithoutLockOnCsr) {
+ DebugManagerStateRestore restore;
+ auto &csr = neoDevice->getUltCommandStreamReceiver<FamilyType>();
+
+ ze_command_queue_desc_t desc = {};
+ desc.mode = ZE_COMMAND_QUEUE_MODE_SYNCHRONOUS;
+ auto commandQueue = std::make_unique<MockCommandQueueHw<FamilyType::gfxCoreFamily>>(device, &csr, &desc);
+
+ auto ownership = csr.obtainUniqueOwnership();
+ auto ctx = typename MockCommandQueueHw<FamilyType::gfxCoreFamily>::CommandListExecutionContext();
+ ctx.lockCSR = &ownership;
+ commandQueue->waitForCommandQueueCompletion(ctx);
+ EXPECT_FALSE(ownership.owns_lock());
+}
 } // namespace ult
 } // namespace L0