1 files changed, 120 insertions, 53 deletions

diff --git a/fs/bcachefs/btree_key_cache.c b/fs/bcachefs/btree_key_cache.c
index 244610b1d0b5..d96188b92db2 100644
--- a/fs/bcachefs/btree_key_cache.c
+++ b/fs/bcachefs/btree_key_cache.c
@@ -101,8 +101,8 @@ static void __bkey_cached_free(struct rcu_pending *pending, struct rcu_head *rcu
 	kmem_cache_free(bch2_key_cache, ck);
 }
 
-static void bkey_cached_free(struct btree_key_cache *bc,
-			     struct bkey_cached *ck)
+static inline void bkey_cached_free_noassert(struct btree_key_cache *bc,
+				      struct bkey_cached *ck)
 {
 	kfree(ck->k);
 	ck->k		= NULL;
@@ -116,6 +116,19 @@ static void bkey_cached_free(struct btree_key_cache *bc,
 	this_cpu_inc(*bc->nr_pending);
 }
 
+static void bkey_cached_free(struct btree_trans *trans,
+			     struct btree_key_cache *bc,
+			     struct bkey_cached *ck)
+{
+	/*
+	 * we'll hit strange issues in the SRCU code if we aren't holding an
+	 * SRCU read lock...
+	 */
+	EBUG_ON(!trans->srcu_held);
+
+	bkey_cached_free_noassert(bc, ck);
+}
+
 static struct bkey_cached *__bkey_cached_alloc(unsigned key_u64s, gfp_t gfp)
 {
 	gfp |= __GFP_ACCOUNT|__GFP_RECLAIMABLE;
@@ -156,7 +169,7 @@ bkey_cached_alloc(struct btree_trans *trans, struct btree_path *path, unsigned k
 	}
 
 	if (ck) {
-		bch2_btree_lock_init(&ck->c, pcpu_readers ? SIX_LOCK_INIT_PCPU : 0);
+		bch2_btree_lock_init(&ck->c, pcpu_readers ? SIX_LOCK_INIT_PCPU : 0, GFP_KERNEL);
 		ck->c.cached = true;
 		goto lock;
 	}
@@ -174,30 +187,28 @@ lock:
 static struct bkey_cached *
 bkey_cached_reuse(struct btree_key_cache *c)
 {
-	struct bucket_table *tbl;
+
+	guard(rcu)();
+	struct bucket_table *tbl = rht_dereference_rcu(c->table.tbl, &c->table);
 	struct rhash_head *pos;
 	struct bkey_cached *ck;
-	unsigned i;
 
-	rcu_read_lock();
-	tbl = rht_dereference_rcu(c->table.tbl, &c->table);
-	for (i = 0; i < tbl->size; i++)
+	for (unsigned i = 0; i < tbl->size; i++)
 		rht_for_each_entry_rcu(ck, pos, tbl, i, hash) {
 			if (!test_bit(BKEY_CACHED_DIRTY, &ck->flags) &&
 			    bkey_cached_lock_for_evict(ck)) {
 				if (bkey_cached_evict(c, ck))
-					goto out;
+					return ck;
 				six_unlock_write(&ck->c.lock);
 				six_unlock_intent(&ck->c.lock);
 			}
 		}
-	ck = NULL;
-out:
-	rcu_read_unlock();
-	return ck;
+	return NULL;
 }
 
-static int btree_key_cache_create(struct btree_trans *trans, struct btree_path *path,
+static int btree_key_cache_create(struct btree_trans *trans,
+				  struct btree_path *path,
+				  struct btree_path *ck_path,
 				  struct bkey_s_c k)
 {
 	struct bch_fs *c = trans->c;
@@ -217,7 +228,7 @@ static int btree_key_cache_create(struct btree_trans *trans, struct btree_path *
 	key_u64s = min(256U, (key_u64s * 3) / 2);
 	key_u64s = roundup_pow_of_two(key_u64s);
 
-	struct bkey_cached *ck = bkey_cached_alloc(trans, path, key_u64s);
+	struct bkey_cached *ck = bkey_cached_alloc(trans, ck_path, key_u64s);
 	int ret = PTR_ERR_OR_ZERO(ck);
 	if (ret)
 		return ret;
@@ -226,26 +237,26 @@ static int btree_key_cache_create(struct btree_trans *trans, struct btree_path *
 		ck = bkey_cached_reuse(bc);
 		if (unlikely(!ck)) {
 			bch_err(c, "error allocating memory for key cache item, btree %s",
-				bch2_btree_id_str(path->btree_id));
-			return -BCH_ERR_ENOMEM_btree_key_cache_create;
+				bch2_btree_id_str(ck_path->btree_id));
+			return bch_err_throw(c, ENOMEM_btree_key_cache_create);
 		}
 	}
 
 	ck->c.level		= 0;
-	ck->c.btree_id		= path->btree_id;
-	ck->key.btree_id	= path->btree_id;
-	ck->key.pos		= path->pos;
+	ck->c.btree_id		= ck_path->btree_id;
+	ck->key.btree_id	= ck_path->btree_id;
+	ck->key.pos		= ck_path->pos;
 	ck->flags		= 1U << BKEY_CACHED_ACCESSED;
 
 	if (unlikely(key_u64s > ck->u64s)) {
-		mark_btree_node_locked_noreset(path, 0, BTREE_NODE_UNLOCKED);
+		mark_btree_node_locked_noreset(ck_path, 0, BTREE_NODE_UNLOCKED);
 
 		struct bkey_i *new_k = allocate_dropping_locks(trans, ret,
 				kmalloc(key_u64s * sizeof(u64), _gfp));
 		if (unlikely(!new_k)) {
 			bch_err(trans->c, "error allocating memory for key cache key, btree %s u64s %u",
 				bch2_btree_id_str(ck->key.btree_id), key_u64s);
-			ret = -BCH_ERR_ENOMEM_btree_key_cache_fill;
+			ret = bch_err_throw(c, ENOMEM_btree_key_cache_fill);
 		} else if (ret) {
 			kfree(new_k);
 			goto err;
@@ -258,32 +269,54 @@ static int btree_key_cache_create(struct btree_trans *trans, struct btree_path *
 
 	bkey_reassemble(ck->k, k);
 
+	ret = bch2_btree_node_lock_write(trans, path, &path_l(path)->b->c);
+	if (unlikely(ret))
+		goto err;
+
 	ret = rhashtable_lookup_insert_fast(&bc->table, &ck->hash, bch2_btree_key_cache_params);
+
+	bch2_btree_node_unlock_write(trans, path, path_l(path)->b);
+
 	if (unlikely(ret)) /* raced with another fill? */
 		goto err;
 
 	atomic_long_inc(&bc->nr_keys);
 	six_unlock_write(&ck->c.lock);
 
-	enum six_lock_type lock_want = __btree_lock_want(path, 0);
+	enum six_lock_type lock_want = __btree_lock_want(ck_path, 0);
 	if (lock_want == SIX_LOCK_read)
 		six_lock_downgrade(&ck->c.lock);
-	btree_path_cached_set(trans, path, ck, (enum btree_node_locked_type) lock_want);
-	path->uptodate = BTREE_ITER_UPTODATE;
+	btree_path_cached_set(trans, ck_path, ck, (enum btree_node_locked_type) lock_want);
+	ck_path->uptodate = BTREE_ITER_UPTODATE;
 	return 0;
 err:
-	bkey_cached_free(bc, ck);
-	mark_btree_node_locked_noreset(path, 0, BTREE_NODE_UNLOCKED);
+	bkey_cached_free(trans, bc, ck);
+	mark_btree_node_locked_noreset(ck_path, 0, BTREE_NODE_UNLOCKED);
 
 	return ret;
 }
 
+static noinline_for_stack void do_trace_key_cache_fill(struct btree_trans *trans,
+						       struct btree_path *ck_path,
+						       struct bkey_s_c k)
+{
+	struct printbuf buf = PRINTBUF;
+
+	bch2_bpos_to_text(&buf, ck_path->pos);
+	prt_char(&buf, ' ');
+	bch2_bkey_val_to_text(&buf, trans->c, k);
+	trace_key_cache_fill(trans, buf.buf);
+	printbuf_exit(&buf);
+}
+
 static noinline int btree_key_cache_fill(struct btree_trans *trans,
-					 struct btree_path *ck_path,
+					 btree_path_idx_t ck_path_idx,
 					 unsigned flags)
 {
+	struct btree_path *ck_path = trans->paths + ck_path_idx;
+
 	if (flags & BTREE_ITER_cached_nofill) {
-		ck_path->uptodate = BTREE_ITER_UPTODATE;
+		ck_path->l[0].b = NULL;
 		return 0;
 	}
 
@@ -293,35 +326,41 @@ static noinline int btree_key_cache_fill(struct btree_trans *trans,
 	int ret;
 
 	bch2_trans_iter_init(trans, &iter, ck_path->btree_id, ck_path->pos,
+			     BTREE_ITER_intent|
 			     BTREE_ITER_key_cache_fill|
 			     BTREE_ITER_cached_nofill);
 	iter.flags &= ~BTREE_ITER_with_journal;
-	k = bch2_btree_iter_peek_slot(&iter);
+	k = bch2_btree_iter_peek_slot(trans, &iter);
 	ret = bkey_err(k);
 	if (ret)
 		goto err;
 
 	/* Recheck after btree lookup, before allocating: */
+	ck_path = trans->paths + ck_path_idx;
 	ret = bch2_btree_key_cache_find(c, ck_path->btree_id, ck_path->pos) ? -EEXIST : 0;
 	if (unlikely(ret))
 		goto out;
 
-	ret = btree_key_cache_create(trans, ck_path, k);
+	ret = btree_key_cache_create(trans, btree_iter_path(trans, &iter), ck_path, k);
 	if (ret)
 		goto err;
+
+	if (trace_key_cache_fill_enabled())
+		do_trace_key_cache_fill(trans, ck_path, k);
 out:
 	/* We're not likely to need this iterator again: */
-	bch2_set_btree_iter_dontneed(&iter);
+	bch2_set_btree_iter_dontneed(trans, &iter);
 err:
 	bch2_trans_iter_exit(trans, &iter);
 	return ret;
 }
 
 static inline int btree_path_traverse_cached_fast(struct btree_trans *trans,
-						  struct btree_path *path)
+						  btree_path_idx_t path_idx)
 {
 	struct bch_fs *c = trans->c;
 	struct bkey_cached *ck;
+	struct btree_path *path = trans->paths + path_idx;
 retry:
 	ck = bch2_btree_key_cache_find(c, path->btree_id, path->pos);
 	if (!ck)
@@ -347,27 +386,32 @@ retry:
 	return 0;
 }
 
-int bch2_btree_path_traverse_cached(struct btree_trans *trans, struct btree_path *path,
+int bch2_btree_path_traverse_cached(struct btree_trans *trans,
+				    btree_path_idx_t path_idx,
 				    unsigned flags)
 {
-	EBUG_ON(path->level);
-
-	path->l[1].b = NULL;
+	EBUG_ON(trans->paths[path_idx].level);
 
 	int ret;
 	do {
-		ret = btree_path_traverse_cached_fast(trans, path);
+		ret = btree_path_traverse_cached_fast(trans, path_idx);
 		if (unlikely(ret == -ENOENT))
-			ret = btree_key_cache_fill(trans, path, flags);
+			ret = btree_key_cache_fill(trans, path_idx, flags);
 	} while (ret == -EEXIST);
 
+	struct btree_path *path = trans->paths + path_idx;
+
 	if (unlikely(ret)) {
 		path->uptodate = BTREE_ITER_NEED_TRAVERSE;
 		if (!bch2_err_matches(ret, BCH_ERR_transaction_restart)) {
 			btree_node_unlock(trans, path, 0);
 			path->l[0].b = ERR_PTR(ret);
 		}
+	} else {
+		BUG_ON(path->uptodate);
+		BUG_ON(!path->nodes_locked);
 	}
+
 	return ret;
 }
 
@@ -392,7 +436,7 @@ static int btree_key_cache_flush_pos(struct btree_trans *trans,
 			     BTREE_ITER_intent);
 	b_iter.flags &= ~BTREE_ITER_with_key_cache;
 
-	ret = bch2_btree_iter_traverse(&c_iter);
+	ret = bch2_btree_iter_traverse(trans, &c_iter);
 	if (ret)
 		goto out;
 
@@ -424,8 +468,15 @@ static int btree_key_cache_flush_pos(struct btree_trans *trans,
 	    !test_bit(JOURNAL_space_low, &c->journal.flags))
 		commit_flags |= BCH_TRANS_COMMIT_no_journal_res;
 
-	ret   = bch2_btree_iter_traverse(&b_iter) ?:
-		bch2_trans_update(trans, &b_iter, ck->k,
+	struct bkey_s_c btree_k = bch2_btree_iter_peek_slot(trans, &b_iter);
+	ret = bkey_err(btree_k);
+	if (ret)
+		goto err;
+
+	/* * Check that we're not violating cache coherency rules: */
+	BUG_ON(bkey_deleted(btree_k.k));
+
+	ret   = bch2_trans_update(trans, &b_iter, ck->k,
 				  BTREE_UPDATE_key_cache_reclaim|
 				  BTREE_UPDATE_internal_snapshot_node|
 				  BTREE_TRIGGER_norun) ?:
@@ -433,7 +484,7 @@ static int btree_key_cache_flush_pos(struct btree_trans *trans,
 				  BCH_TRANS_COMMIT_no_check_rw|
 				  BCH_TRANS_COMMIT_no_enospc|
 				  commit_flags);
-
+err:
 	bch2_fs_fatal_err_on(ret &&
 			     !bch2_err_matches(ret, BCH_ERR_transaction_restart) &&
 			     !bch2_err_matches(ret, BCH_ERR_journal_reclaim_would_deadlock) &&
@@ -469,7 +520,7 @@ evict:
 
 		mark_btree_node_locked_noreset(path, 0, BTREE_NODE_UNLOCKED);
 		if (bkey_cached_evict(&c->btree_key_cache, ck)) {
-			bkey_cached_free(&c->btree_key_cache, ck);
+			bkey_cached_free(trans, &c->btree_key_cache, ck);
 		} else {
 			six_unlock_write(&ck->c.lock);
 			six_unlock_intent(&ck->c.lock);
@@ -583,11 +634,28 @@ void bch2_btree_key_cache_drop(struct btree_trans *trans,
 	}
 
 	bkey_cached_evict(bc, ck);
-	bkey_cached_free(bc, ck);
+	bkey_cached_free(trans, bc, ck);
 
 	mark_btree_node_locked(trans, path, 0, BTREE_NODE_UNLOCKED);
-	btree_path_set_dirty(path, BTREE_ITER_NEED_TRAVERSE);
-	path->should_be_locked = false;
+
+	struct btree_path *path2;
+	unsigned i;
+	trans_for_each_path(trans, path2, i)
+		if (path2->l[0].b == (void *) ck) {
+			/*
+			 * It's safe to clear should_be_locked here because
+			 * we're evicting from the key cache, and we still have
+			 * the underlying btree locked: filling into the key
+			 * cache would require taking a write lock on the btree
+			 * node
+			 */
+			path2->should_be_locked = false;
+			__bch2_btree_path_unlock(trans, path2);
+			path2->l[0].b = ERR_PTR(-BCH_ERR_no_btree_node_drop);
+			btree_path_set_dirty(trans, path2, BTREE_ITER_NEED_TRAVERSE);
+		}
+
+	bch2_trans_verify_locks(trans);
 }
 
 static unsigned long bch2_btree_key_cache_scan(struct shrinker *shrink,
@@ -641,7 +709,7 @@ static unsigned long bch2_btree_key_cache_scan(struct shrinker *shrink,
 			} else if (!bkey_cached_lock_for_evict(ck)) {
 				bc->skipped_lock_fail++;
 			} else if (bkey_cached_evict(bc, ck)) {
-				bkey_cached_free(bc, ck);
+				bkey_cached_free_noassert(bc, ck);
 				bc->freed++;
 				freed++;
 			} else {
@@ -711,7 +779,6 @@ void bch2_fs_btree_key_cache_exit(struct btree_key_cache *bc)
 				rcu_read_unlock();
 				mutex_lock(&bc->table.mutex);
 				mutex_unlock(&bc->table.mutex);
-				rcu_read_lock();
 				continue;
 			}
 			for (i = 0; i < tbl->size; i++)
@@ -755,20 +822,20 @@ int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)
 
 	bc->nr_pending = alloc_percpu(size_t);
 	if (!bc->nr_pending)
-		return -BCH_ERR_ENOMEM_fs_btree_cache_init;
+		return bch_err_throw(c, ENOMEM_fs_btree_cache_init);
 
 	if (rcu_pending_init(&bc->pending[0], &c->btree_trans_barrier, __bkey_cached_free) ||
 	    rcu_pending_init(&bc->pending[1], &c->btree_trans_barrier, __bkey_cached_free))
-		return -BCH_ERR_ENOMEM_fs_btree_cache_init;
+		return bch_err_throw(c, ENOMEM_fs_btree_cache_init);
 
 	if (rhashtable_init(&bc->table, &bch2_btree_key_cache_params))
-		return -BCH_ERR_ENOMEM_fs_btree_cache_init;
+		return bch_err_throw(c, ENOMEM_fs_btree_cache_init);
 
 	bc->table_init_done = true;
 
 	shrink = shrinker_alloc(0, "%s-btree_key_cache", c->name);
 	if (!shrink)
-		return -BCH_ERR_ENOMEM_fs_btree_cache_init;
+		return bch_err_throw(c, ENOMEM_fs_btree_cache_init);
 	bc->shrink = shrink;
 	shrink->count_objects	= bch2_btree_key_cache_count;
 	shrink->scan_objects	= bch2_btree_key_cache_scan;