Skip to content

Commit f9035b0

Browse files
author
Kent Overstreet
committed
bcachefs: Fix refcount leak in check_fix_ptrs()
fsck_err() does a goto fsck_err on error; factor out check_fix_ptr() so that our error label can drop our device ref. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
1 parent bf2b356 commit f9035b0

File tree

1 file changed

+133
-116
lines changed

1 file changed

+133
-116
lines changed

‎fs/bcachefs/buckets.c‎

Lines changed: 133 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -465,143 +465,161 @@ int bch2_update_cached_sectors_list(struct btree_trans *trans, unsigned dev, s64
465465
return bch2_update_replicas_list(trans, &r.e, sectors);
466466
}
467467

468-
int bch2_check_fix_ptrs(struct btree_trans *trans,
469-
enum btree_id btree, unsigned level, struct bkey_s_c k,
470-
enum btree_iter_update_trigger_flags flags)
468+
static int bch2_check_fix_ptr(struct btree_trans *trans,
469+
struct bkey_s_c k,
470+
struct extent_ptr_decoded p,
471+
const union bch_extent_entry *entry,
472+
bool *do_update)
471473
{
472474
struct bch_fs *c = trans->c;
473-
struct bkey_ptrs_c ptrs_c = bch2_bkey_ptrs_c(k);
474-
const union bch_extent_entry *entry_c;
475-
struct extent_ptr_decoded p = { 0 };
476-
bool do_update = false;
477475
struct printbuf buf = PRINTBUF;
478476
int ret = 0;
479477

480-
percpu_down_read(&c->mark_lock);
478+
struct bch_dev *ca = bch2_dev_tryget(c, p.ptr.dev);
479+
if (!ca) {
480+
if (fsck_err(c, ptr_to_invalid_device,
481+
"pointer to missing device %u\n"
482+
"while marking %s",
483+
p.ptr.dev,
484+
(printbuf_reset(&buf),
485+
bch2_bkey_val_to_text(&buf, c, k), buf.buf)))
486+
*do_update = true;
487+
return 0;
488+
}
481489

482-
bkey_for_each_ptr_decode(k.k, ptrs_c, p, entry_c) {
483-
struct bch_dev *ca = bch2_dev_tryget(c, p.ptr.dev);
484-
if (!ca) {
485-
if (fsck_err(c, ptr_to_invalid_device,
486-
"pointer to missing device %u\n"
487-
"while marking %s",
488-
p.ptr.dev,
489-
(printbuf_reset(&buf),
490-
bch2_bkey_val_to_text(&buf, c, k), buf.buf)))
491-
do_update = true;
492-
continue;
493-
}
490+
struct bucket *g = PTR_GC_BUCKET(ca, &p.ptr);
491+
enum bch_data_type data_type = bch2_bkey_ptr_data_type(k, p, entry);
494492

495-
struct bucket *g = PTR_GC_BUCKET(ca, &p.ptr);
496-
enum bch_data_type data_type = bch2_bkey_ptr_data_type(k, p, entry_c);
493+
if (fsck_err_on(!g->gen_valid,
494+
c, ptr_to_missing_alloc_key,
495+
"bucket %u:%zu data type %s ptr gen %u missing in alloc btree\n"
496+
"while marking %s",
497+
p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
498+
bch2_data_type_str(ptr_data_type(k.k, &p.ptr)),
499+
p.ptr.gen,
500+
(printbuf_reset(&buf),
501+
bch2_bkey_val_to_text(&buf, c, k), buf.buf))) {
502+
if (!p.ptr.cached) {
503+
g->gen_valid = true;
504+
g->gen = p.ptr.gen;
505+
} else {
506+
*do_update = true;
507+
}
508+
}
497509

498-
if (fsck_err_on(!g->gen_valid,
499-
c, ptr_to_missing_alloc_key,
500-
"bucket %u:%zu data type %s ptr gen %u missing in alloc btree\n"
501-
"while marking %s",
502-
p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
503-
bch2_data_type_str(ptr_data_type(k.k, &p.ptr)),
504-
p.ptr.gen,
505-
(printbuf_reset(&buf),
506-
bch2_bkey_val_to_text(&buf, c, k), buf.buf))) {
507-
if (!p.ptr.cached) {
508-
g->gen_valid = true;
509-
g->gen = p.ptr.gen;
510-
} else {
511-
do_update = true;
512-
}
510+
if (fsck_err_on(gen_cmp(p.ptr.gen, g->gen) > 0,
511+
c, ptr_gen_newer_than_bucket_gen,
512+
"bucket %u:%zu data type %s ptr gen in the future: %u > %u\n"
513+
"while marking %s",
514+
p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
515+
bch2_data_type_str(ptr_data_type(k.k, &p.ptr)),
516+
p.ptr.gen, g->gen,
517+
(printbuf_reset(&buf),
518+
bch2_bkey_val_to_text(&buf, c, k), buf.buf))) {
519+
if (!p.ptr.cached &&
520+
(g->data_type != BCH_DATA_btree ||
521+
data_type == BCH_DATA_btree)) {
522+
g->gen_valid = true;
523+
g->gen = p.ptr.gen;
524+
g->data_type = 0;
525+
g->dirty_sectors = 0;
526+
g->cached_sectors = 0;
527+
} else {
528+
*do_update = true;
513529
}
530+
}
514531

515-
if (fsck_err_on(gen_cmp(p.ptr.gen, g->gen) > 0,
516-
c, ptr_gen_newer_than_bucket_gen,
517-
"bucket %u:%zu data type %s ptr gen in the future: %u > %u\n"
518-
"while marking %s",
519-
p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
520-
bch2_data_type_str(ptr_data_type(k.k, &p.ptr)),
521-
p.ptr.gen, g->gen,
522-
(printbuf_reset(&buf),
523-
bch2_bkey_val_to_text(&buf, c, k), buf.buf))) {
524-
if (!p.ptr.cached &&
525-
(g->data_type != BCH_DATA_btree ||
526-
data_type == BCH_DATA_btree)) {
527-
g->gen_valid = true;
528-
g->gen = p.ptr.gen;
529-
g->data_type = 0;
530-
g->dirty_sectors = 0;
531-
g->cached_sectors = 0;
532-
} else {
533-
do_update = true;
534-
}
532+
if (fsck_err_on(gen_cmp(g->gen, p.ptr.gen) > BUCKET_GC_GEN_MAX,
533+
c, ptr_gen_newer_than_bucket_gen,
534+
"bucket %u:%zu gen %u data type %s: ptr gen %u too stale\n"
535+
"while marking %s",
536+
p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr), g->gen,
537+
bch2_data_type_str(ptr_data_type(k.k, &p.ptr)),
538+
p.ptr.gen,
539+
(printbuf_reset(&buf),
540+
bch2_bkey_val_to_text(&buf, c, k), buf.buf)))
541+
*do_update = true;
542+
543+
if (fsck_err_on(!p.ptr.cached && gen_cmp(p.ptr.gen, g->gen) < 0,
544+
c, stale_dirty_ptr,
545+
"bucket %u:%zu data type %s stale dirty ptr: %u < %u\n"
546+
"while marking %s",
547+
p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
548+
bch2_data_type_str(ptr_data_type(k.k, &p.ptr)),
549+
p.ptr.gen, g->gen,
550+
(printbuf_reset(&buf),
551+
bch2_bkey_val_to_text(&buf, c, k), buf.buf)))
552+
*do_update = true;
553+
554+
if (data_type != BCH_DATA_btree && p.ptr.gen != g->gen)
555+
goto out;
556+
557+
if (fsck_err_on(bucket_data_type_mismatch(g->data_type, data_type),
558+
c, ptr_bucket_data_type_mismatch,
559+
"bucket %u:%zu gen %u different types of data in same bucket: %s, %s\n"
560+
"while marking %s",
561+
p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr), g->gen,
562+
bch2_data_type_str(g->data_type),
563+
bch2_data_type_str(data_type),
564+
(printbuf_reset(&buf),
565+
bch2_bkey_val_to_text(&buf, c, k), buf.buf))) {
566+
if (data_type == BCH_DATA_btree) {
567+
g->gen_valid = true;
568+
g->gen = p.ptr.gen;
569+
g->data_type = data_type;
570+
g->dirty_sectors = 0;
571+
g->cached_sectors = 0;
572+
} else {
573+
*do_update = true;
535574
}
575+
}
536576

537-
if (fsck_err_on(gen_cmp(g->gen, p.ptr.gen) > BUCKET_GC_GEN_MAX,
538-
c, ptr_gen_newer_than_bucket_gen,
539-
"bucket %u:%zu gen %u data type %s: ptr gen %u too stale\n"
577+
if (p.has_ec) {
578+
struct gc_stripe *m = genradix_ptr(&c->gc_stripes, p.ec.idx);
579+
580+
if (fsck_err_on(!m || !m->alive, c,
581+
ptr_to_missing_stripe,
582+
"pointer to nonexistent stripe %llu\n"
540583
"while marking %s",
541-
p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr), g->gen,
542-
bch2_data_type_str(ptr_data_type(k.k, &p.ptr)),
543-
p.ptr.gen,
584+
(u64) p.ec.idx,
544585
(printbuf_reset(&buf),
545586
bch2_bkey_val_to_text(&buf, c, k), buf.buf)))
546-
do_update = true;
587+
*do_update = true;
547588

548-
if (fsck_err_on(!p.ptr.cached && gen_cmp(p.ptr.gen, g->gen) < 0,
549-
c, stale_dirty_ptr,
550-
"bucket %u:%zu data type %s stale dirty ptr: %u < %u\n"
589+
if (fsck_err_on(m && m->alive && !bch2_ptr_matches_stripe_m(m, p), c,
590+
ptr_to_incorrect_stripe,
591+
"pointer does not match stripe %llu\n"
551592
"while marking %s",
552-
p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
553-
bch2_data_type_str(ptr_data_type(k.k, &p.ptr)),
554-
p.ptr.gen, g->gen,
593+
(u64) p.ec.idx,
555594
(printbuf_reset(&buf),
556595
bch2_bkey_val_to_text(&buf, c, k), buf.buf)))
557-
do_update = true;
596+
*do_update = true;
597+
}
598+
out:
599+
fsck_err:
600+
bch2_dev_put(ca);
601+
printbuf_exit(&buf);
602+
return ret;
603+
}
558604

559-
if (data_type != BCH_DATA_btree && p.ptr.gen != g->gen)
560-
goto next;
605+
int bch2_check_fix_ptrs(struct btree_trans *trans,
606+
enum btree_id btree, unsigned level, struct bkey_s_c k,
607+
enum btree_iter_update_trigger_flags flags)
608+
{
609+
struct bch_fs *c = trans->c;
610+
struct bkey_ptrs_c ptrs_c = bch2_bkey_ptrs_c(k);
611+
const union bch_extent_entry *entry_c;
612+
struct extent_ptr_decoded p = { 0 };
613+
bool do_update = false;
614+
struct printbuf buf = PRINTBUF;
615+
int ret = 0;
561616

562-
if (fsck_err_on(bucket_data_type_mismatch(g->data_type, data_type),
563-
c, ptr_bucket_data_type_mismatch,
564-
"bucket %u:%zu gen %u different types of data in same bucket: %s, %s\n"
565-
"while marking %s",
566-
p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr), g->gen,
567-
bch2_data_type_str(g->data_type),
568-
bch2_data_type_str(data_type),
569-
(printbuf_reset(&buf),
570-
bch2_bkey_val_to_text(&buf, c, k), buf.buf))) {
571-
if (data_type == BCH_DATA_btree) {
572-
g->gen_valid = true;
573-
g->gen = p.ptr.gen;
574-
g->data_type = data_type;
575-
g->dirty_sectors = 0;
576-
g->cached_sectors = 0;
577-
} else {
578-
do_update = true;
579-
}
580-
}
617+
percpu_down_read(&c->mark_lock);
581618

582-
if (p.has_ec) {
583-
struct gc_stripe *m = genradix_ptr(&c->gc_stripes, p.ec.idx);
584-
585-
if (fsck_err_on(!m || !m->alive, c,
586-
ptr_to_missing_stripe,
587-
"pointer to nonexistent stripe %llu\n"
588-
"while marking %s",
589-
(u64) p.ec.idx,
590-
(printbuf_reset(&buf),
591-
bch2_bkey_val_to_text(&buf, c, k), buf.buf)))
592-
do_update = true;
593-
594-
if (fsck_err_on(m && m->alive && !bch2_ptr_matches_stripe_m(m, p), c,
595-
ptr_to_incorrect_stripe,
596-
"pointer does not match stripe %llu\n"
597-
"while marking %s",
598-
(u64) p.ec.idx,
599-
(printbuf_reset(&buf),
600-
bch2_bkey_val_to_text(&buf, c, k), buf.buf)))
601-
do_update = true;
602-
}
603-
next:
604-
bch2_dev_put(ca);
619+
bkey_for_each_ptr_decode(k.k, ptrs_c, p, entry_c) {
620+
ret = bch2_check_fix_ptr(trans, k, p, entry_c, &do_update);
621+
if (ret)
622+
goto err;
605623
}
606624

607625
if (do_update) {
@@ -716,7 +734,6 @@ int bch2_check_fix_ptrs(struct btree_trans *trans,
716734
bch2_btree_node_update_key_early(trans, btree, level - 1, k, new);
717735
}
718736
err:
719-
fsck_err:
720737
percpu_up_read(&c->mark_lock);
721738
printbuf_exit(&buf);
722739
return ret;

0 commit comments

Comments
 (0)