bcache: gc_will_visit_root()

Before, bch_mark_bucket() wasn't checking if gc was going to mark the pointer
for new roots - leading to them being double marked, and triggering an assertion
when marking open buckets if it happened early enough during gc

7 files changed, 50 insertions, 30 deletions

diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 3bda6f09e8e1..4f982e164243 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -1654,9 +1654,10 @@ void bch_btree_node_free(struct cache_set *c, struct btree *b)
 
 	__btree_node_free(c, b);
 
+	/* XX: this isn't right */
 	bch_mark_pointers(c, b, bkey_i_to_s_c_extent(&tmp.k),
 			  -CACHE_BTREE_NODE_SIZE(&c->sb),
-			  false, true);
+			  false, true, false);
 }
 
 static void __bch_btree_set_root(struct cache_set *c, struct btree *b)
@@ -1675,7 +1676,7 @@ static void __bch_btree_set_root(struct cache_set *c, struct btree *b)
 		bool stale = bch_mark_pointers(c, NULL,
 					       bkey_i_to_s_c_extent(&b->key),
 					       CACHE_BTREE_NODE_SIZE(&c->sb),
-					       true, true);
+					       true, true, false);
 
 		BUG_ON(stale);
 	}
diff --git a/drivers/md/bcache/buckets.c b/drivers/md/bcache/buckets.c
index 3c96ce502c10..42b255b9090e 100644
--- a/drivers/md/bcache/buckets.c
+++ b/drivers/md/bcache/buckets.c
@@ -208,15 +208,13 @@ do {								\
 } while (0)
 
 static u8 bch_mark_bucket(struct cache_set *c, struct cache *ca,
-			  struct btree *b,
-			  const struct bch_extent_ptr *ptr,
-			  int sectors, bool dirty, bool metadata)
+			  struct btree *b, const struct bch_extent_ptr *ptr,
+			  int sectors, bool dirty, bool metadata, bool is_gc)
 {
 	struct bucket_mark old, new;
 	unsigned long bucket_nr = PTR_BUCKET_NR(ca, ptr);
 	unsigned saturated;
 	u8 stale;
-	bool is_gc = !b;
 
 	bucket_cmpxchg(&ca->buckets[bucket_nr], old, new, is_gc, ({
 		saturated = 0;
@@ -237,21 +235,25 @@ static u8 bch_mark_bucket(struct cache_set *c, struct cache *ca,
 		if (stale)
 			return stale;
 
-		/*
-		 * Check this after reading bucket mark to guard against
-		 * GC starting between when we check gc_cur_key and when
-		 * the GC zeroes out marks
-		 */
-		if (!is_gc && gc_will_visit_node(c, b))
-			return 0;
-
-		/*
-		 * Disallowed state transition - this means a bkey_cmpxchg()
-		 * operation is racing; just treat it like the pointer was
-		 * already stale
-		 */
-		if (!is_gc && dirty && is_available_bucket(old))
-			return 1;
+		if (!is_gc) {
+			/*
+			 * Check this after reading bucket mark to guard against
+			 * GC starting between when we check gc_cur_key and when
+			 * the GC zeroes out marks
+			 */
+			if (b
+			    ? gc_will_visit_node(c, b)
+			    : gc_will_visit_root(c, BTREE_ID_EXTENTS))
+				return 0;
+
+			/*
+			 * Disallowed state transition - this means a bkey_cmpxchg()
+			 * operation is racing; just treat it like the pointer was
+			 * already stale
+			 */
+			if (dirty && is_available_bucket(old))
+				return 1;
+		}
 
 		BUG_ON((old.dirty_sectors ||
 			old.cached_sectors) &&
@@ -295,7 +297,7 @@ static u8 bch_mark_bucket(struct cache_set *c, struct cache *ca,
  */
 int bch_mark_pointers(struct cache_set *c, struct btree *b,
 		      struct bkey_s_c_extent e, int sectors,
-		      bool fail_if_stale, bool metadata)
+		      bool fail_if_stale, bool metadata, bool is_gc)
 {
 	const struct bch_extent_ptr *ptr, *ptr2;
 	struct cache *ca;
@@ -342,7 +344,7 @@ int bch_mark_pointers(struct cache_set *c, struct btree *b,
 		 *   Fuck me, I hate my life.
 		 */
 		stale = bch_mark_bucket(c, ca, b, ptr, sectors,
-					dirty, metadata);
+					dirty, metadata, is_gc);
 		if (stale && dirty && fail_if_stale)
 			goto stale;
 	}
@@ -356,7 +358,7 @@ stale:
 
 		bch_mark_bucket(c, ca, b, ptr, -sectors,
 				bch_extent_ptr_is_dirty(c, e, ptr),
-				metadata);
+				metadata, is_gc);
 	}
 	rcu_read_unlock();
 
diff --git a/drivers/md/bcache/buckets.h b/drivers/md/bcache/buckets.h
index 5e191ef91812..6e16747c5889 100644
--- a/drivers/md/bcache/buckets.h
+++ b/drivers/md/bcache/buckets.h
@@ -264,6 +264,6 @@ void bch_mark_metadata_bucket(struct cache *, struct bucket *, bool);
 void bch_unmark_open_bucket(struct cache *, struct bucket *);
 
 int bch_mark_pointers(struct cache_set *, struct btree *,
-		      struct bkey_s_c_extent, int, bool, bool);
+		      struct bkey_s_c_extent, int, bool, bool, bool);
 
 #endif /* _BUCKETS_H */
diff --git a/drivers/md/bcache/extents.c b/drivers/md/bcache/extents.c
index 25a63aefb6a8..dbd67497398f 100644
--- a/drivers/md/bcache/extents.c
+++ b/drivers/md/bcache/extents.c
@@ -143,7 +143,7 @@ bool bch_insert_fixup_btree_ptr(struct cache_set *c, struct btree *b,
 
 		stale = bch_mark_pointers(c, b, bkey_i_to_s_c_extent(insert),
 					  CACHE_BTREE_NODE_SIZE(&c->sb),
-					  true, true);
+					  true, true, false);
 		BUG_ON(stale);
 	}
 
@@ -786,7 +786,8 @@ static int bch_add_sectors(struct cache_set *c, struct btree *b,
 		struct bkey_s_c_extent e = bkey_s_c_to_extent(k);
 		int ret;
 
-		ret = bch_mark_pointers(c, b, e, sectors, fail_if_stale, false);
+		ret = bch_mark_pointers(c, b, e, sectors, fail_if_stale,
+					false, false);
 		if (ret)
 			return ret;
 
diff --git a/drivers/md/bcache/gc.c b/drivers/md/bcache/gc.c
index f06debd4cca6..498eb3ad4733 100644
--- a/drivers/md/bcache/gc.c
+++ b/drivers/md/bcache/gc.c
@@ -110,7 +110,7 @@ void __bch_btree_mark_key(struct cache_set *c, int level, struct bkey_s_c k)
 
 		bch_mark_pointers(c, NULL, e, level
 				  ? CACHE_BTREE_NODE_SIZE(&c->sb)
-				  : e.k->size, false, level != 0);
+				  : e.k->size, false, level != 0, true);
 	}
 }
 
