bcachefs: fix failure to relock in bch2_btree_node_mem_alloc()

We weren't always so strict about trans->locked state - but now we are,
and new assertions are shaking some bugs out.

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

diff --git a/fs/bcachefs/btree_cache.c b/fs/bcachefs/btree_cache.c
index cc778d7e769e7556ab7baf20514b5d5c6dfe9f93..063725ecb2b388ff1ace625ec2ff1204e8978fba 100644 (file)
--- a/fs/bcachefs/btree_cache.c
+++ b/fs/bcachefs/btree_cache.c
@@ -159,6 +159,16 @@ struct btree *__bch2_btree_node_mem_alloc(struct bch_fs *c)
        return b;
 }
 
+void bch2_btree_node_to_freelist(struct bch_fs *c, struct btree *b)
+{
+       mutex_lock(&c->btree_cache.lock);
+       list_move(&b->list, &c->btree_cache.freeable);
+       mutex_unlock(&c->btree_cache.lock);
+
+       six_unlock_write(&b->c.lock);
+       six_unlock_intent(&b->c.lock);
+}
+
 /* Btree in memory cache - hash table */
 
 void bch2_btree_node_hash_remove(struct btree_cache *bc, struct btree *b)
@@ -736,6 +746,13 @@ out:
                               start_time);
 
        memalloc_nofs_restore(flags);
+
+       int ret = bch2_trans_relock(trans);
+       if (unlikely(ret)) {
+               bch2_btree_node_to_freelist(c, b);
+               return ERR_PTR(ret);
+       }
+
        return b;
 err:
        mutex_lock(&bc->lock);

diff --git a/fs/bcachefs/btree_cache.h b/fs/bcachefs/btree_cache.h
index c0eb87a057ccb92c43e3c8c8e2a46e050fe5dc6a..f82064007127203e80033359ec4cd4010cccad79 100644 (file)
--- a/fs/bcachefs/btree_cache.h
+++ b/fs/bcachefs/btree_cache.h
@@ -12,6 +12,8 @@ struct btree_iter;
 
 void bch2_recalc_btree_reserve(struct bch_fs *);
 
+void bch2_btree_node_to_freelist(struct bch_fs *, struct btree *);
+
 void bch2_btree_node_hash_remove(struct btree_cache *, struct btree *);
 int __bch2_btree_node_hash_insert(struct btree_cache *, struct btree *);
 int bch2_btree_node_hash_insert(struct btree_cache *, struct btree *,

diff --git a/fs/bcachefs/btree_update_interior.c b/fs/bcachefs/btree_update_interior.c
index b3454d4619e8fb5366f114ecfa741a196c57c533..8fd112026e7a34d7cf4782d0e04c923c4a4a5081 100644 (file)
--- a/fs/bcachefs/btree_update_interior.c
+++ b/fs/bcachefs/btree_update_interior.c
@@ -317,6 +317,12 @@ static struct btree *__bch2_btree_node_alloc(struct btree_trans *trans,
                : 0;
        int ret;
 
+       b = bch2_btree_node_mem_alloc(trans, interior_node);
+       if (IS_ERR(b))
+               return b;
+
+       BUG_ON(b->ob.nr);
+
        mutex_lock(&c->btree_reserve_cache_lock);
        if (c->btree_reserve_cache_nr > nr_reserve) {
                struct btree_alloc *a =
@@ -325,10 +331,9 @@ static struct btree *__bch2_btree_node_alloc(struct btree_trans *trans,
                obs = a->ob;
                bkey_copy(&tmp.k, &a->k);
                mutex_unlock(&c->btree_reserve_cache_lock);
-               goto mem_alloc;
+               goto out;
        }
        mutex_unlock(&c->btree_reserve_cache_lock);
-
 retry:
        ret = bch2_alloc_sectors_start_trans(trans,
                                      c->opts.metadata_target ?:
@@ -341,7 +346,7 @@ retry:
                                          c->opts.metadata_replicas_required),
                                      watermark, 0, cl, &wp);
        if (unlikely(ret))
-               return ERR_PTR(ret);
+               goto err;
 
        if (wp->sectors_free < btree_sectors(c)) {
                struct open_bucket *ob;
@@ -360,19 +365,16 @@ retry:
 
        bch2_open_bucket_get(c, wp, &obs);
        bch2_alloc_sectors_done(c, wp);
-mem_alloc:
-       b = bch2_btree_node_mem_alloc(trans, interior_node);
-       six_unlock_write(&b->c.lock);
-       six_unlock_intent(&b->c.lock);
-
-       /* we hold cannibalize_lock: */
-       BUG_ON(IS_ERR(b));
-       BUG_ON(b->ob.nr);
-
+out:
        bkey_copy(&b->key, &tmp.k);
        b->ob = obs;
+       six_unlock_write(&b->c.lock);
+       six_unlock_intent(&b->c.lock);
 
        return b;
+err:
+       bch2_btree_node_to_freelist(c, b);
+       return ERR_PTR(ret);
 }
 
 static struct btree *bch2_btree_node_alloc(struct btree_update *as,
@@ -2439,6 +2441,9 @@ int bch2_btree_node_update_key(struct btree_trans *trans, struct btree_iter *ite
                }
 
                new_hash = bch2_btree_node_mem_alloc(trans, false);
+               ret = PTR_ERR_OR_ZERO(new_hash);
+               if (ret)
+                       goto err;
        }
 
        path->intent_ref++;
@@ -2446,14 +2451,9 @@ int bch2_btree_node_update_key(struct btree_trans *trans, struct btree_iter *ite
                                           commit_flags, skip_triggers);
        --path->intent_ref;
 
-       if (new_hash) {
-               mutex_lock(&c->btree_cache.lock);
-               list_move(&new_hash->list, &c->btree_cache.freeable);
-               mutex_unlock(&c->btree_cache.lock);
-
-               six_unlock_write(&new_hash->c.lock);
-               six_unlock_intent(&new_hash->c.lock);
-       }
+       if (new_hash)
+               bch2_btree_node_to_freelist(c, new_hash);
+err:
        closure_sync(&cl);
        bch2_btree_cache_cannibalize_unlock(trans);
        return ret;
@@ -2522,6 +2522,10 @@ int bch2_btree_root_alloc_fake_trans(struct btree_trans *trans, enum btree_id id
        b = bch2_btree_node_mem_alloc(trans, false);
        bch2_btree_cache_cannibalize_unlock(trans);
 
+       ret = PTR_ERR_OR_ZERO(b);
+       if (ret)
+               return ret;
+
        set_btree_node_fake(b);
        set_btree_node_need_rewrite(b);
        b->c.level      = level;
@@ -2553,7 +2557,7 @@ int bch2_btree_root_alloc_fake_trans(struct btree_trans *trans, enum btree_id id
 
 void bch2_btree_root_alloc_fake(struct bch_fs *c, enum btree_id id, unsigned level)
 {
-       bch2_trans_run(c, bch2_btree_root_alloc_fake_trans(trans, id, level));
+       bch2_trans_run(c, lockrestart_do(trans, bch2_btree_root_alloc_fake_trans(trans, id, level)));
 }
 
 static void bch2_btree_update_to_text(struct printbuf *out, struct btree_update *as)