This adds lockdep tracking for held btree locks with a single dep_map in btree_trans, i.e. tracking all held btree locks as one object.
This is more practical and more useful than having lockdep track held btree locks individually, because - we can take more locks than lockdep can track (unbounded, now that we have dynamically resizable btree paths) - there's no lock ordering between btree locks for lockdep to track (we do cycle detection) - and this makes it easy to teach lockdep that btree locks are not safe to hold while invoking memory reclaim. The last rule is one that lockdep would never learn, because we only do trylock() from within shrinkers - but we very much do not want to be invoking memory reclaim while holding btree node locks. Signed-off-by: Kent Overstreet <[email protected]> --- fs/bcachefs/btree_iter.c | 30 ++++++++++++++++++++++++------ fs/bcachefs/btree_locking.c | 8 ++++++++ fs/bcachefs/btree_locking.h | 14 ++++++++++++-- fs/bcachefs/btree_types.h | 3 +++ 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c index 80f4a39592b4..eca7634cc423 100644 --- a/fs/bcachefs/btree_iter.c +++ b/fs/bcachefs/btree_iter.c @@ -987,7 +987,7 @@ static int bch2_btree_path_traverse_all(struct btree_trans *trans) bch2_trans_unlock(trans); cond_resched(); - trans->locked = true; + trans_set_locked(trans); trans->last_unlock_ip = 0; if (unlikely(trans->memory_allocation_failure)) { @@ -3088,8 +3088,8 @@ u32 bch2_trans_begin(struct btree_trans *trans) bch2_trans_srcu_unlock(trans); trans->last_begin_ip = _RET_IP_; - trans->locked = true; - trans->last_unlock_ip = 0; + + trans_set_locked(trans); if (trans->restarted) { bch2_btree_path_traverse_all(trans); @@ -3173,6 +3173,12 @@ struct btree_trans *__bch2_trans_get(struct bch_fs *c, unsigned fn_idx) trans->paths_allocated[0] = 1; + static struct lock_class_key lockdep_key; + lockdep_init_map(&trans->dep_map, "bcachefs_btree", &lockdep_key, 0); + + if (IS_ENABLED(CONFIG_LOCKDEP)) + lock_acquire_exclusive(&trans->dep_map, 0, 0, NULL, _THIS_IP_); + if (fn_idx < BCH_TRANSACTIONS_NR) { trans->fn = bch2_btree_transaction_fns[fn_idx]; @@ -3441,7 +3447,19 @@ int bch2_fs_btree_iter_init(struct bch_fs *c) mempool_init_kmalloc_pool(&c->btree_trans_mem_pool, 1, BTREE_TRANS_MEM_MAX) ?: init_srcu_struct(&c->btree_trans_barrier); - if (!ret) - c->btree_trans_barrier_initialized = true; - return ret; + if (ret) + return ret; + +#ifdef CONFIG_LOCKDEP + fs_reclaim_acquire(GFP_KERNEL); + struct btree_trans *trans = bch2_trans_get(c); + lock_acquire_exclusive(&trans->dep_map, 0, 0, NULL, _THIS_IP_); + trans->locked = true; + bch2_trans_put(trans); + fs_reclaim_release(GFP_KERNEL); +#endif + + c->btree_trans_barrier_initialized = true; + return 0; + } diff --git a/fs/bcachefs/btree_locking.c b/fs/bcachefs/btree_locking.c index 16834a6b4c3d..2fdd95359a04 100644 --- a/fs/bcachefs/btree_locking.c +++ b/fs/bcachefs/btree_locking.c @@ -792,6 +792,8 @@ static inline int __bch2_trans_relock(struct btree_trans *trans, bool trace) return bch2_trans_relock_fail(trans, path, &f, trace); } + if (IS_ENABLED(CONFIG_LOCKDEP)) + lock_acquire_exclusive(&trans->dep_map, 0, 0, NULL, _THIS_IP_); trans->locked = true; trans->last_unlock_ip = 0; out: @@ -813,6 +815,9 @@ void bch2_trans_unlock_noassert(struct btree_trans *trans) { __bch2_trans_unlock(trans); + if (trans->locked && IS_ENABLED(CONFIG_LOCKDEP)) + lock_release(&trans->dep_map, _THIS_IP_); + trans->locked = false; trans->last_unlock_ip = _RET_IP_; } @@ -821,6 +826,9 @@ void bch2_trans_unlock(struct btree_trans *trans) { __bch2_trans_unlock(trans); + if (trans->locked && IS_ENABLED(CONFIG_LOCKDEP)) + lock_release(&trans->dep_map, _THIS_IP_); + trans->locked = false; trans->last_unlock_ip = _RET_IP_; } diff --git a/fs/bcachefs/btree_locking.h b/fs/bcachefs/btree_locking.h index 2530fdb29b15..af7b3d663a65 100644 --- a/fs/bcachefs/btree_locking.h +++ b/fs/bcachefs/btree_locking.h @@ -194,19 +194,29 @@ int bch2_six_check_for_deadlock(struct six_lock *lock, void *p); /* lock: */ +static inline void trans_set_locked(struct btree_trans *trans) +{ + if (!trans->locked) { + if (IS_ENABLED(CONFIG_LOCKDEP)) + lock_acquire_exclusive(&trans->dep_map, 0, 0, NULL, _THIS_IP_); + trans->locked = true; + trans->last_unlock_ip = 0; + } +} + static inline int __btree_node_lock_nopath(struct btree_trans *trans, struct btree_bkey_cached_common *b, enum six_lock_type type, bool lock_may_not_fail, unsigned long ip) { - int ret; + trans_set_locked(trans); trans->lock_may_not_fail = lock_may_not_fail; trans->lock_must_abort = false; trans->locking = b; - ret = six_lock_ip_waiter(&b->lock, type, &trans->locking_wait, + int ret = six_lock_ip_waiter(&b->lock, type, &trans->locking_wait, bch2_six_check_for_deadlock, trans, ip); WRITE_ONCE(trans->locking, NULL); WRITE_ONCE(trans->locking_wait.start_time, 0); diff --git a/fs/bcachefs/btree_types.h b/fs/bcachefs/btree_types.h index c9c9864a87b0..994e45c8dbeb 100644 --- a/fs/bcachefs/btree_types.h +++ b/fs/bcachefs/btree_types.h @@ -522,6 +522,9 @@ struct btree_trans { unsigned journal_u64s; unsigned extra_disk_res; /* XXX kill */ +#ifdef CONFIG_DEBUG_LOCK_ALLOC + struct lockdep_map dep_map; +#endif /* Entries before this are zeroed out on every bch2_trans_get() call */ struct list_head list; -- 2.45.2
