bcachefs: Add a counter for btree_trans restarts
authorKent Overstreet <kent.overstreet@gmail.com>
Sun, 17 Jul 2022 23:35:38 +0000 (19:35 -0400)
committerKent Overstreet <kent.overstreet@linux.dev>
Sun, 22 Oct 2023 21:09:36 +0000 (17:09 -0400)
This will help us improve nested transactions - we need to add
assertions that whenever an inner transaction handles a restart, it
still returns -EINTR to the outer transaction.

This also adds nested_lockrestart_do() and nested_commit_do() which use
the new counters to correctly return -EINTR when the transaction was
restarted.

Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
fs/bcachefs/btree_iter.c
fs/bcachefs/btree_iter.h
fs/bcachefs/btree_types.h
fs/bcachefs/btree_update.h

index 30958cbb95322a7f030b0905761ff15704d2efd8..45ecd196bceb2b064c1f95d074c2415c61ab2aa3 100644 (file)
@@ -3188,7 +3188,7 @@ void *bch2_trans_kmalloc(struct btree_trans *trans, size_t size)
  * node may return EINTR when the trylock fails. When this occurs
  * bch2_trans_begin() should be called and the transaction retried.
  */
-void bch2_trans_begin(struct btree_trans *trans)
+u32 bch2_trans_begin(struct btree_trans *trans)
 {
        struct btree_path *path;
 
@@ -3234,11 +3234,20 @@ void bch2_trans_begin(struct btree_trans *trans)
                bch2_trans_relock(trans);
        }
 
+       trans->last_restarted_ip = _RET_IP_;
        if (trans->restarted)
                bch2_btree_path_traverse_all(trans);
 
-       trans->restarted = false;
        trans->last_begin_time = ktime_get_ns();
+       return trans->restart_count;
+}
+
+void bch2_trans_verify_not_restarted(struct btree_trans *trans, u32 restart_count)
+{
+       bch2_trans_inconsistent_on(trans_was_restarted(trans, restart_count), trans,
+               "trans->restart_count %u, should be %u, last restarted by %ps\n",
+               trans->restart_count, restart_count,
+               (void *) trans->last_restarted_ip);
 }
 
 static void bch2_trans_alloc_paths(struct btree_trans *trans, struct bch_fs *c)
index 209b89dd1d2b69d0c3a282a6f20f0bb2436514d1..c2f5afc9eeb97dd90c42e88f1529b9c4440c03e4 100644 (file)
@@ -203,10 +203,18 @@ void bch2_path_put(struct btree_trans *, struct btree_path *, bool);
 bool bch2_trans_relock(struct btree_trans *);
 void bch2_trans_unlock(struct btree_trans *);
 
+static inline int trans_was_restarted(struct btree_trans *trans, u32 restart_count)
+{
+       return restart_count != trans->restart_count ? -EINTR : 0;
+}
+
+void bch2_trans_verify_not_restarted(struct btree_trans *, u32);
+
 __always_inline
 static inline int btree_trans_restart(struct btree_trans *trans)
 {
        trans->restarted = true;
+       trans->restart_count++;
        bch2_trans_unlock(trans);
        return -EINTR;
 }
@@ -321,7 +329,7 @@ static inline void set_btree_iter_dontneed(struct btree_iter *iter)
 }
 
 void *bch2_trans_kmalloc(struct btree_trans *, size_t);
-void bch2_trans_begin(struct btree_trans *);
+u32 bch2_trans_begin(struct btree_trans *);
 
 static inline struct btree *
 __btree_iter_peek_node_and_restart(struct btree_trans *trans, struct btree_iter *iter)
@@ -394,6 +402,42 @@ __bch2_btree_iter_peek_and_restart(struct btree_trans *trans,
        return k;
 }
 
+#define lockrestart_do(_trans, _do)                                    \
+({                                                                     \
+       int _ret;                                                       \
+                                                                       \
+       do {                                                            \
+               bch2_trans_begin(_trans);                               \
+               _ret = (_do);                                           \
+       } while (_ret == -EINTR);                                       \
+                                                                       \
+       _ret;                                                           \
+})
+
+/*
+ * nested_lockrestart_do(), nested_commit_do():
+ *
+ * These are like lockrestart_do() and commit_do(), with two differences:
+ *
+ *  - We don't call bch2_trans_begin() unless we had a transaction restart
+ *  - We return -EINTR if we succeeded after a transaction restart
+ */
+#define nested_lockrestart_do(_trans, _do)                             \
+({                                                                     \
+       u32 _restart_count, _orig_restart_count;                        \
+       int _ret;                                                       \
+                                                                       \
+       _restart_count = _orig_restart_count = (_trans)->restart_count; \
+                                                                       \
+       while ((_ret = (_do)) == -EINTR)                                \
+               _restart_count = bch2_trans_begin(_trans);              \
+                                                                       \
+       if (!_ret)                                                      \
+               bch2_trans_verify_not_restarted(_trans, _restart_count);\
+                                                                       \
+       _ret ?: trans_was_restarted(_trans, _orig_restart_count);       \
+})
+
 #define for_each_btree_key2(_trans, _iter, _btree_id,                  \
                            _start, _flags, _k, _do)                    \
 ({                                                                     \
index 64f4bc8913e85f3e09e440f2a635b6634e2d9172..0650a35581828bd8b67a674ba631e6991ef8a026 100644 (file)
@@ -410,6 +410,9 @@ struct btree_trans {
        bool                    memory_allocation_failure:1;
        bool                    journal_transaction_names:1;
        bool                    journal_replay_not_finished:1;
+       u32                     restart_count;
+       unsigned long           last_restarted_ip;
+
        /*
         * For when bch2_trans_update notices we'll be splitting a compressed
         * extent:
index 1c3dd012cae890dce9a3d38e61c4e5b1f119a029..9b5a8b18b01be959a0d59081b6a14e007de0fd62 100644 (file)
@@ -106,22 +106,14 @@ static inline int bch2_trans_commit(struct btree_trans *trans,
        return __bch2_trans_commit(trans);
 }
 
-#define lockrestart_do(_trans, _do)                                    \
-({                                                                     \
-       int _ret;                                                       \
-                                                                       \
-       do {                                                            \
-               bch2_trans_begin(_trans);                               \
-               _ret = (_do);                                           \
-       } while (_ret == -EINTR);                                       \
-                                                                       \
-       _ret;                                                           \
-})
-
 #define commit_do(_trans, _disk_res, _journal_seq, _flags, _do)        \
        lockrestart_do(_trans, _do ?: bch2_trans_commit(_trans, (_disk_res),\
                                        (_journal_seq), (_flags)))
 
+#define nested_commit_do(_trans, _disk_res, _journal_seq, _flags, _do) \
+       nested_lockrestart_do(_trans, _do ?: bch2_trans_commit(_trans, (_disk_res),\
+                                       (_journal_seq), (_flags)))
+
 #define bch2_trans_do(_c, _disk_res, _journal_seq, _flags, _do)                \
 ({                                                                     \
        struct btree_trans trans;                                       \