Skip to content

Commit af8561b

Browse files
committed
events: unlock notifier event on failed event
When a device notifies that an event failed, the wait event eventually ends up in pocl_update_event_finished. This function will lock the command queue. But since the notify event is locked in common.c:broadcast, a lock order violation can occur as the command queue is locked after the notifier event. This is in contrast to the lock order in other places where where it is first cq then event. This commit solves this by unlocking the notifier event before calling pocl_update_event_failed.
1 parent 0a517b0 commit af8561b

File tree

9 files changed

+87
-13
lines changed

9 files changed

+87
-13
lines changed

lib/CL/devices/basic/basic.c

+10-1
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,16 @@ pocl_basic_notify (cl_device_id device, cl_event event, cl_event finished)
571571

572572
if (finished->status < CL_COMPLETE)
573573
{
574-
pocl_update_event_failed_locked (event);
574+
/* Unlock the finished event in order to prevent a lock order violation
575+
* with the command queue that will be locked during
576+
* pocl_update_event_failed.
577+
*/
578+
pocl_unlock_events_inorder (event, finished);
579+
pocl_update_event_failed (CL_FAILED, NULL, 0, event, NULL);
580+
/* Lock events in this order to avoid a lock order violation between
581+
* the finished/notifier and event/wait events.
582+
*/
583+
pocl_lock_events_inorder (finished, event);
575584
return;
576585
}
577586

lib/CL/devices/hsa/pocl-hsa.c

+10-1
Original file line numberDiff line numberDiff line change
@@ -1613,7 +1613,16 @@ pocl_hsa_notify (cl_device_id device, cl_event event, cl_event finished)
16131613

16141614
if (finished->status < CL_COMPLETE)
16151615
{
1616-
pocl_update_event_failed_locked (event);
1616+
/* Unlock the finished event in order to prevent a lock order violation
1617+
* with the command queue that will be locked during
1618+
* pocl_update_event_failed.
1619+
*/
1620+
pocl_unlock_events_inorder (event, finished);
1621+
pocl_update_event_failed (CL_FAILED, NULL, 0, event, NULL);
1622+
/* Lock events in this order to avoid a lock order violation between
1623+
* the finished/notifier and event/wait events.
1624+
*/
1625+
pocl_lock_events_inorder (finished, event);
16171626
return;
16181627
}
16191628

lib/CL/devices/proxy/pocl_proxy.cpp

+10-1
Original file line numberDiff line numberDiff line change
@@ -1750,7 +1750,16 @@ pocl_proxy_notify (cl_device_id device, cl_event event, cl_event finished)
17501750

17511751
if (finished->status < CL_COMPLETE)
17521752
{
1753-
pocl_update_event_failed_locked (event);
1753+
/* Unlock the finished event in order to prevent a lock order violation
1754+
* with the command queue that will be locked during
1755+
* pocl_update_event_failed.
1756+
*/
1757+
pocl_unlock_events_inorder(event, finished);
1758+
pocl_update_event_failed(CL_FAILED, NULL, 0, event, NULL);
1759+
/* Lock events in this order to avoid a lock order violation between
1760+
* the finished/notifier and event/wait events.
1761+
*/
1762+
pocl_lock_events_inorder(finished, event);
17541763
return;
17551764
}
17561765

lib/CL/devices/pthread/pthread.c

+10-1
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,16 @@ pocl_pthread_notify (cl_device_id device, cl_event event, cl_event finished)
258258

259259
if (finished->status < CL_COMPLETE)
260260
{
261-
pocl_update_event_failed_locked (event);
261+
/* Unlock the finished event in order to prevent a lock order violation
262+
* with the command queue that will be locked during
263+
* pocl_update_event_failed.
264+
*/
265+
pocl_unlock_events_inorder (event, finished);
266+
pocl_update_event_failed (CL_FAILED, NULL, 0, event, NULL);
267+
/* Lock events in this order to avoid a lock order violation between
268+
* the finished/notifier and event/wait events.
269+
*/
270+
pocl_lock_events_inorder (finished, event);
262271
return;
263272
}
264273

lib/CL/devices/remote/remote.c

+10-1
Original file line numberDiff line numberDiff line change
@@ -1595,7 +1595,16 @@ pocl_remote_notify (cl_device_id device, cl_event event, cl_event finished)
15951595

