[PATCH] lockdep: introduce lockdep_assert_not_held()

[PATCH] lockdep: introduce lockdep_assert_not_held()

[David Sterba]

Add an assertion that a lock is not held, suitable for the following
(simplified) usecase in filesystems:

- filesystem write
  - lock(&big_filesystem_lock)
  - kmalloc(GFP_KERNEL)
    - trigger dirty data write to get more memory
      - find dirty pages
      - call filesystem write
        - lock(&big_filesystem_lock)
	  deadlock

The cause here is the use of GFP_KERNEL that does not exclude poking
filesystems to allow freeing some memory. Such scenario is a bug, so the
use of GFP_NOFS is the right flag.

The annotation can help catch such bugs during development because
the actual deadlock could be hard to hit in practice.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 Documentation/locking/lockdep-design.txt | 5 +++++
 include/linux/lockdep.h                  | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/Documentation/locking/lockdep-design.txt b/Documentation/locking/lockdep-design.txt
index 39fae143c9cb..8b3013aaf518 100644
--- a/Documentation/locking/lockdep-design.txt
+++ b/Documentation/locking/lockdep-design.txt
@@ -211,6 +211,11 @@ that nobody tampered with the lock, e.g. kernel/sched/sched.h
 	lockdep_unpin_lock(&rq->lock, rf->cookie);
   }
 
+In some contexts it is useful to assert that a given lock is not held.  A
+typical example are filesystems that must avoid recursion to their code when
+a memory allocation triggers write of dirty data. When the allocation is done
+with a lock taken, re-entering the code would cause a deadlock.
+
 While comments about locking requirements might provide useful information,
 the runtime checks performed by annotations are invaluable when debugging
 locking problems and they carry the same level of details when inspecting
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 6e2377e6c1d6..a6682104bd95 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -397,6 +397,10 @@ extern void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie);
 		WARN_ON_ONCE(debug_locks && !lockdep_is_held(l));	\
 	} while (0)
 
+#define lockdep_assert_not_held(l)	do {			\
+		WARN_ON(debug_locks && lockdep_is_held(l));	\
+	} while (0)
+
 #define lockdep_recursing(tsk)	((tsk)->lockdep_recursion)
 
 #define lockdep_pin_lock(l)	lock_pin_lock(&(l)->dep_map)
@@ -469,6 +473,7 @@ struct lockdep_map { };
 #define lockdep_assert_held_exclusive(l)	do { (void)(l); } while (0)
 #define lockdep_assert_held_read(l)		do { (void)(l); } while (0)
 #define lockdep_assert_held_once(l)		do { (void)(l); } while (0)
+#define lockdep_assert_not_held(l)		do { (void)(l); } while (0)
 
 #define lockdep_recursing(tsk)			(0)
 
-- 
2.21.0

Re: [PATCH] lockdep: introduce lockdep_assert_not_held()

[David Sterba]

On Thu, Jun 13, 2019 at 03:36:04PM +0200, David Sterba wrote:
> Add an assertion that a lock is not held, suitable for the following
> (simplified) usecase in filesystems:
> 
> - filesystem write
>   - lock(&big_filesystem_lock)
>   - kmalloc(GFP_KERNEL)
>     - trigger dirty data write to get more memory
>       - find dirty pages
>       - call filesystem write
>         - lock(&big_filesystem_lock)
> 	  deadlock
> 
> The cause here is the use of GFP_KERNEL that does not exclude poking
> filesystems to allow freeing some memory. Such scenario is a bug, so the
> use of GFP_NOFS is the right flag.
> 
> The annotation can help catch such bugs during development because
> the actual deadlock could be hard to hit in practice.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Any comments on that? I just found another case with convoluted
callstacks where the lockdep assertion would catch the potential lock up
earlier than under the testing load.