Thread (51 messages) 51 messages, 4 authors, 2017-01-19

Re: [PATCH v4 15/15] lockdep: Crossrelease feature documentation

From: Peter Zijlstra <peterz@infradead.org>
Date: 2017-01-10 20:08:53
Also in: lkml

First off my sincere apologies for being so horribly slow with this :/

I did spend some time thinking about this thing during the Christmas
holidays, but have not yet managed to write a coherent text on it like I
promised I'd do.

That said; I think I now mostly understand what and why.

But I still feel this document is very hard to read and presents things
backwards.
+Let's take a look at more complicated example.
+
+   TASK X			   TASK Y
+   ------			   ------
+   acquire B
+
+   release B
+
+   acquire C
+
+   release C
+   (1)
+   fork Y
+				   acquire AX
+   acquire D
+   /* A dependency 'AX -> D' exists */
+				   acquire F
+   release D
+				   acquire G
+				   /* A dependency 'F -> G' exists */
+   acquire E
+   /* A dependency 'AX -> E' exists */
+				   acquire H
+				   /* A dependency 'G -> H' exists */
+   release E
+				   release H
+   release AX held by Y
+				   release G
+
+				   release F
+
+   where AX, B, C,..., H are different lock classes, and a suffix 'X' is
+   added on crosslocks.
+
+Does a dependency 'AX -> B' exist? Nope.
I think the above without the "fork Y" line is a much more interesting
example, because then the answer becomes: maybe.

This all boils down to the asynchonous nature of the primitive. There is
no well defined point other than what is observed (as I think you tried
to point out in our earlier exchanges).

The "acquire AX" point is entirely random wrt any action in other
threads, _however_ the time between "acquire" and "release" of any
'lock' is the only time we can be certain of things.
+==============
+Implementation
+==============
+
+Data structures
+---------------
+
+Crossrelease feature introduces two main data structures.
+
+1. pend_lock
I'm not sure 'pending' is the right name here, but I'll consider that
more when I review the code patches.
+
+   This is an array embedded in task_struct, for keeping locks queued so
+   that real dependencies can be added using them at commit step. Since
+   it's local data, it can be accessed locklessly in the owner context.
+   The array is filled at acquire step and consumed at commit step. And
+   it's managed in circular manner.
+
+2. cross_lock
+
+   This is a global linked list, for keeping all crosslocks in progress.
+   The list grows at acquire step and is shrunk at release step.
FWIW, this is a perfect example of why I say the document is written
backwards. At this point there is no demonstrated need or use for this
list.
+
+CONCLUSION
+
+Crossrelease feature introduces two main data structures.
+
+1. A pend_lock array for queueing typical locks in circular manner.
+2. A cross_lock linked list for managing crosslocks in progress.
+
+
+How crossrelease works
+----------------------
+
+Let's take a look at how crossrelease feature works step by step,
+starting from how lockdep works without crossrelease feaure.
+
+
+Let's look at how commit works for crosslocks.
+
+   AX's RELEASE CONTEXT		   AX's ACQUIRE CONTEXT
+   --------------------		   --------------------
+				   acquire AX
+				   /*
+				    * 1. Mark AX as started
+				    *
+				    * (No queuing for crosslocks)
+				    *
+				    * In pend_lock: Empty
+				    * In graph: Empty
+				    */
+
+   (serialized by some means e.g. barrier)
+
+   acquire D
+   /*
+    * (No marking for typical locks)
+    *
+    * 1. Queue D
+    *
+    * In pend_lock: D
+    * In graph: Empty
+    */
+				   acquire B
+				   /*
+				    * (No marking for typical locks)
+				    *
+				    * 1. Queue B
+				    *
+				    * In pend_lock: B
+				    * In graph: Empty
+				    */
+   release D
+   /*
+    * (No commit for typical locks)
+    *
+    * In pend_lock: D
+    * In graph: Empty
+    */
+				   acquire C
+				   /*
+				    * (No marking for typical locks)
+				    *
+				    * 1. Add 'B -> C' of TT type
+				    * 2. Queue C
+				    *
+				    * In pend_lock: B, C
+				    * In graph: 'B -> C'
+				    */
+   acquire E
+   /*
+    * (No marking for typical locks)
+    *
+    * 1. Queue E
+    *
+    * In pend_lock: D, E
+    * In graph: 'B -> C'
+    */
+				   acquire D
+				   /*
+				    * (No marking for typical locks)
+				    *
+				    * 1. Add 'C -> D' of TT type
+				    * 2. Queue D
+				    *
+				    * In pend_lock: B, C, D
+				    * In graph: 'B -> C', 'C -> D'
+				    */
+   release E
+   /*
+    * (No commit for typical locks)
+    *
+    * In pend_lock: D, E
+    * In graph: 'B -> C', 'C -> D'
+    */
+				   release D
+				   /*
+				    * (No commit for typical locks)
+				    *
+				    * In pend_lock: B, C, D
+				    * In graph: 'B -> C', 'C -> D'
+				    */
+   release AX
+   /*
+    * 1. Commit AX (= Add 'AX -> ?')
+    *   a. What queued since AX was marked: D, E
+    *   b. Add 'AX -> D' of CT type
+    *   c. Add 'AX -> E' of CT type
OK, so commit adds multiple dependencies, that makes more sense.
Previously I understood commit to only add a single dependency, which
does not make sense (except in the special case where there is but one).

I dislike how I have to reconstruct this from an example instead of
first having had the rules stated though.
+    *
+    * In pend_lock: D, E
+    * In graph: 'B -> C', 'C -> D',
+    *           'AX -> D', 'AX -> E'
+    */
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help