bcachefs: trans_for_each_path_safe()

bch2_btree_trans_to_text() is used on btree_trans objects that are owned
by different threads - when printing out deadlock cycles - so we need a
safe version of trans_for_each_path(), else we race with seeing a
btree_path that was just allocated and not fully initialized:

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
index f0d0b64a55a40c5450bf67c1b6a0a80fff1e2dee..4830d203b37b336e6e0d33dc289860e2970e851f 100644 (file)
--- a/fs/bcachefs/btree_iter.c
+++ b/fs/bcachefs/btree_iter.c
@@ -2914,6 +2914,10 @@ static void bch2_trans_alloc_paths(struct btree_trans *trans, struct bch_fs *c)
 #endif
        if (!p)
                p = mempool_alloc(&trans->c->btree_paths_pool, GFP_NOFS);
+       /*
+        * paths need to be zeroed, bch2_check_for_deadlock looks at paths in
+        * other threads
+        */
 
        trans->paths            = p; p += paths_bytes;
        trans->updates          = p; p += updates_bytes;
@@ -3111,7 +3115,7 @@ void bch2_btree_trans_to_text(struct printbuf *out, struct btree_trans *trans)
        struct btree_path *path;
        struct btree_bkey_cached_common *b;
        static char lock_types[] = { 'r', 'i', 'w' };
-       unsigned l;
+       unsigned l, idx;
 
        if (!out->nr_tabstops) {
                printbuf_tabstop_push(out, 16);
@@ -3120,7 +3124,7 @@ void bch2_btree_trans_to_text(struct printbuf *out, struct btree_trans *trans)
 
        prt_printf(out, "%i %s\n", trans->locking_wait.task->pid, trans->fn);
 
-       trans_for_each_path(trans, path) {
+       trans_for_each_path_safe(trans, path, idx) {
                if (!path->nodes_locked)
                        continue;
 

diff --git a/fs/bcachefs/btree_iter.h b/fs/bcachefs/btree_iter.h
index 0cfb8af3d0e1ff4569502b5b8c6e91bfa634679b..9a4dbf358fe566d74f10cc600fda99839a80891a 100644 (file)
--- a/fs/bcachefs/btree_iter.h
+++ b/fs/bcachefs/btree_iter.h
@@ -89,6 +89,35 @@ __trans_next_path(struct btree_trans *trans, unsigned idx)
 #define trans_for_each_path(_trans, _path)                             \
        trans_for_each_path_from(_trans, _path, 0)
 
+static inline struct btree_path *
+__trans_next_path_safe(struct btree_trans *trans, unsigned *idx)
+{
+       u64 l;
+
+       if (*idx == BTREE_ITER_MAX)
+               return NULL;
+
+       l = trans->paths_allocated >> *idx;
+       if (!l)
+               return NULL;
+
+       *idx += __ffs64(l);
+       EBUG_ON(*idx >= BTREE_ITER_MAX);
+       return &trans->paths[*idx];
+}
+
+/*
+ * This version is intended to be safe for use on a btree_trans that is owned by
+ * another thread, for bch2_btree_trans_to_text();
+ */
+#define trans_for_each_path_safe_from(_trans, _path, _idx, _start)     \
+       for (_idx = _start;                                             \
+            (_path = __trans_next_path_safe((_trans), &_idx));         \
+            _idx++)
+
+#define trans_for_each_path_safe(_trans, _path, _idx)                  \
+       trans_for_each_path_safe_from(_trans, _path, _idx, 0)
+
 static inline struct btree_path *next_btree_path(struct btree_trans *trans, struct btree_path *path)
 {
        unsigned idx = path ? path->sorted_idx + 1 : 0;

diff --git a/fs/bcachefs/btree_locking.c b/fs/bcachefs/btree_locking.c
index 6e1306add4431d686dfb03ee9f7fede7605ecdf4..1f4eca898ab7fb12724200febd3e3bc415e64d55 100644 (file)
--- a/fs/bcachefs/btree_locking.c
+++ b/fs/bcachefs/btree_locking.c
@@ -255,6 +255,7 @@ int bch2_check_for_deadlock(struct btree_trans *trans, struct printbuf *cycle)
        struct trans_waiting_for_lock *top;
        struct btree_bkey_cached_common *b;
        struct btree_path *path;
+       unsigned path_idx;
        int ret;
 
        if (trans->lock_must_abort) {
@@ -273,12 +274,12 @@ next:
 
        top = &g.g[g.nr - 1];
 
-       trans_for_each_path_from(top->trans, path, top->path_idx) {
+       trans_for_each_path_safe_from(top->trans, path, path_idx, top->path_idx) {
                if (!path->nodes_locked)
                        continue;
 
-               if (top->path_idx != path->idx) {
-                       top->path_idx           = path->idx;
+               if (path_idx != top->path_idx) {
+                       top->path_idx           = path_idx;
                        top->level              = 0;
                        top->lock_start_time    = 0;
                }