Re: [PATCH v2] locking/lockdep: Revise Documentation/locking/crossrelease.txt

2017-11-09 Thread Byungchul Park

On 11/10/2017 4:30 PM, Ingo Molnar wrote:


* Byungchul Park  wrote:


 Event C depends on event A.
 Event A depends on event B.
 Event B depends on event C.
  
-   NOTE: Precisely speaking, a dependency is one between whether a

-   waiter for an event can be woken up and whether another waiter for
-   another event can be woken up. However from now on, we will describe
-   a dependency as if it's one between an event and another event for
-   simplicity.


Why was this explanation removed?


-Lockdep tries to detect a deadlock by checking dependencies created by
-lock operations, acquire and release. Waiting for a lock corresponds to
-waiting for an event, and releasing a lock corresponds to triggering an
-event in the previous section.
+Lockdep tries to detect a deadlock by checking circular dependencies
+created by lock operations, acquire and release, which are wait and
+event respectively.


What? You changed a readable paragraph into an unreadable one.

Sorry, this text needs to be acked by someone with good English skills, and I
don't have the time right now to fix it all up. Please send minimal, obvious
typo/grammar fixes only.


I will send one including minimal fixes at the next spin.

--
Thanks,
Byungchul


Re: [PATCH v2] locking/lockdep: Revise Documentation/locking/crossrelease.txt

2017-11-09 Thread Ingo Molnar

* Byungchul Park  wrote:

> Event C depends on event A.
> Event A depends on event B.
> Event B depends on event C.
>  
> -   NOTE: Precisely speaking, a dependency is one between whether a
> -   waiter for an event can be woken up and whether another waiter for
> -   another event can be woken up. However from now on, we will describe
> -   a dependency as if it's one between an event and another event for
> -   simplicity.

Why was this explanation removed?

> -Lockdep tries to detect a deadlock by checking dependencies created by
> -lock operations, acquire and release. Waiting for a lock corresponds to
> -waiting for an event, and releasing a lock corresponds to triggering an
> -event in the previous section.
> +Lockdep tries to detect a deadlock by checking circular dependencies
> +created by lock operations, acquire and release, which are wait and
> +event respectively.

What? You changed a readable paragraph into an unreadable one.

Sorry, this text needs to be acked by someone with good English skills, and I 
don't have the time right now to fix it all up. Please send minimal, obvious
typo/grammar fixes only.

Thanks,

Ingo


Re: [PATCH v2] locking/lockdep: Revise Documentation/locking/crossrelease.txt

2017-11-08 Thread Byungchul Park
On Thu, Nov 09, 2017 at 04:20:36PM +0900, Byungchul Park wrote:
> Changes from v1
> - Run several tools checking english spell and grammar over the text.
> - Simplify the document more.

Checker tools also reported other words e.g. crosslock, crossrelease,
lockdep, mutex, lockless, and so on, but I left them unchanged since
I thought it's better to leave it. Please let me know if I was wrong.

Thanks,
Byungchul

