bcachefs: Call bch2_path_put_nokeep() before bch2_path_put()

bch2_path_put_nokeep() is sketchy, and we should consider removing it:
it unconditionally frees btree_paths once their ref hits 0.

The assumption is that we only use it for paths that have never been
visible outside the btree core btree code; i.e. higher level code will
never be making assumptions about locking based on these paths.

However, there's subtle brokenness with this approach:

 - If we call bch2_path_put(), then bch2_path_put_nokeep(),
   bch2_path_put() may free the first path on the assumption that we we
   have another path keeping a node locked - but then
   bch2_path_put_nokeep() just unconditionally frees it.

The same bug may arise if we're calling bch2_path_put() and
bch2_path_put_nokeep() on the same (refcounted) path, or two adjacent
paths that point to the same btree node.

This patch hacks around one of these bugs by calling
bch2_path_put_nokeep() first in bch2_trans_iter_exit.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
index 7b3e7f9368d1da02ac7c7312bc8c525dfeb1b7cd..0a62f55a3aa8a13e1062ccac256eb74ff5438ee4 100644 (file)
--- a/fs/bcachefs/btree_iter.c
+++ b/fs/bcachefs/btree_iter.c
@@ -2722,12 +2722,12 @@ static inline void btree_path_list_add(struct btree_trans *trans,
 
 void bch2_trans_iter_exit(struct btree_trans *trans, struct btree_iter *iter)
 {
-       if (iter->path)
-               bch2_path_put(trans, iter->path,
-                             iter->flags & BTREE_ITER_INTENT);
        if (iter->update_path)
                bch2_path_put_nokeep(trans, iter->update_path,
                              iter->flags & BTREE_ITER_INTENT);
+       if (iter->path)
+               bch2_path_put(trans, iter->path,
+                             iter->flags & BTREE_ITER_INTENT);
        if (iter->key_cache_path)
                bch2_path_put(trans, iter->key_cache_path,
                              iter->flags & BTREE_ITER_INTENT);