bcachefs: don't hold btree locks during btree readsbtree-iter-locking

4 files changed, 51 insertions, 49 deletions

diff --git a/fs/bcachefs/btree_cache.c b/fs/bcachefs/btree_cache.c
index 798f7be00cec..2215dcd13afc 100644
--- a/fs/bcachefs/btree_cache.c
+++ b/fs/bcachefs/btree_cache.c
@@ -577,19 +577,22 @@ err:
 
 /* Slowpath, don't want it inlined into btree_iter_traverse() */
 static noinline struct btree *bch2_btree_node_fill(struct bch_fs *c,
-						   struct btree_iter *iter,
-						   const struct bkey_i *k,
-						   unsigned level,
-						   enum six_lock_type lock_type)
+				struct btree_iter *iter,
+				const struct bkey_i *k,
+				unsigned level,
+				enum six_lock_type lock_type,
+				bool sync)
 {
 	struct btree_cache *bc = &c->btree_cache;
 	struct btree *b;
+	u32 seq;
 
 	/*
 	 * Parent node must be locked, else we could read in a btree node that's
 	 * been freed:
 	 */
 	BUG_ON(!btree_node_locked(iter, level + 1));
+	BUG_ON(level >= BTREE_MAX_DEPTH);
 
 	b = bch2_btree_node_mem_alloc(c);
 	if (IS_ERR(b))
@@ -612,22 +615,27 @@ static noinline struct btree *bch2_btree_node_fill(struct bch_fs *c,
 	}
 
 	/*
-	 * If the btree node wasn't cached, we can't drop our lock on
-	 * the parent until after it's added to the cache - because
-	 * otherwise we could race with a btree_split() freeing the node
-	 * we're trying to lock.
-	 *
-	 * But the deadlock described below doesn't exist in this case,
-	 * so it's safe to not drop the parent lock until here:
+	 * btree node has been added to cache, drop all btree locks before
+	 * doing IO:
 	 */
-	if (btree_node_read_locked(iter, level + 1))
-		btree_node_unlock(iter, level + 1);
+	set_btree_node_read_in_flight(b);
 
-	bch2_btree_node_read(c, b, true);
 	six_unlock_write(&b->lock);
+	seq = b->lock.state.seq;
+	six_unlock_intent(&b->lock);
+
+	bch2_btree_iter_unlock(iter);
+
+	bch2_btree_node_read(c, b, sync);
+
+	if (!bch2_btree_iter_relock(iter))
+		return ERR_PTR(-EINTR);
 
-	if (lock_type == SIX_LOCK_read)
-		six_lock_downgrade(&b->lock);
+	if (!sync)
+		return NULL;
+
+	if (!six_relock_type(&b->lock, lock_type, seq))
+		return ERR_PTR(-EINTR);
 
 	return b;
 }
@@ -855,12 +863,13 @@ out_upgrade:
 	goto out;
 }
 
