From: Dan Carpenter <dan.carpenter@linaro.org>
To: djwong@kernel.org
Cc: "Darrick J. Wong" <djwong@kernel.org>, linux-xfs@vger.kernel.org
Subject: [bug report] xfs: repair free space btrees
Date: Thu, 4 Apr 2024 10:32:32 +0300 [thread overview]
Message-ID: <4e8b2fc3-838a-458e-b306-2f8a0062ba76@moroto.mountain> (raw)
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
next reply other threads:[~2024-04-04 7:32 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-04 7:32 Dan Carpenter [this message]
2024-04-04 20:55 ` [bug report] xfs: repair free space btrees Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4e8b2fc3-838a-458e-b306-2f8a0062ba76@moroto.mountain \
--to=dan.carpenter@linaro.org \
--cc=djwong@kernel.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).