From cf862a580c003614835a81dab8d1ae790545213f Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Mon, 30 Nov 2020 20:12:41 -0800 Subject: [PATCH 1/6] [SYCL][L0] Fix memory leak in event pool. Memeory leak was caused by two errors. 1. incorrectly setting the number of live events in the event pool initially. 2. never increment the number of live events afterwards. Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index efbf971d6945b..2cd8ce9e3b803 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -250,12 +250,15 @@ _pi_context::getFreeSlotInExistingOrNewPool(ze_event_pool_handle_t &ZePool, &ZeDevices[0], &ZeEventPool)) return ZeRes; NumEventsAvailableInEventPool[ZeEventPool] = MaxNumEventsPerPool - 1; - NumEventsLiveInEventPool[ZeEventPool] = MaxNumEventsPerPool; + NumEventsLiveInEventPool[ZeEventPool] = 1; } else { std::lock_guard NumEventsAvailableInEventPoolGuard( NumEventsAvailableInEventPoolMutex); Index = MaxNumEventsPerPool - NumEventsAvailableInEventPool[ZeEventPool]; --NumEventsAvailableInEventPool[ZeEventPool]; + std::lock_guard NumEventsLiveInEventPoolGuard( + NumEventsLiveInEventPoolMutex, std::adopt_lock); + NumEventsLiveInEventPool[ZeEventPool]++; } ZePool = ZeEventPool; return ZE_RESULT_SUCCESS; From 0aa785236f3ea7a4fe5bd8e95c298ed44134952d Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Mon, 30 Nov 2020 21:52:25 -0800 Subject: [PATCH 2/6] fixed piEventRelease to decrement live events properly Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 2cd8ce9e3b803..0aaa1ca5c85d7 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -257,7 +257,7 @@ _pi_context::getFreeSlotInExistingOrNewPool(ze_event_pool_handle_t &ZePool, Index = MaxNumEventsPerPool - NumEventsAvailableInEventPool[ZeEventPool]; --NumEventsAvailableInEventPool[ZeEventPool]; std::lock_guard NumEventsLiveInEventPoolGuard( - NumEventsLiveInEventPoolMutex, std::adopt_lock); + NumEventsLiveInEventPoolMutex); NumEventsLiveInEventPool[ZeEventPool]++; } ZePool = ZeEventPool; @@ -268,6 +268,7 @@ ze_result_t _pi_context::decrementAliveEventsInPool(ze_event_pool_handle_t ZePool) { std::lock_guard Lock(NumEventsLiveInEventPoolMutex); --NumEventsLiveInEventPool[ZePool]; + ++NumEventsAvailableInEventPool[ZePool]; if (NumEventsLiveInEventPool[ZePool] == 0) { return zeEventPoolDestroy(ZePool); } @@ -3565,11 +3566,11 @@ pi_result piEventRelease(pi_event Event) { } ZE_CALL(zeEventDestroy(Event->ZeEvent)); - auto Context = Event->Context; - ZE_CALL(Context->decrementAliveEventsInPool(Event->ZeEventPool)); - delete Event; } + auto Context = Event->Context; + ZE_CALL(Context->decrementAliveEventsInPool(Event->ZeEventPool)); + return PI_SUCCESS; } From 8a4567052649fd10ba83130f66e2e68251f4bc5c Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Mon, 30 Nov 2020 23:33:12 -0800 Subject: [PATCH 3/6] revert piEventRelease change Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 0aaa1ca5c85d7..310c0334b079c 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -3566,10 +3566,11 @@ pi_result piEventRelease(pi_event Event) { } ZE_CALL(zeEventDestroy(Event->ZeEvent)); + auto Context = Event->Context; + ZE_CALL(Context->decrementAliveEventsInPool(Event->ZeEventPool)); + delete Event; } - auto Context = Event->Context; - ZE_CALL(Context->decrementAliveEventsInPool(Event->ZeEventPool)); return PI_SUCCESS; } From 5a2ba3efc5b533340b8300afc4c66c08317e948b Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Wed, 2 Dec 2020 12:21:42 -0800 Subject: [PATCH 4/6] [SYCL][L0] Added a finalize() function in pi_context to fix memory leaks Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 18 ++++++++++++------ sycl/plugins/level_zero/pi_level_zero.hpp | 3 +++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 310c0334b079c..bf1b4e16d540b 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -250,15 +250,12 @@ _pi_context::getFreeSlotInExistingOrNewPool(ze_event_pool_handle_t &ZePool, &ZeDevices[0], &ZeEventPool)) return ZeRes; NumEventsAvailableInEventPool[ZeEventPool] = MaxNumEventsPerPool - 1; - NumEventsLiveInEventPool[ZeEventPool] = 1; + NumEventsLiveInEventPool[ZeEventPool] = MaxNumEventsPerPool; } else { std::lock_guard NumEventsAvailableInEventPoolGuard( NumEventsAvailableInEventPoolMutex); Index = MaxNumEventsPerPool - NumEventsAvailableInEventPool[ZeEventPool]; --NumEventsAvailableInEventPool[ZeEventPool]; - std::lock_guard NumEventsLiveInEventPoolGuard( - NumEventsLiveInEventPoolMutex); - NumEventsLiveInEventPool[ZeEventPool]++; } ZePool = ZeEventPool; return ZE_RESULT_SUCCESS; @@ -268,7 +265,6 @@ ze_result_t _pi_context::decrementAliveEventsInPool(ze_event_pool_handle_t ZePool) { std::lock_guard Lock(NumEventsLiveInEventPoolMutex); --NumEventsLiveInEventPool[ZePool]; - ++NumEventsAvailableInEventPool[ZePool]; if (NumEventsLiveInEventPool[ZePool] == 0) { return zeEventPoolDestroy(ZePool); } @@ -441,6 +437,14 @@ pi_result _pi_context::initialize() { return PI_SUCCESS; } +void _pi_context::finalize() { + // This function is called when pi_context is deallocated, piContextRelase. + // There could be some memory that may have not been deallocated. + // For example, zeEventPool could be still alive. + if (ZeEventPool && NumEventsLiveInEventPool[ZeEventPool]) + zeEventPoolDestroy(ZeEventPool); +} + pi_result _pi_queue::resetCommandListFenceEntry(ze_command_list_handle_t ZeCommandList, bool MakeAvailable) { @@ -1814,6 +1818,9 @@ pi_result piContextRelease(pi_context Context) { assert(Context); if (--(Context->RefCount) == 0) { + // Clean up any live memory associated with Context + Context->finalize(); + auto ZeContext = Context->ZeContext; // Destroy the command list used for initializations ZE_CALL(zeCommandListDestroy(Context->ZeCommandListInit)); @@ -3568,7 +3575,6 @@ pi_result piEventRelease(pi_event Event) { auto Context = Event->Context; ZE_CALL(Context->decrementAliveEventsInPool(Event->ZeEventPool)); - delete Event; } diff --git a/sycl/plugins/level_zero/pi_level_zero.hpp b/sycl/plugins/level_zero/pi_level_zero.hpp index 4570d73f9cfa8..5e7cea31007c8 100644 --- a/sycl/plugins/level_zero/pi_level_zero.hpp +++ b/sycl/plugins/level_zero/pi_level_zero.hpp @@ -216,6 +216,9 @@ struct _pi_context : _pi_object { // Initialize the PI context. pi_result initialize(); + // Finalize the PI context + void finalize(); + // A L0 context handle is primarily used during creation and management of // resources that may be used by multiple devices. ze_context_handle_t ZeContext; From e58c42aa15a89de38019b35fd11d965b43d7ce18 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Fri, 4 Dec 2020 10:15:00 -0800 Subject: [PATCH 5/6] moved de-initialize code into finalize. added mutex guard before check live event in the pool Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 26 ++++++++++++----------- sycl/plugins/level_zero/pi_level_zero.hpp | 2 +- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index bf1b4e16d540b..67808765e44bc 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -437,12 +437,24 @@ pi_result _pi_context::initialize() { return PI_SUCCESS; } -void _pi_context::finalize() { +pi_result _pi_context::finalize() { // This function is called when pi_context is deallocated, piContextRelase. // There could be some memory that may have not been deallocated. // For example, zeEventPool could be still alive. + std::lock_guard NumEventsLiveInEventPoolGuard( + NumEventsLiveInEventPoolMutex, std::adopt_lock); if (ZeEventPool && NumEventsLiveInEventPool[ZeEventPool]) zeEventPoolDestroy(ZeEventPool); + + // Destroy the command list used for initializations + ZE_CALL(zeCommandListDestroy(ZeCommandListInit)); + + // Destruction of some members of pi_context uses L0 context + // and therefore it must be valid at that point. + // Technically it should be placed to the destructor of pi_context + // but this makes API error handling more complex. + ZE_CALL(zeContextDestroy(ZeContext)); + return PI_SUCCESS; } pi_result @@ -1820,17 +1832,7 @@ pi_result piContextRelease(pi_context Context) { if (--(Context->RefCount) == 0) { // Clean up any live memory associated with Context Context->finalize(); - - auto ZeContext = Context->ZeContext; - // Destroy the command list used for initializations - ZE_CALL(zeCommandListDestroy(Context->ZeCommandListInit)); delete Context; - - // Destruction of some members of pi_context uses L0 context - // and therefore it must be valid at that point. - // Technically it should be placed to the destructor of pi_context - // but this makes API error handling more complex. - ZE_CALL(zeContextDestroy(ZeContext)); } return PI_SUCCESS; } @@ -3575,9 +3577,9 @@ pi_result piEventRelease(pi_event Event) { auto Context = Event->Context; ZE_CALL(Context->decrementAliveEventsInPool(Event->ZeEventPool)); + delete Event; } - return PI_SUCCESS; } diff --git a/sycl/plugins/level_zero/pi_level_zero.hpp b/sycl/plugins/level_zero/pi_level_zero.hpp index 5e7cea31007c8..fb1ff0fecc7e0 100644 --- a/sycl/plugins/level_zero/pi_level_zero.hpp +++ b/sycl/plugins/level_zero/pi_level_zero.hpp @@ -217,7 +217,7 @@ struct _pi_context : _pi_object { pi_result initialize(); // Finalize the PI context - void finalize(); + pi_result finalize(); // A L0 context handle is primarily used during creation and management of // resources that may be used by multiple devices. From 7001353f6f16b0fa006a0fc777e63d6631f110d5 Mon Sep 17 00:00:00 2001 From: Byoungro So Date: Mon, 7 Dec 2020 09:57:23 -0800 Subject: [PATCH 6/6] return the finalize result Signed-off-by: Byoungro So --- sycl/plugins/level_zero/pi_level_zero.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 67808765e44bc..51084bba12967 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -1831,8 +1831,9 @@ pi_result piContextRelease(pi_context Context) { assert(Context); if (--(Context->RefCount) == 0) { // Clean up any live memory associated with Context - Context->finalize(); + pi_result Result = Context->finalize(); delete Context; + return Result; } return PI_SUCCESS; }