[SYCL][L0] Fix memory leak in pi Queue #2905
Conversation
Retain Queue whenever a pi_event associates a pi_queue. Release the associated pi_queue when a pi_event is released. Signed-off-by: Byoungro So <[email protected]>
…mutex Signed-off-by: Byoungro So <[email protected]>
Signed-off-by: Byoungro So <[email protected]>
|
@smaslov-intel Please review this PR. Thanks. |
smaslov-intel
left a comment
smaslov-intel
left a comment
There was a problem hiding this comment.
I certainly like this approach better than #2899, so please abandon that one.
Signed-off-by: Byoungro So <[email protected]>
Signed-off-by: Byoungro So <[email protected]>
Signed-off-by: Byoungro So <[email protected]>
Signed-off-by: Byoungro So <[email protected]>
|
Vào Th 4, 6 thg 1, 2021 lúc 23:10 smaslov-intel <[email protected]>
đã viết:
… ***@***.**** commented on this pull request.
------------------------------
In sycl/plugins/level_zero/pi_level_zero.cpp
<#2905 (comment)>:
> @@ -1995,8 +2020,7 @@ pi_result piQueueRetain(pi_queue Queue) {
pi_result piQueueRelease(pi_queue Queue) {
PI_ASSERT(Queue, PI_INVALID_QUEUE);
- // Lock automatically releases when this goes out of scope.
- std::lock_guard<std::mutex> lock(Queue->PiQueueMutex);
+ Queue->PiQueueMutex.lock();
I see. Can we maybe do smth like this:
{
need_delete = false
lock_gaurd()
if (...) {
need_delete = true
}
}
if (need_delete) delete;
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2905 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AQBNS7ZTZ5OIQL6LQ7EGT3LSYSDQRANCNFSM4VAXZBQA>
.
|
Hi I am sorry, I don't understand this comments. Can you speak in English? |
Signed-off-by: Byoungro So <[email protected]>
smaslov-intel
left a comment
smaslov-intel
left a comment
There was a problem hiding this comment.
LGTM but there are fails in the tests
Thanks for approval. It could be related to the lock_guard. I will look into it today. Thanks, Sergey. |
|
|
||
| // Lock automatically releases when this goes out of scope. | ||
| std::lock_guard<std::mutex> lock(Queue->PiQueueMutex); | ||
|
|
There was a problem hiding this comment.
I can't move this up because piEventsWait is trying to acquire the lock. So reverting this change.
There was a problem hiding this comment.
you can still have this lock specially for createEventAndAssociateQueue, before line 4318
There was a problem hiding this comment.
Yes, that's what did. I moved it inside the { } so the lock_guard is only used for createEventAndAssociateQueue. There is a separate lock_guard in the original line below.
|
|
||
| // Lock automatically releases when this goes out of scope. | ||
| std::lock_guard<std::mutex> lock(Queue->PiQueueMutex); | ||
|
|
There was a problem hiding this comment.
Same here. I can't move the lock here because of piEventsWait below.
Signed-off-by: Byoungro So <[email protected]>
4 participants
Retain Queue whenever a pi_event associates a pi_queue.
Release the associated pi_queue when a pi_event is released.
Signed-off-by: Byoungro So [email protected]