bcache: fix handling of PTR_LOST_DEV keys

If we lose all copies of a key, we want to fail the read, not
treat it as a hole in the keyspace, so don't mark the key as
deleted. Instead, set the key type to error.

This fixes a regression from "New bkey format".

Also, instead of having bch_extent_normalize() set a key's
deleted flag, just return true if the key should be dropped.
If the key is an extent and has no pointers, it becomes a
discard.

This could come up in bch_flag_key_bad(). If a cached key
points to a device that has gone bad, we end up dropping all
pointers from the key. This would cause us to insert a deleted
key, which triggers a BUG_ON ever since "Don't insert deleted
keys with nonzero size". What we want instead is to discard
this range of the keyspace -- we just lost some cached data,
which is not an error.

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

3 files changed, 26 insertions, 25 deletions

diff --git a/drivers/md/bcache/extents.c b/drivers/md/bcache/extents.c
index 6af7d34cee88..6e92e3e82743 100644
--- a/drivers/md/bcache/extents.c
+++ b/drivers/md/bcache/extents.c
@@ -1376,32 +1376,24 @@ bool bch_extent_normalize(struct cache_set *c, struct bkey *k)
 	bool cached;
 
 	switch (k->type) {
-	case KEY_TYPE_DELETED:
 	case KEY_TYPE_ERROR:
-		break;
-
-	case KEY_TYPE_DISCARD:
-		if (!k->version)
-			set_bkey_deleted(k);
-		break;
+		return false;
 
+	case KEY_TYPE_DELETED:
 	case KEY_TYPE_COOKIE:
-		set_bkey_deleted(k);
-		break;
+		return true;
+
+	case KEY_TYPE_DISCARD:
+		return !k->version;
 
 	case BCH_EXTENT:
 		e = bkey_i_to_extent(k);
 
-		if (!e->k.size) {
-			set_bkey_deleted(&e->k);
-			return true;
-		}
-
 		/*
 		 * Preserve cached status since its stored in the
 		 * first pointer
 		 */
-		cached = EXTENT_CACHED(&e->v);
+		cached = bch_extent_ptrs(e) && EXTENT_CACHED(&e->v);
 
 		bch_extent_drop_stale(c, &e->k);
 
@@ -1425,20 +1417,24 @@ bool bch_extent_normalize(struct cache_set *c, struct bkey *k)
 			if (PTR_DEV(ptr) != PTR_LOST_DEV)
 				have_data = true;
 
-		if (!have_data)
+		if (!have_data) {
 			bch_set_extent_ptrs(e, 0);
-
-		if (!bch_extent_ptrs(e))
-			k->type = KEY_TYPE_DELETED;
-		else
+			if (cached) {
+				k->type = KEY_TYPE_DISCARD;
+				if (!k->version)
+					return true;
+			} else {
+				k->type = KEY_TYPE_ERROR;
+			}
+		} else {
 			SET_EXTENT_CACHED(&e->v, cached);
+		}
 
-		break;
+		return false;
 	default:
 		BUG();
+		return false;
 	}
-
-	return bkey_deleted(k);
 }
 
 /*
diff --git a/drivers/md/bcache/io.c b/drivers/md/bcache/io.c
index cbaa60c3e762..139686e45090 100644
--- a/drivers/md/bcache/io.c
+++ b/drivers/md/bcache/io.c
@@ -475,7 +475,8 @@ static void __bch_write(struct closure *cl)
 		bio_set_op_attrs(n, REQ_OP_WRITE, 0);
 		bch_submit_bbio_replicas(n, op->c, k, ptrs_from, false);
 
-		bch_extent_normalize(op->c, k);
+		BUG_ON(bch_extent_normalize(op->c, k));
+
 		bch_check_mark_super(op->c, k, false);
 
 		/*
diff --git a/drivers/md/bcache/move.c b/drivers/md/bcache/move.c
index 5503a461c573..3eff18c5845a 100644
--- a/drivers/md/bcache/move.c
+++ b/drivers/md/bcache/move.c
@@ -1188,7 +1188,11 @@ static int bch_flag_key_bad(struct btree_iter *iter,
 	 * bch_extent_normalize() needs to know how to turn a key with only
 	 * pointers to PTR_LOST_DEV into a KEY_BAD() key anyways - because we
 	 * might have a cached pointer that will go stale later - so just call
-	 * it here:
+	 * it here.
+	 *
+	 * If the key was cached, we may have dropped all pointers above --
+	 * in this case, bch_extent_normalize() will change the key type to
+	 * DISCARD.
 	 */
 	bch_extent_normalize(c, &e->k);