bcachefs: Clean up key merging

This patch simplifies the key merging code by getting rid of partial
merges - it's simpler and saner if we just don't merge extents when
they'd overflow k->size.

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

diff --git a/fs/bcachefs/bkey_methods.c b/fs/bcachefs/bkey_methods.c
index cf2e054cca2f75c63f677528bfcd3148a5bc8889..ff9d770aabea452e738884a8670d09e99c9ce8b9 100644 (file)
--- a/fs/bcachefs/bkey_methods.c
+++ b/fs/bcachefs/bkey_methods.c
@@ -84,7 +84,7 @@ static void key_type_inline_data_to_text(struct printbuf *out, struct bch_fs *c,
        .val_to_text    = key_type_inline_data_to_text, \
 }
 
-static const struct bkey_ops bch2_bkey_ops[] = {
+const struct bkey_ops bch2_bkey_ops[] = {
 #define x(name, nr) [KEY_TYPE_##name]  = bch2_bkey_ops_##name,
        BCH_BKEY_TYPES()
 #undef x
@@ -292,24 +292,11 @@ bool bch2_bkey_normalize(struct bch_fs *c, struct bkey_s k)
                : false;
 }
 
-enum merge_result bch2_bkey_merge(struct bch_fs *c,
-                                 struct bkey_s l, struct bkey_s r)
+bool bch2_bkey_merge(struct bch_fs *c, struct bkey_s l, struct bkey_s_c r)
 {
        const struct bkey_ops *ops = &bch2_bkey_ops[l.k->type];
-       enum merge_result ret;
 
-       if (bch2_key_merging_disabled ||
-           !ops->key_merge ||
-           l.k->type != r.k->type ||
-           bversion_cmp(l.k->version, r.k->version) ||
-           bpos_cmp(l.k->p, bkey_start_pos(r.k)))
-               return BCH_MERGE_NOMERGE;
-
-       ret = ops->key_merge(c, l, r);
-
-       if (ret != BCH_MERGE_NOMERGE)
-               l.k->needs_whiteout |= r.k->needs_whiteout;
-       return ret;
+       return bch2_bkey_maybe_mergable(l.k, r.k) && ops->key_merge(c, l, r);
 }
 
 static const struct old_bkey_type {

diff --git a/fs/bcachefs/bkey_methods.h b/fs/bcachefs/bkey_methods.h
index bfa6f112aeed17519677d96bc41ca9177f91abe4..3012035db1a33b3acd902563c0a7ea70c2718108 100644 (file)
--- a/fs/bcachefs/bkey_methods.h
+++ b/fs/bcachefs/bkey_methods.h
@@ -11,17 +11,6 @@ enum btree_node_type;
 
 extern const char * const bch2_bkey_types[];
 
-enum merge_result {
-       BCH_MERGE_NOMERGE,
-
-       /*
-        * The keys were mergeable, but would have overflowed size - so instead
-        * l was changed to the maximum size, and both keys were modified:
-        */
-       BCH_MERGE_PARTIAL,
-       BCH_MERGE_MERGE,
-};
-
 struct bkey_ops {
        /* Returns reason for being invalid if invalid, else NULL: */
        const char *    (*key_invalid)(const struct bch_fs *,
@@ -30,13 +19,14 @@ struct bkey_ops {
                                       struct bkey_s_c);
        void            (*swab)(struct bkey_s);
        bool            (*key_normalize)(struct bch_fs *, struct bkey_s);
-       enum merge_result (*key_merge)(struct bch_fs *,
-                                      struct bkey_s, struct bkey_s);
+       bool            (*key_merge)(struct bch_fs *, struct bkey_s, struct bkey_s_c);
        void            (*compat)(enum btree_id id, unsigned version,
                                  unsigned big_endian, int write,
                                  struct bkey_s);
 };
 
+extern const struct bkey_ops bch2_bkey_ops[];
+
 const char *bch2_bkey_val_invalid(struct bch_fs *, struct bkey_s_c);
 const char *__bch2_bkey_invalid(struct bch_fs *, struct bkey_s_c,
                                enum btree_node_type);
@@ -57,8 +47,17 @@ void bch2_bkey_swab_val(struct bkey_s);
 
 bool bch2_bkey_normalize(struct bch_fs *, struct bkey_s);
 
-enum merge_result bch2_bkey_merge(struct bch_fs *,
-                                 struct bkey_s, struct bkey_s);
+static inline bool bch2_bkey_maybe_mergable(const struct bkey *l, const struct bkey *r)
+{
+       return l->type == r->type &&
+               !bversion_cmp(l->version, r->version) &&
+               !bpos_cmp(l->p, bkey_start_pos(r)) &&
+               (u64) l->size + r->size <= KEY_SIZE_MAX &&
+               bch2_bkey_ops[l->type].key_merge &&
+               !bch2_key_merging_disabled;
+}
+
+bool bch2_bkey_merge(struct bch_fs *, struct bkey_s, struct bkey_s_c);
 
 void bch2_bkey_renumber(enum btree_node_type, struct bkey_packed *, int);
 

diff --git a/fs/bcachefs/extents.c b/fs/bcachefs/extents.c
index 1f28dea26ca25b196d1a68effd0207fa67f76e28..2ced3557e13b8872c6e96a5a37fc9783fb7e50fc 100644 (file)
--- a/fs/bcachefs/extents.c
+++ b/fs/bcachefs/extents.c
@@ -229,17 +229,16 @@ void bch2_extent_to_text(struct printbuf *out, struct bch_fs *c,
        bch2_bkey_ptrs_to_text(out, c, k);
 }
 
-enum merge_result bch2_extent_merge(struct bch_fs *c,
-                                   struct bkey_s _l, struct bkey_s _r)
+bool bch2_extent_merge(struct bch_fs *c, struct bkey_s _l, struct bkey_s_c _r)
 {
        struct bkey_s_extent l = bkey_s_to_extent(_l);
-       struct bkey_s_extent r = bkey_s_to_extent(_r);
+       struct bkey_s_c_extent r = bkey_s_c_to_extent(_r);
        union bch_extent_entry *en_l = l.v->start;
-       union bch_extent_entry *en_r = r.v->start;
+       const union bch_extent_entry *en_r = r.v->start;
        struct bch_extent_crc_unpacked crc_l, crc_r;
 
        if (bkey_val_u64s(l.k) != bkey_val_u64s(r.k))
-               return BCH_MERGE_NOMERGE;
+               return false;
 
        crc_l = bch2_extent_crc_unpack(l.k, NULL);
 
@@ -247,7 +246,7 @@ enum merge_result bch2_extent_merge(struct bch_fs *c,
                en_r = vstruct_idx(r.v, (u64 *) en_l - l.v->_data);
 
                if (extent_entry_type(en_l) != extent_entry_type(en_r))
-                       return BCH_MERGE_NOMERGE;
+                       return false;
 
                switch (extent_entry_type(en_l)) {
                case BCH_EXTENT_ENTRY_ptr: {
@@ -258,20 +257,20 @@ enum merge_result bch2_extent_merge(struct bch_fs *c,
                        if (lp->offset + crc_l.compressed_size != rp->offset ||
                            lp->dev                     != rp->dev ||
                            lp->gen                     != rp->gen)
-                               return BCH_MERGE_NOMERGE;
+                               return false;
 
                        /* We don't allow extents to straddle buckets: */
                        ca = bch_dev_bkey_exists(c, lp->dev);
 
                        if (PTR_BUCKET_NR(ca, lp) != PTR_BUCKET_NR(ca, rp))
-                               return BCH_MERGE_NOMERGE;
+                               return false;
 
                        break;
                }
                case BCH_EXTENT_ENTRY_stripe_ptr:
                        if (en_l->stripe_ptr.block      != en_r->stripe_ptr.block ||
                            en_l->stripe_ptr.idx        != en_r->stripe_ptr.idx)
-                               return BCH_MERGE_NOMERGE;
+                               return false;
                        break;
                case BCH_EXTENT_ENTRY_crc32:
                case BCH_EXTENT_ENTRY_crc64:
@@ -282,30 +281,30 @@ enum merge_result bch2_extent_merge(struct bch_fs *c,
                        if (crc_l.csum_type             != crc_r.csum_type ||
                            crc_l.compression_type      != crc_r.compression_type ||
                            crc_l.nonce                 != crc_r.nonce)
-                               return BCH_MERGE_NOMERGE;
+                               return false;
 
                        if (crc_l.offset + crc_l.live_size != crc_l.compressed_size ||
                            crc_r.offset)
-                               return BCH_MERGE_NOMERGE;
+                               return false;
 
                        if (!bch2_checksum_mergeable(crc_l.csum_type))
-                               return BCH_MERGE_NOMERGE;
+                               return false;
 
                        if (crc_is_compressed(crc_l))
-                               return BCH_MERGE_NOMERGE;
+                               return false;
 
                        if (crc_l.csum_type &&
                            crc_l.uncompressed_size +
                            crc_r.uncompressed_size > c->sb.encoded_extent_max)
-                               return BCH_MERGE_NOMERGE;
+                               return false;
 
                        if (crc_l.uncompressed_size + crc_r.uncompressed_size >
                            bch2_crc_field_size_max[extent_entry_type(en_l)])
-                               return BCH_MERGE_NOMERGE;
+                               return false;
 
                        break;
                default:
-                       return BCH_MERGE_NOMERGE;
+                       return false;
                }
        }
 
@@ -333,8 +332,7 @@ enum merge_result bch2_extent_merge(struct bch_fs *c,
        }
 
        bch2_key_resize(l.k, l.k->size + r.k->size);
-
-       return BCH_MERGE_MERGE;
+       return true;
 }
 
 /* KEY_TYPE_reservation: */
@@ -362,25 +360,17 @@ void bch2_reservation_to_text(struct printbuf *out, struct bch_fs *c,
               r.v->nr_replicas);
 }
 
-enum merge_result bch2_reservation_merge(struct bch_fs *c,
-                                        struct bkey_s _l, struct bkey_s _r)
+bool bch2_reservation_merge(struct bch_fs *c, struct bkey_s _l, struct bkey_s_c _r)
 {
        struct bkey_s_reservation l = bkey_s_to_reservation(_l);
-       struct bkey_s_reservation r = bkey_s_to_reservation(_r);
+       struct bkey_s_c_reservation r = bkey_s_c_to_reservation(_r);
 
        if (l.v->generation != r.v->generation ||
            l.v->nr_replicas != r.v->nr_replicas)
-               return BCH_MERGE_NOMERGE;
-
-       if ((u64) l.k->size + r.k->size > KEY_SIZE_MAX) {
-               bch2_key_resize(l.k, KEY_SIZE_MAX);
-               bch2_cut_front_s(l.k->p, r.s);
-               return BCH_MERGE_PARTIAL;
-       }
+               return false;
 
        bch2_key_resize(l.k, l.k->size + r.k->size);
-
-       return BCH_MERGE_MERGE;
+       return true;
 }
 
 /* Extent checksum entries: */

diff --git a/fs/bcachefs/extents.h b/fs/bcachefs/extents.h
index 9999805f955ebfff87cf066aff9457c976d9c3db..3f6224f75ce82f7a8a25fe75796758e524af968f 100644 (file)
--- a/fs/bcachefs/extents.h
+++ b/fs/bcachefs/extents.h
@@ -394,8 +394,7 @@ void bch2_btree_ptr_v2_compat(enum btree_id, unsigned, unsigned,
 
 const char *bch2_extent_invalid(const struct bch_fs *, struct bkey_s_c);
 void bch2_extent_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
-enum merge_result bch2_extent_merge(struct bch_fs *,
-                                   struct bkey_s, struct bkey_s);
+bool bch2_extent_merge(struct bch_fs *, struct bkey_s, struct bkey_s_c);
 
 #define bch2_bkey_ops_extent (struct bkey_ops) {               \
        .key_invalid    = bch2_extent_invalid,                  \
@@ -409,8 +408,7 @@ enum merge_result bch2_extent_merge(struct bch_fs *,
 
 const char *bch2_reservation_invalid(const struct bch_fs *, struct bkey_s_c);
 void bch2_reservation_to_text(struct printbuf *, struct bch_fs *, struct bkey_s_c);
-enum merge_result bch2_reservation_merge(struct bch_fs *,
-                                        struct bkey_s, struct bkey_s);
+bool bch2_reservation_merge(struct bch_fs *, struct bkey_s, struct bkey_s_c);
 
 #define bch2_bkey_ops_reservation (struct bkey_ops) {          \
        .key_invalid    = bch2_reservation_invalid,             \

diff --git a/fs/bcachefs/reflink.c b/fs/bcachefs/reflink.c
index 6aa37726341d068cc5297870e6d3873a1fceca49..ead31f9e31aa0b1bbc540dfde36183afe71fc722 100644 (file)
--- a/fs/bcachefs/reflink.c
+++ b/fs/bcachefs/reflink.c
@@ -42,24 +42,22 @@ void bch2_reflink_p_to_text(struct printbuf *out, struct bch_fs *c,
        pr_buf(out, "idx %llu", le64_to_cpu(p.v->idx));
 }
 
-enum merge_result bch2_reflink_p_merge(struct bch_fs *c,
-                                      struct bkey_s _l, struct bkey_s _r)
+bool bch2_reflink_p_merge(struct bch_fs *c, struct bkey_s _l, struct bkey_s_c _r)
 {
        struct bkey_s_reflink_p l = bkey_s_to_reflink_p(_l);
-       struct bkey_s_reflink_p r = bkey_s_to_reflink_p(_r);
+       struct bkey_s_c_reflink_p r = bkey_s_c_to_reflink_p(_r);
 
-       if (le64_to_cpu(l.v->idx) + l.k->size != le64_to_cpu(r.v->idx))
-               return BCH_MERGE_NOMERGE;
+       /*
+        * Disabled for now, the triggers code needs to be reworked for merging
+        * of reflink pointers to work:
+        */
+       return false;
 
-       if ((u64) l.k->size + r.k->size > KEY_SIZE_MAX) {
-               bch2_key_resize(l.k, KEY_SIZE_MAX);
-               bch2_cut_front_s(l.k->p, _r);
-               return BCH_MERGE_PARTIAL;
-       }
+       if (le64_to_cpu(l.v->idx) + l.k->size != le64_to_cpu(r.v->idx))
+               return false;
 
        bch2_key_resize(l.k, l.k->size + r.k->size);
-
-       return BCH_MERGE_MERGE;
+       return true;
 }
 
 /* indirect extents */

diff --git a/fs/bcachefs/reflink.h b/fs/bcachefs/reflink.h
index bfc785619ee89d17270fa75342c85fe4f2712d54..68c5cb5a2780ddd1552d41d03229e145f1d14d3c 100644 (file)
--- a/fs/bcachefs/reflink.h
+++ b/fs/bcachefs/reflink.h
@@ -5,8 +5,7 @@
 const char *bch2_reflink_p_invalid(const struct bch_fs *, struct bkey_s_c);
 void bch2_reflink_p_to_text(struct printbuf *, struct bch_fs *,
                            struct bkey_s_c);
-enum merge_result bch2_reflink_p_merge(struct bch_fs *,
-                                      struct bkey_s, struct bkey_s);
+bool bch2_reflink_p_merge(struct bch_fs *, struct bkey_s, struct bkey_s_c);
 
 #define bch2_bkey_ops_reflink_p (struct bkey_ops) {            \
        .key_invalid    = bch2_reflink_p_invalid,               \