[bug report] xfs: repair free space btrees

Dan Carpenter
2024-04-04T07:32:38Z

Hello Darrick J. Wong,

Commit 4bdfd7d15747 ("xfs: repair free space btrees") from Dec 15,
2023 (linux-next), leads to the following Smatch static checker
warning:

fs/xfs/scrub/alloc_repair.c:781 xrep_abt_build_new_trees()
warn: missing unwind goto?

fs/xfs/scrub/alloc_repair.c
702 STATIC int
703 xrep_abt_build_new_trees(
704 struct xrep_abt *ra)
705 {
706 struct xfs_scrub *sc = ra->sc;
707 struct xfs_btree_cur *bno_cur;
708 struct xfs_btree_cur *cnt_cur;
709 struct xfs_perag *pag = sc->sa.pag;
710 bool needs_resort = false;
711 int error;
712
713 /*
714 * Sort the free extents by length so that we can set up the free space
715 * btrees in as few extents as possible. This reduces the amount of
716 * deferred rmap / free work we have to do at the end.
717 */
718 error = xrep_cntbt_sort_records(ra, false);
719 if (error)
720 return error;
721
722 /*
723 * Prepare to construct the new btree by reserving disk space for the
724 * new btree and setting up all the accounting information we'll need
725 * to root the new btree while it's under construction and before we
726 * attach it to the AG header.
727 */
728 xrep_newbt_init_bare(&ra->new_bnobt, sc);
729 xrep_newbt_init_bare(&ra->new_cntbt, sc);
730
731 ra->new_bnobt.bload.get_records = xrep_abt_get_records;
732 ra->new_cntbt.bload.get_records = xrep_abt_get_records;
733
734 ra->new_bnobt.bload.claim_block = xrep_abt_claim_block;
735 ra->new_cntbt.bload.claim_block = xrep_abt_claim_block;
736
737 /* Allocate cursors for the staged btrees. */
738 bno_cur = xfs_bnobt_init_cursor(sc->mp, NULL, NULL, pag);
739 xfs_btree_stage_afakeroot(bno_cur, &ra->new_bnobt.afake);
740
741 cnt_cur = xfs_cntbt_init_cursor(sc->mp, NULL, NULL, pag);
742 xfs_btree_stage_afakeroot(cnt_cur, &ra->new_cntbt.afake);
743
744 /* Last chance to abort before we start committing fixes. */
745 if (xchk_should_terminate(sc, &error))
746 goto err_cur;
747
748 /* Reserve the space we'll need for the new btrees. */
749 error = xrep_abt_reserve_space(ra, bno_cur, cnt_cur, &needs_resort);
750 if (error)
751 goto err_cur;
752
753 /*
754 * If we need to re-sort the free extents by length, do so so that we
755 * can put the records into the cntbt in the correct order.
756 */
757 if (needs_resort) {
758 error = xrep_cntbt_sort_records(ra, needs_resort);
759 if (error)
760 goto err_cur;
761 }
762
763 /*
764 * Due to btree slack factors, it's possible for a new btree to be one
765 * level taller than the old btree. Update the alternate incore btree
766 * height so that we don't trip the verifiers when writing the new
767 * btree blocks to disk.
768 */
769 pag->pagf_repair_bno_level = ra->new_bnobt.bload.btree_height;
770 pag->pagf_repair_cnt_level = ra->new_cntbt.bload.btree_height;
771
772 /* Load the free space by length tree. */
773 ra->array_cur = XFARRAY_CURSOR_INIT;
774 ra->longest = 0;
775 error = xfs_btree_bload(cnt_cur, &ra->new_cntbt.bload, ra);
776 if (error)
777 goto err_levels;
^^^^^^^^^^^^^^^^
778
779 error = xrep_bnobt_sort_records(ra);
780 if (error)
--> 781 return error;
^^^^^^^^^^^^^
Should this be a goto err_levels?

782
783 /* Load the free space by block number tree. */
784 ra->array_cur = XFARRAY_CURSOR_INIT;
785 error = xfs_btree_bload(bno_cur, &ra->new_bnobt.bload, ra);
786 if (error)
787 goto err_levels;
788
789 /*
790 * Install the new btrees in the AG header. After this point the old
791 * btrees are no longer accessible and the new trees are live.
792 */
793 xfs_allocbt_commit_staged_btree(bno_cur, sc->tp, sc->sa.agf_bp);
794 xfs_btree_del_cursor(bno_cur, 0);
795 xfs_allocbt_commit_staged_btree(cnt_cur, sc->tp, sc->sa.agf_bp);
796 xfs_btree_del_cursor(cnt_cur, 0);
797
798 /* Reset the AGF counters now that we've changed the btree shape. */
799 error = xrep_abt_reset_counters(ra);
800 if (error)
801 goto err_newbt;
802
803 /* Dispose of any unused blocks and the accounting information. */
804 xrep_abt_dispose_reservations(ra, error);
805
806 return xrep_roll_ag_trans(sc);
807
808 err_levels:
809 pag->pagf_repair_bno_level = 0;
810 pag->pagf_repair_cnt_level = 0;
811 err_cur:
812 xfs_btree_del_cursor(cnt_cur, error);
813 xfs_btree_del_cursor(bno_cur, error);
814 err_newbt:
815 xrep_abt_dispose_reservations(ra, error);
816 return error;
817 }

regards,
dan carpenter