From 693f2ff38480e2c893ea5fd786ecc45433bf952f Mon Sep 17 00:00:00 2001 From: Slawomir Milczarek Date: Thu, 18 Mar 2021 15:16:58 +0000 Subject: [PATCH] Ensure shared handle be closed once on Linux Related-To: NEO-5644 Signed-off-by: Slawomir Milczarek --- level_zero/core/source/image/image_hw.inl | 2 +- opencl/source/sharings/va/va_surface.cpp | 2 +- .../linux/drm_memory_manager_tests.cpp | 19 +++++++++++++++++++ .../memory_manager/graphics_allocation.h | 1 + shared/source/memory_manager/memory_manager.h | 2 +- .../os_agnostic_memory_manager.h | 2 -- .../os_interface/linux/drm_memory_manager.cpp | 12 +++++++----- .../os_interface/linux/drm_memory_manager.h | 2 +- 8 files changed, 31 insertions(+), 11 deletions(-) diff --git a/level_zero/core/source/image/image_hw.inl b/level_zero/core/source/image/image_hw.inl index 7b2249e146..334f028d26 100644 --- a/level_zero/core/source/image/image_hw.inl +++ b/level_zero/core/source/image/image_hw.inl @@ -75,7 +75,7 @@ ze_result_t ImageCoreFamily::initialize(Device *device, const ze_ NEO::AllocationProperties properties(device->getRootDeviceIndex(), true, imgInfo, NEO::GraphicsAllocation::AllocationType::SHARED_IMAGE, device->getNEODevice()->getDeviceBitfield()); allocation = device->getNEODevice()->getMemoryManager()->createGraphicsAllocationFromSharedHandle(externalMemoryImportDesc->fd, properties, false); - device->getNEODevice()->getMemoryManager()->closeSharedHandle(externalMemoryImportDesc->fd); + device->getNEODevice()->getMemoryManager()->closeSharedHandle(allocation); } else { return ZE_RESULT_ERROR_UNSUPPORTED_ENUMERATION; } diff --git a/opencl/source/sharings/va/va_surface.cpp b/opencl/source/sharings/va/va_surface.cpp index b25c3c9450..a1c6ebec41 100644 --- a/opencl/source/sharings/va/va_surface.cpp +++ b/opencl/source/sharings/va/va_surface.cpp @@ -118,7 +118,7 @@ Image *VASurface::createSharedVaSurface(Context *context, VASharingFunctions *sh auto alloc = memoryManager->createGraphicsAllocationFromSharedHandle(sharedHandle, properties, false); - memoryManager->closeSharedHandle(sharedHandle); + memoryManager->closeSharedHandle(alloc); lock.unlock(); diff --git a/opencl/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp b/opencl/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp index d6e225953d..74943fb18e 100644 --- a/opencl/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp +++ b/opencl/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp @@ -573,6 +573,25 @@ TEST_F(DrmMemoryManagerWithExplicitExpectationsTest, givenDrmMemoryManagerCreate EXPECT_NE(nullptr, drmMemoryManger.peekGemCloseWorker()); } +TEST_F(DrmMemoryManagerTest, GivenAllocationWhenClosingSharedHandleThenSucceeds) { + mock->ioctl_expected.primeFdToHandle = 1; + mock->ioctl_expected.gemWait = 1; + mock->ioctl_expected.gemClose = 1; + + osHandle handle = 1u; + this->mock->outputHandle = 2u; + size_t size = 4096u; + AllocationProperties properties(rootDeviceIndex, false, size, GraphicsAllocation::AllocationType::SHARED_BUFFER, false, {}); + + auto graphicsAllocation = memoryManager->createGraphicsAllocationFromSharedHandle(handle, properties, false); + EXPECT_EQ(handle, graphicsAllocation->peekSharedHandle()); + + memoryManager->closeSharedHandle(graphicsAllocation); + EXPECT_EQ(Sharing::nonSharedResource, graphicsAllocation->peekSharedHandle()); + + memoryManager->freeGraphicsMemory(graphicsAllocation); +} + TEST_F(DrmMemoryManagerTest, GivenAllocationWhenFreeingThenSucceeds) { mock->ioctl_expected.gemUserptr = 1; mock->ioctl_expected.gemWait = 1; diff --git a/shared/source/memory_manager/graphics_allocation.h b/shared/source/memory_manager/graphics_allocation.h index a65c37b570..24bc04f62b 100644 --- a/shared/source/memory_manager/graphics_allocation.h +++ b/shared/source/memory_manager/graphics_allocation.h @@ -183,6 +183,7 @@ class GraphicsAllocation : public IDNode { void decReuseCount() { sharingInfo.reuseCount--; } uint32_t peekReuseCount() const { return sharingInfo.reuseCount; } osHandle peekSharedHandle() const { return sharingInfo.sharedHandle; } + void setSharedHandle(osHandle handle) { sharingInfo.sharedHandle = handle; } void setAllocationType(AllocationType allocationType); AllocationType getAllocationType() const { return allocationType; } diff --git a/shared/source/memory_manager/memory_manager.h b/shared/source/memory_manager/memory_manager.h index 40c0ebd55d..d9771b1dfe 100644 --- a/shared/source/memory_manager/memory_manager.h +++ b/shared/source/memory_manager/memory_manager.h @@ -87,7 +87,7 @@ class MemoryManager { virtual bool verifyHandle(osHandle handle, uint32_t rootDeviceIndex, bool) { return true; } virtual GraphicsAllocation *createGraphicsAllocationFromSharedHandle(osHandle handle, const AllocationProperties &properties, bool requireSpecificBitness) = 0; - virtual void closeSharedHandle(osHandle handle){}; + virtual void closeSharedHandle(GraphicsAllocation *graphicsAllocation){}; virtual GraphicsAllocation *createGraphicsAllocationFromNTHandle(void *handle, uint32_t rootDeviceIndex) = 0; virtual bool mapAuxGpuVA(GraphicsAllocation *graphicsAllocation); diff --git a/shared/source/memory_manager/os_agnostic_memory_manager.h b/shared/source/memory_manager/os_agnostic_memory_manager.h index 306bf038d6..57c5553278 100644 --- a/shared/source/memory_manager/os_agnostic_memory_manager.h +++ b/shared/source/memory_manager/os_agnostic_memory_manager.h @@ -18,8 +18,6 @@ class MemoryAllocation : public GraphicsAllocation { size_t sizeToFree = 0; const bool uncacheable; - void setSharedHandle(osHandle handle) { sharingInfo.sharedHandle = handle; } - MemoryAllocation(uint32_t rootDeviceIndex, AllocationType allocationType, void *cpuPtrIn, uint64_t gpuAddress, uint64_t baseAddress, size_t sizeIn, MemoryPool::Type pool, size_t maxOsContextCount) : MemoryAllocation(rootDeviceIndex, 1, allocationType, cpuPtrIn, gpuAddress, baseAddress, sizeIn, pool, maxOsContextCount) {} diff --git a/shared/source/os_interface/linux/drm_memory_manager.cpp b/shared/source/os_interface/linux/drm_memory_manager.cpp index b5d4589ad8..0d9b779f6e 100644 --- a/shared/source/os_interface/linux/drm_memory_manager.cpp +++ b/shared/source/os_interface/linux/drm_memory_manager.cpp @@ -628,8 +628,12 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromSharedHandle(o return drmAllocation; } -void DrmMemoryManager::closeSharedHandle(osHandle handle) { - closeFunction(handle); +void DrmMemoryManager::closeSharedHandle(GraphicsAllocation *gfxAllocation) { + DrmAllocation *drmAllocation = static_cast(gfxAllocation); + if (drmAllocation->peekSharedHandle() != Sharing::nonSharedResource) { + closeFunction(drmAllocation->peekSharedHandle()); + drmAllocation->setSharedHandle(Sharing::nonSharedResource); + } } GraphicsAllocation *DrmMemoryManager::createPaddedAllocation(GraphicsAllocation *inputGraphicsAllocation, size_t sizeWithPadding) { @@ -707,9 +711,7 @@ void DrmMemoryManager::freeGraphicsMemoryImpl(GraphicsAllocation *gfxAllocation) for (auto bo : bos) { unreference(bo, bo && bo->isReused ? false : true); } - if (gfxAllocation->peekSharedHandle() != Sharing::nonSharedResource) { - closeFunction(gfxAllocation->peekSharedHandle()); - } + closeSharedHandle(gfxAllocation); } releaseGpuRange(gfxAllocation->getReservedAddressPtr(), gfxAllocation->getReservedAddressSize(), gfxAllocation->getRootDeviceIndex()); diff --git a/shared/source/os_interface/linux/drm_memory_manager.h b/shared/source/os_interface/linux/drm_memory_manager.h index 51c334ec80..0f8495a9b0 100644 --- a/shared/source/os_interface/linux/drm_memory_manager.h +++ b/shared/source/os_interface/linux/drm_memory_manager.h @@ -36,7 +36,7 @@ class DrmMemoryManager : public MemoryManager { void handleFenceCompletion(GraphicsAllocation *allocation) override; GraphicsAllocation *createGraphicsAllocationFromExistingStorage(AllocationProperties &properties, void *ptr, MultiGraphicsAllocation &multiGraphicsAllocation) override; GraphicsAllocation *createGraphicsAllocationFromSharedHandle(osHandle handle, const AllocationProperties &properties, bool requireSpecificBitness) override; - void closeSharedHandle(osHandle handle) override; + void closeSharedHandle(GraphicsAllocation *gfxAllocation) override; GraphicsAllocation *createPaddedAllocation(GraphicsAllocation *inputGraphicsAllocation, size_t sizeWithPadding) override; GraphicsAllocation *createGraphicsAllocationFromNTHandle(void *handle, uint32_t rootDeviceIndex) override { return nullptr; }