Events deadlock #226

bibrak · 2024-10-30T09:20:59Z

Creates an internal DAG of the events at runtime. From that DAG maintains a topological sort. If there is a cycle in the DAG, adding that dependency in the topological sort will return error hence detecting the deadlock.

source/layers/validation/checkers/events_deadlock/zel_events_deadlock_checker.h

bmyates · 2024-10-30T18:43:20Z

source/layers/validation/checkers/events_deadlock/zel_events_deadlock_checker.cpp

+}
+
+ze_result_t
+eventsDeadlockChecker::ZEeventsDeadlockChecker::zeCommandListAppendMemoryCopyPrologue(


I would be insterested in seeing a high level outline of how the algorithm works before getting to implementation of specific functions

Which APIs will add things to graph

Which APIs will remove things from graph

Which APIs will execute deadlock detection mechanism

bmyates · 2024-10-30T18:47:50Z

source/layers/validation/checkers/events_deadlock/zel_events_deadlock_checker.cpp

+
+ze_result_t
+eventsDeadlockChecker::ZEeventsDeadlockChecker::zeCommandListAppendMemoryCopyPrologue(
+    ze_command_list_handle_t hCommandList, ///< [in] handle of command list


I think you may need to consider the specific properties of the command list to determine what to do with it.

For instance, a command list with ZE_COMMAND_LIST_FLAG_IN_ORDER has an implicit dependency between all things appended to it.

An immediate command list executes immediately and you probably want to do deadlock deetection immediately

a non-immediate command list does not execute until it is explicitly submittted, and you may want to delay deadlock detection until that point. (Or maybe not -not really sure on this one!)

The deadlock detection happens as the API (action) is called from the user app (doesn't matter how the driver stages it in queues). The detection is only based on the hSignalEvent, and any phWaitEvents events.

bmyates · 2024-10-30T18:48:32Z

source/layers/validation/checkers/events_deadlock/zel_events_deadlock_checker.cpp

+
+        if (eventToDagID.find(hSignalEvent) != eventToDagID.end()) {
+            if (eventToDagID[hSignalEvent] != invalidDagID) {
+                // This event already exists in the DAG. Get the DAG node ID.


just a nitpick, it's not really a DAG if there is a deadlock - because then there is a cycle.

Yes, when a cycle is added the underlying DAG structure will not accept that .add() and will return error. This way we know there is a cycle and hence a deadlock.

bmyates · 2024-10-30T18:51:14Z

samples/zello_events_deadlock/zello_events_deadlock.cpp

+    SUCCESS_OR_TERMINATE(zeMemAllocDevice(context, &device_desc, buffer_size, 0, pDevice, &device_mem_ptr));
+
+    std::cout << "\n\n\n";
+


There is an api zeEventHostSignal that allows signalling events from host. If that API were called at any point in time for event[2], then everything would work.

There is also the possibility that a later gpu command signals this event, and then there is also no problem in that case.

This seems intractable to me. I don't know that it is solvable because there is no way to predict what will happen at some point in the infinite future

But even in that case won't it be an ill formed use of the L0 API? Like a single action is being signaled twice? And even if it were ok then we will have an infinite cycle and a deadlock the second time it is triggered.

bmyates · 2024-10-30T18:53:05Z

source/layers/validation/checkers/events_deadlock/zel_events_deadlock_checker.h

+
+      private:
+        // events point from/out to a DAG node. This map stores the DAG ID for each event (if there is one).
+        std::unordered_map<ze_event_handle_t, int> eventToDagID;


events can be reused many times and can be associated with many different command list actions. It seems like this is limiting events to only being used once

Yes, I have yet to recycle them. I will have to come up with something like eventToDagID[event] = invalidDagID; when there is a reset called or myriad of those ways that could happen that I am not aware of and will need help later.

bmyates · 2024-10-30T18:57:45Z

source/layers/validation/checkers/events_deadlock/zel_events_deadlock_checker.cpp

+                std::cout << "\tDAG: Adding edge from " << dagID << " to " << this_action_new_node_id << std::endl;
+            } else {
+                std::cerr << "eventsDeadlockChecker: zeCommandListAppendMemoryCopyPrologue: Error: Wait event not found in eventToDagID map" << std::endl;
+                std::terminate();


If something goes wrong we sohuld return an error instead of terminating.

We also need to make sure any error messages we log are understandable to someone that is not familiar with the internals here

rwmcguir

We need the original Apache 2.0 LICENSE file from the XLA project put into the thirdparty/xla folder

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

rwmcguir · 2024-11-06T03:46:42Z

third_party/xla/graphcycles.h

+
+// SPDX-License-Identifier: Apache-2.0
+
+// Copyright (C) 2024 Intel Corporation


@lisanna-dettwyler a question in our other normal source files we have nomenclature like
//* @file graphcycles.h
in our comments. Should we insert these?
from legal perspective this is not necessary, but is infrastructure using this for anyreason?

rwmcguir

From Licensing perspective this looks good.
You have my informal +1 on License basis...

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

aravindksg · 2024-11-08T22:00:20Z

source/layers/validation/README.md

@@ -18,6 +18,7 @@ By default, no validation modes will be enabled. The individual validation modes

 - `ZE_ENABLE_PARAMETER_VALIDATION`
 - `ZE_ENABLE_HANDLE_LIFETIME`
+- `ZEL_ENABLE_EVENTSDEADLOCK_CHECKER`


did we want to rename this to simply "events_checker" ? that way it can perform deadlock detection and any other checks on events as well in future

That is a good idea.

aravindksg · 2024-11-08T22:00:41Z

source/layers/validation/README.md

@@ -48,6 +49,9 @@ This mode maintains an internal mapping of each handle type to a state structure
 - Additional per handle state checks added as needed
    - Example - Check ze_cmdlist_handle_t open or closed

+### `ZEL_ENABLE_EVENTSDEADLOCK_CHECKER`
+
+The Events Deadlock Checker is designed to detect potential deadlocks that might occur due to improper event usage in the Level Zero API. It prints out wairning messages for user when it detects a potential deadlock.


typo "wairning"

aravindksg · 2024-11-08T22:19:47Z

source/layers/validation/checkers/events_deadlock/zel_events_deadlock_checker.h

+namespace validation_layer {
+
+constexpr uint32_t invalidDagID = std::numeric_limits<uint32_t>::max();
+constexpr ze_event_handle_t invalidEventAddress = std::numeric_limits<ze_event_handle_t>::max();


what is max of an event_handle?

I think the max value of the pointer?

aravindksg · 2024-11-08T23:00:08Z

source/layers/validation/checkers/events_deadlock/zel_events_deadlock_checker.cpp

+        eventToDagID.erase(hEvent);
+    }
+
+    return ZE_RESULT_SUCCESS;


general question: how is epilogue call handled? if the call to driver returns an error then does epilogue get called? also does epilogue need to return the return value from the driver call?

@nrspruit do you know?

regardless of the result, the epilogue will be called like here:
https://github.com/oneapi-src/level-zero/blob/master/source/layers/validation/ze_valddi.cpp#L49 .

Perhaps we could expand this to pass in the "result" to the epilogue such that a check does not occur or don't call epilogue given an error?

Yes, either not call epilogue altogether if error in the actual zeCall or pass the error in the epilogue and then its as per the checker logic and behavior what it needs to do in case of error.

In the later case, then it will have to change API for all checkers for all zeAPI calls. Or just for the ZeEventDestroy()?

Again, in the former case it will be only for ZeEventDestroy() or all ZeAPI calls?

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

bibrak changed the title ~~Events deadlock~~ [Draft] [RFC] Events deadlock Oct 30, 2024

nrspruit reviewed Oct 30, 2024

View reviewed changes

source/layers/validation/checkers/events_deadlock/zel_events_deadlock_checker.h Outdated Show resolved Hide resolved

bmyates reviewed Oct 30, 2024

View reviewed changes

rwmcguir requested changes Nov 1, 2024

View reviewed changes

bibrak added 21 commits November 4, 2024 12:23

feature: events deadlock detection in validation layer

c3a928d

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

ab4d9e8

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

3fc842a

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

39bfa01

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

580efc1

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

bea254d

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

08e18ef

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

297ed53

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

e88c11d

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

ca00b13

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

cf4effc

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

6fc9b08

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

4c5d5dc

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

2a41d12

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

4663b9b

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

b377cca

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

3c6697e

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

e751378

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

861c206

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

4ca971c

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

a7b4e5d

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

dcaf532

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

bibrak force-pushed the events_deadlock branch from 00217b0 to dcaf532 Compare November 4, 2024 22:19

bibrak added 3 commits November 4, 2024 15:38

feature: events deadlock detection in validation layer

7433d7c

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

5d24b5b

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

64ebf67

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

bibrak changed the title ~~[Draft] [RFC] Events deadlock~~ [Draft] Events deadlock Nov 5, 2024

bibrak changed the title ~~[Draft] Events deadlock~~ Events deadlock Nov 5, 2024

bibrak added 4 commits November 5, 2024 10:24

feature: events deadlock detection in validation layer

a66309f

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

ae901c7

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

eb5678b

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

9164b43

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

rwmcguir reviewed Nov 6, 2024

View reviewed changes

bibrak added 3 commits November 6, 2024 10:03

feature: events deadlock detection in validation layer

fb78371

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

a0ac094

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

3091196

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

aravindksg reviewed Nov 8, 2024

View reviewed changes

bibrak added 3 commits November 11, 2024 22:05

feature: events deadlock detection in validation layer

abde7db

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

c7ea2dc

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

bf2f727

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

bibrak requested a review from rwmcguir November 12, 2024 17:18

rwmcguir previously approved these changes Nov 12, 2024

View reviewed changes

feature: events deadlock detection in validation layer

2fd7b30

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

bibrak dismissed rwmcguir’s stale review via 2fd7b30 November 12, 2024 18:37

bibrak added 6 commits November 12, 2024 13:54

feature: events deadlock detection in validation layer

bc3bfa4

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

fe01ae6

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

0b99ac9

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

25bfb0e

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

1ecc1ef

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

feature: events deadlock detection in validation layer

7323cc3

Related-To: NEO-12810 Signed-off-by: Chandio, Bibrak Qamar <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Events deadlock #226

Events deadlock #226

bibrak commented Oct 30, 2024

bmyates Oct 30, 2024

bmyates Oct 30, 2024

bibrak Oct 30, 2024

bmyates Oct 30, 2024

bibrak Oct 30, 2024

bmyates Oct 30, 2024 •

edited

Loading

bibrak Oct 30, 2024 •

edited

Loading

bmyates Oct 30, 2024

bibrak Oct 30, 2024

bmyates Oct 30, 2024

rwmcguir left a comment

rwmcguir Nov 6, 2024

rwmcguir left a comment

aravindksg Nov 8, 2024

bibrak Nov 8, 2024

bibrak Nov 12, 2024

aravindksg Nov 8, 2024

bibrak Nov 8, 2024

aravindksg Nov 8, 2024

bibrak Nov 8, 2024

aravindksg Nov 8, 2024

aravindksg Nov 8, 2024

nrspruit Nov 9, 2024

bibrak Nov 9, 2024

		SUCCESS_OR_TERMINATE(zeMemAllocDevice(context, &device_desc, buffer_size, 0, pDevice, &device_mem_ptr));

		std::cout << "\n\n\n";


		// SPDX-License-Identifier: Apache-2.0

		// Copyright (C) 2024 Intel Corporation

Events deadlock #226

Are you sure you want to change the base?

Events deadlock #226

Conversation

bibrak commented Oct 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmyates Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

bibrak Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwmcguir left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwmcguir left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmyates Oct 30, 2024 •

edited

Loading

bibrak Oct 30, 2024 •

edited

Loading