From cb448c6c355fe04396b2353e949ed5ff6c42ddff Mon Sep 17 00:00:00 2001 From: Maciej Bielski Date: Fri, 15 Dec 2023 14:53:51 +0000 Subject: [PATCH] fix: add missing registerIpcExportedAllocation Unify `getIpcMemHandle()` and `getIpcMemHandles()` and fix the issue of missing IPC handles registration while reducing copy-pasted code. Also replace `boHandle` with `boHandleWrapper` to leverage `refCount` properly and avoid double-free. Related-To: NEO-8904 Signed-off-by: Maciej Bielski --- .../core/source/context/context_imp.cpp | 104 ++++++++---------- level_zero/core/source/context/context_imp.h | 1 + .../os_interface/linux/drm_memory_manager.cpp | 2 +- ...m_memory_manager_localmem_prelim_tests.cpp | 2 +- 4 files changed, 50 insertions(+), 59 deletions(-) diff --git a/level_zero/core/source/context/context_imp.cpp b/level_zero/core/source/context/context_imp.cpp index 94eea31786..3156f38506 100644 --- a/level_zero/core/source/context/context_imp.cpp +++ b/level_zero/core/source/context/context_imp.cpp @@ -591,34 +591,6 @@ void ContextImp::setIPCHandleData(NEO::GraphicsAllocation *graphicsAllocation, u } } -ze_result_t ContextImp::getIpcMemHandle(const void *ptr, - ze_ipc_mem_handle_t *pIpcHandle) { - NEO::SvmAllocationData *allocData = this->driverHandle->svmAllocsManager->getSVMAlloc(ptr); - if (allocData) { - auto *memoryManager = driverHandle->getMemoryManager(); - auto *graphicsAllocation = allocData->gpuAllocations.getDefaultGraphicsAllocation(); - - uint64_t handle = 0; - int ret = graphicsAllocation->createInternalHandle(memoryManager, 0u, handle); - if (ret < 0) { - return ZE_RESULT_ERROR_OUT_OF_HOST_MEMORY; - } - - memoryManager->registerIpcExportedAllocation(graphicsAllocation); - - IpcMemoryData &ipcData = *reinterpret_cast(pIpcHandle->data); - auto type = allocData->memoryType; - uint8_t ipcType = 0; - if (type == InternalMemoryType::hostUnifiedMemory) { - ipcType = static_cast(InternalIpcMemoryType::hostUnifiedMemory); - } - setIPCHandleData(graphicsAllocation, handle, ipcData, reinterpret_cast(ptr), ipcType); - - return ZE_RESULT_SUCCESS; - } - return ZE_RESULT_ERROR_INVALID_ARGUMENT; -} - ze_result_t ContextImp::getIpcHandleFromFd(uint64_t handle, ze_ipc_mem_handle_t *pIpcHandle) { std::map::iterator ipcHandleIterator; auto lock = this->driverHandle->lockIPCHandleMap(); @@ -645,43 +617,61 @@ ze_result_t ContextImp::getFdFromIpcHandle(ze_ipc_mem_handle_t ipcHandle, uint64 return ZE_RESULT_SUCCESS; } -ze_result_t ContextImp::getIpcMemHandles(const void *ptr, - uint32_t *numIpcHandles, - ze_ipc_mem_handle_t *pIpcHandles) { +ze_result_t ContextImp::getIpcMemHandlesImpl(const void *ptr, + uint32_t *numIpcHandles, + ze_ipc_mem_handle_t *pIpcHandles) { NEO::SvmAllocationData *allocData = this->driverHandle->svmAllocsManager->getSVMAlloc(ptr); - if (allocData) { - auto alloc = allocData->gpuAllocations.getDefaultGraphicsAllocation(); - uint32_t numHandles = alloc->getNumHandles(); - UNRECOVERABLE_IF(numIpcHandles == nullptr); + if (!allocData) { + return ZE_RESULT_ERROR_INVALID_ARGUMENT; + } + auto memoryManager = this->driverHandle->getMemoryManager(); + auto alloc = allocData->gpuAllocations.getDefaultGraphicsAllocation(); + + if (numIpcHandles) { + uint32_t numHandles = alloc->getNumHandles(); if (*numIpcHandles == 0 || *numIpcHandles > numHandles) { *numIpcHandles = numHandles; } + } - if (pIpcHandles == nullptr) { - return ZE_RESULT_SUCCESS; - } - - auto type = allocData->memoryType; - auto ipcType = InternalIpcMemoryType::deviceUnifiedMemory; - if (type == InternalMemoryType::hostUnifiedMemory) { - ipcType = InternalIpcMemoryType::hostUnifiedMemory; - } - - for (uint32_t i = 0; i < *numIpcHandles; i++) { - uint64_t handle = 0; - int ret = allocData->gpuAllocations.getDefaultGraphicsAllocation()->createInternalHandle(this->driverHandle->getMemoryManager(), i, handle); - if (ret < 0) { - return ZE_RESULT_ERROR_OUT_OF_HOST_MEMORY; - } - - IpcMemoryData &ipcData = *reinterpret_cast(pIpcHandles[i].data); - setIPCHandleData(alloc, handle, ipcData, reinterpret_cast(ptr), static_cast(ipcType)); - } - + if (pIpcHandles == nullptr) { return ZE_RESULT_SUCCESS; } - return ZE_RESULT_ERROR_INVALID_ARGUMENT; + + auto type = allocData->memoryType; + auto ipcType = InternalIpcMemoryType::deviceUnifiedMemory; + if (type == InternalMemoryType::hostUnifiedMemory) { + ipcType = InternalIpcMemoryType::hostUnifiedMemory; + } + + uint32_t loopCount = numIpcHandles ? *numIpcHandles : 1u; + for (uint32_t i = 0u; i < loopCount; i++) { + uint64_t handle = 0; + int ret = alloc->createInternalHandle(memoryManager, i, handle); + if (ret < 0) { + return ZE_RESULT_ERROR_OUT_OF_HOST_MEMORY; + } + + memoryManager->registerIpcExportedAllocation(alloc); + + IpcMemoryData &ipcData = *reinterpret_cast(pIpcHandles[i].data); + setIPCHandleData(alloc, handle, ipcData, reinterpret_cast(ptr), static_cast(ipcType)); + } + + return ZE_RESULT_SUCCESS; +} + +ze_result_t ContextImp::getIpcMemHandle(const void *ptr, + ze_ipc_mem_handle_t *pIpcHandle) { + return getIpcMemHandlesImpl(ptr, nullptr, pIpcHandle); +} + +ze_result_t ContextImp::getIpcMemHandles(const void *ptr, + uint32_t *numIpcHandles, + ze_ipc_mem_handle_t *pIpcHandles) { + DEBUG_BREAK_IF(numIpcHandles == nullptr); + return getIpcMemHandlesImpl(ptr, numIpcHandles, pIpcHandles); } ze_result_t ContextImp::openIpcMemHandle(ze_device_handle_t hDevice, diff --git a/level_zero/core/source/context/context_imp.h b/level_zero/core/source/context/context_imp.h index 8d572de804..12b6cb87cc 100644 --- a/level_zero/core/source/context/context_imp.h +++ b/level_zero/core/source/context/context_imp.h @@ -176,6 +176,7 @@ struct ContextImp : Context { ze_result_t checkMemSizeLimit(Device *inDevice, size_t size, bool relaxedSizeAllowed, void **ptr); protected: + ze_result_t getIpcMemHandlesImpl(const void *ptr, uint32_t *numIpcHandles, ze_ipc_mem_handle_t *pIpcHandles); void setIPCHandleData(NEO::GraphicsAllocation *graphicsAllocation, uint64_t handle, IpcMemoryData &ipcData, uint64_t ptrAddress, uint8_t type); bool isAllocationSuitableForCompression(const StructuresLookupTable &structuresLookupTable, Device &device, size_t allocSize); size_t getPageAlignedSizeRequired(size_t size, NEO::HeapIndex *heapRequired, size_t *pageSizeRequired); diff --git a/shared/source/os_interface/linux/drm_memory_manager.cpp b/shared/source/os_interface/linux/drm_memory_manager.cpp index 43c2825d71..d9f45e9f0f 100644 --- a/shared/source/os_interface/linux/drm_memory_manager.cpp +++ b/shared/source/os_interface/linux/drm_memory_manager.cpp @@ -834,7 +834,7 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromMultipleShared auto patIndex = drm.getPatIndex(nullptr, properties.allocationType, CacheRegion::defaultRegion, CachePolicy::writeBack, false, MemoryPoolHelper::isSystemMemoryPool(memoryPool)); auto boHandleWrapper = reuseSharedAllocation ? BufferObjectHandleWrapper{boHandle} : tryToGetBoHandleWrapperWithSharedOwnership(boHandle); - bo = new (std::nothrow) BufferObject(properties.rootDeviceIndex, &drm, patIndex, boHandle, size, maxOsContextCount); + bo = new (std::nothrow) BufferObject(properties.rootDeviceIndex, &drm, patIndex, std::move(boHandleWrapper), size, maxOsContextCount); i++; } bos.push_back(bo); diff --git a/shared/test/unit_test/os_interface/linux/drm_memory_manager_localmem_prelim_tests.cpp b/shared/test/unit_test/os_interface/linux/drm_memory_manager_localmem_prelim_tests.cpp index be06c00cdf..272ba2228b 100644 --- a/shared/test/unit_test/os_interface/linux/drm_memory_manager_localmem_prelim_tests.cpp +++ b/shared/test/unit_test/os_interface/linux/drm_memory_manager_localmem_prelim_tests.cpp @@ -2462,7 +2462,7 @@ TEST_F(DrmMemoryManagerTestPrelim, whenCreatingAllocationFromMultipleSharedHandl TEST_F(DrmMemoryManagerTestPrelim, whenCreatingAllocationFromMultipleSharedHandlesWithNoSharingResourcesThenDifferentAllocationsAreReturned) { mock->ioctlExpected.primeFdToHandle = 4; mock->ioctlExpected.gemWait = 2; - mock->ioctlExpected.gemClose = 4; + mock->ioctlExpected.gemClose = 2; std::vector handles{6, 7}; size_t size = 65536u * 2;