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>

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 {

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);
 

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: */

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,		\

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 */

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,		\