Skip to content

Commit 3c5d0b7

Browse files
author
Kent Overstreet
committed
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 <[email protected]>
1 parent 1dceae4 commit 3c5d0b7

File tree

3 files changed

+44
-21
lines changed

3 files changed

+44
-21
lines changed

fs/bcachefs/btree_cache.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,16 @@ struct btree *__bch2_btree_node_mem_alloc(struct bch_fs *c)
159159
return b;
160160
}
161161

162+
void bch2_btree_node_to_freelist(struct bch_fs *c, struct btree *b)
163+
{
164+
mutex_lock(&c->btree_cache.lock);
165+
list_move(&b->list, &c->btree_cache.freeable);
166+
mutex_unlock(&c->btree_cache.lock);
167+
168+
six_unlock_write(&b->c.lock);
169+
six_unlock_intent(&b->c.lock);
170+
}
171+
162172
/* Btree in memory cache - hash table */
163173

164174
void bch2_btree_node_hash_remove(struct btree_cache *bc, struct btree *b)
@@ -736,6 +746,13 @@ struct btree *bch2_btree_node_mem_alloc(struct btree_trans *trans, bool pcpu_rea
736746
start_time);
737747

738748
memalloc_nofs_restore(flags);
749+
750+
int ret = bch2_trans_relock(trans);
751+
if (unlikely(ret)) {
752+
bch2_btree_node_to_freelist(c, b);
753+
return ERR_PTR(ret);
754+
}
755+
739756
return b;
740757
err:
741758
mutex_lock(&bc->lock);

fs/bcachefs/btree_cache.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ struct btree_iter;
1212

1313
void bch2_recalc_btree_reserve(struct bch_fs *);
1414

15+
void bch2_btree_node_to_freelist(struct bch_fs *, struct btree *);
16+
1517
void bch2_btree_node_hash_remove(struct btree_cache *, struct btree *);
1618
int __bch2_btree_node_hash_insert(struct btree_cache *, struct btree *);
1719
int bch2_btree_node_hash_insert(struct btree_cache *, struct btree *,

fs/bcachefs/btree_update_interior.c

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,12 @@ static struct btree *__bch2_btree_node_alloc(struct btree_trans *trans,
317317
: 0;
318318
int ret;
319319

320+
b = bch2_btree_node_mem_alloc(trans, interior_node);
321+
if (IS_ERR(b))
322+
return b;
323+
324+
BUG_ON(b->ob.nr);
325+
320326
mutex_lock(&c->btree_reserve_cache_lock);
321327
if (c->btree_reserve_cache_nr > nr_reserve) {
322328
struct btree_alloc *a =
@@ -325,10 +331,9 @@ static struct btree *__bch2_btree_node_alloc(struct btree_trans *trans,
325331
obs = a->ob;
326332
bkey_copy(&tmp.k, &a->k);
327333
mutex_unlock(&c->btree_reserve_cache_lock);
328-
goto mem_alloc;
334+
goto out;
329335
}
330336
mutex_unlock(&c->btree_reserve_cache_lock);
331-
332337
retry:
333338
ret = bch2_alloc_sectors_start_trans(trans,
334339
c->opts.metadata_target ?:
@@ -341,7 +346,7 @@ static struct btree *__bch2_btree_node_alloc(struct btree_trans *trans,
341346
c->opts.metadata_replicas_required),
342347
watermark, 0, cl, &wp);
343348
if (unlikely(ret))
344-
return ERR_PTR(ret);
349+
goto err;
345350

346351
if (wp->sectors_free < btree_sectors(c)) {
347352
struct open_bucket *ob;
@@ -360,19 +365,16 @@ static struct btree *__bch2_btree_node_alloc(struct btree_trans *trans,
360365

361366
bch2_open_bucket_get(c, wp, &obs);
362367
bch2_alloc_sectors_done(c, wp);
363-
mem_alloc:
364-
b = bch2_btree_node_mem_alloc(trans, interior_node);
365-
six_unlock_write(&b->c.lock);
366-
six_unlock_intent(&b->c.lock);
367-
368-
/* we hold cannibalize_lock: */
369-
BUG_ON(IS_ERR(b));
370-
BUG_ON(b->ob.nr);
371-
368+
out:
372369
bkey_copy(&b->key, &tmp.k);
373370
b->ob = obs;
371+
six_unlock_write(&b->c.lock);
372+
six_unlock_intent(&b->c.lock);
374373

375374
return b;
375+
err:
376+
bch2_btree_node_to_freelist(c, b);
377+
return ERR_PTR(ret);
376378
}
377379

378380
static struct btree *bch2_btree_node_alloc(struct btree_update *as,
@@ -2439,21 +2441,19 @@ int bch2_btree_node_update_key(struct btree_trans *trans, struct btree_iter *ite
24392441
}
24402442

24412443
new_hash = bch2_btree_node_mem_alloc(trans, false);
2444+
ret = PTR_ERR_OR_ZERO(new_hash);
2445+
if (ret)
2446+
goto err;
24422447
}
24432448

24442449
path->intent_ref++;
24452450
ret = __bch2_btree_node_update_key(trans, iter, b, new_hash, new_key,
24462451
commit_flags, skip_triggers);
24472452
--path->intent_ref;
24482453

2449-
if (new_hash) {
2450-
mutex_lock(&c->btree_cache.lock);
2451-
list_move(&new_hash->list, &c->btree_cache.freeable);
2452-
mutex_unlock(&c->btree_cache.lock);
2453-
2454-
six_unlock_write(&new_hash->c.lock);
2455-
six_unlock_intent(&new_hash->c.lock);
2456-
}
2454+
if (new_hash)
2455+
bch2_btree_node_to_freelist(c, new_hash);
2456+
err:
24572457
closure_sync(&cl);
24582458
bch2_btree_cache_cannibalize_unlock(trans);
24592459
return ret;
@@ -2522,6 +2522,10 @@ int bch2_btree_root_alloc_fake_trans(struct btree_trans *trans, enum btree_id id
25222522
b = bch2_btree_node_mem_alloc(trans, false);
25232523
bch2_btree_cache_cannibalize_unlock(trans);
25242524

2525+
ret = PTR_ERR_OR_ZERO(b);
2526+
if (ret)
2527+
return ret;
2528+
25252529
set_btree_node_fake(b);
25262530
set_btree_node_need_rewrite(b);
25272531
b->c.level = level;
@@ -2553,7 +2557,7 @@ int bch2_btree_root_alloc_fake_trans(struct btree_trans *trans, enum btree_id id
25532557

25542558
void bch2_btree_root_alloc_fake(struct bch_fs *c, enum btree_id id, unsigned level)
25552559
{
2556-
bch2_trans_run(c, bch2_btree_root_alloc_fake_trans(trans, id, level));
2560+
bch2_trans_run(c, lockrestart_do(trans, bch2_btree_root_alloc_fake_trans(trans, id, level)));
25572561
}
25582562

25592563
static void bch2_btree_update_to_text(struct printbuf *out, struct btree_update *as)

0 commit comments

Comments
 (0)