bcache: refactor some extent crc code a bit

3 files changed, 98 insertions, 77 deletions

diff --git a/drivers/md/bcache/extents.c b/drivers/md/bcache/extents.c
index 8310b8cb898a..0beda6a36f55 100644
--- a/drivers/md/bcache/extents.c
+++ b/drivers/md/bcache/extents.c
@@ -164,7 +164,7 @@ static bool crc_cmp(union bch_extent_crc *l, union bch_extent_crc *r)
 }
 
 /* Increment pointers after @crc by crc's offset until the next crc entry: */
-void extent_adjust_pointers(struct bkey_s_extent e, union bch_extent_crc *crc)
+void bch_extent_crc_narrow_pointers(struct bkey_s_extent e, union bch_extent_crc *crc)
 {
 	union bch_extent_entry *entry;
 
@@ -176,7 +176,86 @@ void extent_adjust_pointers(struct bkey_s_extent e, union bch_extent_crc *crc)
 	}
 }
 
-static void extent_cleanup_crcs(struct bkey_s_extent e)
+/*
+ * We're writing another replica for this extent, so while we've got the data in
+ * memory we'll be computing a new checksum for the currently live data.
+ *
+ * If there are other replicas we aren't moving, and they are checksummed but
+ * not compressed, we can modify them to point to only the data that is
+ * currently live (so that readers won't have to bounce) while we've got the
+ * checksum we need:
+ *
+ * XXX: to guard against data being corrupted while in memory, instead of
+ * recomputing the checksum here, it would be better in the read path to instead
+ * of computing the checksum of the entire extent:
+ *
+ * | extent                              |
+ *
+ * compute the checksums of the live and dead data separately
+ * | dead data || live data || dead data |
+ *
+ * and then verify that crc_dead1 + crc_live + crc_dead2 == orig_crc, and then
+ * use crc_live here (that we verified was correct earlier)
+ */
+void bch_extent_narrow_crcs(struct bkey_s_extent e)
+{
+	union bch_extent_crc *crc;
+	bool have_wide = false, have_narrow = false;
+	u64 csum = 0;
+	unsigned csum_type = 0;
+
+	extent_for_each_crc(e, crc) {
+		if (crc_to_64(crc).compression_type)
+			continue;
+
+		if (crc_to_64(crc).uncompressed_size != e.k->size) {
+			have_wide = true;
+		} else {
+			have_narrow = true;
+			csum = crc_to_64(crc).csum;
+			csum_type = crc_to_64(crc).csum_type;
+		}
+	}
+
+	if (!have_wide || !have_narrow)
+		return;
+
+	extent_for_each_crc(e, crc) {
+		if (crc_to_64(crc).compression_type)
+			continue;
+
+		if (crc_to_64(crc).uncompressed_size != e.k->size) {
+			switch (extent_crc_type(crc)) {
+			case BCH_EXTENT_CRC_NONE:
+				BUG();
+			case BCH_EXTENT_CRC32:
+				if (bch_crc_size[csum_type] > sizeof(crc->crc32.csum))
+					continue;
+
+				bch_extent_crc_narrow_pointers(e, crc);
+				crc->crc32.compressed_size	= e.k->size;
+				crc->crc32.uncompressed_size	= e.k->size;
+				crc->crc32.offset		= 0;
+				crc->crc32.csum_type		= csum_type;
+				crc->crc32.csum			= csum;
+				break;
+			case BCH_EXTENT_CRC64:
+				if (bch_crc_size[csum_type] > sizeof(crc->crc64.csum))
+					continue;
+
+				bch_extent_crc_narrow_pointers(e, crc);
+				crc->crc64.compressed_size	= e.k->size;
+				crc->crc64.uncompressed_size	= e.k->size;
+				crc->crc64.offset		= 0;
+				crc->crc64.csum_type		= csum_type;
+				crc->crc64.csum			= csum;
+				break;
+			}
+		}
+	}
+}
+
+void bch_extent_drop_redundant_crcs(struct bkey_s_extent e)
 {
 	union bch_extent_entry *entry = e.v->start;
 	union bch_extent_crc *crc, *prev = NULL;
@@ -209,7 +288,7 @@ static void extent_cleanup_crcs(struct bkey_s_extent e)
 		    !crc_to_64(crc).csum_type &&
 		    !crc_to_64(crc).compression_type){
 			/* null crc entry: */
-			extent_adjust_pointers(e, crc);
+			bch_extent_crc_narrow_pointers(e, crc);
 			goto drop;
 		}
 
@@ -226,12 +305,6 @@ drop:
 	EBUG_ON(bkey_val_u64s(e.k) && !bch_extent_nr_ptrs(e.c));
 }
 
-void bch_extent_drop_ptr(struct bkey_s_extent e, struct bch_extent_ptr *ptr)
-{
-	__bch_extent_drop_ptr(e, ptr);
-	extent_cleanup_crcs(e);
-}
-
 static bool should_drop_ptr(const struct cache_set *c,
 			    struct bkey_s_c_extent e,
 			    const struct bch_extent_ptr *ptr)
@@ -261,7 +334,7 @@ void bch_extent_drop_stale(struct cache_set *c, struct bkey_s_extent e)
 	rcu_read_unlock();
 
 	if (dropped)
-		extent_cleanup_crcs(e);
+		bch_extent_drop_redundant_crcs(e);
 }
 
 static bool bch_ptr_normalize(struct btree_keys *bk, struct bkey_s k)
@@ -1805,7 +1878,7 @@ static void __extent_sort_ptrs(struct cache_member_rcu *mi,
 	}
 
 	/* Sort done - now drop redundant crc entries: */