> -8<-
> >From 412bc9eb0d22791f70f7364bda189feb41899ff9 Mon Sep 17 00:00:00 2001
> From: Byungchul Park 
> Date: Thu, 9 Nov 2017 16:12:23 +0900
> Subject: [PATCH v2] locking/lockdep: Revise 
> Documentation/locking/crossrelease.txt
> 
> Revise Documentation/locking/crossrelease.txt to enhance its readability.
> 
> Signed-off-by: Byungchul Park 
> ---
>  Documentation/locking/crossrelease.txt | 492 
> +++--
>  1 file changed, 227 insertions(+), 265 deletions(-)
> 
> diff --git a/Documentation/locking/crossrelease.txt 
> b/Documentation/locking/crossrelease.txt
> index bdf1423..11e3e3b 100644
> --- a/Documentation/locking/crossrelease.txt
> +++ b/Documentation/locking/crossrelease.txt
> @@ -12,10 +12,10 @@ Contents:
>  
>   (*) Limitation
>  
> - - Limit lockdep
> + - Limiting lockdep
>   - Pros from the limitation
>   - Cons from the limitation
> - - Relax the limitation
> + - Relaxing the limitation
>  
>   (*) Crossrelease
>  
> @@ -30,9 +30,9 @@ Contents:
>   (*) Optimizations
>  
>   - Avoid duplication
> - - Lockless for hot paths
> + - Make hot paths lockless
>  
> - (*) APPENDIX A: What lockdep does to work aggresively
> + (*) APPENDIX A: How to add dependencies aggressively
>  
>   (*) APPENDIX B: How to avoid adding false dependencies
>  
> @@ -51,36 +51,30 @@ also impossible due to the same reason.
>  
>  For example:
>  
> -   A context going to trigger event C is waiting for event A to happen.
> -   A context going to trigger event A is waiting for event B to happen.
> -   A context going to trigger event B is waiting for event C to happen.
> +   A context going to trigger event C is waiting for event A.
> +   A context going to trigger event A is waiting for event B.
> +   A context going to trigger event B is waiting for event C.
>  
> -A deadlock occurs when these three wait operations run at the same time,
> -because event C cannot be triggered if event A does not happen, which in
> -turn cannot be triggered if event B does not happen, which in turn
> -cannot be triggered if event C does not happen. After all, no event can
> -be triggered since any of them never meets its condition to wake up.
> +A deadlock occurs when the three waiters run at the same time, because
> +event C cannot be triggered if event A does not happen, which in turn
> +cannot be triggered if event B does not happen, which in turn cannot be
> +triggered if event C does not happen. After all, no event can be
> +triggered since any of them never meets its condition to wake up.
>  
> -A dependency might exist between two waiters and a deadlock might happen
> -due to an incorrect releationship between dependencies. Thus, we must
> -define what a dependency is first. A dependency exists between them if:
> +A dependency exists between two waiters, and a deadlock happens due to
> +an incorrect relationship between dependencies. Thus, we must define
> +what a dependency is first. A dependency exists between waiters if:
>  
> 1. There are two waiters waiting for each event at a given time.
> 2. The only way to wake up each waiter is to trigger its event.
> 3. Whether one can be woken up depends on whether the other can.
>  
> -Each wait in the example creates its dependency like:
> +Each waiter in the example creates its dependency like:
>  
> Event C depends on event A.
> Event A depends on event B.
> Event B depends on event C.
>  
> -   NOTE: Precisely speaking, a dependency is one between whether a
> -   waiter for an event can be woken up and whether another waiter for
> -   another event can be woken up. However from now on, we will describe
> -   a dependency as if it's one between an event and another event for
> -   simplicity.
> -
>  And they form circular dependencies like:
>  
>  -> C -> A -> B -
> @@ -101,19 +95,18 @@ Circular dependencies cause a deadlock.
>  How lockdep works
>  -
>  
> -Lockdep tries to detect a deadlock by checking dependencies created by
> -lock operations, acquire and release. Waiting for a lock corresponds to
> -waiting for an event, and releasing a lock corresponds to triggering an
> -event in the previous section.
> +Lockdep tries

[PATCH v2] locking/lockdep: Revise Documentation/locking/crossrelease.txt

2017-11-08 Thread Byungchul Park
Changes from v1
- Run several tools checking english spell and grammar over the text.
- Simplify the document more.

-8<-
>From 412bc9eb0d22791f70f7364bda189feb41899ff9 Mon Sep 17 00:00:00 2001
From: Byungchul Park 
Date: Thu, 9 Nov 2017 16:12:23 +0900
Subject: [PATCH v2] locking/lockdep: Revise 
Documentation/locking/crossrelease.txt

Revise Documentation/locking/crossrelease.txt to enhance its readability.

Signed-off-by: Byungchul Park 
---
 Documentation/locking/crossrelease.txt | 492 +++--
 1 file changed, 227 insertions(+), 265 deletions(-)

