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