@@ -200,7 +200,7 @@ static int bch_gc_btree(struct cache_set *c, enum btree_id btree_id)
 	__bch_btree_mark_key(c, b->level + 1, bkey_i_to_s_c(&b->key));
 
 	write_seqcount_begin(&c->gc_cur_lock);
-	c->gc_cur_level = b->level + 1;
+	c->gc_cur_level = U8_MAX;
 	write_seqcount_end(&c->gc_cur_lock);
 	spin_unlock(&c->btree_root_lock);
 	return 0;
diff --git a/drivers/md/bcache/gc.h b/drivers/md/bcache/gc.h
index b3d60bd2bfc9..867444512640 100644
--- a/drivers/md/bcache/gc.h
+++ b/drivers/md/bcache/gc.h
@@ -53,4 +53,20 @@ static inline bool gc_will_visit_node(struct cache_set *c,
 	return ret;
 }
 
+static inline bool gc_will_visit_root(struct cache_set *c, enum btree_id id)
+{
+	unsigned seq;
+	bool ret;
+
+	do {
+		seq = read_seqcount_begin(&c->gc_cur_lock);
+		ret = id != c->gc_cur_btree
+			? id > c->gc_cur_btree
+			: c->gc_cur_level != U8_MAX;
+	} while (read_seqcount_retry(&c->gc_cur_lock, seq));
+
+	return ret;
+
+}
+
 #endif
diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
index 2625e8d4b550..47f5cc2987bc 100644
--- a/drivers/md/bcache/journal.c
+++ b/drivers/md/bcache/journal.c
@@ -939,7 +939,7 @@ static int bch_journal_replay_key(struct cache_set *c, enum btree_id id,
 	 */
 	if (do_subtract)
 		bch_mark_pointers(c, NULL, bkey_i_to_s_c_extent(&temp.key),
-				  -temp.key.k.size, false, false);
+				  -temp.key.k.size, false, false, true);
 
 	return 0;
 }