diff mbox series

[v4,1/7] btrfs: Add macros for annotating wait events with lockdep

Message ID 20220725221150.3959022-2-iangelak@fb.com (mailing list archive)
State New, archived
Headers show
Series btrfs: Annotate wait events with lockdep | expand

Commit Message

Ioannis Angelakopoulos July 25, 2022, 10:11 p.m. UTC
Introduce four macros that are used to annotate wait events in btrfs code
with lockdep; 1) the btrfs_lockdep_init_map, 2) the btrfs_lockdep_acquire,
3) the btrfs_lockdep_release, and 4) the btrfs_might_wait_for_event macros.

The btrfs_lockdep_init_map macro is used to initialize a lockdep map.

The btrfs_lockdep_<acquire,release> macros are used by threads to take the
lockdep map as readers (shared lock) and release it, respectively.

The btrfs_might_wait_for_event macro is used by threads to take the
lockdep map as writers (exclusive lock) and release it.

In general, the lockdep annotation for wait events works as follows:

The condition for a wait event can be modified and signaled at the same
time by multiple threads. These threads hold the lockdep map as readers
when they enter a context in which blocking would prevent signaling the
condition. Frequently, this occurs when a thread violates a condition
(lockdep map acquire), before restoring it and signaling it at a later
point (lockdep map release).

The threads that block on the wait event take the lockdep map as writers
(exclusive lock). These threads have to block until all the threads that
hold the lockdep map as readers signal the condition for the wait event and
release the lockdep map.

The lockdep annotation is used to warn about potential deadlock scenarios
that involve the threads that modify and signal the wait event condition
and threads that block on the wait event. A simple example is illustrated
below:

Without lockdep:

TA                                        TB
cond = false
                                          lock(A)
                                          wait_event(w, cond)
                                          unlock(A)
lock(A)
cond = true
signal(w)
unlock(A)

With lockdep:

TA                                        TB
rwsem_acquire_read(lockdep_map)
cond = false
                                          lock(A)
                                          rwsem_acquire(lockdep_map)
                                          rwsem_release(lockdep_map)
                                          wait_event(w, cond)
                                          unlock(A)
lock(A)
cond = true
signal(w)
unlock(A)
rwsem_release(lockdep_map)

In the second case, with the lockdep annotation, lockdep would warn about
an ABBA deadlock, while the first case would just deadlock at some point.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Ioannis Angelakopoulos <iangelak@fb.com>
---
 fs/btrfs/ctree.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

Comments