diff --git a/Documentation/locking/crossrelease.txt 
b/Documentation/locking/crossrelease.txt
index bdf1423..11e3e3b 100644
--- a/Documentation/locking/crossrelease.txt
+++ b/Documentation/locking/crossrelease.txt
@@ -12,10 +12,10 @@ Contents:
 
  (*) Limitation
 
- - Limit lockdep
+ - Limiting lockdep
  - Pros from the limitation
  - Cons from the limitation
- - Relax the limitation
+ - Relaxing the limitation
 
  (*) Crossrelease
 
@@ -30,9 +30,9 @@ Contents:
  (*) Optimizations
 
  - Avoid duplication
- - Lockless for hot paths
+ - Make hot paths lockless
 
- (*) APPENDIX A: What lockdep does to work aggresively
+ (*) APPENDIX A: How to add dependencies aggressively
 
  (*) APPENDIX B: How to avoid adding false dependencies
 
@@ -51,36 +51,30 @@ also impossible due to the same reason.
 
 For example:
 
-   A context going to trigger event C is waiting for event A to happen.
-   A context going to trigger event A is waiting for event B to happen.
-   A context going to trigger event B is waiting for event C to happen.
+   A context going to trigger event C is waiting for event A.
+   A context going to trigger event A is waiting for event B.
+   A context going to trigger event B is waiting for event C.
 
-A deadlock occurs when these three wait operations run at the same time,
-because event C cannot be triggered if event A does not happen, which in
-turn cannot be triggered if event B does not happen, which in turn
-cannot be triggered if event C does not happen. After all, no event can
-be triggered since any of them never meets its condition to wake up.
+A deadlock occurs when the three waiters run at the same time, because
+event C cannot be triggered if event A does not happen, which in turn
+cannot be triggered if event B does not happen, which in turn cannot be
+triggered if event C does not happen. After all, no event can be
+triggered since any of them never meets its condition to wake up.
 
-A dependency might exist between two waiters and a deadlock might happen
-due to an incorrect releationship between dependencies. Thus, we must
-define what a dependency is first. A dependency exists between them if:
+A dependency exists between two waiters, and a deadlock happens due to
+an incorrect relationship between dependencies. Thus, we must define
+what a dependency is first. A dependency exists between waiters if:
 
1. There are two waiters waiting for each event at a given time.
2. The only way to wake up each waiter is to trigger its event.
3. Whether one can be woken up depends on whether the other can.
 
-Each wait in the example creates its dependency like:
+Each waiter in the example creates its dependency like:
 
Event C depends on event A.
Event A depends on event B.
Event B depends on event C.
 
-   NOTE: Precisely speaking, a dependency is one between whether a
-   waiter for an event can be woken up and whether another waiter for
-   another event can be woken up. However from now on, we will describe
-   a dependency as if it's one between an event and another event for
-   simplicity.
-
 And they form circular dependencies like:
 
 -> C -> A -> B -
@@ -101,19 +95,18 @@ Circular dependencies cause a deadlock.
 How lockdep works
 -
 
-Lockdep tries to detect a deadlock by checking dependencies created by
-lock operations, acquire and release. Waiting for a lock corresponds to
-waiting for an event, and releasing a lock corresponds to triggering an
-event in the previous section.
+Lockdep tries to detect a deadlock by checking circular dependencies
+created by lock operations, acquire and release, which are wait and
+event respectively.
 
 In short, lockdep does:
 
1. Detect a new dependency.
-   2. Add the dependency into a global graph.
+   2. Add the dependency to a global graph.
3. Check if that makes dependencies circular.
-   4. Report a deadlock or its possibility if so.
+   4. Report the deadlock if so.
 
-For example, consider a graph built by lockdep that looks like:
+For example, the graph has been built like:
 
A -> B -
\
@@ -123,7 +116,7 @@ For example, consider a graph built by lockdep that looks 
like:
 
where A, B,..., E are different lock classes.
 
-Lockdep will add a dependency into the graph on detection of a new
+Lockdep will add a dependency to the grap