bcachefs: Make sure we're releasing btree iterators

This wasn't originally required, but this is the model we're moving
towards.

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

diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
index 5e220284b0b34249fec2759362c6420d9154fd7c..cc5d6389899c86046582275d258e86941b993a32 100644 (file)
--- a/fs/bcachefs/btree_iter.c
+++ b/fs/bcachefs/btree_iter.c
@@ -1729,7 +1729,12 @@ static inline void __bch2_trans_iter_free(struct btree_trans *trans,
 int bch2_trans_iter_put(struct btree_trans *trans,
                        struct btree_iter *iter)
 {
-       int ret = btree_iter_err(iter);
+       int ret;
+
+       if (IS_ERR_OR_NULL(iter))
+               return 0;
+
+       ret = btree_iter_err(iter);
 
        if (!(trans->iters_touched & (1ULL << iter->idx)) &&
            !(iter->flags & BTREE_ITER_KEEP_UNTIL_COMMIT))
@@ -1742,6 +1747,9 @@ int bch2_trans_iter_put(struct btree_trans *trans,
 int bch2_trans_iter_free(struct btree_trans *trans,
                         struct btree_iter *iter)
 {
+       if (IS_ERR_OR_NULL(iter))
+               return 0;
+
        trans->iters_touched &= ~(1ULL << iter->idx);
 
        return bch2_trans_iter_put(trans, iter);
@@ -1981,8 +1989,8 @@ struct btree_iter *bch2_trans_copy_iter(struct btree_trans *trans,
 
        trans->iters_live |= 1ULL << iter->idx;
        /*
-        * Don't mark it as touched, we don't need to preserve this iter since
-        * it's cheap to copy it again:
+        * We don't need to preserve this iter since it's cheap to copy it
+        * again - this will cause trans_iter_put() to free it right away:
         */
        trans->iters_touched &= ~(1ULL << iter->idx);
 

diff --git a/fs/bcachefs/fs-common.c b/fs/bcachefs/fs-common.c
index 96f7bbe0a3eddf2483e9067805743f7e73519488..878419d409927c7d33902993d0f37a4b5357e362 100644 (file)
--- a/fs/bcachefs/fs-common.c
+++ b/fs/bcachefs/fs-common.c
@@ -19,14 +19,15 @@ int bch2_create_trans(struct btree_trans *trans, u64 dir_inum,
                      struct posix_acl *acl)
 {
        struct bch_fs *c = trans->c;
-       struct btree_iter *dir_iter;
+       struct btree_iter *dir_iter = NULL;
        struct bch_hash_info hash = bch2_hash_info_init(c, new_inode);
        u64 now = bch2_current_time(trans->c);
        int ret;
 
        dir_iter = bch2_inode_peek(trans, dir_u, dir_inum, BTREE_ITER_INTENT);
-       if (IS_ERR(dir_iter))
-               return PTR_ERR(dir_iter);
+       ret = PTR_ERR_OR_ZERO(dir_iter);
+       if (ret)
+               goto err;
 
        bch2_inode_init_late(new_inode, now, uid, gid, mode, rdev, dir_u);
 
@@ -37,20 +38,20 @@ int bch2_create_trans(struct btree_trans *trans, u64 dir_inum,
                                BLOCKDEV_INODE_MAX, 0,
                                &c->unused_inode_hint);
        if (ret)
-               return ret;
+               goto err;
 
        if (default_acl) {
                ret = bch2_set_acl_trans(trans, new_inode, &hash,
                                         default_acl, ACL_TYPE_DEFAULT);
                if (ret)
-                       return ret;
+                       goto err;
        }
 
        if (acl) {
                ret = bch2_set_acl_trans(trans, new_inode, &hash,
                                         acl, ACL_TYPE_ACCESS);
                if (ret)
-                       return ret;
+                       goto err;
        }
 
        if (name) {
@@ -62,48 +63,55 @@ int bch2_create_trans(struct btree_trans *trans, u64 dir_inum,
 
                ret = bch2_inode_write(trans, dir_iter, dir_u);
                if (ret)
-                       return ret;
+                       goto err;
 
                ret = bch2_dirent_create(trans, dir_inum, &dir_hash,
                                         mode_to_type(new_inode->bi_mode),
                                         name, new_inode->bi_inum,
                                         BCH_HASH_SET_MUST_CREATE);
                if (ret)
-                       return ret;
+                       goto err;
        }
-
-       return 0;
+err:
+       bch2_trans_iter_put(trans, dir_iter);
+       return ret;
 }
 
 int bch2_link_trans(struct btree_trans *trans, u64 dir_inum,
                    u64 inum, struct bch_inode_unpacked *dir_u,
                    struct bch_inode_unpacked *inode_u, const struct qstr *name)
 {
-       struct btree_iter *dir_iter, *inode_iter;
+       struct btree_iter *dir_iter = NULL, *inode_iter = NULL;
        struct bch_hash_info dir_hash;
        u64 now = bch2_current_time(trans->c);
+       int ret;
 
        inode_iter = bch2_inode_peek(trans, inode_u, inum, BTREE_ITER_INTENT);
-       if (IS_ERR(inode_iter))
-               return PTR_ERR(inode_iter);
+       ret = PTR_ERR_OR_ZERO(inode_iter);
+       if (ret)
+               goto err;
 
        inode_u->bi_ctime = now;
        bch2_inode_nlink_inc(inode_u);
 
        dir_iter = bch2_inode_peek(trans, dir_u, dir_inum, 0);
-       if (IS_ERR(dir_iter))
-               return PTR_ERR(dir_iter);
+       ret = PTR_ERR_OR_ZERO(dir_iter);
+       if (ret)
+               goto err;
 
        dir_u->bi_mtime = dir_u->bi_ctime = now;
 
        dir_hash = bch2_hash_info_init(trans->c, dir_u);
-       bch2_trans_iter_put(trans, dir_iter);
 
-       return bch2_dirent_create(trans, dir_inum, &dir_hash,
+       ret =   bch2_dirent_create(trans, dir_inum, &dir_hash,
                                  mode_to_type(inode_u->bi_mode),
                                  name, inum, BCH_HASH_SET_MUST_CREATE) ?:
                bch2_inode_write(trans, dir_iter, dir_u) ?:
                bch2_inode_write(trans, inode_iter, inode_u);
+err:
+       bch2_trans_iter_put(trans, dir_iter);
+       bch2_trans_iter_put(trans, inode_iter);
+       return ret;
 }
 
 int bch2_unlink_trans(struct btree_trans *trans,
@@ -111,39 +119,49 @@ int bch2_unlink_trans(struct btree_trans *trans,
                      struct bch_inode_unpacked *inode_u,
                      const struct qstr *name)
 {
-       struct btree_iter *dir_iter, *dirent_iter, *inode_iter;
+       struct btree_iter *dir_iter = NULL, *dirent_iter = NULL,
+                         *inode_iter = NULL;
        struct bch_hash_info dir_hash;
        u64 inum, now = bch2_current_time(trans->c);
        struct bkey_s_c k;
+       int ret;
 
        dir_iter = bch2_inode_peek(trans, dir_u, dir_inum, BTREE_ITER_INTENT);
-       if (IS_ERR(dir_iter))
-               return PTR_ERR(dir_iter);
+       ret = PTR_ERR_OR_ZERO(dir_iter);
+       if (ret)
+               goto err;
 
        dir_hash = bch2_hash_info_init(trans->c, dir_u);
 
        dirent_iter = __bch2_dirent_lookup_trans(trans, dir_inum, &dir_hash,
                                                 name, BTREE_ITER_INTENT);
-       if (IS_ERR(dirent_iter))
-               return PTR_ERR(dirent_iter);
+       ret = PTR_ERR_OR_ZERO(dirent_iter);
+       if (ret)
+               goto err;
 
        k = bch2_btree_iter_peek_slot(dirent_iter);
        inum = le64_to_cpu(bkey_s_c_to_dirent(k).v->d_inum);
 
        inode_iter = bch2_inode_peek(trans, inode_u, inum, BTREE_ITER_INTENT);
-       if (IS_ERR(inode_iter))
-               return PTR_ERR(inode_iter);
+       ret = PTR_ERR_OR_ZERO(inode_iter);
+       if (ret)
+               goto err;
 
        dir_u->bi_mtime = dir_u->bi_ctime = inode_u->bi_ctime = now;
        dir_u->bi_nlink -= S_ISDIR(inode_u->bi_mode);
        bch2_inode_nlink_dec(inode_u);
 
-       return  (S_ISDIR(inode_u->bi_mode)
+       ret =   (S_ISDIR(inode_u->bi_mode)
                 ? bch2_empty_dir_trans(trans, inum)
                 : 0) ?:
                bch2_dirent_delete_at(trans, &dir_hash, dirent_iter) ?:
                bch2_inode_write(trans, dir_iter, dir_u) ?:
                bch2_inode_write(trans, inode_iter, inode_u);
+err:
+       bch2_trans_iter_put(trans, inode_iter);
+       bch2_trans_iter_put(trans, dirent_iter);
+       bch2_trans_iter_put(trans, dir_iter);
+       return ret;
 }
 
 bool bch2_reinherit_attrs(struct bch_inode_unpacked *dst_u,
@@ -179,24 +197,26 @@ int bch2_rename_trans(struct btree_trans *trans,
                      const struct qstr *dst_name,
                      enum bch_rename_mode mode)
 {
-       struct btree_iter *src_dir_iter, *dst_dir_iter = NULL;
-       struct btree_iter *src_inode_iter, *dst_inode_iter = NULL;
+       struct btree_iter *src_dir_iter = NULL, *dst_dir_iter = NULL;
+       struct btree_iter *src_inode_iter = NULL, *dst_inode_iter = NULL;
        struct bch_hash_info src_hash, dst_hash;
        u64 src_inode, dst_inode, now = bch2_current_time(trans->c);
        int ret;
 
        src_dir_iter = bch2_inode_peek(trans, src_dir_u, src_dir,
                                       BTREE_ITER_INTENT);
-       if (IS_ERR(src_dir_iter))
-               return PTR_ERR(src_dir_iter);
+       ret = PTR_ERR_OR_ZERO(src_dir_iter);
+       if (ret)
+               goto err;
 
        src_hash = bch2_hash_info_init(trans->c, src_dir_u);
 
        if (dst_dir != src_dir) {
                dst_dir_iter = bch2_inode_peek(trans, dst_dir_u, dst_dir,
                                               BTREE_ITER_INTENT);
-               if (IS_ERR(dst_dir_iter))
-                       return PTR_ERR(dst_dir_iter);
+               ret = PTR_ERR_OR_ZERO(dst_dir_iter);
+               if (ret)
+                       goto err;
 
                dst_hash = bch2_hash_info_init(trans->c, dst_dir_u);
        } else {
@@ -211,38 +231,48 @@ int bch2_rename_trans(struct btree_trans *trans,
                                 dst_name, &dst_inode,
                                 mode);
        if (ret)
-               return ret;
+               goto err;
 
        src_inode_iter = bch2_inode_peek(trans, src_inode_u, src_inode,
                                         BTREE_ITER_INTENT);
-       if (IS_ERR(src_inode_iter))
-               return PTR_ERR(src_inode_iter);
+       ret = PTR_ERR_OR_ZERO(src_inode_iter);
+       if (ret)
+               goto err;
 
        if (dst_inode) {
                dst_inode_iter = bch2_inode_peek(trans, dst_inode_u, dst_inode,
                                                 BTREE_ITER_INTENT);
-               if (IS_ERR(dst_inode_iter))
-                       return PTR_ERR(dst_inode_iter);
+               ret = PTR_ERR_OR_ZERO(dst_inode_iter);
+               if (ret)
+                       goto err;
        }
 
        if (mode == BCH_RENAME_OVERWRITE) {
                if (S_ISDIR(src_inode_u->bi_mode) !=
-                   S_ISDIR(dst_inode_u->bi_mode))
-                       return -ENOTDIR;
+                   S_ISDIR(dst_inode_u->bi_mode)) {
+                       ret = -ENOTDIR;
+                       goto err;
+               }
 
                if (S_ISDIR(dst_inode_u->bi_mode) &&
-                   bch2_empty_dir_trans(trans, dst_inode))
-                       return -ENOTEMPTY;
+                   bch2_empty_dir_trans(trans, dst_inode)) {
+                       ret = -ENOTEMPTY;
+                       goto err;
+               }
        }
 
        if (bch2_reinherit_attrs(src_inode_u, dst_dir_u) &&
-           S_ISDIR(src_inode_u->bi_mode))
-               return -EXDEV;
+           S_ISDIR(src_inode_u->bi_mode)) {
+               ret = -EXDEV;
+               goto err;
+       }
 
        if (mode == BCH_RENAME_EXCHANGE &&
            bch2_reinherit_attrs(dst_inode_u, src_dir_u) &&
-           S_ISDIR(dst_inode_u->bi_mode))
-               return -EXDEV;
+           S_ISDIR(dst_inode_u->bi_mode)) {
+               ret = -EXDEV;
+               goto err;
+       }
 
        if (S_ISDIR(src_inode_u->bi_mode)) {
                src_dir_u->bi_nlink--;
@@ -270,7 +300,7 @@ int bch2_rename_trans(struct btree_trans *trans,
        if (dst_inode)
                dst_inode_u->bi_ctime   = now;
 
-       return  bch2_inode_write(trans, src_dir_iter, src_dir_u) ?:
+       ret =   bch2_inode_write(trans, src_dir_iter, src_dir_u) ?:
                (src_dir != dst_dir
                 ? bch2_inode_write(trans, dst_dir_iter, dst_dir_u)
                 : 0 ) ?:
@@ -278,4 +308,10 @@ int bch2_rename_trans(struct btree_trans *trans,
                (dst_inode
                 ? bch2_inode_write(trans, dst_inode_iter, dst_inode_u)
                 : 0 );
+err:
+       bch2_trans_iter_put(trans, dst_inode_iter);
+       bch2_trans_iter_put(trans, src_inode_iter);
+       bch2_trans_iter_put(trans, dst_dir_iter);
+       bch2_trans_iter_put(trans, src_dir_iter);
+       return ret;
 }

diff --git a/fs/bcachefs/inode.c b/fs/bcachefs/inode.c
index bd44ef3842cb26dbb7d2950f4a911dfcb958adba..c40ff6fc7ae29b7f388dc843e372612971ccbdd3 100644 (file)
--- a/fs/bcachefs/inode.c
+++ b/fs/bcachefs/inode.c
@@ -362,16 +362,16 @@ int bch2_inode_create(struct btree_trans *trans,
                      struct bch_inode_unpacked *inode_u,
                      u64 min, u64 max, u64 *hint)
 {
-       struct bch_fs *c = trans->c;
        struct bkey_inode_buf *inode_p;
-       struct btree_iter *iter;
+       struct btree_iter *iter = NULL;
+       struct bkey_s_c k;
        u64 start;
        int ret;
 
        if (!max)
                max = ULLONG_MAX;
 
-       if (c->opts.inodes_32bit)
+       if (trans->c->opts.inodes_32bit)
                max = min_t(u64, max, U32_MAX);
 
        start = READ_ONCE(*hint);
@@ -382,48 +382,37 @@ int bch2_inode_create(struct btree_trans *trans,
        inode_p = bch2_trans_kmalloc(trans, sizeof(*inode_p));
        if (IS_ERR(inode_p))
                return PTR_ERR(inode_p);
-
-       iter = bch2_trans_get_iter(trans,
-                       BTREE_ID_INODES, POS(start, 0),
-                       BTREE_ITER_SLOTS|BTREE_ITER_INTENT);
-       if (IS_ERR(iter))
-               return PTR_ERR(iter);
 again:
-       while (1) {
-               struct bkey_s_c k = bch2_btree_iter_peek_slot(iter);
-
-               ret = bkey_err(k);
-               if (ret)
-                       return ret;
+       for_each_btree_key(trans, iter, BTREE_ID_INODES, POS(start, 0),
+                          BTREE_ITER_SLOTS|BTREE_ITER_INTENT, k, ret) {
+               if (iter->pos.inode > max)
+                       break;
 
-               switch (k.k->type) {
-               case KEY_TYPE_inode:
-                       /* slot used */
-                       if (iter->pos.inode >= max)
-                               goto out;
+               if (k.k->type != KEY_TYPE_inode)
+                       goto found_slot;
+       }
 
-                       bch2_btree_iter_next_slot(iter);
-                       break;
+       bch2_trans_iter_put(trans, iter);
 
-               default:
-                       *hint                   = k.k->p.inode;
-                       inode_u->bi_inum        = k.k->p.inode;
-                       inode_u->bi_generation  = bkey_generation(k);
+       if (ret)
+               return ret;
 
-                       bch2_inode_pack(inode_p, inode_u);
-                       bch2_trans_update(trans, iter, &inode_p->inode.k_i, 0);
-                       return 0;
-               }
-       }
-out:
        if (start != min) {
                /* Retry from start */
                start = min;
-               bch2_btree_iter_set_pos(iter, POS(start, 0));
                goto again;
        }
 
        return -ENOSPC;
+found_slot:
+       *hint                   = k.k->p.inode;
+       inode_u->bi_inum        = k.k->p.inode;
+       inode_u->bi_generation  = bkey_generation(k);
+
+       bch2_inode_pack(inode_p, inode_u);
+       bch2_trans_update(trans, iter, &inode_p->inode.k_i, 0);
+       bch2_trans_iter_put(trans, iter);
+       return 0;
 }
 
 int bch2_inode_rm(struct bch_fs *c, u64 inode_nr)
@@ -518,14 +507,13 @@ int bch2_inode_find_by_inum_trans(struct btree_trans *trans, u64 inode_nr,
        k = bch2_btree_iter_peek_slot(iter);
        ret = bkey_err(k);
        if (ret)
-               return ret;
+               goto err;
 
        ret = k.k->type == KEY_TYPE_inode
                ? bch2_inode_unpack(bkey_s_c_to_inode(k), inode)
                : -ENOENT;
-
+err:
        bch2_trans_iter_put(trans, iter);
-
        return ret;
 }
 

diff --git a/fs/bcachefs/reflink.c b/fs/bcachefs/reflink.c
index 3b8c74ca3725fc1d294c9179bd021e844e6eedd6..d78a3d5f72465d32da21df13b21dab6af04b766c 100644 (file)
--- a/fs/bcachefs/reflink.c
+++ b/fs/bcachefs/reflink.c
@@ -128,10 +128,9 @@ static int bch2_make_extent_indirect(struct btree_trans *trans,
 
        bch2_trans_update(trans, extent_iter, &r_p->k_i, 0);
 err:
-       if (!IS_ERR(reflink_iter)) {
+       if (!IS_ERR(reflink_iter))
                c->reflink_hint = reflink_iter->pos.offset;
-               bch2_trans_iter_put(trans, reflink_iter);
-       }
+       bch2_trans_iter_put(trans, reflink_iter);
 
        return ret;
 }

diff --git a/fs/bcachefs/str_hash.h b/fs/bcachefs/str_hash.h
index cf6ecd963a7bec793d49299a6d465c5179679879..0710d0bbe36d993713d5eeb0f2421fbd51747720 100644 (file)
--- a/fs/bcachefs/str_hash.h
+++ b/fs/bcachefs/str_hash.h
@@ -262,10 +262,8 @@ int bch2_hash_set(struct btree_trans *trans,
        if (!ret)
                ret = -ENOSPC;
 out:
-       if (!IS_ERR_OR_NULL(slot))
-               bch2_trans_iter_put(trans, slot);
-       if (!IS_ERR_OR_NULL(iter))
-               bch2_trans_iter_put(trans, iter);
+       bch2_trans_iter_put(trans, slot);
+       bch2_trans_iter_put(trans, iter);
 
        return ret;
 found: