Skip to content

Commit

Permalink
bcachefs: try_alloc_bucket() now uses bch2_check_discard_freespace_key()
Browse files Browse the repository at this point in the history
check_discard_freespace_key() was doing all the same checks as
try_alloc_bucket(), but with repair.

Signed-off-by: Kent Overstreet <[email protected]>
  • Loading branch information
Kent Overstreet committed Nov 15, 2024
1 parent fc37ce1 commit feb21a9
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 113 deletions.
80 changes: 45 additions & 35 deletions fs/bcachefs/alloc_background.c
Original file line number Diff line number Diff line change
Expand Up @@ -1332,51 +1332,53 @@ int bch2_check_alloc_hole_bucket_gens(struct btree_trans *trans,
return ret;
}

static noinline_for_stack int bch2_check_discard_freespace_key(struct btree_trans *trans,
struct btree_iter *iter)
int bch2_check_discard_freespace_key(struct btree_trans *trans, struct btree_iter *iter, u8 *gen)
{
struct bch_fs *c = trans->c;
struct btree_iter alloc_iter;
struct bkey_s_c alloc_k;
struct bch_alloc_v4 a_convert;
const struct bch_alloc_v4 *a;
u64 genbits;
struct bpos pos;
enum bch_data_type state = iter->btree_id == BTREE_ID_need_discard
? BCH_DATA_need_discard
: BCH_DATA_free;
struct printbuf buf = PRINTBUF;
int ret;

pos = iter->pos;
pos.offset &= ~(~0ULL << 56);
genbits = iter->pos.offset & (~0ULL << 56);
struct bpos bucket = iter->pos;
bucket.offset &= ~(~0ULL << 56);
u64 genbits = iter->pos.offset & (~0ULL << 56);

alloc_k = bch2_bkey_get_iter(trans, &alloc_iter, BTREE_ID_alloc, pos, 0);
ret = bkey_err(alloc_k);
struct btree_iter alloc_iter;
struct bkey_s_c alloc_k = bch2_bkey_get_iter(trans, &alloc_iter, BTREE_ID_alloc, bucket, BTREE_ITER_cached);
int ret = bkey_err(alloc_k);
if (ret)
return ret;

if (fsck_err_on(!bch2_dev_bucket_exists(c, pos),
trans, need_discard_freespace_key_to_invalid_dev_bucket,
"entry in %s btree for nonexistant dev:bucket %llu:%llu",
bch2_btree_id_str(iter->btree_id), pos.inode, pos.offset))
goto delete;
if (!bch2_dev_bucket_exists(c, bucket)) {
if (fsck_err(trans, need_discard_freespace_key_to_invalid_dev_bucket,
"entry in %s btree for nonexistant dev:bucket %llu:%llu",
bch2_btree_id_str(iter->btree_id), bucket.inode, bucket.offset))
goto delete;
ret = 1;
goto out;
}

a = bch2_alloc_to_v4(alloc_k, &a_convert);
struct bch_alloc_v4 a_convert;
const struct bch_alloc_v4 *a = bch2_alloc_to_v4(alloc_k, &a_convert);

if (a->data_type != state ||
(state == BCH_DATA_free &&
genbits != alloc_freespace_genbits(*a))) {
if (fsck_err(trans, need_discard_freespace_key_bad,
"%s\n incorrectly set at %s:%llu:%llu:0 (free %u, genbits %llu should be %llu)",
(bch2_bkey_val_to_text(&buf, c, alloc_k), buf.buf),
bch2_btree_id_str(iter->btree_id),
iter->pos.inode,
iter->pos.offset,
a->data_type == state,
genbits >> 56, alloc_freespace_genbits(*a) >> 56))
goto delete;
ret = 1;
goto out;
}

if (fsck_err_on(a->data_type != state ||
(state == BCH_DATA_free &&
genbits != alloc_freespace_genbits(*a)),
trans, need_discard_freespace_key_bad,
"%s\n incorrectly set at %s:%llu:%llu:0 (free %u, genbits %llu should be %llu)",
(bch2_bkey_val_to_text(&buf, c, alloc_k), buf.buf),
bch2_btree_id_str(iter->btree_id),
iter->pos.inode,
iter->pos.offset,
a->data_type == state,
genbits >> 56, alloc_freespace_genbits(*a) >> 56))
goto delete;
*gen = a->gen;
out:
fsck_err:
bch2_set_btree_iter_dontneed(&alloc_iter);
Expand All @@ -1386,10 +1388,18 @@ static noinline_for_stack int bch2_check_discard_freespace_key(struct btree_tran
delete:
ret = bch2_btree_bit_mod_iter(trans, iter, false) ?:
bch2_trans_commit(trans, NULL, NULL,
BCH_TRANS_COMMIT_no_enospc);
BCH_TRANS_COMMIT_no_enospc) ?:
1;
goto out;
}

