From f5513b6a1daea43866061f5330b2c28beb241021 Mon Sep 17 00:00:00 2001 From: "Zdanowicz, Zbigniew" Date: Fri, 26 Jan 2018 16:53:18 +0100 Subject: [PATCH] Handle host pointer not meeting memory manager criteria Change-Id: I65eec6083f1d8bb7b5f46e1a2e015aa6fd7f3d9f --- Jenkinsfile | 2 +- runtime/mem_obj/buffer.cpp | 55 +++++++++-------- runtime/mem_obj/buffer.h | 6 +- runtime/memory_manager/memory_manager.h | 2 +- unit_tests/api/cl_api_tests.cpp | 39 +++++++++++- unit_tests/api/cl_api_tests.h | 27 ++++++++- unit_tests/api/cl_create_buffer_tests.cpp | 72 ++++++++++++++++++++++- unit_tests/mocks/mock_device.cpp | 7 +++ unit_tests/mocks/mock_device.h | 7 ++- unit_tests/mocks/mock_memory_manager.h | 24 +++++++- 10 files changed, 207 insertions(+), 34 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 9e14fd2f3b..0d77450150 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -2,4 +2,4 @@ neoDependenciesRev='730226-753' strategy='EQUAL' allowedF=49 -allowedCD=367 +allowedCD=366 diff --git a/runtime/mem_obj/buffer.cpp b/runtime/mem_obj/buffer.cpp index 2bd127653b..13f7badccb 100644 --- a/runtime/mem_obj/buffer.cpp +++ b/runtime/mem_obj/buffer.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -94,9 +94,9 @@ Buffer *Buffer::create(Context *context, bool copyMemoryFromHostPtr = false; MemoryManager *memoryManager = context->getMemoryManager(); - DEBUG_BREAK_IF(!memoryManager); + UNRECOVERABLE_IF(!memoryManager); - checkMemory(flags, size, hostPtr, errcodeRet, zeroCopy, allocateMemory, copyMemoryFromHostPtr); + checkMemory(flags, size, hostPtr, errcodeRet, zeroCopy, allocateMemory, copyMemoryFromHostPtr, memoryManager); if (hostPtr && context->isProvidingPerformanceHints()) { if (zeroCopy) { @@ -113,30 +113,27 @@ Buffer *Buffer::create(Context *context, } if (errcodeRet == CL_SUCCESS) { while (true) { - if (memoryManager) { - if (flags & CL_MEM_USE_HOST_PTR) { - memory = context->getSVMAllocsManager()->getSVMAlloc(hostPtr); - if (memory) { - zeroCopy = true; - isHostPtrSVM = true; - copyMemoryFromHostPtr = false; - allocateMemory = false; - } + if (flags & CL_MEM_USE_HOST_PTR) { + memory = context->getSVMAllocsManager()->getSVMAlloc(hostPtr); + if (memory) { + zeroCopy = true; + isHostPtrSVM = true; + copyMemoryFromHostPtr = false; + allocateMemory = false; } - if (allocateMemory) { - memory = memoryManager->createGraphicsAllocationWithRequiredBitness(size, nullptr, true); - if (context->isProvidingPerformanceHints()) { - context->providePerformanceHint(CL_CONTEXT_DIAGNOSTICS_LEVEL_GOOD_INTEL, CL_BUFFER_NEEDS_ALLOCATE_MEMORY); - } - } else { - if (!memory) { - //Host ptr was not created with clSVMAlloc - create graphic allocation - memory = memoryManager->createGraphicsAllocationWithRequiredBitness(size, hostPtr, true); - } + } + if (allocateMemory) { + memory = memoryManager->createGraphicsAllocationWithRequiredBitness(size, nullptr, true); + if (context->isProvidingPerformanceHints()) { + context->providePerformanceHint(CL_CONTEXT_DIAGNOSTICS_LEVEL_GOOD_INTEL, CL_BUFFER_NEEDS_ALLOCATE_MEMORY); } } else { - DEBUG_BREAK_IF(true); + if (!memory) { + //Host ptr was not created with clSVMAlloc - create graphic allocation + memory = memoryManager->createGraphicsAllocationWithRequiredBitness(size, hostPtr, true); + } } + if (!memory) { errcodeRet = CL_OUT_OF_HOST_MEMORY; break; @@ -191,15 +188,23 @@ void Buffer::checkMemory(cl_mem_flags flags, cl_int &errcodeRet, bool &isZeroCopy, bool &allocateMemory, - bool ©MemoryFromHostPtr) { + bool ©MemoryFromHostPtr, + MemoryManager *memMngr) { errcodeRet = CL_SUCCESS; isZeroCopy = false; allocateMemory = false; copyMemoryFromHostPtr = false; + uintptr_t minAddress = 0; + auto memRestrictions = memMngr->getAlignedMallocRestrictions(); + if (memRestrictions) { + minAddress = memRestrictions->minAddress; + } if (flags & CL_MEM_USE_HOST_PTR) { if (hostPtr) { - if (alignUp(hostPtr, MemoryConstants::cacheLineSize) != hostPtr || alignUp(size, MemoryConstants::cacheLineSize) != size) { + if (alignUp(hostPtr, MemoryConstants::cacheLineSize) != hostPtr || + alignUp(size, MemoryConstants::cacheLineSize) != size || + minAddress > reinterpret_cast(hostPtr)) { allocateMemory = true; isZeroCopy = false; copyMemoryFromHostPtr = true; diff --git a/runtime/mem_obj/buffer.h b/runtime/mem_obj/buffer.h index b9664e33a4..4ceee0ab8b 100644 --- a/runtime/mem_obj/buffer.h +++ b/runtime/mem_obj/buffer.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -28,6 +28,7 @@ namespace OCLRT { class Buffer; +class MemoryManager; typedef Buffer *(*BufferCreatFunc)(Context *context, cl_mem_flags flags, @@ -125,7 +126,8 @@ class Buffer : public MemObj { cl_int &errcodeRet, bool &isZeroCopy, bool &allocateMemory, - bool ©MemoryFromHostPtr); + bool ©MemoryFromHostPtr, + MemoryManager *memMngr); }; template diff --git a/runtime/memory_manager/memory_manager.h b/runtime/memory_manager/memory_manager.h index 6b33d2da6d..ed7940349f 100644 --- a/runtime/memory_manager/memory_manager.h +++ b/runtime/memory_manager/memory_manager.h @@ -78,7 +78,7 @@ class MemoryManager { MemoryManager(bool enable64kbpages); virtual ~MemoryManager(); - void *allocateSystemMemory(size_t size, size_t alignment); + MOCKABLE_VIRTUAL void *allocateSystemMemory(size_t size, size_t alignment); virtual GraphicsAllocation *allocateGraphicsMemory(size_t size) { return allocateGraphicsMemory(size, static_cast(0u)); diff --git a/unit_tests/api/cl_api_tests.cpp b/unit_tests/api/cl_api_tests.cpp index 845ee0c7fb..303ab04970 100644 --- a/unit_tests/api/cl_api_tests.cpp +++ b/unit_tests/api/cl_api_tests.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -24,7 +24,9 @@ #include "runtime/command_queue/command_queue.h" #include "runtime/helpers/options.h" #include "unit_tests/mocks/mock_context.h" +#include "unit_tests/mocks/mock_device.h" #include "unit_tests/mocks/mock_kernel.h" +#include "unit_tests/mocks/mock_memory_manager.h" namespace OCLRT { @@ -63,4 +65,39 @@ void api_fixture::TearDown() { PlatformFixture::TearDown(); MemoryManagementFixture::TearDown(); } + +void api_fixture_using_aligned_memory_manager::SetUp() { + retVal = CL_SUCCESS; + retSize = 0; + + MemoryManagementFixture::SetUp(); + + device = Device::create(*platformDevices); + Device *devPtr = reinterpret_cast(device); + cl_device_id clDevice = devPtr; + + context = Context::create(nullptr, DeviceVector(&clDevice, 1), nullptr, nullptr, retVal); + EXPECT_EQ(CL_SUCCESS, retVal); + Context *ctxPtr = reinterpret_cast(context); + + commandQueue = new CommandQueue(context, devPtr, 0); + + program = new MockProgram(ctxPtr); + Program *prgPtr = reinterpret_cast(program); + + kernel = new MockKernel(prgPtr, *program->MockProgram::getKernelInfo(), *devPtr); + ASSERT_NE(nullptr, kernel); + BuiltInFixture::SetUp(devPtr); +} + +void api_fixture_using_aligned_memory_manager::TearDown() { + delete kernel; + delete commandQueue; + context->release(); + program->release(); + delete device; + + BuiltInFixture::TearDown(); + MemoryManagementFixture::TearDown(); +} } // namespace OCLRT diff --git a/unit_tests/api/cl_api_tests.h b/unit_tests/api/cl_api_tests.h index 8df33d938c..303ab41376 100644 --- a/unit_tests/api/cl_api_tests.h +++ b/unit_tests/api/cl_api_tests.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -26,6 +26,9 @@ #include "unit_tests/fixtures/platform_fixture.h" #include "unit_tests/fixtures/built_in_fixture.h" #include "runtime/api/api.h" +#include "test.h" + +#include namespace OCLRT { @@ -33,6 +36,7 @@ class CommandQueue; class Context; class MockKernel; class MockProgram; +class MockAlignedMallocManagerDevice; struct api_fixture : public MemoryManagementFixture, public PlatformFixture, @@ -65,4 +69,25 @@ struct api_tests : public api_fixture, api_fixture::TearDown(); } }; + +struct api_fixture_using_aligned_memory_manager : public MemoryManagementFixture, + public BuiltInFixture { + using BuiltInFixture::SetUp; + + public: + virtual void SetUp(); + virtual void TearDown(); + + cl_int retVal; + size_t retSize; + + CommandQueue *commandQueue; + Context *context; + MockKernel *kernel; + MockProgram *program; + MockAlignedMallocManagerDevice *device; +}; + +using api_test_using_aligned_memory_manager = Test; + } // namespace OCLRT diff --git a/unit_tests/api/cl_create_buffer_tests.cpp b/unit_tests/api/cl_create_buffer_tests.cpp index 24e32d0ab2..668b0ebbe4 100644 --- a/unit_tests/api/cl_create_buffer_tests.cpp +++ b/unit_tests/api/cl_create_buffer_tests.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Intel Corporation + * Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -22,6 +22,12 @@ #include "cl_api_tests.h" #include "runtime/context/context.h" +#include "runtime/command_queue/command_queue.h" +#include "runtime/mem_obj/buffer.h" +#include "unit_tests/mocks/mock_device.h" +#include "unit_tests/mocks/mock_kernel.h" +#include "unit_tests/mocks/mock_memory_manager.h" +#include "unit_tests/mocks/mock_program.h" using namespace OCLRT; @@ -174,4 +180,68 @@ TEST_F(clCreateBufferTests, noRet) { delete[] pHostMem; } + +using clCreateBufferTestsWithRestrictions = api_test_using_aligned_memory_manager; + +TEST_F(clCreateBufferTestsWithRestrictions, givenMemoryManagerRestrictionsWhenMinIsLesserThanHostPtrThenUseZeroCopy) { + std::unique_ptr hostMem(nullptr); + unsigned char *destMem = nullptr; + cl_mem_flags flags = CL_MEM_USE_HOST_PTR; + const unsigned int bufferSize = MemoryConstants::pageSize * 3; + const unsigned int destBufferSize = MemoryConstants::pageSize; + cl_mem buffer = nullptr; + + uintptr_t minAddress = 0; + MockAllocSysMemAgnosticMemoryManager *memMngr = reinterpret_cast(device->getMemoryManager()); + memMngr->ptrRestrictions = &memMngr->testRestrictions; + EXPECT_EQ(minAddress, memMngr->ptrRestrictions->minAddress); + + hostMem.reset(new unsigned char[bufferSize]); + + destMem = hostMem.get(); + destMem += MemoryConstants::pageSize; + destMem -= (reinterpret_cast(destMem) % MemoryConstants::pageSize); + + buffer = clCreateBuffer(context, flags, destBufferSize, destMem, &retVal); + + ASSERT_EQ(CL_SUCCESS, retVal); + EXPECT_NE(nullptr, buffer); + + Buffer *bufferObj = OCLRT::castToObject(buffer); + EXPECT_TRUE(bufferObj->isMemObjZeroCopy()); + + retVal = clReleaseMemObject(buffer); + EXPECT_EQ(CL_SUCCESS, retVal); +} + +TEST_F(clCreateBufferTestsWithRestrictions, givenMemoryManagerRestrictionsWhenMinIsLesserThanHostPtrThenCreateCopy) { + std::unique_ptr hostMem(nullptr); + unsigned char *destMem = nullptr; + cl_mem_flags flags = CL_MEM_USE_HOST_PTR; + const unsigned int realBufferSize = MemoryConstants::pageSize * 3; + const unsigned int destBufferSize = MemoryConstants::pageSize; + cl_mem buffer = nullptr; + + MockAllocSysMemAgnosticMemoryManager *memMngr = reinterpret_cast(device->getMemoryManager()); + memMngr->ptrRestrictions = &memMngr->testRestrictions; + + hostMem.reset(new unsigned char[realBufferSize]); + + destMem = hostMem.get(); + destMem += MemoryConstants::pageSize; + destMem -= (reinterpret_cast(destMem) % MemoryConstants::pageSize); + memMngr->ptrRestrictions->minAddress = reinterpret_cast(destMem) + 1; + + buffer = clCreateBuffer(context, flags, destBufferSize, destMem, &retVal); + + ASSERT_EQ(CL_SUCCESS, retVal); + EXPECT_NE(nullptr, buffer); + + Buffer *bufferObj = OCLRT::castToObject(buffer); + EXPECT_FALSE(bufferObj->isMemObjZeroCopy()); + + retVal = clReleaseMemObject(buffer); + EXPECT_EQ(CL_SUCCESS, retVal); +} + } // namespace ULT diff --git a/unit_tests/mocks/mock_device.cpp b/unit_tests/mocks/mock_device.cpp index 1c1d069880..e41292d0e4 100644 --- a/unit_tests/mocks/mock_device.cpp +++ b/unit_tests/mocks/mock_device.cpp @@ -90,3 +90,10 @@ OCLRT::FailMemoryManager::FailMemoryManager(int32_t fail) : MemoryManager(false) agnostic = new OsAgnosticMemoryManager(false); this->fail = fail; } + +MockAlignedMallocManagerDevice::MockAlignedMallocManagerDevice(const HardwareInfo &hwInfo, bool isRootDevice) : MockDevice(hwInfo, isRootDevice) { + //delete regular OsAgnosticMemoryManager + delete memoryManager; + //and create specific + memoryManager = new MockAllocSysMemAgnosticMemoryManager(); +} diff --git a/unit_tests/mocks/mock_device.h b/unit_tests/mocks/mock_device.h index 8e1a406161..5c5384c459 100644 --- a/unit_tests/mocks/mock_device.h +++ b/unit_tests/mocks/mock_device.h @@ -39,8 +39,8 @@ extern bool initialized; class MockDevice : public Device { public: - using Device::initializeCaps; using Device::commandStreamReceiver; + using Device::initializeCaps; void setOSTime(OSTime *osTime); void setDriverInfo(DriverInfo *driverInfo); @@ -183,4 +183,9 @@ class FailDeviceAfterOne : public Device { } }; +class MockAlignedMallocManagerDevice : public MockDevice { + public: + MockAlignedMallocManagerDevice(const HardwareInfo &hwInfo, bool isRootDevice = true); +}; + } // namespace OCLRT diff --git a/unit_tests/mocks/mock_memory_manager.h b/unit_tests/mocks/mock_memory_manager.h index 3557607267..0bda87a53e 100644 --- a/unit_tests/mocks/mock_memory_manager.h +++ b/unit_tests/mocks/mock_memory_manager.h @@ -1,5 +1,5 @@ /* -* Copyright (c) 2017, Intel Corporation +* Copyright (c) 2017 - 2018, Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -45,4 +45,26 @@ class GMockMemoryManager : public OsAgnosticMemoryManager { // cleanAllocationList call defined in MemoryManager. bool MemoryManagerCleanAllocationList(uint32_t waitTaskCount, uint32_t allocationType) { return MemoryManager::cleanAllocationList(waitTaskCount, allocationType); } }; + +class MockAllocSysMemAgnosticMemoryManager : public OsAgnosticMemoryManager { + public: + MockAllocSysMemAgnosticMemoryManager() : OsAgnosticMemoryManager() { + ptrRestrictions = nullptr; + testRestrictions.minAddress = 0; + } + + AlignedMallocRestrictions *getAlignedMallocRestrictions() override { + return ptrRestrictions; + } + + void *allocateSystemMemory(size_t size, size_t alignment) override { + constexpr size_t minAlignment = 16; + alignment = std::max(alignment, minAlignment); + return alignedMalloc(size, alignment); + } + + AlignedMallocRestrictions testRestrictions; + AlignedMallocRestrictions *ptrRestrictions; +}; + } // namespace OCLRT