From 1be887979bc12a6c88b33b0d53dfdc369bfa9d49 Mon Sep 17 00:00:00 2001 From: Kent Overstreet Date: Sun, 9 Oct 2022 03:32:17 -0400 Subject: [PATCH] bcachefs: Handle dropping pointers in data_update path Cached pointers are generally dropped, not moved: this led to an assertion firing in the data update path when there were no new replicas being written. This path adds a data_options field for pointers to be dropped, and tweaks move_extent() to check if we're only dropping pointers, not writing new ones, before kicking off a data update operation. Signed-off-by: Kent Overstreet --- fs/bcachefs/data_update.c | 23 ++++++++++++++-- fs/bcachefs/data_update.h | 2 ++ fs/bcachefs/move.c | 56 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/fs/bcachefs/data_update.c b/fs/bcachefs/data_update.c index 0b6f765bcad9..c606f075688f 100644 --- a/fs/bcachefs/data_update.c +++ b/fs/bcachefs/data_update.c @@ -331,8 +331,9 @@ int bch2_data_update_init(struct bch_fs *c, struct data_update *m, i = 0; bkey_for_each_ptr_decode(k.k, ptrs, p, entry) { - if (p.ptr.cached) - m->data_opts.rewrite_ptrs &= ~(1U << i); + if (((1U << i) & m->data_opts.rewrite_ptrs) && + p.ptr.cached) + BUG(); if (!((1U << i) & m->data_opts.rewrite_ptrs)) bch2_dev_list_add_dev(&m->op.devs_have, p.ptr.dev); @@ -368,5 +369,23 @@ int bch2_data_update_init(struct bch_fs *c, struct data_update *m, m->op.nr_replicas = m->op.nr_replicas_required = hweight32(m->data_opts.rewrite_ptrs) + m->data_opts.extra_replicas; + + BUG_ON(!m->op.nr_replicas); return 0; } + +void bch2_data_update_opts_normalize(struct bkey_s_c k, struct data_update_opts *opts) +{ + struct bkey_ptrs_c ptrs = bch2_bkey_ptrs_c(k); + const struct bch_extent_ptr *ptr; + unsigned i = 0; + + bkey_for_each_ptr(ptrs, ptr) { + if ((opts->rewrite_ptrs & (1U << i)) && ptr->cached) { + opts->kill_ptrs |= 1U << i; + opts->rewrite_ptrs ^= 1U << i; + } + + i++; + } +} diff --git a/fs/bcachefs/data_update.h b/fs/bcachefs/data_update.h index ee38bd655af1..5d8690795959 100644 --- a/fs/bcachefs/data_update.h +++ b/fs/bcachefs/data_update.h @@ -10,6 +10,7 @@ struct moving_context; struct data_update_opts { unsigned rewrite_ptrs; + unsigned kill_ptrs; u16 target; u8 extra_replicas; unsigned btree_insert_flags; @@ -35,5 +36,6 @@ int bch2_data_update_init(struct bch_fs *, struct data_update *, struct write_point_specifier, struct bch_io_opts, struct data_update_opts, enum btree_id, struct bkey_s_c); +void bch2_data_update_opts_normalize(struct bkey_s_c, struct data_update_opts *); #endif /* _BCACHEFS_DATA_UPDATE_H */ diff --git a/fs/bcachefs/move.c b/fs/bcachefs/move.c index 0486c7e14c56..f00c57c8e7a3 100644 --- a/fs/bcachefs/move.c +++ b/fs/bcachefs/move.c @@ -183,7 +183,52 @@ void bch_move_stats_init(struct bch_move_stats *stats, char *name) scnprintf(stats->name, sizeof(stats->name), "%s", name); } +static int bch2_extent_drop_ptrs(struct btree_trans *trans, + struct btree_iter *iter, + struct bkey_s_c k, + struct data_update_opts data_opts) +{ + struct bch_fs *c = trans->c; + struct bkey_i *n; + int ret; + + n = bch2_trans_kmalloc(trans, bkey_bytes(k.k)); + ret = PTR_ERR_OR_ZERO(n); + if (ret) + return ret; + + bkey_reassemble(n, k); + + while (data_opts.kill_ptrs) { + unsigned i = 0, drop = __fls(data_opts.kill_ptrs); + struct bch_extent_ptr *ptr; + + bch2_bkey_drop_ptrs(bkey_i_to_s(n), ptr, i++ == drop); + data_opts.kill_ptrs ^= 1U << drop; + } + + /* + * If the new extent no longer has any pointers, bch2_extent_normalize() + * will do the appropriate thing with it (turning it into a + * KEY_TYPE_error key, or just a discard if it was a cached extent) + */ + bch2_extent_normalize(c, bkey_i_to_s(n)); + + /* + * Since we're not inserting through an extent iterator + * (BTREE_ITER_ALL_SNAPSHOTS iterators aren't extent iterators), + * we aren't using the extent overwrite path to delete, we're + * just using the normal key deletion path: + */ + if (bkey_deleted(&n->k)) + n->k.size = 0; + + return bch2_trans_update(trans, iter, n, BTREE_UPDATE_INTERNAL_SNAPSHOT_NODE) ?: + bch2_trans_commit(trans, NULL, NULL, BTREE_INSERT_NOFAIL); +} + static int bch2_move_extent(struct btree_trans *trans, + struct btree_iter *iter, struct moving_context *ctxt, struct bch_io_opts io_opts, enum btree_id btree_id, @@ -198,6 +243,15 @@ static int bch2_move_extent(struct btree_trans *trans, unsigned sectors = k.k->size, pages; int ret = -ENOMEM; + bch2_data_update_opts_normalize(k, &data_opts); + + if (!data_opts.rewrite_ptrs && + !data_opts.extra_replicas) { + if (data_opts.kill_ptrs) + return bch2_extent_drop_ptrs(trans, iter, k, data_opts); + return 0; + } + if (!percpu_ref_tryget_live(&c->writes)) return -EROFS; @@ -429,7 +483,7 @@ static int __bch2_move_data(struct moving_context *ctxt, bch2_bkey_buf_reassemble(&sk, c, k); k = bkey_i_to_s_c(sk.k); - ret2 = bch2_move_extent(&trans, ctxt, io_opts, + ret2 = bch2_move_extent(&trans, &iter, ctxt, io_opts, btree_id, k, data_opts); if (ret2) { if (bch2_err_matches(ret2, BCH_ERR_transaction_restart)) -- 2.20.1