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>
This commit is contained in:
Kent Overstreet 2023-11-12 20:35:51 -05:00
parent cd404e5b05
commit e56978c80d
3 changed files with 24 additions and 140 deletions

View file

@ -1797,7 +1797,6 @@ struct btree *bch2_btree_iter_next_node(struct btree_iter *iter)
inline bool bch2_btree_iter_advance(struct btree_iter *iter) inline bool bch2_btree_iter_advance(struct btree_iter *iter)
{ {
if (likely(!(iter->flags & BTREE_ITER_ALL_LEVELS))) {
struct bpos pos = iter->k.p; struct bpos pos = iter->k.p;
bool ret = !(iter->flags & BTREE_ITER_ALL_SNAPSHOTS bool ret = !(iter->flags & BTREE_ITER_ALL_SNAPSHOTS
? bpos_eq(pos, SPOS_MAX) ? bpos_eq(pos, SPOS_MAX)
@ -1807,13 +1806,6 @@ inline bool bch2_btree_iter_advance(struct btree_iter *iter)
pos = bkey_successor(iter, pos); pos = bkey_successor(iter, pos);
bch2_btree_iter_set_pos(iter, pos); bch2_btree_iter_set_pos(iter, pos);
return ret; return ret;
} else {
if (!btree_path_node(iter->path, iter->path->level))
return true;
iter->advanced = true;
return false;
}
} }
inline bool bch2_btree_iter_rewind(struct btree_iter *iter) 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; struct bpos iter_pos;
int ret; int ret;
EBUG_ON(iter->flags & BTREE_ITER_ALL_LEVELS);
EBUG_ON((iter->flags & BTREE_ITER_FILTER_SNAPSHOTS) && bkey_eq(end, POS_MAX)); EBUG_ON((iter->flags & BTREE_ITER_FILTER_SNAPSHOTS) && bkey_eq(end, POS_MAX));
if (iter->update_path) { if (iter->update_path) {
@ -2203,103 +2194,6 @@ struct bkey_s_c bch2_btree_iter_peek_upto(struct btree_iter *iter, struct bpos e
goto out_no_locked; 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 * bch2_btree_iter_next() - returns first key greater than iterator's current
* position * 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(iter);
bch2_btree_iter_verify_entry_exit(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)); EBUG_ON(iter->path->level && (iter->flags & BTREE_ITER_WITH_KEY_CACHE));
/* extents can't span inode numbers: */ /* extents can't span inode numbers: */

View file

@ -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_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_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) static inline struct bkey_s_c bch2_btree_iter_peek(struct btree_iter *iter)
{ {
return bch2_btree_iter_peek_upto(iter, SPOS_MAX); 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 btree_id,
unsigned flags) 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)) && if (!(flags & (BTREE_ITER_ALL_SNAPSHOTS|BTREE_ITER_NOT_EXTENTS)) &&
btree_id_is_extents(btree_id)) btree_id_is_extents(btree_id))
flags |= BTREE_ITER_IS_EXTENTS; 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, static inline struct bkey_s_c bch2_btree_iter_peek_prev_type(struct btree_iter *iter,
unsigned flags) unsigned flags)
{ {
BUG_ON(flags & BTREE_ITER_ALL_LEVELS);
return 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_prev(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, static inline struct bkey_s_c bch2_btree_iter_peek_type(struct btree_iter *iter,
unsigned flags) unsigned flags)
{ {
return flags & BTREE_ITER_ALL_LEVELS ? bch2_btree_iter_peek_all_levels(iter) : return flags & BTREE_ITER_SLOTS ? bch2_btree_iter_peek_slot(iter) :
flags & BTREE_ITER_SLOTS ? bch2_btree_iter_peek_slot(iter) :
bch2_btree_iter_peek(iter); bch2_btree_iter_peek(iter);
} }

View file

@ -185,33 +185,32 @@ struct btree_node_iter {
* Iterate over all possible positions, synthesizing deleted keys for holes: * 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_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 * Indicates that intent locks should be taken on leaf nodes, because we expect
* to be doing updates: * 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: * 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 * Used in bch2_btree_iter_traverse(), to indicate whether we're searching for
* @pos or the first key strictly greater than @pos * @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_IS_EXTENTS = 1 << 3;
static const __maybe_unused u16 BTREE_ITER_NOT_EXTENTS = 1 << 5; static const __maybe_unused u16 BTREE_ITER_NOT_EXTENTS = 1 << 4;
static const __maybe_unused u16 BTREE_ITER_CACHED = 1 << 6; static const __maybe_unused u16 BTREE_ITER_CACHED = 1 << 5;
static const __maybe_unused u16 BTREE_ITER_WITH_KEY_CACHE = 1 << 7; static const __maybe_unused u16 BTREE_ITER_WITH_KEY_CACHE = 1 << 6;
static const __maybe_unused u16 BTREE_ITER_WITH_UPDATES = 1 << 8; static const __maybe_unused u16 BTREE_ITER_WITH_UPDATES = 1 << 7;
static const __maybe_unused u16 BTREE_ITER_WITH_JOURNAL = 1 << 9; static const __maybe_unused u16 BTREE_ITER_WITH_JOURNAL = 1 << 8;
static const __maybe_unused u16 __BTREE_ITER_ALL_SNAPSHOTS = 1 << 10; static const __maybe_unused u16 __BTREE_ITER_ALL_SNAPSHOTS = 1 << 9;
static const __maybe_unused u16 BTREE_ITER_ALL_SNAPSHOTS = 1 << 11; static const __maybe_unused u16 BTREE_ITER_ALL_SNAPSHOTS = 1 << 10;
static const __maybe_unused u16 BTREE_ITER_FILTER_SNAPSHOTS = 1 << 12; static const __maybe_unused u16 BTREE_ITER_FILTER_SNAPSHOTS = 1 << 11;
static const __maybe_unused u16 BTREE_ITER_NOPRESERVE = 1 << 13; static const __maybe_unused u16 BTREE_ITER_NOPRESERVE = 1 << 12;
static const __maybe_unused u16 BTREE_ITER_CACHED_NOFILL = 1 << 14; static const __maybe_unused u16 BTREE_ITER_CACHED_NOFILL = 1 << 13;
static const __maybe_unused u16 BTREE_ITER_KEY_CACHE_FILL = 1 << 15; static const __maybe_unused u16 BTREE_ITER_KEY_CACHE_FILL = 1 << 14;
#define __BTREE_ITER_FLAGS_END 16 #define __BTREE_ITER_FLAGS_END 15
enum btree_path_uptodate { enum btree_path_uptodate {
BTREE_ITER_UPTODATE = 0, BTREE_ITER_UPTODATE = 0,