Commit a56c6171 authored by Kent Overstreet's avatar Kent Overstreet

bcachefs: Make backpointer fsck wb flush check more rigorous

backpointers fsck now always runs in rw mode - the btree is being
modified while it runs, by e.g. copygc, rebalance, the discard worker,
the invalidate worker.

We could find a missing backpointer, flush the btree write buffer, and
then on the next iteration find a new key at the exact same position -
which will most likely need another write buffer flush.

Hence, we have to check for an exact match on last_flushed, not just the
pos.
Signed-off-by: default avatarKent Overstreet <kent.overstreet@linux.dev>
parent 0f64a6da
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
#include "bbpos.h" #include "bbpos.h"
#include "alloc_background.h" #include "alloc_background.h"
#include "backpointers.h" #include "backpointers.h"
#include "bkey_buf.h"
#include "btree_cache.h" #include "btree_cache.h"
#include "btree_update.h" #include "btree_update.h"
#include "btree_update_interior.h" #include "btree_update_interior.h"
...@@ -404,25 +405,23 @@ int bch2_check_btree_backpointers(struct bch_fs *c) ...@@ -404,25 +405,23 @@ int bch2_check_btree_backpointers(struct bch_fs *c)
return ret; return ret;
} }
struct bpos_level {
unsigned level;
struct bpos pos;
};
static int check_bp_exists(struct btree_trans *trans, static int check_bp_exists(struct btree_trans *trans,
struct bpos bucket, struct bpos bucket,
struct bch_backpointer bp, struct bch_backpointer bp,
struct bkey_s_c orig_k, struct bkey_s_c orig_k,
struct bpos bucket_start, struct bpos bucket_start,
struct bpos bucket_end, struct bpos bucket_end,
struct bpos_level *last_flushed) struct bkey_buf *last_flushed)
{ {
struct bch_fs *c = trans->c; struct bch_fs *c = trans->c;
struct btree_iter bp_iter = { NULL }; struct btree_iter bp_iter = { NULL };
struct printbuf buf = PRINTBUF; struct printbuf buf = PRINTBUF;
struct bkey_s_c bp_k; struct bkey_s_c bp_k;
struct bkey_buf tmp;
int ret; int ret;
bch2_bkey_buf_init(&tmp);
if (bpos_lt(bucket, bucket_start) || if (bpos_lt(bucket, bucket_start) ||
bpos_gt(bucket, bucket_end)) bpos_gt(bucket, bucket_end))
return 0; return 0;
...@@ -439,8 +438,11 @@ static int check_bp_exists(struct btree_trans *trans, ...@@ -439,8 +438,11 @@ static int check_bp_exists(struct btree_trans *trans,
if (bp_k.k->type != KEY_TYPE_backpointer || if (bp_k.k->type != KEY_TYPE_backpointer ||
memcmp(bkey_s_c_to_backpointer(bp_k).v, &bp, sizeof(bp))) { memcmp(bkey_s_c_to_backpointer(bp_k).v, &bp, sizeof(bp))) {
if (last_flushed->level != bp.level || if (!bpos_eq(orig_k.k->p, last_flushed->k->k.p) ||
!bpos_eq(last_flushed->pos, orig_k.k->p)) { bkey_bytes(orig_k.k) != bkey_bytes(&last_flushed->k->k) ||
memcmp(orig_k.v, &last_flushed->k->v, bkey_val_bytes(orig_k.k))) {
bch2_bkey_buf_reassemble(&tmp, c, orig_k);
if (bp.level) { if (bp.level) {
bch2_trans_unlock(trans); bch2_trans_unlock(trans);
bch2_btree_interior_updates_flush(c); bch2_btree_interior_updates_flush(c);
...@@ -450,8 +452,7 @@ static int check_bp_exists(struct btree_trans *trans, ...@@ -450,8 +452,7 @@ static int check_bp_exists(struct btree_trans *trans,
if (ret) if (ret)
goto err; goto err;
last_flushed->level = bp.level; bch2_bkey_buf_copy(last_flushed, c, tmp.k);
last_flushed->pos = orig_k.k->p;
ret = -BCH_ERR_transaction_restart_write_buffer_flush; ret = -BCH_ERR_transaction_restart_write_buffer_flush;
goto out; goto out;
} }
...@@ -461,6 +462,7 @@ static int check_bp_exists(struct btree_trans *trans, ...@@ -461,6 +462,7 @@ static int check_bp_exists(struct btree_trans *trans,
err: err:
fsck_err: fsck_err:
bch2_trans_iter_exit(trans, &bp_iter); bch2_trans_iter_exit(trans, &bp_iter);
bch2_bkey_buf_exit(&tmp, c);
printbuf_exit(&buf); printbuf_exit(&buf);
return ret; return ret;
missing: missing:
...@@ -482,7 +484,7 @@ static int check_extent_to_backpointers(struct btree_trans *trans, ...@@ -482,7 +484,7 @@ static int check_extent_to_backpointers(struct btree_trans *trans,
enum btree_id btree, unsigned level, enum btree_id btree, unsigned level,
struct bpos bucket_start, struct bpos bucket_start,
struct bpos bucket_end, struct bpos bucket_end,
struct bpos_level *last_flushed, struct bkey_buf *last_flushed,
struct bkey_s_c k) struct bkey_s_c k)
{ {
struct bch_fs *c = trans->c; struct bch_fs *c = trans->c;
...@@ -516,7 +518,7 @@ static int check_btree_root_to_backpointers(struct btree_trans *trans, ...@@ -516,7 +518,7 @@ static int check_btree_root_to_backpointers(struct btree_trans *trans,
enum btree_id btree_id, enum btree_id btree_id,
struct bpos bucket_start, struct bpos bucket_start,
struct bpos bucket_end, struct bpos bucket_end,
struct bpos_level *last_flushed, struct bkey_buf *last_flushed,
int *level) int *level)
{ {
struct bch_fs *c = trans->c; struct bch_fs *c = trans->c;
...@@ -621,10 +623,13 @@ static int bch2_check_extents_to_backpointers_pass(struct btree_trans *trans, ...@@ -621,10 +623,13 @@ static int bch2_check_extents_to_backpointers_pass(struct btree_trans *trans,
struct btree_iter iter; struct btree_iter iter;
enum btree_id btree_id; enum btree_id btree_id;
struct bkey_s_c k; struct bkey_s_c k;
struct bkey_buf last_flushed;
int ret = 0; int ret = 0;
bch2_bkey_buf_init(&last_flushed);
bkey_init(&last_flushed.k->k);
for (btree_id = 0; btree_id < btree_id_nr_alive(c); btree_id++) { for (btree_id = 0; btree_id < btree_id_nr_alive(c); btree_id++) {
struct bpos_level last_flushed = { UINT_MAX, POS_MIN };
int level, depth = btree_type_has_ptrs(btree_id) ? 0 : 1; int level, depth = btree_type_has_ptrs(btree_id) ? 0 : 1;
ret = commit_do(trans, NULL, NULL, ret = commit_do(trans, NULL, NULL,
...@@ -669,6 +674,7 @@ static int bch2_check_extents_to_backpointers_pass(struct btree_trans *trans, ...@@ -669,6 +674,7 @@ static int bch2_check_extents_to_backpointers_pass(struct btree_trans *trans,
} }
} }
bch2_bkey_buf_exit(&last_flushed, c);
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment