Commit f1625637 authored by Kent Overstreet's avatar Kent Overstreet
Browse files

bcachefs: Assert that we don't lock nodes when !trans->locked 



We rely on the trans->locked to know if a trans has nodes locked for
assertions about deadlocks; there can't be more than one trans in the
same process that is locked.

Signed-off-by: default avatarKent Overstreet <kent.overstreet@linux.dev>
parent a8cdf0ff

fs/bcachefs/btree_locking.h

+3 −4
Original line number Diff line number Diff line
@@ -218,13 +218,11 @@ static inline int __btree_node_lock_nopath(struct btree_trans *trans, 
					 bool lock_may_not_fail,

					 unsigned long ip)

{

	int ret;



	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);
@@ -284,6 +282,7 @@ static inline int btree_node_lock(struct btree_trans *trans, 
	int ret = 0;



	EBUG_ON(level >= BTREE_MAX_DEPTH);

	bch2_trans_verify_not_unlocked(trans);



	if (likely(six_trylock_type(&b->lock, type)) ||

	    btree_node_lock_increment(trans, b, level, (enum btree_node_locked_type) type) ||

fs/bcachefs/btree_update_interior.c

+12 −12
Original line number Diff line number Diff line
@@ -731,6 +731,18 @@ static void btree_update_nodes_written(struct btree_update *as) 
	bch2_fs_fatal_err_on(ret && !bch2_journal_error(&c->journal), c,

			     "%s", bch2_err_str(ret));

err:

	/*

	 * Ensure transaction is unlocked before using btree_node_lock_nopath()

	 * (the use of which is always suspect, we need to work on removing this

	 * in the future)

	 *

	 * It should be, but bch2_path_get_unlocked_mut() -> bch2_path_get()

	 * calls bch2_path_upgrade(), before we call path_make_mut(), so we may

	 * rarely end up with a locked path besides the one we have here:

	 */

	bch2_trans_unlock(trans);

	bch2_trans_begin(trans);



	/*

	 * We have to be careful because another thread might be getting ready

	 * to free as->b and calling btree_update_reparent() on us - we'll
@@ -750,18 +762,6 @@ static void btree_update_nodes_written(struct btree_update *as) 
		 * we're in journal error state:

		 */



		/*

		 * Ensure transaction is unlocked before using

		 * btree_node_lock_nopath() (the use of which is always suspect,

		 * we need to work on removing this in the future)

		 *

		 * It should be, but bch2_path_get_unlocked_mut() -> bch2_path_get()

		 * calls bch2_path_upgrade(), before we call path_make_mut(), so

		 * we may rarely end up with a locked path besides the one we

		 * have here:

		 */

		bch2_trans_unlock(trans);

		bch2_trans_begin(trans);

		btree_path_idx_t path_idx = bch2_path_get_unlocked_mut(trans,

						as->btree_id, b->c.level, b->key.k.p);

		struct btree_path *path = trans->paths + path_idx;

fs/bcachefs/btree_update_interior.h

+2 −0
Original line number Diff line number Diff line
@@ -159,6 +159,8 @@ static inline int bch2_foreground_maybe_merge(struct btree_trans *trans, 
					      unsigned level,

					      unsigned flags)

{

	bch2_trans_verify_not_unlocked(trans);



	return  bch2_foreground_maybe_merge_sibling(trans, path, level, flags,

						    btree_prev_sib) ?:

		bch2_foreground_maybe_merge_sibling(trans, path, level, flags,