15961596
if (finished->status < CL_COMPLETE)
15971597
{
1598-
pocl_update_event_failed_locked (event);
1598+
/* Unlock the finished event in order to prevent a lock order violation
1599+
* with the command queue that will be locked during
1600+
* pocl_update_event_failed.
1601+
*/
1602+
pocl_unlock_events_inorder (event, finished);
1603+
pocl_update_event_failed (CL_FAILED, NULL, 0, event, NULL);
1604+
/* Lock events in this order to avoid a lock order violation between
1605+
* the finished/notifier and event/wait events.
1606+
*/
1607+
pocl_lock_events_inorder (finished, event);
15991608
return;
16001609
}
16011610

lib/CL/devices/tbb/tbb.c

+13-3
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,19 @@ void pocl_tbb_notify(cl_device_id device, cl_event event, cl_event finished) {
286286
cl_bool wake_thread = CL_FALSE;
287287
_cl_command_node *node = event->command;
288288

289-
if (finished->status < CL_COMPLETE) {
290-
pocl_update_event_failed_locked (event);
291-
return;
289+
if (finished->status < CL_COMPLETE)
290+
{
291+
/* Unlock the finished event in order to prevent a lock order violation
292+
* with the command queue that will be locked during
293+
* pocl_update_event_failed.
294+
*/
295+
pocl_unlock_events_inorder (event, finished);
296+
pocl_update_event_failed (CL_FAILED, NULL, 0, event, NULL);
297+
/* Lock events in this order to avoid a lock order violation between
298+
* the finished/notifier and event/wait events.
299+
*/
300+
pocl_lock_events_inorder (finished, event);
301+
return;
292302
}
293303

294304
if (node->state != POCL_COMMAND_READY)

lib/CL/devices/tce/tce_common.cc

+12-2
Original file line numberDiff line numberDiff line change
@@ -1170,8 +1170,18 @@ pocl_tce_notify (cl_device_id device, cl_event event, cl_event finished)
11701170
{
11711171
_cl_command_node *node = event->command;
11721172

1173-
if (finished->status < CL_COMPLETE) {
1174-
pocl_update_event_failed_locked(event);
1173+
if (finished->status < CL_COMPLETE)
1174+
{
1175+
/* Unlock the finished event in order to prevent a lock order violation
1176+
* with the command queue that will be locked during
1177+
* pocl_update_event_failed.
1178+
*/
1179+
pocl_unlock_events_inorder(event, finished);
1180+
pocl_update_event_failed(CL_FAILED, NULL, 0, event, NULL);
1181+
/* Lock events in this order to avoid a lock order violation between
1182+
* the finished/notifier and event/wait events.
1183+
*/
1184+
pocl_lock_events_inorder(finished, event);
11751185
return;
11761186
}
11771187

lib/CL/devices/vulkan/pocl-vulkan.c

+10-1
Original file line numberDiff line numberDiff line change
@@ -2935,7 +2935,16 @@ pocl_vulkan_notify (cl_device_id device, cl_event event, cl_event finished)
29352935

29362936
if (finished->status < CL_COMPLETE)
29372937
{
2938-
pocl_update_event_failed_locked (event);
2938+
/* Unlock the finished event in order to prevent a lock order violation
2939+
* with the command queue that will be locked during
2940+
* pocl_update_event_failed.
2941+
*/
2942+
pocl_unlock_events_inorder (event, finished);
2943+
pocl_update_event_failed (CL_FAILED, NULL, 0, event, NULL);
2944+
/* Lock events in this order to avoid a lock order violation between
2945+
* the finished/notifier and event/wait events.
2946+
*/
2947+
pocl_lock_events_inorder (finished, event);
29392948
return;
29402949
}
29412950

lib/CL/pocl_util.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ void pocl_aligned_free (void *ptr);
9292
/* locks / unlocks two events in order of their event-id.
9393
* This avoids any potential deadlocks of threads should
9494
* they try to lock events in opposite order. */
95-
void pocl_lock_events_inorder (cl_event ev1, cl_event ev2);
96-
void pocl_unlock_events_inorder (cl_event ev1, cl_event ev2);
95+
POCL_EXPORT void pocl_lock_events_inorder (cl_event ev1, cl_event ev2);
96+
POCL_EXPORT void pocl_unlock_events_inorder (cl_event ev1, cl_event ev2);
9797

9898
/* Function for creating events */
9999
cl_int pocl_create_event (cl_event *event,

0 commit comments

Comments
 (0)