diff options
author | Kent Overstreet <kent.overstreet@gmail.com> | 2022-08-11 20:14:54 -0400 |
---|---|---|
committer | Kent Overstreet <kent.overstreet@gmail.com> | 2022-08-11 20:47:24 -0400 |
commit | 2c5c54f44ab14b394c7c3a55109c7d61a1fe928a (patch) | |
tree | 80031769918853751a6cac5617d04567d10ca121 | |
parent | 9539f293d5103f489ced43efc7c3427c0831f74c (diff) |
bcachefs: Track the maximum btree_paths ever allocated by each transaction
We need a way to check if the machinery for handling btree_paths with in
a transaction is behaving reasonably, as it often has not been - we've
had bugs with transaction path overflows caused by duplicate paths and
plenty of other things.
This patch tracks, per transaction fn, the most btree paths ever
allocated by that transaction and makes it available in debugfs.
Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
-rw-r--r-- | fs/bcachefs/bcachefs.h | 3 | ||||
-rw-r--r-- | fs/bcachefs/btree_iter.c | 77 | ||||
-rw-r--r-- | fs/bcachefs/btree_iter.h | 2 | ||||
-rw-r--r-- | fs/bcachefs/debug.c | 30 |
4 files changed, 85 insertions, 27 deletions
diff --git a/fs/bcachefs/bcachefs.h b/fs/bcachefs/bcachefs.h index c23cc33b67e2..a5bf808763e0 100644 --- a/fs/bcachefs/bcachefs.h +++ b/fs/bcachefs/bcachefs.h @@ -532,7 +532,10 @@ struct btree_debug { #define BCH_TRANSACTIONS_NR 128 struct btree_transaction_stats { + struct mutex lock; struct time_stats lock_hold_times; + unsigned nr_max_paths; + char *max_paths_text; }; struct bch_fs_pcpu { diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c index 226b9497044d..6f02cddbab91 100644 --- a/fs/bcachefs/btree_iter.c +++ b/fs/bcachefs/btree_iter.c @@ -1872,42 +1872,69 @@ void bch2_dump_trans_updates(struct btree_trans *trans) printbuf_exit(&buf); } -noinline __cold -void bch2_dump_trans_paths_updates(struct btree_trans *trans) +void bch2_btree_path_to_text(struct printbuf *out, struct btree_path *path) +{ + prt_printf(out, "path: idx %2u ref %u:%u %c %c btree=%s l=%u pos ", + path->idx, path->ref, path->intent_ref, + path->preserve ? 'P' : ' ', + path->should_be_locked ? 'S' : ' ', + bch2_btree_ids[path->btree_id], + path->level); + bch2_bpos_to_text(out, path->pos); + + prt_printf(out, " locks %u", path->nodes_locked); +#ifdef CONFIG_BCACHEFS_DEBUG + prt_printf(out, " %pS", (void *) path->ip_allocated); +#endif + prt_newline(out); +} + +void bch2_trans_paths_to_text(struct printbuf *out, struct btree_trans *trans) { struct btree_path *path; - struct printbuf buf = PRINTBUF; unsigned idx; - trans_for_each_path_inorder(trans, path, idx) { - printbuf_reset(&buf); + trans_for_each_path_inorder(trans, path, idx) + bch2_btree_path_to_text(out, path); +} - bch2_bpos_to_text(&buf, path->pos); +noinline __cold +void bch2_dump_trans_paths_updates(struct btree_trans *trans) +{ + struct printbuf buf = PRINTBUF; - printk(KERN_ERR "path: idx %2u ref %u:%u %c %c btree=%s l=%u pos %s locks %u %pS\n", - path->idx, path->ref, path->intent_ref, - path->preserve ? 'P' : ' ', - path->should_be_locked ? 'S' : ' ', - bch2_btree_ids[path->btree_id], - path->level, - buf.buf, - path->nodes_locked, -#ifdef CONFIG_BCACHEFS_DEBUG - (void *) path->ip_allocated -#else - NULL -#endif - ); - } + bch2_trans_paths_to_text(&buf, trans); + printk(KERN_ERR "%s", buf.buf); printbuf_exit(&buf); bch2_dump_trans_updates(trans); } +noinline +static void bch2_trans_update_max_paths(struct btree_trans *trans) +{ + struct btree_transaction_stats *s = btree_trans_stats(trans); + struct printbuf buf = PRINTBUF; + + bch2_trans_paths_to_text(&buf, trans); + + if (!buf.allocation_failure) { + mutex_lock(&s->lock); + if (s->nr_max_paths < hweight64(trans->paths_allocated)) { + s->nr_max_paths = hweight64(trans->paths_allocated); + swap(s->max_paths_text, buf.buf); + } + mutex_unlock(&s->lock); + } + + printbuf_exit(&buf); +} + static struct btree_path *btree_path_alloc(struct btree_trans *trans, struct btree_path *pos) { + struct btree_transaction_stats *s = btree_trans_stats(trans); struct btree_path *path; unsigned idx; @@ -1920,6 +1947,9 @@ static struct btree_path *btree_path_alloc(struct btree_trans *trans, idx = __ffs64(~trans->paths_allocated); trans->paths_allocated |= 1ULL << idx; + if (s && unlikely(hweight64(trans->paths_allocated) > s->nr_max_paths)) + bch2_trans_update_max_paths(trans); + path = &trans->paths[idx]; path->idx = idx; @@ -3471,9 +3501,12 @@ void bch2_fs_btree_iter_exit(struct bch_fs *c) int bch2_fs_btree_iter_init(struct bch_fs *c) { - unsigned nr = BTREE_ITER_MAX; + unsigned i, nr = BTREE_ITER_MAX; int ret; + for (i = 0; i < ARRAY_SIZE(c->btree_transaction_stats); i++) + mutex_init(&c->btree_transaction_stats[i].lock); + INIT_LIST_HEAD(&c->btree_trans_list); mutex_init(&c->btree_trans_lock); diff --git a/fs/bcachefs/btree_iter.h b/fs/bcachefs/btree_iter.h index 24b99cf20e13..de9c584916a1 100644 --- a/fs/bcachefs/btree_iter.h +++ b/fs/bcachefs/btree_iter.h @@ -535,6 +535,8 @@ __bch2_btree_iter_peek_and_restart(struct btree_trans *trans, /* new multiple iterator interface: */ void bch2_trans_updates_to_text(struct printbuf *, struct btree_trans *); +void bch2_btree_path_to_text(struct printbuf *, struct btree_path *); +void bch2_trans_paths_to_text(struct printbuf *, struct btree_trans *); void bch2_dump_trans_updates(struct btree_trans *); void bch2_dump_trans_paths_updates(struct btree_trans *); void __bch2_trans_init(struct btree_trans *, struct bch_fs *, diff --git a/fs/bcachefs/debug.c b/fs/bcachefs/debug.c index 413a577d5730..781cb22e6304 100644 --- a/fs/bcachefs/debug.c +++ b/fs/bcachefs/debug.c @@ -672,7 +672,29 @@ static ssize_t lock_held_stats_read(struct file *file, char __user *buf, prt_printf(&i->buf, "%s: ", c->btree_transaction_fns[i->iter]); prt_newline(&i->buf); printbuf_indent_add(&i->buf, 2); - bch2_time_stats_to_text(&i->buf, &s->lock_hold_times); + + mutex_lock(&s->lock); + + if (IS_ENABLED(CONFIG_BCACHEFS_LOCK_TIME_STATS)) { + prt_printf(&i->buf, "Lock hold times:"); + prt_newline(&i->buf); + + printbuf_indent_add(&i->buf, 2); + bch2_time_stats_to_text(&i->buf, &s->lock_hold_times); + printbuf_indent_sub(&i->buf, 2); + } + + if (s->max_paths_text) { + prt_printf(&i->buf, "Maximum allocated btree paths (%u):", s->nr_max_paths); + prt_newline(&i->buf); + + printbuf_indent_add(&i->buf, 2); + prt_str_indented(&i->buf, s->max_paths_text); + printbuf_indent_sub(&i->buf, 2); + } + + mutex_unlock(&s->lock); + printbuf_indent_sub(&i->buf, 2); prt_newline(&i->buf); i->iter++; @@ -719,10 +741,8 @@ void bch2_fs_debug_init(struct bch_fs *c) debugfs_create_file("journal_pins", 0400, c->fs_debug_dir, c->btree_debug, &journal_pins_ops); - if (IS_ENABLED(CONFIG_BCACHEFS_LOCK_TIME_STATS)) { - debugfs_create_file("btree_transaction_stats", 0400, c->fs_debug_dir, - c, &lock_held_stats_op); - } + debugfs_create_file("btree_transaction_stats", 0400, c->fs_debug_dir, + c, &lock_held_stats_op); c->btree_debug_dir = debugfs_create_dir("btrees", c->fs_debug_dir); if (IS_ERR_OR_NULL(c->btree_debug_dir)) |