From e941ae7d3afc68127adef917a2b779dabe83fdfe Mon Sep 17 00:00:00 2001 From: Kent Overstreet Date: Sun, 17 Jul 2022 19:35:38 -0400 Subject: [PATCH] bcachefs: Add a counter for btree_trans restarts This will help us improve nested transactions - we need to add assertions that whenever an inner transaction handles a restart, it still returns -EINTR to the outer transaction. This also adds nested_lockrestart_do() and nested_commit_do() which use the new counters to correctly return -EINTR when the transaction was restarted. Signed-off-by: Kent Overstreet --- fs/bcachefs/btree_iter.c | 13 +++++++++-- fs/bcachefs/btree_iter.h | 46 +++++++++++++++++++++++++++++++++++++- fs/bcachefs/btree_types.h | 3 +++ fs/bcachefs/btree_update.h | 16 ++++--------- 4 files changed, 63 insertions(+), 15 deletions(-) diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c index 30958cbb95322..45ecd196bceb2 100644 --- a/fs/bcachefs/btree_iter.c +++ b/fs/bcachefs/btree_iter.c @@ -3188,7 +3188,7 @@ void *bch2_trans_kmalloc(struct btree_trans *trans, size_t size) * node may return EINTR when the trylock fails. When this occurs * bch2_trans_begin() should be called and the transaction retried. */ -void bch2_trans_begin(struct btree_trans *trans) +u32 bch2_trans_begin(struct btree_trans *trans) { struct btree_path *path; @@ -3234,11 +3234,20 @@ void bch2_trans_begin(struct btree_trans *trans) bch2_trans_relock(trans); } + trans->last_restarted_ip = _RET_IP_; if (trans->restarted) bch2_btree_path_traverse_all(trans); - trans->restarted = false; trans->last_begin_time = ktime_get_ns(); + return trans->restart_count; +} + +void bch2_trans_verify_not_restarted(struct btree_trans *trans, u32 restart_count) +{ + bch2_trans_inconsistent_on(trans_was_restarted(trans, restart_count), trans, + "trans->restart_count %u, should be %u, last restarted by %ps\n", + trans->restart_count, restart_count, + (void *) trans->last_restarted_ip); } static void bch2_trans_alloc_paths(struct btree_trans *trans, struct bch_fs *c) diff --git a/fs/bcachefs/btree_iter.h b/fs/bcachefs/btree_iter.h index 209b89dd1d2b6..c2f5afc9eeb97 100644 --- a/fs/bcachefs/btree_iter.h +++ b/fs/bcachefs/btree_iter.h @@ -203,10 +203,18 @@ void bch2_path_put(struct btree_trans *, struct btree_path *, bool); bool bch2_trans_relock(struct btree_trans *); void bch2_trans_unlock(struct btree_trans *); +static inline int trans_was_restarted(struct btree_trans *trans, u32 restart_count) +{ + return restart_count != trans->restart_count ? -EINTR : 0; +} + +void bch2_trans_verify_not_restarted(struct btree_trans *, u32); + __always_inline static inline int btree_trans_restart(struct btree_trans *trans) { trans->restarted = true; + trans->restart_count++; bch2_trans_unlock(trans); return -EINTR; } @@ -321,7 +329,7 @@ static inline void set_btree_iter_dontneed(struct btree_iter *iter) } void *bch2_trans_kmalloc(struct btree_trans *, size_t); -void bch2_trans_begin(struct btree_trans *); +u32 bch2_trans_begin(struct btree_trans *); static inline struct btree * __btree_iter_peek_node_and_restart(struct btree_trans *trans, struct btree_iter *iter) @@ -394,6 +402,42 @@ __bch2_btree_iter_peek_and_restart(struct btree_trans *trans, return k; } +#define lockrestart_do(_trans, _do) \ +({ \ + int _ret; \ + \ + do { \ + bch2_trans_begin(_trans); \ + _ret = (_do); \ + } while (_ret == -EINTR); \ + \ + _ret; \ +}) + +/* + * nested_lockrestart_do(), nested_commit_do(): + * + * These are like lockrestart_do() and commit_do(), with two differences: + * + * - We don't call bch2_trans_begin() unless we had a transaction restart + * - We return -EINTR if we succeeded after a transaction restart + */ +#define nested_lockrestart_do(_trans, _do) \ +({ \ + u32 _restart_count, _orig_restart_count; \ + int _ret; \ + \ + _restart_count = _orig_restart_count = (_trans)->restart_count; \ + \ + while ((_ret = (_do)) == -EINTR) \ + _restart_count = bch2_trans_begin(_trans); \ + \ + if (!_ret) \ + bch2_trans_verify_not_restarted(_trans, _restart_count);\ + \ + _ret ?: trans_was_restarted(_trans, _orig_restart_count); \ +}) + #define for_each_btree_key2(_trans, _iter, _btree_id, \ _start, _flags, _k, _do) \ ({ \ diff --git a/fs/bcachefs/btree_types.h b/fs/bcachefs/btree_types.h index 64f4bc8913e85..0650a35581828 100644 --- a/fs/bcachefs/btree_types.h +++ b/fs/bcachefs/btree_types.h @@ -410,6 +410,9 @@ struct btree_trans { bool memory_allocation_failure:1; bool journal_transaction_names:1; bool journal_replay_not_finished:1; + u32 restart_count; + unsigned long last_restarted_ip; + /* * For when bch2_trans_update notices we'll be splitting a compressed * extent: diff --git a/fs/bcachefs/btree_update.h b/fs/bcachefs/btree_update.h index 1c3dd012cae89..9b5a8b18b01be 100644 --- a/fs/bcachefs/btree_update.h +++ b/fs/bcachefs/btree_update.h @@ -106,22 +106,14 @@ static inline int bch2_trans_commit(struct btree_trans *trans, return __bch2_trans_commit(trans); } -#define lockrestart_do(_trans, _do) \ -({ \ - int _ret; \ - \ - do { \ - bch2_trans_begin(_trans); \ - _ret = (_do); \ - } while (_ret == -EINTR); \ - \ - _ret; \ -}) - #define commit_do(_trans, _disk_res, _journal_seq, _flags, _do) \ lockrestart_do(_trans, _do ?: bch2_trans_commit(_trans, (_disk_res),\ (_journal_seq), (_flags))) +#define nested_commit_do(_trans, _disk_res, _journal_seq, _flags, _do) \ + nested_lockrestart_do(_trans, _do ?: bch2_trans_commit(_trans, (_disk_res),\ + (_journal_seq), (_flags))) + #define bch2_trans_do(_c, _disk_res, _journal_seq, _flags, _do) \ ({ \ struct btree_trans trans; \ -- 2.30.2