bcachefs: Kill BTREE_ITER_ALL_LEVELS

As discussed in the previous patch, BTREE_ITER_ALL_LEVELS appears to be
racy with concurrent interior node updates - and perhaps it is fixable,
but it's tricky and unnecessary.

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

diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
index bf4d2ade45355c7a03eda9b83735ff798a5c4ca6..f58e03e3e038712ad4f490145177907ef66903fc 100644 (file)
--- a/fs/bcachefs/btree_iter.c
+++ b/fs/bcachefs/btree_iter.c
@@ -1797,23 +1797,15 @@ err:
 
 inline bool bch2_btree_iter_advance(struct btree_iter *iter)
 {
-       if (likely(!(iter->flags & BTREE_ITER_ALL_LEVELS))) {
-               struct bpos pos = iter->k.p;
-               bool ret = !(iter->flags & BTREE_ITER_ALL_SNAPSHOTS
-                            ? bpos_eq(pos, SPOS_MAX)
-                            : bkey_eq(pos, SPOS_MAX));
-
-               if (ret && !(iter->flags & BTREE_ITER_IS_EXTENTS))
-                       pos = bkey_successor(iter, pos);
-               bch2_btree_iter_set_pos(iter, pos);
-               return ret;
-       } else {
-               if (!btree_path_node(iter->path, iter->path->level))
-                       return true;
+       struct bpos pos = iter->k.p;
+       bool ret = !(iter->flags & BTREE_ITER_ALL_SNAPSHOTS
+                    ? bpos_eq(pos, SPOS_MAX)
+                    : bkey_eq(pos, SPOS_MAX));
 
-               iter->advanced = true;
-               return false;
-       }
+       if (ret && !(iter->flags & BTREE_ITER_IS_EXTENTS))
+               pos = bkey_successor(iter, pos);
+       bch2_btree_iter_set_pos(iter, pos);
+       return ret;
 }
 
 inline bool bch2_btree_iter_rewind(struct btree_iter *iter)