David Sterba Aug. 2, 2022, 5:33 p.m. UTC | #1
On Mon, Jul 25, 2022 at 03:11:46PM -0700, Ioannis Angelakopoulos wrote:
> Introduce four macros that are used to annotate wait events in btrfs code
> with lockdep; 1) the btrfs_lockdep_init_map, 2) the btrfs_lockdep_acquire,
> 3) the btrfs_lockdep_release, and 4) the btrfs_might_wait_for_event macros.
> 
> The btrfs_lockdep_init_map macro is used to initialize a lockdep map.
> 
> The btrfs_lockdep_<acquire,release> macros are used by threads to take the
> lockdep map as readers (shared lock) and release it, respectively.
> 
> The btrfs_might_wait_for_event macro is used by threads to take the
> lockdep map as writers (exclusive lock) and release it.
> 
> In general, the lockdep annotation for wait events works as follows:
> 
> The condition for a wait event can be modified and signaled at the same
> time by multiple threads. These threads hold the lockdep map as readers
> when they enter a context in which blocking would prevent signaling the
> condition. Frequently, this occurs when a thread violates a condition
> (lockdep map acquire), before restoring it and signaling it at a later
> point (lockdep map release).
> 
> The threads that block on the wait event take the lockdep map as writers
> (exclusive lock). These threads have to block until all the threads that
> hold the lockdep map as readers signal the condition for the wait event and
> release the lockdep map.
> 
> The lockdep annotation is used to warn about potential deadlock scenarios
> that involve the threads that modify and signal the wait event condition
> and threads that block on the wait event. A simple example is illustrated
> below:
> 
> Without lockdep:
> 
> TA                                        TB
> cond = false
>                                           lock(A)
>                                           wait_event(w, cond)
>                                           unlock(A)
> lock(A)
> cond = true
> signal(w)
> unlock(A)
> 
> With lockdep:
> 
> TA                                        TB
> rwsem_acquire_read(lockdep_map)
> cond = false
>                                           lock(A)
>                                           rwsem_acquire(lockdep_map)
>                                           rwsem_release(lockdep_map)
>                                           wait_event(w, cond)
>                                           unlock(A)
> lock(A)
> cond = true
> signal(w)
> unlock(A)
> rwsem_release(lockdep_map)
> 
> In the second case, with the lockdep annotation, lockdep would warn about
> an ABBA deadlock, while the first case would just deadlock at some point.
> 
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Ioannis Angelakopoulos <iangelak@fb.com>
> ---
>  fs/btrfs/ctree.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 4db85b9dc7ed..f2169d01c66e 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1175,6 +1175,51 @@ enum {
>  	BTRFS_ROOT_UNFINISHED_DROP,
>  };
>  
> +/*
> + * Lockdep annotation for wait events.
> + *
> + * @b: The struct where the lockdep map is defined
> + * @lock: The lockdep map corresponding to a wait event
> + *
> + * This macro is used to annotate a wait event. In this case a thread acquires
> + * the lockdep map as writer (exclusive lock) because it has to block until all
> + * the threads that hold the lock as readers signal the condition for the wait
> + * event and release their locks.
> + */
> +#define btrfs_might_wait_for_event(b, lock)					\

Please don't use sinle letter varaibles or macro parameters, I've
renamed it to 'owner' in all macros.
diff mbox series

Patch

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 4db85b9dc7ed..f2169d01c66e 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1175,6 +1175,51 @@  enum {
 	BTRFS_ROOT_UNFINISHED_DROP,
 };
 
+/*
+ * Lockdep annotation for wait events.
+ *
+ * @b: The struct where the lockdep map is defined
+ * @lock: The lockdep map corresponding to a wait event
+ *
+ * This macro is used to annotate a wait event. In this case a thread acquires
+ * the lockdep map as writer (exclusive lock) because it has to block until all
+ * the threads that hold the lock as readers signal the condition for the wait
+ * event and release their locks.
+ */
+#define btrfs_might_wait_for_event(b, lock)					\
+	do {									\
+		rwsem_acquire(&b->lock##_map, 0, 0, _THIS_IP_);			\
+		rwsem_release(&b->lock##_map, _THIS_IP_);			\
+	} while (0)
+
+/*
+ * Protection for the resource/condition of a wait event.
+ *
+ * @b: The struct where the lockdep map is defined
+ * @lock: The lockdep map corresponding to a wait event
+ *
+ * Many threads can modify the condition for the wait event at the same time
+ * and signal the threads that block on the wait event. The threads that
+ * modify the condition and do the signaling acquire the lock as readers
+ * (shared lock).
+ */
+#define btrfs_lockdep_acquire(b, lock)						\
+	rwsem_acquire_read(&b->lock##_map, 0, 0, _THIS_IP_)
+
+/*
+ * Used after signaling the condition for a wait event to release the
+ * lockdep map held by a reader thread.
+ */
+#define btrfs_lockdep_release(b, lock)						\
+	rwsem_release(&b->lock##_map, _THIS_IP_)
+
+/* Initialization of the lockdep map */
+#define btrfs_lockdep_init_map(b, lock)                                        \
+	do {									\
+		static struct lock_class_key lock##_key;			\
+		lockdep_init_map(&b->lock##_map, #lock, &lock##_key, 0);	\
+	} while (0)
+
 static inline void btrfs_wake_unfinished_drop(struct btrfs_fs_info *fs_info)
 {
 	clear_and_wake_up_bit(BTRFS_FS_UNFINISHED_DROPS, &fs_info->flags);