Skip to content

Commit 92c0b1d

Browse files
committed
[SYCL][PL][L0] Fixes problem with possible deep recursion.
Fixes an issue where cleanupAfterEvent could potentially very deeply recurse releasing events from dependent wait lists. This algorithm does not recurse at all, but adds to a list of events to release, and iterates until the list is empty.
1 parent 697c018 commit 92c0b1d

File tree

2 files changed

+29
-10
lines changed

2 files changed

+29
-10
lines changed

sycl/plugins/level_zero/pi_level_zero.cpp

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,8 @@ static void printZeEventList(_pi_ze_event_list_t &PiZeEventList) {
827827
zePrint("\n");
828828
}
829829

830-
pi_result _pi_ze_event_list_t::releaseAndDestroyPiZeEventList() {
830+
pi_result _pi_ze_event_list_t::releaseAndDestroyPiZeEventList(
831+
std::list<pi_event> &EventsToBeReleased) {
831832
// acquire a lock before reading the length and list fields.
832833
// Acquire the lock, copy the needed data locally, and reset
833834
// the fields, then release the lock.
@@ -854,7 +855,8 @@ pi_result _pi_ze_event_list_t::releaseAndDestroyPiZeEventList() {
854855
}
855856

856857
for (pi_uint32 I = 0; I < LocLength; I++) {
857-
piEventRelease(LocPiEventList[I]);
858+
// Add the event to be released to the list
859+
EventsToBeReleased.push_back(LocPiEventList[I]);
858860
}
859861

860862
if (LocZeEventList != nullptr) {
@@ -3755,6 +3757,7 @@ pi_result piEventGetProfilingInfo(pi_event Event, pi_profiling_info ParamName,
37553757
// This currently recycles the associate command list, and also makes
37563758
// sure to release any kernel that may have been used by the event.
37573759
static void cleanupAfterEvent(pi_event Event) {
3760+
zePrint("cleanupAfterEvent entry\n");
37583761
// The implementation of this is slightly tricky. The same event
37593762
// can be referred to by multiple threads, so it is possible to
37603763
// have a race condition between the read of fields of the event,
@@ -3798,11 +3801,25 @@ static void cleanupAfterEvent(pi_event Event) {
37983801
}
37993802
}
38003803

3801-
// Had to make sure lock of Event's Queue has been released before
3802-
// the code starts to release the event wait list, as that could
3803-
// potentially cause recursive calls to cleanupAfterEvent, and
3804-
// run into a problem trying to do multiple locks on the same Queue.
3805-
Event->WaitList.releaseAndDestroyPiZeEventList();
3804+
// Make a list of all the dependent events that must have signalled
3805+
// because this event was dependent on them. This list will be appended
3806+
// to as we walk it so that this algorithm doesn't go recursive
3807+
// due to dependent events themselves being dependent on other events
3808+
// forming a potentially very deep tree, and deep recursion. That
3809+
// turned out to be a significant problem with the recursive code
3810+
// that preceded this implementation.
3811+
3812+
std::list<pi_event> EventsToBeReleased;
3813+
3814+
Event->WaitList.releaseAndDestroyPiZeEventList(EventsToBeReleased);
3815+
3816+
while (!EventsToBeReleased.empty()) {
3817+
pi_event DepEvent = EventsToBeReleased.front();
3818+
EventsToBeReleased.pop_front();
3819+
3820+
DepEvent->WaitList.releaseAndDestroyPiZeEventList(EventsToBeReleased);
3821+
piEventRelease(DepEvent);
3822+
}
38063823
zePrint("cleanupAfterEvent exit\n");
38073824
}
38083825

sycl/plugins/level_zero/pi_level_zero.hpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -509,9 +509,11 @@ struct _pi_ze_event_list_t {
509509
const pi_event *EventList,
510510
pi_queue CurQueue);
511511

512-
// Release all the events in this object's PiEventList, and destroy
513-
// the data structures it contains.
514-
pi_result releaseAndDestroyPiZeEventList();
512+
// Add all the events in this object's PiEventList to the end
513+
// of the list EventsToBeReleased. Destroy pi_ze_event_list_t data
514+
// structure fields making it look empty.
515+
pi_result
516+
releaseAndDestroyPiZeEventList(std::list<pi_event> &EventsToBeReleased);
515517

516518
// Had to create custom assignment operator because the mutex is
517519
// not assignment copyable. Just field by field copy of the other

0 commit comments

Comments
 (0)