-void bch2_btree_node_prefetch(struct bch_fs *c, const struct bkey_i *k,
-			      unsigned level, enum btree_id btree_id)
+int bch2_btree_node_prefetch(struct bch_fs *c, const struct bkey_i *k,
+			     unsigned level, enum btree_id btree_id)
 {
 	struct btree_cache *bc = &c->btree_cache;
 	struct btree *b;
 
+	BUG_ON(!btree_node_locked(iter, level + 1));
 	BUG_ON(level >= BTREE_MAX_DEPTH);
 
 	rcu_read_lock();
@@ -870,27 +879,12 @@ void bch2_btree_node_prefetch(struct bch_fs *c, const struct bkey_i *k,
 	if (b)
 		return;
 
-	b = bch2_btree_node_mem_alloc(c);
+	b = bch2_btree_node_fill(c, iter, k, level, SIX_LOCK_read, false);
 	if (IS_ERR(b))
-		return;
-
-	bkey_copy(&b->key, k);
-	if (bch2_btree_node_hash_insert(bc, b, level, btree_id)) {
-		/* raced with another fill: */
-
-		/* mark as unhashed... */
-		bkey_i_to_extent(&b->key)->v._data[0] = 0;
-
-		mutex_lock(&bc->lock);
-		list_add(&b->list, &bc->freeable);
-		mutex_unlock(&bc->lock);
-		goto out;
-	}
+		return PTR_ERR(b);
 
-	bch2_btree_node_read(c, b, false);
-out:
-	six_unlock_write(&b->lock);
-	six_unlock_intent(&b->lock);
+	BUG_ON(b);
+	return 0;
 }
 
 int bch2_print_btree_node(struct bch_fs *c, struct btree *b,
diff --git a/fs/bcachefs/btree_cache.h b/fs/bcachefs/btree_cache.h
index 43109d086479..33a3c95d7626 100644
--- a/fs/bcachefs/btree_cache.h
+++ b/fs/bcachefs/btree_cache.h
@@ -29,8 +29,8 @@ struct btree *bch2_btree_node_get_sibling(struct bch_fs *, struct btree_iter *,
 					  struct btree *, bool,
 					  enum btree_node_sibling);
 
-void bch2_btree_node_prefetch(struct bch_fs *, const struct bkey_i *,
-			      unsigned, enum btree_id);
+int bch2_btree_node_prefetch(struct bch_fs *, const struct bkey_i *,
+			     unsigned, enum btree_id);
 
 void bch2_fs_btree_cache_exit(struct bch_fs *);
 int bch2_fs_btree_cache_init(struct bch_fs *);
diff --git a/fs/bcachefs/btree_io.c b/fs/bcachefs/btree_io.c
index 847dfd685eac..22a8f50f37ed 100644
--- a/fs/bcachefs/btree_io.c
+++ b/fs/bcachefs/btree_io.c
@@ -1385,7 +1385,8 @@ start:
 
 	bch2_time_stats_update(&c->times[BCH_TIME_btree_read], rb->start_time);
 	bio_put(&rb->bio);
-	clear_btree_node_read_in_flight(b);
+
+	clear_bit_unlock(BTREE_NODE_read_in_flight, &b->flags);
 	wake_up_bit(&b->flags, BTREE_NODE_read_in_flight);
 }
 
@@ -1413,11 +1414,15 @@ void bch2_btree_node_read(struct bch_fs *c, struct btree *b,
 	int ret;
 
 	trace_btree_read(c, b);
+	set_btree_node_read_in_flight(b);
 
 	ret = bch2_btree_pick_ptr(c, b, NULL, &pick);
 	if (bch2_fs_fatal_err_on(ret <= 0, c,
 			"btree node read error: no device to read from")) {
 		set_btree_node_read_error(b);
+
+		clear_bit_unlock(BTREE_NODE_read_in_flight, &b->flags);
+		wake_up_bit(&b->flags, BTREE_NODE_read_in_flight);
 		return;
 	}
 
@@ -1437,8 +1442,6 @@ void bch2_btree_node_read(struct bch_fs *c, struct btree *b,
 	bio->bi_private		= b;
 	bch2_bio_map(bio, b->data);
 
-	set_btree_node_read_in_flight(b);
-
 	if (rb->have_ioref) {
 		this_cpu_add(ca->io_done->sectors[READ][BCH_DATA_BTREE],
 			     bio_sectors(bio));
diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
index 09f360712a23..bed16685aa2e 100644
--- a/fs/bcachefs/btree_iter.c
+++ b/fs/bcachefs/btree_iter.c
@@ -813,7 +813,7 @@ static inline int btree_iter_lock_root(struct btree_iter *iter,
 }
 
 noinline
-static void btree_iter_prefetch(struct btree_iter *iter)
+static int btree_iter_prefetch(struct btree_iter *iter)
 {
 	struct btree_iter_level *l = &iter->l[iter->level];
 	struct btree_node_iter node_iter = l->iter;
@@ -823,8 +823,9 @@ static void btree_iter_prefetch(struct btree_iter *iter)
 		? (iter->level > 1 ? 0 :  2)
 		: (iter->level > 1 ? 1 : 16);
 	bool was_locked = btree_node_locked(iter, iter->level);
+	int ret = 0;
 
-	while (nr) {
+	while (nr && !ret) {
 		if (!bch2_btree_node_relock(iter, iter->level))
 			return;
 
@@ -834,13 +835,15 @@ static void btree_iter_prefetch(struct btree_iter *iter)
 			break;
 
 		bch2_bkey_unpack(l->b, &tmp.k, k);
-		bch2_btree_node_prefetch(iter->c, &tmp.k,
+		ret = bch2_btree_node_prefetch(iter->c, &tmp.k,
 					 iter->level - 1,
 					 iter->btree_id);
 	}
 
 	if (!was_locked)
 		btree_node_unlock(iter, iter->level);
+
+	return ret;
 }
 
 static inline int btree_iter_down(struct btree_iter *iter)
@@ -862,12 +865,14 @@ static inline int btree_iter_down(struct btree_iter *iter)
 
 	mark_btree_node_locked(iter, level, lock_type);
 	btree_iter_node_set(iter, b);
-
-	if (iter->flags & BTREE_ITER_PREFETCH)
-		btree_iter_prefetch(iter);
-
 	iter->level = level;
 
+	if (iter->flags & BTREE_ITER_PREFETCH) {
+		int ret = btree_iter_prefetch(iter);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }