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 <kent.overstreet@linux.dev>

diff --git a/fs/bcachefs/data_update.c b/fs/bcachefs/data_update.c
index 0b6f765..c606f07 100644 (file)
--- 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 ee38bd6..5d86907 100644 (file)
--- 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 0486c7e..f00c57c 100644 (file)
--- 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))