From 8cad3e2f73f5c6ad39e9da5564382a2a737a201c Mon Sep 17 00:00:00 2001 From: Kent Overstreet Date: Sun, 22 Sep 2019 19:10:21 -0400 Subject: [PATCH] bcachefs: Use cached iterators for inode updates This switches inode updates to use cached btree iterators - which should be a nice performance boost, since lock contention on the inodes btree can be a bottleneck on multithreaded workloads. Signed-off-by: Kent Overstreet Signed-off-by: Kent Overstreet --- fs/bcachefs/btree_key_cache.c | 10 ++-- fs/bcachefs/btree_key_cache.h | 3 + fs/bcachefs/inode.c | 104 +++++++++++++++++++++------------- 3 files changed, 72 insertions(+), 45 deletions(-) diff --git a/fs/bcachefs/btree_key_cache.c b/fs/bcachefs/btree_key_cache.c index 1be01035869f3..52b657030755b 100644 --- a/fs/bcachefs/btree_key_cache.c +++ b/fs/bcachefs/btree_key_cache.c @@ -28,8 +28,8 @@ static const struct rhashtable_params bch2_btree_key_cache_params = { }; __flatten -static inline struct bkey_cached * -btree_key_cache_find(struct bch_fs *c, enum btree_id btree_id, struct bpos pos) +inline struct bkey_cached * +bch2_btree_key_cache_find(struct bch_fs *c, enum btree_id btree_id, struct bpos pos) { struct bkey_cached_key key = { .btree_id = btree_id, @@ -218,7 +218,7 @@ int bch2_btree_iter_traverse_cached(struct btree_iter *iter) goto fill; } retry: - ck = btree_key_cache_find(c, iter->btree_id, iter->pos); + ck = bch2_btree_key_cache_find(c, iter->btree_id, iter->pos); if (!ck) { if (iter->flags & BTREE_ITER_CACHED_NOCREATE) { iter->l[0].b = NULL; @@ -415,7 +415,7 @@ int bch2_btree_key_cache_flush(struct btree_trans *trans, struct bkey_cached_key key = { id, pos }; /* Fastpath - assume it won't be found: */ - if (!btree_key_cache_find(c, id, pos)) + if (!bch2_btree_key_cache_find(c, id, pos)) return 0; return btree_key_cache_flush_pos(trans, key, 0, true); @@ -462,7 +462,7 @@ bool bch2_btree_insert_key_cached(struct btree_trans *trans, void bch2_btree_key_cache_verify_clean(struct btree_trans *trans, enum btree_id id, struct bpos pos) { - BUG_ON(btree_key_cache_find(trans->c, id, pos)); + BUG_ON(bch2_btree_key_cache_find(trans->c, id, pos)); } #endif diff --git a/fs/bcachefs/btree_key_cache.h b/fs/bcachefs/btree_key_cache.h index b1756c6c622ce..d448264abcc89 100644 --- a/fs/bcachefs/btree_key_cache.h +++ b/fs/bcachefs/btree_key_cache.h @@ -1,6 +1,9 @@ #ifndef _BCACHEFS_BTREE_KEY_CACHE_H #define _BCACHEFS_BTREE_KEY_CACHE_H +struct bkey_cached * +bch2_btree_key_cache_find(struct bch_fs *, enum btree_id, struct bpos); + int bch2_btree_iter_traverse_cached(struct btree_iter *); bool bch2_btree_insert_key_cached(struct btree_trans *, diff --git a/fs/bcachefs/inode.c b/fs/bcachefs/inode.c index 71670f415d66d..631c60bb2facf 100644 --- a/fs/bcachefs/inode.c +++ b/fs/bcachefs/inode.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include "bcachefs.h" +#include "btree_key_cache.h" #include "bkey_methods.h" #include "btree_update.h" #include "error.h" @@ -189,11 +190,11 @@ struct btree_iter *bch2_inode_peek(struct btree_trans *trans, int ret; iter = bch2_trans_get_iter(trans, BTREE_ID_INODES, POS(0, inum), - BTREE_ITER_SLOTS|flags); + BTREE_ITER_CACHED|flags); if (IS_ERR(iter)) return iter; - k = bch2_btree_iter_peek_slot(iter); + k = bch2_btree_iter_peek_cached(iter); ret = bkey_err(k); if (ret) goto err; @@ -390,7 +391,17 @@ again: if (bkey_cmp(iter->pos, POS(0, max)) > 0) break; - if (k.k->type != KEY_TYPE_inode) + /* + * There's a potential cache coherency issue with the btree key + * cache code here - we're iterating over the btree, skipping + * that cache. We should never see an empty slot that isn't + * actually empty due to a pending update in the key cache + * because the update that creates the inode isn't done with a + * cached iterator, but - better safe than sorry, check the + * cache before using a slot: + */ + if (k.k->type != KEY_TYPE_inode && + !bch2_btree_key_cache_find(trans->c, BTREE_ID_INODES, iter->pos)) goto found_slot; } @@ -424,6 +435,8 @@ int bch2_inode_rm(struct bch_fs *c, u64 inode_nr) struct bkey_i_inode_generation delete; struct bpos start = POS(inode_nr, 0); struct bpos end = POS(inode_nr + 1, 0); + struct bkey_s_c k; + u64 bi_generation; int ret; /* @@ -444,51 +457,62 @@ int bch2_inode_rm(struct bch_fs *c, u64 inode_nr) return ret; bch2_trans_init(&trans, c, 0, 0); +retry: + bch2_trans_begin(&trans); + + bi_generation = 0; + + ret = bch2_btree_key_cache_flush(&trans, BTREE_ID_INODES, POS(0, inode_nr)); + if (ret) { + if (ret != -EINTR) + bch_err(c, "error flushing btree key cache: %i", ret); + goto err; + } iter = bch2_trans_get_iter(&trans, BTREE_ID_INODES, POS(0, inode_nr), BTREE_ITER_SLOTS|BTREE_ITER_INTENT); - do { - struct bkey_s_c k = bch2_btree_iter_peek_slot(iter); - u32 bi_generation = 0; + k = bch2_btree_iter_peek_slot(iter); - ret = bkey_err(k); - if (ret) - break; + ret = bkey_err(k); + if (ret) + goto err; - bch2_fs_inconsistent_on(k.k->type != KEY_TYPE_inode, c, - "inode %llu not found when deleting", - inode_nr); + bch2_fs_inconsistent_on(k.k->type != KEY_TYPE_inode, c, + "inode %llu not found when deleting", + inode_nr); - switch (k.k->type) { - case KEY_TYPE_inode: { - struct bch_inode_unpacked inode_u; + switch (k.k->type) { + case KEY_TYPE_inode: { + struct bch_inode_unpacked inode_u; - if (!bch2_inode_unpack(bkey_s_c_to_inode(k), &inode_u)) - bi_generation = inode_u.bi_generation + 1; - break; - } - case KEY_TYPE_inode_generation: { - struct bkey_s_c_inode_generation g = - bkey_s_c_to_inode_generation(k); - bi_generation = le32_to_cpu(g.v->bi_generation); - break; - } - } + if (!bch2_inode_unpack(bkey_s_c_to_inode(k), &inode_u)) + bi_generation = inode_u.bi_generation + 1; + break; + } + case KEY_TYPE_inode_generation: { + struct bkey_s_c_inode_generation g = + bkey_s_c_to_inode_generation(k); + bi_generation = le32_to_cpu(g.v->bi_generation); + break; + } + } - if (!bi_generation) { - bkey_init(&delete.k); - delete.k.p.offset = inode_nr; - } else { - bkey_inode_generation_init(&delete.k_i); - delete.k.p.offset = inode_nr; - delete.v.bi_generation = cpu_to_le32(bi_generation); - } + if (!bi_generation) { + bkey_init(&delete.k); + delete.k.p.offset = inode_nr; + } else { + bkey_inode_generation_init(&delete.k_i); + delete.k.p.offset = inode_nr; + delete.v.bi_generation = cpu_to_le32(bi_generation); + } - bch2_trans_update(&trans, iter, &delete.k_i, 0); + bch2_trans_update(&trans, iter, &delete.k_i, 0); - ret = bch2_trans_commit(&trans, NULL, NULL, - BTREE_INSERT_NOFAIL); - } while (ret == -EINTR); + ret = bch2_trans_commit(&trans, NULL, NULL, + BTREE_INSERT_NOFAIL); +err: + if (ret == -EINTR) + goto retry; bch2_trans_exit(&trans); return ret; @@ -502,11 +526,11 @@ int bch2_inode_find_by_inum_trans(struct btree_trans *trans, u64 inode_nr, int ret; iter = bch2_trans_get_iter(trans, BTREE_ID_INODES, - POS(0, inode_nr), BTREE_ITER_SLOTS); + POS(0, inode_nr), BTREE_ITER_CACHED); if (IS_ERR(iter)) return PTR_ERR(iter); - k = bch2_btree_iter_peek_slot(iter); + k = bch2_btree_iter_peek_cached(iter); ret = bkey_err(k); if (ret) goto err; -- 2.30.2