-	extent_cleanup_crcs(dst);
+	bch_extent_drop_redundant_crcs(dst);
 
 	memcpy(src.v, dst.v, bkey_val_bytes(dst.k));
 	set_bkey_val_u64s(src.k, bkey_val_u64s(dst.k));
diff --git a/drivers/md/bcache/extents.h b/drivers/md/bcache/extents.h
index 70f04235a110..3024fb539335 100644
--- a/drivers/md/bcache/extents.h
+++ b/drivers/md/bcache/extents.h
@@ -400,17 +400,27 @@ static inline unsigned bkey_extent_is_compressed(struct cache_set *c,
 	return ret;
 }
 
-void extent_adjust_pointers(struct bkey_s_extent, union bch_extent_crc *);
+void bch_extent_narrow_crcs(struct bkey_s_extent);
+void bch_extent_drop_redundant_crcs(struct bkey_s_extent);
 
 /* Doesn't cleanup redundant crcs */
 static inline void __bch_extent_drop_ptr(struct bkey_s_extent e,
 					 struct bch_extent_ptr *ptr)
 {
+	EBUG_ON(ptr < &e.v->start->ptr ||
+		ptr >= &extent_entry_last(e)->ptr);
+	EBUG_ON(ptr->type != 1 << BCH_EXTENT_ENTRY_ptr);
 	memmove(ptr, ptr + 1, (void *) extent_entry_last(e) - (void *) (ptr + 1));
 	e.k->u64s -= sizeof(*ptr) / sizeof(u64);
 }
 
-void bch_extent_drop_ptr(struct bkey_s_extent, struct bch_extent_ptr *);
+static inline void bch_extent_drop_ptr(struct bkey_s_extent e,
+				       struct bch_extent_ptr *ptr)
+{
+	__bch_extent_drop_ptr(e, ptr);
+	bch_extent_drop_redundant_crcs(e);
+}
+
 bool bch_extent_has_device(struct bkey_s_c_extent, unsigned);
 
 bool bch_cut_front(struct bpos, struct bkey_i *);
diff --git a/drivers/md/bcache/io.c b/drivers/md/bcache/io.c
index 57755e875ac2..9e4dae1c95d6 100644
--- a/drivers/md/bcache/io.c
+++ b/drivers/md/bcache/io.c
@@ -806,63 +806,6 @@ static void bch_write_endio(struct bio *bio)
 	closure_put(cl);
 }
 
-/*
- * We're writing another replica for this extent, so while we've got the data in
- * memory we'll be computing a new checksum for the currently live data.
- *
- * If there are other replicas we aren't moving, and they are checksummed but
- * not compressed, we can modify them to point to only the data that is
- * currently live (so that readers won't have to bounce) while we've got the
- * checksum we need:
- *
- * XXX: to guard against data being corrupted while in memory, instead of
- * recomputing the checksum here, it would be better in the read path to instead
- * of computing the checksum of the entire extent:
- *
- * | extent                              |
- *
- * compute the checksums of the live and dead data separately
- * | dead data || live data || dead data |
- *
- * and then verify that crc_dead1 + crc_live + crc_dead2 == orig_crc, and then
- * use crc_live here (that we verified was correct earlier)
- */
-static void extent_cleanup_checksums(struct bkey_s_extent e,
-				     u64 csum, unsigned csum_type)
-{
-	union bch_extent_crc *crc;
-
-	extent_for_each_crc(e, crc)
-		switch (extent_crc_type(crc)) {
-		case BCH_EXTENT_CRC_NONE:
-			BUG();
-		case BCH_EXTENT_CRC32:
-			if (crc->crc32.compression_type != BCH_COMPRESSION_NONE ||
-			    bch_crc_size[csum_type] > sizeof(crc->crc32.csum))
-				continue;
-
-			extent_adjust_pointers(e, crc);
-			crc->crc32.compressed_size	= e.k->size;
-			crc->crc32.uncompressed_size	= e.k->size;
-			crc->crc32.offset		= 0;
-			crc->crc32.csum_type		= csum_type;
-			crc->crc32.csum		= csum;
-			break;
-		case BCH_EXTENT_CRC64:
-			if (crc->crc64.compression_type != BCH_COMPRESSION_NONE ||
-			    bch_crc_size[csum_type] > sizeof(crc->crc64.csum))
-				continue;
-
-			extent_adjust_pointers(e, crc);
-			crc->crc64.compressed_size	= e.k->size;
-			crc->crc64.uncompressed_size	= e.k->size;
-			crc->crc64.offset		= 0;
-			crc->crc64.csum_type		= csum_type;
-			crc->crc64.csum		= csum;
-			break;
-		}
-}
-
 static int bch_write_extent(struct bch_write_op *op,
 			    struct open_bucket *ob,
 			    struct bkey_i_extent *e,
@@ -954,13 +897,6 @@ static int bch_write_extent(struct bch_write_op *op,
 		 * under it
 		 */
 		csum = bch_checksum_bio(bio, csum_type);
-
-		/*
-		 * If possible, adjust existing pointers to only point to
-		 * currently live data, while we have the checksum for that
-		 * data:
-		 */
-		extent_cleanup_checksums(extent_i_to_s(e), csum, csum_type);
 #if 0
 		if (compression_type != BCH_COMPRESSION_NONE)
 			pr_info("successfully compressed %u -> %u",
@@ -976,6 +912,8 @@ static int bch_write_extent(struct bch_write_op *op,
 		bch_alloc_sectors_done(op->c, op->wp,
 				       e, op->nr_replicas,
 				       ob, bio_sectors(bio));
+
+		bch_extent_narrow_crcs(extent_i_to_s(e));
 	} else {
 		if (e->k.size > ob->sectors_free)
 			bch_key_resize(&e->k, ob->sectors_free);