bcachefs: trans->locked
authorKent Overstreet <kent.overstreet@linux.dev>
Tue, 9 Apr 2024 23:57:08 +0000 (19:57 -0400)
committerKent Overstreet <kent.overstreet@linux.dev>
Wed, 8 May 2024 21:29:19 +0000 (17:29 -0400)
Add a field for tracking whether a transaction object holds btree locks,
and assertions to verify state.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
fs/bcachefs/btree_iter.c
fs/bcachefs/btree_iter.h
fs/bcachefs/btree_locking.c
fs/bcachefs/btree_types.h

index bb037e4da299050974fbedd0cebf93516d37ee5c..224001d8470d69c1d756251070e9ecf1f9e85dad 100644 (file)
@@ -999,6 +999,7 @@ retry_all:
 
        bch2_trans_unlock(trans);
        cond_resched();
+       trans->locked = true;
 
        if (unlikely(trans->memory_allocation_failure)) {
                struct closure cl;
@@ -3023,7 +3024,8 @@ u32 bch2_trans_begin(struct btree_trans *trans)
        if (!trans->restarted &&
            (need_resched() ||
             time_after64(now, trans->last_begin_time + BTREE_TRANS_MAX_LOCK_HOLD_TIME_NS))) {
-               drop_locks_do(trans, (cond_resched(), 0));
+               bch2_trans_unlock(trans);
+               cond_resched();
                now = local_clock();
        }
        trans->last_begin_time = now;
@@ -3033,6 +3035,8 @@ u32 bch2_trans_begin(struct btree_trans *trans)
                bch2_trans_srcu_unlock(trans);
 
        trans->last_begin_ip = _RET_IP_;
+       trans->locked  = true;
+
        if (trans->restarted) {
                bch2_btree_path_traverse_all(trans);
                trans->notrace_relock_fail = false;
@@ -3090,7 +3094,7 @@ struct btree_trans *__bch2_trans_get(struct bch_fs *c, unsigned fn_idx)
                         */
                        BUG_ON(pos_task &&
                               pid == pos_task->pid &&
-                              bch2_trans_locked(pos));
+                              pos->locked);
 
                        if (pos_task && pid < pos_task->pid) {
                                list_add_tail(&trans->list, &pos->list);
@@ -3106,6 +3110,7 @@ got_trans:
        trans->last_begin_time  = local_clock();
        trans->fn_idx           = fn_idx;
        trans->locking_wait.task = current;
+       trans->locked           = true;
        trans->journal_replay_not_finished =
                unlikely(!test_bit(JOURNAL_REPLAY_DONE, &c->journal.flags)) &&
                atomic_inc_not_zero(&c->journal_keys.ref);
index 0d44d613e4ac16353e9a585d96f05c66c17cbe43..e4786610539029bb672939d50e7521bd242ca669 100644 (file)
@@ -286,7 +286,6 @@ int bch2_trans_relock(struct btree_trans *);
 int bch2_trans_relock_notrace(struct btree_trans *);
 void bch2_trans_unlock(struct btree_trans *);
 void bch2_trans_unlock_long(struct btree_trans *);
-bool bch2_trans_locked(struct btree_trans *);
 
 static inline int trans_was_restarted(struct btree_trans *trans, u32 restart_count)
 {
index 9a3ab56f6cfcb7ad6b581417989fc2756382dd1c..7824a7a2e50f7059b5732153edfa96b9de03330f 100644 (file)
@@ -490,8 +490,6 @@ static inline bool btree_path_get_locks(struct btree_trans *trans,
        if (path->uptodate == BTREE_ITER_NEED_RELOCK)
                path->uptodate = BTREE_ITER_UPTODATE;
 
-       bch2_trans_verify_locks(trans);
-
        return path->uptodate < BTREE_ITER_NEED_RELOCK;
 }
 
@@ -607,7 +605,9 @@ bool bch2_btree_path_relock_norestart(struct btree_trans *trans, struct btree_pa
 {
        struct get_locks_fail f;
 
-       return btree_path_get_locks(trans, path, false, &f);
+       bool ret = btree_path_get_locks(trans, path, false, &f);
+       bch2_trans_verify_locks(trans);
+       return ret;
 }
 
 int __bch2_btree_path_relock(struct btree_trans *trans,
@@ -630,7 +630,9 @@ bool bch2_btree_path_upgrade_noupgrade_sibs(struct btree_trans *trans,
 
        path->locks_want = new_locks_want;
 
-       return btree_path_get_locks(trans, path, true, f);
+       bool ret = btree_path_get_locks(trans, path, true, f);
+       bch2_trans_verify_locks(trans);
+       return ret;
 }
 
 bool __bch2_btree_path_upgrade(struct btree_trans *trans,
@@ -638,8 +640,9 @@ bool __bch2_btree_path_upgrade(struct btree_trans *trans,
                               unsigned new_locks_want,
                               struct get_locks_fail *f)
 {
-       if (bch2_btree_path_upgrade_noupgrade_sibs(trans, path, new_locks_want, f))
-               return true;
+       bool ret = bch2_btree_path_upgrade_noupgrade_sibs(trans, path, new_locks_want, f);
+       if (ret)
+               goto out;
 
        /*
         * XXX: this is ugly - we'd prefer to not be mucking with other
@@ -673,8 +676,9 @@ bool __bch2_btree_path_upgrade(struct btree_trans *trans,
                                btree_path_get_locks(trans, linked, true, NULL);
                        }
        }
-
-       return false;
+out:
+       bch2_trans_verify_locks(trans);
+       return ret;
 }
 
 void __bch2_btree_path_downgrade(struct btree_trans *trans,
@@ -774,6 +778,8 @@ static inline int __bch2_trans_relock(struct btree_trans *trans, bool trace)
 
        if (unlikely(trans->restarted))
                return -((int) trans->restarted);
+       if (unlikely(trans->locked))
+               goto out;
 
        trans_for_each_path(trans, path, i) {
                struct get_locks_fail f;
@@ -783,6 +789,8 @@ static inline int __bch2_trans_relock(struct btree_trans *trans, bool trace)
                        return bch2_trans_relock_fail(trans, path, &f, trace);
        }
 
+       trans->locked = true;
+out:
        bch2_trans_verify_locks(trans);
        return 0;
 }
@@ -800,11 +808,17 @@ int bch2_trans_relock_notrace(struct btree_trans *trans)
 void bch2_trans_unlock_noassert(struct btree_trans *trans)
 {
        __bch2_trans_unlock(trans);
+
+       trans->locked = false;
+       trans->last_unlock_ip = _RET_IP_;
 }
 
 void bch2_trans_unlock(struct btree_trans *trans)
 {
        __bch2_trans_unlock(trans);
+
+       trans->locked = false;
+       trans->last_unlock_ip = _RET_IP_;
 }
 
 void bch2_trans_unlock_long(struct btree_trans *trans)
@@ -813,17 +827,6 @@ void bch2_trans_unlock_long(struct btree_trans *trans)
        bch2_trans_srcu_unlock(trans);
 }
 
-bool bch2_trans_locked(struct btree_trans *trans)
-{
-       struct btree_path *path;
-       unsigned i;
-
-       trans_for_each_path(trans, path, i)
-               if (path->nodes_locked)
-                       return true;
-       return false;
-}
-
 int __bch2_trans_mutex_lock(struct btree_trans *trans,
                            struct mutex *lock)
 {
@@ -865,6 +868,17 @@ void bch2_btree_path_verify_locks(struct btree_path *path)
        }
 }
 
+static bool bch2_trans_locked(struct btree_trans *trans)
+{
+       struct btree_path *path;
+       unsigned i;
+
+       trans_for_each_path(trans, path, i)
+               if (path->nodes_locked)
+                       return true;
+       return false;
+}
+
 void bch2_trans_verify_locks(struct btree_trans *trans)
 {
        struct btree_path *path;
index 123abeec4ce9cf7c1082393f1ab1a2b823c89fba..a973ba6264d3be6957b41912ea00dce99524248b 100644 (file)
@@ -469,6 +469,8 @@ struct btree_trans {
        u8                      lock_must_abort;
        bool                    lock_may_not_fail:1;
        bool                    srcu_held:1;
+       bool                    locked:1;
+       bool                    write_locked:1;
        bool                    used_mempool:1;
        bool                    in_traverse_all:1;
        bool                    paths_sorted:1;
@@ -476,13 +478,13 @@ struct btree_trans {
        bool                    journal_transaction_names:1;
        bool                    journal_replay_not_finished:1;
        bool                    notrace_relock_fail:1;
-       bool                    write_locked:1;
        enum bch_errcode        restarted:16;
        u32                     restart_count;
 
        u64                     last_begin_time;
        unsigned long           last_begin_ip;
        unsigned long           last_restarted_ip;
+       unsigned long           last_unlock_ip;
        unsigned long           srcu_lock_time;
 
        const char              *fn;