From 972a79aaaee237db8d9f3b479b3bec8697b4f2b4 Mon Sep 17 00:00:00 2001 From: "Milczarek, Slawomir" Date: Fri, 10 May 2019 17:29:06 +0200 Subject: [PATCH] Reduced a scope of the lock for AUB file stream Related-To: NEO-2747 Change-Id: Ic164900f5898df35af74ccff9c31f8296dcf12fd --- runtime/aub_mem_dump/aub_mem_dump.h | 4 +- .../aub_command_stream_receiver.cpp | 4 +- .../aub_command_stream_receiver_hw_base.inl | 11 ++++-- .../command_stream/aub_file_stream_tests.cpp | 39 +++++++++++++++++-- unit_tests/mocks/mock_aub_file_stream.h | 2 +- 5 files changed, 48 insertions(+), 12 deletions(-) diff --git a/runtime/aub_mem_dump/aub_mem_dump.h b/runtime/aub_mem_dump/aub_mem_dump.h index b1df12c2e5..1c208c26e1 100644 --- a/runtime/aub_mem_dump/aub_mem_dump.h +++ b/runtime/aub_mem_dump/aub_mem_dump.h @@ -139,11 +139,11 @@ struct AubFileStream : public AubStream { MOCKABLE_VIRTUAL void expectMemory(uint64_t physAddress, const void *memory, size_t size, uint32_t addressSpace, uint32_t compareOperation); MOCKABLE_VIRTUAL bool addComment(const char *message); - MOCKABLE_VIRTUAL std::unique_lock lockStream(); + MOCKABLE_VIRTUAL std::unique_lock lockStream(); std::ofstream fileHandle; std::string fileName; - std::recursive_mutex mutex; + std::mutex mutex; }; template diff --git a/runtime/command_stream/aub_command_stream_receiver.cpp b/runtime/command_stream/aub_command_stream_receiver.cpp index 7981b9c6ec..59e0680df8 100644 --- a/runtime/command_stream/aub_command_stream_receiver.cpp +++ b/runtime/command_stream/aub_command_stream_receiver.cpp @@ -277,8 +277,8 @@ bool AubFileStream::addComment(const char *message) { return true; } -std::unique_lock AubFileStream::lockStream() { - return std::unique_lock(mutex); +std::unique_lock AubFileStream::lockStream() { + return std::unique_lock(mutex); } } // namespace AubMemDump diff --git a/runtime/command_stream/aub_command_stream_receiver_hw_base.inl b/runtime/command_stream/aub_command_stream_receiver_hw_base.inl index c0d8946307..55c9245813 100644 --- a/runtime/command_stream/aub_command_stream_receiver_hw_base.inl +++ b/runtime/command_stream/aub_command_stream_receiver_hw_base.inl @@ -155,6 +155,8 @@ const std::string AUBCommandStreamReceiverHw::getFileName() { template void AUBCommandStreamReceiverHw::initializeEngine() { + auto streamLocked = getAubStream()->lockStream(); + if (hardwareContextController) { hardwareContextController->initialize(); return; @@ -290,8 +292,6 @@ FlushStamp AUBCommandStreamReceiverHw::flush(BatchBuffer &batchBuffer } } - auto streamLocked = getAubStream()->lockStream(); - initializeEngine(); // Write our batch buffer @@ -380,6 +380,8 @@ bool AUBCommandStreamReceiverHw::addPatchInfoComments() { template void AUBCommandStreamReceiverHw::submitBatchBuffer(uint64_t batchBufferGpuAddress, const void *batchBuffer, size_t batchBufferSize, uint32_t memoryBank, uint64_t entryBits) { + auto streamLocked = getAubStream()->lockStream(); + if (hardwareContextController) { if (batchBufferSize) { hardwareContextController->submit(batchBufferGpuAddress, batchBuffer, batchBufferSize, memoryBank, MemoryConstants::pageSize64k); @@ -619,7 +621,6 @@ bool AUBCommandStreamReceiverHw::writeMemory(GraphicsAllocation &gfxA } bool ownsLock = !gfxAllocation.isLocked(); - uint64_t gpuAddress; void *cpuAddress; size_t size; @@ -627,12 +628,16 @@ bool AUBCommandStreamReceiverHw::writeMemory(GraphicsAllocation &gfxA return false; } + auto streamLocked = getAubStream()->lockStream(); + if (aubManager) { this->writeMemoryWithAubManager(gfxAllocation); } else { writeMemory(gpuAddress, cpuAddress, size, this->getMemoryBank(&gfxAllocation), this->getPPGTTAdditionalBits(&gfxAllocation)); } + streamLocked.unlock(); + if (gfxAllocation.isLocked() && ownsLock) { this->getMemoryManager()->unlockResource(&gfxAllocation); } diff --git a/unit_tests/command_stream/aub_file_stream_tests.cpp b/unit_tests/command_stream/aub_file_stream_tests.cpp index 69aa301977..97b3388168 100644 --- a/unit_tests/command_stream/aub_file_stream_tests.cpp +++ b/unit_tests/command_stream/aub_file_stream_tests.cpp @@ -171,18 +171,49 @@ HWTEST_F(AubFileStreamTests, givenAubCommandStreamReceiverWhenReopenFileIsCalled EXPECT_TRUE(mockAubFileStream->lockStreamCalled); } -HWTEST_F(AubFileStreamTests, givenAubCommandStreamReceiverWhenFlushIsCalledThenFileStreamShouldBeLocked) { +HWTEST_F(AubFileStreamTests, givenAubCommandStreamReceiverWhenInitializeEngineIsCalledThenFileStreamShouldBeLocked) { + auto mockAubFileStream = std::make_unique(); + auto aubExecutionEnvironment = getEnvironment>(true, true, true); + auto aubCsr = aubExecutionEnvironment->template getCsr>(); + + aubCsr->stream = static_cast(mockAubFileStream.get()); + + aubCsr->initializeEngine(); + EXPECT_TRUE(mockAubFileStream->lockStreamCalled); +} + +HWTEST_F(AubFileStreamTests, givenAubCommandStreamReceiverWhenSubmitBatchBufferIsCalledThenFileStreamShouldBeLocked) { auto mockAubFileStream = std::make_unique(); auto aubExecutionEnvironment = getEnvironment>(true, true, true); auto aubCsr = aubExecutionEnvironment->template getCsr>(); LinearStream cs(aubExecutionEnvironment->commandBuffer); + BatchBuffer batchBuffer{cs.getGraphicsAllocation(), 0, 0, nullptr, false, false, QueueThrottle::MEDIUM, cs.getUsed(), &cs}; aubCsr->stream = static_cast(mockAubFileStream.get()); - BatchBuffer batchBuffer{cs.getGraphicsAllocation(), 0, 0, nullptr, false, false, QueueThrottle::MEDIUM, cs.getUsed(), &cs}; - ResidencyContainer allocationsForResidency = {}; + auto pBatchBuffer = ptrOffset(batchBuffer.commandBufferAllocation->getUnderlyingBuffer(), batchBuffer.startOffset); + auto batchBufferGpuAddress = ptrOffset(batchBuffer.commandBufferAllocation->getGpuAddress(), batchBuffer.startOffset); + auto currentOffset = batchBuffer.usedSize; + auto sizeBatchBuffer = currentOffset - batchBuffer.startOffset; - aubCsr->flush(batchBuffer, allocationsForResidency); + aubCsr->initializeEngine(); + mockAubFileStream->lockStreamCalled = false; + + aubCsr->submitBatchBuffer(batchBufferGpuAddress, pBatchBuffer, sizeBatchBuffer, aubCsr->getMemoryBank(batchBuffer.commandBufferAllocation), + aubCsr->getPPGTTAdditionalBits(batchBuffer.commandBufferAllocation)); + EXPECT_TRUE(mockAubFileStream->lockStreamCalled); +} + +HWTEST_F(AubFileStreamTests, givenAubCommandStreamReceiverWhenWriteMemoryIsCalledThenFileStreamShouldBeLocked) { + auto mockAubFileStream = std::make_unique(); + auto aubExecutionEnvironment = getEnvironment>(true, true, true); + auto aubCsr = aubExecutionEnvironment->template getCsr>(); + + aubCsr->stream = static_cast(mockAubFileStream.get()); + + MockGraphicsAllocation allocation(reinterpret_cast(0x1000), 0x1000); + + aubCsr->writeMemory(allocation); EXPECT_TRUE(mockAubFileStream->lockStreamCalled); } diff --git a/unit_tests/mocks/mock_aub_file_stream.h b/unit_tests/mocks/mock_aub_file_stream.h index 6cb0177bc8..941c48bf1c 100644 --- a/unit_tests/mocks/mock_aub_file_stream.h +++ b/unit_tests/mocks/mock_aub_file_stream.h @@ -42,7 +42,7 @@ struct MockAubFileStream : public AUBCommandStreamReceiver::AubFileStream { void flush() override { flushCalled = true; } - std::unique_lock lockStream() override { + std::unique_lock lockStream() override { lockStreamCalled = true; return AUBCommandStreamReceiver::AubFileStream::lockStream(); }