static int bch2_check_discard_freespace_key_fsck(struct btree_trans *trans, struct btree_iter *iter)
{
u8 gen;
int ret = bch2_check_discard_freespace_key(trans, iter, &gen);
return ret < 0 ? ret : 0;
}

/*
* We've already checked that generation numbers in the bucket_gens btree are
* valid for buckets that exist; this just checks for keys for nonexistent
Expand Down Expand Up @@ -1544,7 +1554,7 @@ int bch2_check_alloc_info(struct bch_fs *c)
ret = for_each_btree_key(trans, iter,
BTREE_ID_need_discard, POS_MIN,
BTREE_ITER_prefetch, k,
bch2_check_discard_freespace_key(trans, &iter));
bch2_check_discard_freespace_key_fsck(trans, &iter));
if (ret)
goto err;

Expand All @@ -1557,7 +1567,7 @@ int bch2_check_alloc_info(struct bch_fs *c)
break;

ret = bkey_err(k) ?:
bch2_check_discard_freespace_key(trans, &iter);
bch2_check_discard_freespace_key_fsck(trans, &iter);
if (bch2_err_matches(ret, BCH_ERR_transaction_restart)) {
ret = 0;
continue;
Expand Down
2 changes: 2 additions & 0 deletions fs/bcachefs/alloc_background.h
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,8 @@ int bch2_alloc_key_to_dev_counters(struct btree_trans *, struct bch_dev *,
int bch2_trigger_alloc(struct btree_trans *, enum btree_id, unsigned,
struct bkey_s_c, struct bkey_s,
enum btree_iter_update_trigger_flags);

int bch2_check_discard_freespace_key(struct btree_trans *, struct btree_iter *, u8 *);
int bch2_check_alloc_info(struct bch_fs *);
int bch2_check_alloc_to_lru_refs(struct bch_fs *);
void bch2_dev_do_discards(struct bch_dev *);
Expand Down
93 changes: 15 additions & 78 deletions fs/bcachefs/alloc_foreground.c
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,8 @@ static inline unsigned open_buckets_reserved(enum bch_watermark watermark)
}

static struct open_bucket *__try_alloc_bucket(struct bch_fs *c, struct bch_dev *ca,
u64 bucket,
u64 bucket, u8 gen,
enum bch_watermark watermark,
const struct bch_alloc_v4 *a,
struct bucket_alloc_state *s,
struct closure *cl)
{
Expand Down Expand Up @@ -261,7 +260,7 @@ static struct open_bucket *__try_alloc_bucket(struct bch_fs *c, struct bch_dev *
ob->valid = true;
ob->sectors_free = ca->mi.bucket_size;
ob->dev = ca->dev_idx;
ob->gen = a->gen;
ob->gen = gen;
ob->bucket = bucket;
spin_unlock(&ob->lock);

Expand All @@ -282,98 +281,36 @@ static struct open_bucket *try_alloc_bucket(struct btree_trans *trans, struct bc
struct closure *cl)
{
struct bch_fs *c = trans->c;
struct btree_iter iter = { NULL };
struct bkey_s_c k;
struct open_bucket *ob;
struct bch_alloc_v4 a_convert;
const struct bch_alloc_v4 *a;
u64 b = freespace_iter->pos.offset & ~(~0ULL << 56);
unsigned genbits = freespace_iter->pos.offset >> 56;
struct printbuf buf = PRINTBUF;
int ret;

if (b < ca->mi.first_bucket || b >= ca->mi.nbuckets) {
prt_printf(&buf, "freespace btree has bucket outside allowed range %u-%llu\n"
" freespace key ",
ca->mi.first_bucket, ca->mi.nbuckets);
bch2_bkey_to_text(&buf, &freespace_iter->k);
bch2_trans_inconsistent(trans, "%s", buf.buf);
ob = ERR_PTR(-EIO);
goto err;
}

k = bch2_bkey_get_iter(trans, &iter,
BTREE_ID_alloc, POS(ca->dev_idx, b),
BTREE_ITER_cached);
ret = bkey_err(k);
if (ret) {
ob = ERR_PTR(ret);
goto err;
}

a = bch2_alloc_to_v4(k, &a_convert);

if (a->data_type != BCH_DATA_free) {
if (c->curr_recovery_pass <= BCH_RECOVERY_PASS_check_alloc_info) {
ob = NULL;
goto err;
}
u8 gen;

prt_printf(&buf, "non free bucket in freespace btree\n"
" freespace key ");
bch2_bkey_to_text(&buf, &freespace_iter->k);
prt_printf(&buf, "\n ");
bch2_bkey_val_to_text(&buf, c, k);
bch2_trans_inconsistent(trans, "%s", buf.buf);
ob = ERR_PTR(-EIO);
goto err;
}

if (genbits != (alloc_freespace_genbits(*a) >> 56) &&
c->curr_recovery_pass > BCH_RECOVERY_PASS_check_alloc_info) {
prt_printf(&buf, "bucket in freespace btree with wrong genbits (got %u should be %llu)\n"
" freespace key ",
genbits, alloc_freespace_genbits(*a) >> 56);
bch2_bkey_to_text(&buf, &freespace_iter->k);
prt_printf(&buf, "\n ");
bch2_bkey_val_to_text(&buf, c, k);
bch2_trans_inconsistent(trans, "%s", buf.buf);
ob = ERR_PTR(-EIO);
goto err;
}
int ret = bch2_check_discard_freespace_key(trans, freespace_iter, &gen);
if (ret < 0)
return ERR_PTR(ret);
if (ret)
return NULL;

if (c->curr_recovery_pass <= BCH_RECOVERY_PASS_check_extents_to_backpointers) {
if (unlikely(c->curr_recovery_pass <= BCH_RECOVERY_PASS_check_extents_to_backpointers)) {
struct bch_backpointer bp;
struct bpos bp_pos = POS_MIN;

ret = bch2_get_next_backpointer(trans, ca, POS(ca->dev_idx, b), -1,
&bp_pos, &bp,
BTREE_ITER_nopreserve);
if (ret) {
ob = ERR_PTR(ret);
goto err;
}
if (ret)
return ERR_PTR(ret);

if (!bkey_eq(bp_pos, POS_MAX)) {
/*
* Bucket may have data in it - we don't call
* bc2h_trans_inconnsistent() because fsck hasn't
* bch2_trans_inconsistent() because fsck hasn't
* finished yet
*/
ob = NULL;
goto err;
return NULL;
}
}

ob = __try_alloc_bucket(c, ca, b, watermark, a, s, cl);
if (!ob)
bch2_set_btree_iter_dontneed(&iter);
err:
if (iter.path)
bch2_set_btree_iter_dontneed(&iter);
bch2_trans_iter_exit(trans, &iter);
printbuf_exit(&buf);
return ob;
return __try_alloc_bucket(c, ca, b, gen, watermark, s, cl);
}

/*
Expand Down Expand Up @@ -452,7 +389,7 @@ bch2_bucket_alloc_early(struct btree_trans *trans,

s->buckets_seen++;

ob = __try_alloc_bucket(trans->c, ca, k.k->p.offset, watermark, a, s, cl);
ob = __try_alloc_bucket(trans->c, ca, k.k->p.offset, a->gen, watermark, s, cl);
next:
bch2_set_btree_iter_dontneed(&citer);
bch2_trans_iter_exit(trans, &citer);
Expand Down

0 comments on commit feb21a9

Please sign in to comment.