Skip to content

Commit cb6055e

Browse files
author
Kent Overstreet
committed
bcachefs: Handle race between stripe reuse, invalidate_stripe_to_dev
When creating a new stripe, we may reuse an existing stripe that has some empty and some nonempty blocks. Generally, the existing stripe won't change underneath us - except for block sector counts, which we copy to the new key in ec_stripe_key_update. But the device removal path can now invalidate stripe pointers to a device, and that can race with stripe reuse. Change ec_stripe_key_update() to check for and resolve this inconsistency. Signed-off-by: Kent Overstreet <[email protected]>
1 parent b1e5622 commit cb6055e

File tree

2 files changed

+55
-28
lines changed

2 files changed

+55
-28
lines changed

fs/bcachefs/ec.c

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,47 +1206,62 @@ void bch2_do_stripe_deletes(struct bch_fs *c)
12061206
/* stripe creation: */
12071207

12081208
static int ec_stripe_key_update(struct btree_trans *trans,
1209-
struct bkey_i_stripe *new,
1210-
bool create)
1209+
struct bkey_i_stripe *old,
1210+
struct bkey_i_stripe *new)
12111211
{
12121212
struct bch_fs *c = trans->c;
1213-
struct btree_iter iter;
1214-
struct bkey_s_c k;
1215-
int ret;
1213+
bool create = !old;
12161214

1217-
k = bch2_bkey_get_iter(trans, &iter, BTREE_ID_stripes,
1218-
new->k.p, BTREE_ITER_intent);
1219-
ret = bkey_err(k);
1215+
struct btree_iter iter;
1216+
struct bkey_s_c k = bch2_bkey_get_iter(trans, &iter, BTREE_ID_stripes,
1217+
new->k.p, BTREE_ITER_intent);
1218+
int ret = bkey_err(k);
12201219
if (ret)
12211220
goto err;
12221221

1223-
if (k.k->type != (create ? KEY_TYPE_deleted : KEY_TYPE_stripe)) {
1224-
bch2_fs_inconsistent(c, "error %s stripe: got existing key type %s",
1225-
create ? "creating" : "updating",
1226-
bch2_bkey_types[k.k->type]);
1222+
if (bch2_fs_inconsistent_on(k.k->type != (create ? KEY_TYPE_deleted : KEY_TYPE_stripe),
1223+
c, "error %s stripe: got existing key type %s",
1224+
create ? "creating" : "updating",
1225+
bch2_bkey_types[k.k->type])) {
12271226
ret = -EINVAL;
12281227
goto err;
12291228
}
12301229

12311230
if (k.k->type == KEY_TYPE_stripe) {
1232-
const struct bch_stripe *old = bkey_s_c_to_stripe(k).v;
1233-
unsigned i;
1231+
const struct bch_stripe *v = bkey_s_c_to_stripe(k).v;
12341232

1235-
if (old->nr_blocks != new->v.nr_blocks) {
1236-
bch_err(c, "error updating stripe: nr_blocks does not match");
1237-
ret = -EINVAL;
1238-
goto err;
1239-
}
1233+
BUG_ON(old->v.nr_blocks != new->v.nr_blocks);
1234+
BUG_ON(old->v.nr_blocks != v->nr_blocks);
1235+
1236+
for (unsigned i = 0; i < new->v.nr_blocks; i++) {
1237+
unsigned sectors = stripe_blockcount_get(v, i);
1238+
1239+
if (!bch2_extent_ptr_eq(old->v.ptrs[i], new->v.ptrs[i]) && sectors) {
1240+
struct printbuf buf = PRINTBUF;
12401241

1241-
for (i = 0; i < new->v.nr_blocks; i++) {
1242-
unsigned v = stripe_blockcount_get(old, i);
1242+
prt_printf(&buf, "stripe changed nonempty block %u", i);
1243+
prt_str(&buf, "\nold: ");
1244+
bch2_bkey_val_to_text(&buf, c, k);
1245+
prt_str(&buf, "\nnew: ");
1246+
bch2_bkey_val_to_text(&buf, c, bkey_i_to_s_c(&new->k_i));
1247+
bch2_fs_inconsistent(c, "%s", buf.buf);
1248+
printbuf_exit(&buf);
1249+
ret = -EINVAL;
1250+
goto err;
1251+
}
12431252

1244-
BUG_ON(v &&
1245-
(old->ptrs[i].dev != new->v.ptrs[i].dev ||
1246-
old->ptrs[i].gen != new->v.ptrs[i].gen ||
1247-
old->ptrs[i].offset != new->v.ptrs[i].offset));
1253+
/*
1254+
* If the stripe ptr changed underneath us, it must have
1255+
* been dev_remove_stripes() -> * invalidate_stripe_to_dev()
1256+
*/
1257+
if (!bch2_extent_ptr_eq(old->v.ptrs[i], v->ptrs[i])) {
1258+
BUG_ON(v->ptrs[i].dev != BCH_SB_MEMBER_INVALID);
1259+
1260+
if (bch2_extent_ptr_eq(old->v.ptrs[i], new->v.ptrs[i]))
1261+
new->v.ptrs[i].dev = BCH_SB_MEMBER_INVALID;
1262+
}
12481263

1249-
stripe_blockcount_set(&new->v, i, v);
1264+
stripe_blockcount_set(&new->v, i, sectors);
12501265
}
12511266
}
12521267

@@ -1508,8 +1523,10 @@ static void ec_stripe_create(struct ec_stripe_new *s)
15081523
BCH_TRANS_COMMIT_no_check_rw|
15091524
BCH_TRANS_COMMIT_no_enospc,
15101525
ec_stripe_key_update(trans,
1511-
bkey_i_to_stripe(&s->new_stripe.key),
1512-
!s->have_existing_stripe));
1526+
s->have_existing_stripe
1527+
? bkey_i_to_stripe(&s->existing_stripe.key)
1528+
: NULL,
1529+
bkey_i_to_stripe(&s->new_stripe.key)));
15131530
bch_err_msg(c, ret, "creating stripe key");
15141531
if (ret) {
15151532
goto err;

fs/bcachefs/extents.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,16 @@ void bch2_bkey_ptrs_to_text(struct printbuf *, struct bch_fs *,
695695
int bch2_bkey_ptrs_validate(struct bch_fs *, struct bkey_s_c,
696696
enum bch_validate_flags);
697697

698+
static inline bool bch2_extent_ptr_eq(struct bch_extent_ptr ptr1,
699+
struct bch_extent_ptr ptr2)
700+
{
701+
return (ptr1.cached == ptr2.cached &&
702+
ptr1.unwritten == ptr2.unwritten &&
703+
ptr1.offset == ptr2.offset &&
704+
ptr1.dev == ptr2.dev &&
705+
ptr1.dev == ptr2.dev);
706+
}
707+
698708
void bch2_ptr_swab(struct bkey_s);
699709

700710
const struct bch_extent_rebalance *bch2_bkey_rebalance_opts(struct bkey_s_c);

0 commit comments

Comments
 (0)