bcachefs: Fix implementation of KEY_TYPE_error

When force-removing a device, we were silently dropping extents that we
no longer had pointers for - we should have been switching them to
KEY_TYPE_error, so that reads for data that was lost return errors.

This patch adds the logic for switching a key to KEY_TYPE_error to
bch2_bkey_drop_ptr(), and improves the logic somewhat.

Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>

diff --git a/fs/bcachefs/btree_gc.c b/fs/bcachefs/btree_gc.c
index f14667390e2ceea4be5423ed2b9c4416f3200708..dbf44704d0fc3e9fa696421ec9eb4c6bf926bd0d 100644 (file)
--- a/fs/bcachefs/btree_gc.c
+++ b/fs/bcachefs/btree_gc.c
@@ -1767,7 +1767,6 @@ static int bch2_gc_btree_gens(struct bch_fs *c, enum btree_id btree_id)
                        bch2_bkey_buf_reassemble(&sk, c, k);
                        bch2_extent_normalize(c, bkey_i_to_s(sk.k));
 
-
                        commit_err =
                                bch2_trans_update(&trans, &iter, sk.k, 0) ?:
                                bch2_trans_commit(&trans, NULL, NULL,

diff --git a/fs/bcachefs/extents.c b/fs/bcachefs/extents.c
index 966d6ef4179391fd907fcdf10992a76d1f2d2455..7f1a5c81ef0986e8dec79696834988f67d3514dd 100644 (file)
--- a/fs/bcachefs/extents.c
+++ b/fs/bcachefs/extents.c
@@ -479,7 +479,7 @@ restart_narrow_pointers:
 
        bkey_for_each_ptr_decode(&k->k, ptrs, p, i)
                if (can_narrow_crc(p.crc, n)) {
-                       bch2_bkey_drop_ptr(bkey_i_to_s(k), &i->ptr);
+                       __bch2_bkey_drop_ptr(bkey_i_to_s(k), &i->ptr);
                        p.ptr.offset += p.crc.offset;
                        p.crc = n;
                        bch2_extent_ptr_decoded_append(k, &p);
@@ -784,41 +784,85 @@ static union bch_extent_entry *extent_entry_prev(struct bkey_ptrs ptrs,
        return i;
 }
 
-union bch_extent_entry *bch2_bkey_drop_ptr(struct bkey_s k,
-                                          struct bch_extent_ptr *ptr)
+static void extent_entry_drop(struct bkey_s k, union bch_extent_entry *entry)
+{
+       union bch_extent_entry *next = extent_entry_next(entry);
+
+       /* stripes have ptrs, but their layout doesn't work with this code */
+       BUG_ON(k.k->type == KEY_TYPE_stripe);
+
+       memmove_u64s_down(entry, next,
+                         (u64 *) bkey_val_end(k) - (u64 *) next);
+       k.k->u64s -= (u64 *) next - (u64 *) entry;
+}
+
+/*
+ * Returns pointer to the next entry after the one being dropped:
+ */
+union bch_extent_entry *__bch2_bkey_drop_ptr(struct bkey_s k,
+                                            struct bch_extent_ptr *ptr)
 {
        struct bkey_ptrs ptrs = bch2_bkey_ptrs(k);
-       union bch_extent_entry *dst, *src, *prev;
+       union bch_extent_entry *entry = to_entry(ptr), *next;
+       union bch_extent_entry *ret = entry;
        bool drop_crc = true;
 
        EBUG_ON(ptr < &ptrs.start->ptr ||
                ptr >= &ptrs.end->ptr);
        EBUG_ON(ptr->type != 1 << BCH_EXTENT_ENTRY_ptr);
 
-       src = extent_entry_next(to_entry(ptr));
-       if (src != ptrs.end &&
-           !extent_entry_is_crc(src))
-               drop_crc = false;
-
-       dst = to_entry(ptr);
-       while ((prev = extent_entry_prev(ptrs, dst))) {
-               if (extent_entry_is_ptr(prev))
+       for (next = extent_entry_next(entry);
+            next != ptrs.end;
+            next = extent_entry_next(next)) {
+               if (extent_entry_is_crc(next)) {
                        break;
-
-               if (extent_entry_is_crc(prev)) {
-                       if (drop_crc)
-                               dst = prev;
+               } else if (extent_entry_is_ptr(next)) {
+                       drop_crc = false;
                        break;
                }
+       }
+
+       extent_entry_drop(k, entry);
 
-               dst = prev;
+       while ((entry = extent_entry_prev(ptrs, entry))) {
+               if (extent_entry_is_ptr(entry))
+                       break;
+
+               if ((extent_entry_is_crc(entry) && drop_crc) ||
+                   extent_entry_is_stripe_ptr(entry)) {
+                       ret = (void *) ret - extent_entry_bytes(entry);
+                       extent_entry_drop(k, entry);
+               }
        }
 
-       memmove_u64s_down(dst, src,
-                         (u64 *) ptrs.end - (u64 *) src);
-       k.k->u64s -= (u64 *) src - (u64 *) dst;
+       return ret;
+}
+
+union bch_extent_entry *bch2_bkey_drop_ptr(struct bkey_s k,
+                                          struct bch_extent_ptr *ptr)
+{
+       bool have_dirty = bch2_bkey_dirty_devs(k.s_c).nr;
+       union bch_extent_entry *ret =
+               __bch2_bkey_drop_ptr(k, ptr);
+
+       /*
+        * If we deleted all the dirty pointers and there's still cached
+        * pointers, we could set the cached pointers to dirty if they're not
+        * stale - but to do that correctly we'd need to grab an open_bucket
+        * reference so that we don't race with bucket reuse:
+        */
+       if (have_dirty &&
+           !bch2_bkey_dirty_devs(k.s_c).nr) {
+               k.k->type = KEY_TYPE_error;
+               set_bkey_val_u64s(k.k, 0);
+               ret = NULL;
+       } else if (!bch2_bkey_nr_ptrs(k.s_c)) {
+               k.k->type = KEY_TYPE_deleted;
+               set_bkey_val_u64s(k.k, 0);
+               ret = NULL;
+       }
 
-       return dst;
+       return ret;
 }
 
 void bch2_bkey_drop_device(struct bkey_s k, unsigned dev)
@@ -888,10 +932,6 @@ bool bch2_extent_normalize(struct bch_fs *c, struct bkey_s k)
                ptr->cached &&
                ptr_stale(bch_dev_bkey_exists(c, ptr->dev), ptr));
 
-       /* will only happen if all pointers were cached: */
-       if (!bch2_bkey_nr_ptrs(k.s_c))
-               k.k->type = KEY_TYPE_deleted;
-
        return bkey_deleted(k.k);
 }
 

diff --git a/fs/bcachefs/extents.h b/fs/bcachefs/extents.h
index afd3067bb64eb83be7c16f954094f9c148c72be7..9c2567274a2b8d286707d6b1b3594b3d04007ac3 100644 (file)
--- a/fs/bcachefs/extents.h
+++ b/fs/bcachefs/extents.h
@@ -78,12 +78,12 @@ static inline size_t extent_entry_u64s(const union bch_extent_entry *entry)
 
 static inline bool extent_entry_is_ptr(const union bch_extent_entry *e)
 {
-       switch (extent_entry_type(e)) {
-       case BCH_EXTENT_ENTRY_ptr:
-               return true;
-       default:
-               return false;
-       }
+       return extent_entry_type(e) == BCH_EXTENT_ENTRY_ptr;
+}
+
+static inline bool extent_entry_is_stripe_ptr(const union bch_extent_entry *e)
+{
+       return extent_entry_type(e) == BCH_EXTENT_ENTRY_stripe_ptr;
 }
 
 static inline bool extent_entry_is_crc(const union bch_extent_entry *e)
@@ -578,6 +578,8 @@ void bch2_bkey_extent_entry_drop(struct bkey_i *, union bch_extent_entry *);
 void bch2_bkey_append_ptr(struct bkey_i *, struct bch_extent_ptr);
 void bch2_extent_ptr_decoded_append(struct bkey_i *,
                                    struct extent_ptr_decoded *);
+union bch_extent_entry *__bch2_bkey_drop_ptr(struct bkey_s,
+                                            struct bch_extent_ptr *);
 union bch_extent_entry *bch2_bkey_drop_ptr(struct bkey_s,
                                           struct bch_extent_ptr *);
 

diff --git a/fs/bcachefs/move.c b/fs/bcachefs/move.c
index fbb6c043ad9bd67ce5e56456b7797551dddf7c04..0db0ce503cd5a0a5d9a2be1c57d0c05e6cd6ec70 100644 (file)
--- a/fs/bcachefs/move.c
+++ b/fs/bcachefs/move.c
@@ -195,7 +195,7 @@ int bch2_migrate_index_update(struct bch_write_op *op)
                                extent_for_each_ptr(extent_i_to_s(new), new_ptr)
                                        new_ptr->cached = true;
 
-                       bch2_bkey_drop_ptr(bkey_i_to_s(insert), old_ptr);
+                       __bch2_bkey_drop_ptr(bkey_i_to_s(insert), old_ptr);
                }
 
                extent_for_each_ptr_decode(extent_i_to_s(new), p, entry) {