@@ -2064,7 +2056,6 @@ struct bkey_s_c bch2_btree_iter_peek_upto(struct btree_iter *iter, struct bpos e
        struct bpos iter_pos;
        int ret;
 
-       EBUG_ON(iter->flags & BTREE_ITER_ALL_LEVELS);
        EBUG_ON((iter->flags & BTREE_ITER_FILTER_SNAPSHOTS) && bkey_eq(end, POS_MAX));
 
        if (iter->update_path) {
@@ -2203,103 +2194,6 @@ end:
        goto out_no_locked;
 }
 
-/**
- * bch2_btree_iter_peek_all_levels() - returns the first key greater than or
- * equal to iterator's current position, returning keys from every level of the
- * btree. For keys at different levels of the btree that compare equal, the key
- * from the lower level (leaf) is returned first.
- * @iter:      iterator to peek from
- *
- * Returns:    key if found, or an error extractable with bkey_err().
- */
-struct bkey_s_c bch2_btree_iter_peek_all_levels(struct btree_iter *iter)
-{
-       struct btree_trans *trans = iter->trans;
-       struct bkey_s_c k;
-       int ret;
-
-       EBUG_ON(iter->path->cached);
-       bch2_btree_iter_verify(iter);
-       BUG_ON(iter->path->level < iter->min_depth);
-       BUG_ON(!(iter->flags & BTREE_ITER_ALL_SNAPSHOTS));
-       EBUG_ON(!(iter->flags & BTREE_ITER_ALL_LEVELS));
-
-       while (1) {
-               iter->path = bch2_btree_path_set_pos(trans, iter->path, iter->pos,
-                                       iter->flags & BTREE_ITER_INTENT,
-                                       btree_iter_ip_allocated(iter));
-
-               ret = bch2_btree_path_traverse(trans, iter->path, iter->flags);
-               if (unlikely(ret)) {
-                       /* ensure that iter->k is consistent with iter->pos: */
-                       bch2_btree_iter_set_pos(iter, iter->pos);
-                       k = bkey_s_c_err(ret);
-                       goto out_no_locked;
-               }
-
-               /* Already at end? */
-               if (!btree_path_node(iter->path, iter->path->level)) {
-                       k = bkey_s_c_null;
-                       goto out_no_locked;
-               }
-
-               k = btree_path_level_peek_all(trans->c,
-                               &iter->path->l[iter->path->level], &iter->k);
-
-               /* Check if we should go up to the parent node: */
-               if (!k.k ||
-                   (iter->advanced &&
-                    bpos_eq(path_l(iter->path)->b->key.k.p, iter->pos))) {
-                       iter->pos = path_l(iter->path)->b->key.k.p;
-                       btree_path_set_level_up(trans, iter->path);
-                       iter->advanced = false;
-                       continue;
-               }
-
-               /*
-                * Check if we should go back down to a leaf:
-                * If we're not in a leaf node, we only return the current key
-                * if it exactly matches iter->pos - otherwise we first have to
-                * go back to the leaf:
-                */
-               if (iter->path->level != iter->min_depth &&
-                   (iter->advanced ||
-                    !k.k ||
-                    !bpos_eq(iter->pos, k.k->p))) {
-                       btree_path_set_level_down(trans, iter->path, iter->min_depth);
-                       iter->pos = bpos_successor(iter->pos);
-                       iter->advanced = false;
-                       continue;
-               }
-
-               /* Check if we should go to the next key: */
-               if (iter->path->level == iter->min_depth &&
-                   iter->advanced &&
-                   k.k &&
-                   bpos_eq(iter->pos, k.k->p)) {
-                       iter->pos = bpos_successor(iter->pos);
-                       iter->advanced = false;
-                       continue;
-               }
-
-               if (iter->advanced &&
-                   iter->path->level == iter->min_depth &&
-                   !bpos_eq(k.k->p, iter->pos))
-                       iter->advanced = false;
-
-               BUG_ON(iter->advanced);
-               BUG_ON(!k.k);
-               break;
-       }
-
-       iter->pos = k.k->p;
-       btree_path_set_should_be_locked(iter->path);
-out_no_locked:
-       bch2_btree_iter_verify(iter);
-
-       return k;
-}
-
 /**
  * bch2_btree_iter_next() - returns first key greater than iterator's current
  * position
@@ -2466,7 +2360,6 @@ struct bkey_s_c bch2_btree_iter_peek_slot(struct btree_iter *iter)
 
        bch2_btree_iter_verify(iter);
        bch2_btree_iter_verify_entry_exit(iter);
-       EBUG_ON(iter->flags & BTREE_ITER_ALL_LEVELS);
        EBUG_ON(iter->path->level && (iter->flags & BTREE_ITER_WITH_KEY_CACHE));
 
        /* extents can't span inode numbers: */

diff --git a/fs/bcachefs/btree_iter.h b/fs/bcachefs/btree_iter.h
index eaffced4c132b538f2ffc1909f315f4b26902615..7286f92b1974dd3c58c9b67e8379c87b2c0262cf 100644 (file)
--- a/fs/bcachefs/btree_iter.h
+++ b/fs/bcachefs/btree_iter.h
@@ -348,8 +348,6 @@ struct btree *bch2_btree_iter_next_node(struct btree_iter *);
 struct bkey_s_c bch2_btree_iter_peek_upto(struct btree_iter *, struct bpos);
 struct bkey_s_c bch2_btree_iter_next(struct btree_iter *);
 
-struct bkey_s_c bch2_btree_iter_peek_all_levels(struct btree_iter *);
-
 static inline struct bkey_s_c bch2_btree_iter_peek(struct btree_iter *iter)
 {
        return bch2_btree_iter_peek_upto(iter, SPOS_MAX);
@@ -408,9 +406,6 @@ static inline unsigned __bch2_btree_iter_flags(struct btree_trans *trans,
                                               unsigned btree_id,
                                               unsigned flags)
 {
-       if (flags & BTREE_ITER_ALL_LEVELS)
-               flags |= BTREE_ITER_ALL_SNAPSHOTS|__BTREE_ITER_ALL_SNAPSHOTS;
-
        if (!(flags & (BTREE_ITER_ALL_SNAPSHOTS|BTREE_ITER_NOT_EXTENTS)) &&
            btree_id_is_extents(btree_id))
                flags |= BTREE_ITER_IS_EXTENTS;
@@ -606,8 +601,6 @@ u32 bch2_trans_begin(struct btree_trans *);
 static inline struct bkey_s_c bch2_btree_iter_peek_prev_type(struct btree_iter *iter,
                                                             unsigned flags)
 {
-       BUG_ON(flags & BTREE_ITER_ALL_LEVELS);
-
        return  flags & BTREE_ITER_SLOTS      ? bch2_btree_iter_peek_slot(iter) :
                                                bch2_btree_iter_peek_prev(iter);
 }
@@ -615,8 +608,7 @@ static inline struct bkey_s_c bch2_btree_iter_peek_prev_type(struct btree_iter *
 static inline struct bkey_s_c bch2_btree_iter_peek_type(struct btree_iter *iter,
                                                        unsigned flags)
 {
-       return  flags & BTREE_ITER_ALL_LEVELS ? bch2_btree_iter_peek_all_levels(iter) :
-               flags & BTREE_ITER_SLOTS      ? bch2_btree_iter_peek_slot(iter) :
+       return  flags & BTREE_ITER_SLOTS      ? bch2_btree_iter_peek_slot(iter) :
                                                bch2_btree_iter_peek(iter);
 }
 

diff --git a/fs/bcachefs/btree_types.h b/fs/bcachefs/btree_types.h
index b667da4e8403648c21e31ace7aa203abb838860f..7f17c23326c7a7503eff51c9f6fc7f61fd54001d 100644 (file)
--- a/fs/bcachefs/btree_types.h
+++ b/fs/bcachefs/btree_types.h
@@ -185,33 +185,32 @@ struct btree_node_iter {
  * Iterate over all possible positions, synthesizing deleted keys for holes:
  */
 static const __maybe_unused u16 BTREE_ITER_SLOTS               = 1 << 0;
-static const __maybe_unused u16 BTREE_ITER_ALL_LEVELS          = 1 << 1;
 /*
  * Indicates that intent locks should be taken on leaf nodes, because we expect
  * to be doing updates:
  */
-static const __maybe_unused u16 BTREE_ITER_INTENT              = 1 << 2;
+static const __maybe_unused u16 BTREE_ITER_INTENT              = 1 << 1;
 /*
  * Causes the btree iterator code to prefetch additional btree nodes from disk:
  */
-static const __maybe_unused u16 BTREE_ITER_PREFETCH            = 1 << 3;
+static const __maybe_unused u16 BTREE_ITER_PREFETCH            = 1 << 2;
 /*
  * Used in bch2_btree_iter_traverse(), to indicate whether we're searching for
  * @pos or the first key strictly greater than @pos
  */
-static const __maybe_unused u16 BTREE_ITER_IS_EXTENTS          = 1 << 4;
-static const __maybe_unused u16 BTREE_ITER_NOT_EXTENTS         = 1 << 5;
-static const __maybe_unused u16 BTREE_ITER_CACHED              = 1 << 6;
-static const __maybe_unused u16 BTREE_ITER_WITH_KEY_CACHE      = 1 << 7;
-static const __maybe_unused u16 BTREE_ITER_WITH_UPDATES                = 1 << 8;
-static const __maybe_unused u16 BTREE_ITER_WITH_JOURNAL                = 1 << 9;
-static const __maybe_unused u16 __BTREE_ITER_ALL_SNAPSHOTS     = 1 << 10;
-static const __maybe_unused u16 BTREE_ITER_ALL_SNAPSHOTS       = 1 << 11;
-static const __maybe_unused u16 BTREE_ITER_FILTER_SNAPSHOTS    = 1 << 12;
-static const __maybe_unused u16 BTREE_ITER_NOPRESERVE          = 1 << 13;
-static const __maybe_unused u16 BTREE_ITER_CACHED_NOFILL       = 1 << 14;
-static const __maybe_unused u16 BTREE_ITER_KEY_CACHE_FILL      = 1 << 15;
-#define __BTREE_ITER_FLAGS_END                                        16
+static const __maybe_unused u16 BTREE_ITER_IS_EXTENTS          = 1 << 3;
+static const __maybe_unused u16 BTREE_ITER_NOT_EXTENTS         = 1 << 4;
+static const __maybe_unused u16 BTREE_ITER_CACHED              = 1 << 5;
+static const __maybe_unused u16 BTREE_ITER_WITH_KEY_CACHE      = 1 << 6;
+static const __maybe_unused u16 BTREE_ITER_WITH_UPDATES                = 1 << 7;
+static const __maybe_unused u16 BTREE_ITER_WITH_JOURNAL                = 1 << 8;
+static const __maybe_unused u16 __BTREE_ITER_ALL_SNAPSHOTS     = 1 << 9;
+static const __maybe_unused u16 BTREE_ITER_ALL_SNAPSHOTS       = 1 << 10;
+static const __maybe_unused u16 BTREE_ITER_FILTER_SNAPSHOTS    = 1 << 11;
+static const __maybe_unused u16 BTREE_ITER_NOPRESERVE          = 1 << 12;
+static const __maybe_unused u16 BTREE_ITER_CACHED_NOFILL       = 1 << 13;
+static const __maybe_unused u16 BTREE_ITER_KEY_CACHE_FILL      = 1 << 14;
+#define __BTREE_ITER_FLAGS_END                                        15
 
 enum btree_path_uptodate {
        BTREE_ITER_UPTODATE             = 0,