Merge tag 'scrub-detect-inobt-gaps-6.4_2023-04-11' of git://git.kernel.org/pub/scm...
authorDave Chinner <david@fromorbit.com>
Thu, 13 Apr 2023 21:10:05 +0000 (07:10 +1000)
committerDave Chinner <dchinner@redhat.com>
Thu, 13 Apr 2023 21:10:05 +0000 (07:10 +1000)
xfs: detect incorrect gaps in inode btree [v24.5]

This series continues the corrections for a couple of problems I found
in the inode btree scrubber.  The first problem is that we don't
directly check the inobt records have a direct correspondence with the
finobt records, and vice versa.  The second problem occurs on
filesystems with sparse inode chunks -- the cross-referencing we do
detects sparseness, but it doesn't actually check the consistency
between the inobt hole records and the rmap data.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Dave Chinner <david@fromorbit.com>
fs/xfs/libxfs/xfs_ialloc.c
fs/xfs/libxfs/xfs_ialloc.h
fs/xfs/scrub/ialloc.c

index 0d2980accd3ce1be2b38cf9a964d2e588da3536e..a16d5de16933f4ebe0aff4ff787dae97cedc52ac 100644 (file)
@@ -1978,8 +1978,6 @@ xfs_difree_inobt(
         */
        if (!xfs_has_ikeep(mp) && rec.ir_free == XFS_INOBT_ALL_FREE &&
            mp->m_sb.sb_inopblock <= XFS_INODES_PER_CHUNK) {
-               struct xfs_perag        *pag = agbp->b_pag;
-
                xic->deleted = true;
                xic->first_ino = XFS_AGINO_TO_INO(mp, pag->pag_agno,
                                rec.ir_startino);
@@ -2643,44 +2641,50 @@ xfs_ialloc_read_agi(
        return 0;
 }
 
-/* Is there an inode record covering a given range of inode numbers? */
-int
-xfs_ialloc_has_inode_record(
-       struct xfs_btree_cur    *cur,
-       xfs_agino_t             low,
-       xfs_agino_t             high,
-       bool                    *exists)
+/* How many inodes are backed by inode clusters ondisk? */
+STATIC int
+xfs_ialloc_count_ondisk(
+       struct xfs_btree_cur            *cur,
+       xfs_agino_t                     low,
+       xfs_agino_t                     high,
+       unsigned int                    *allocated)
 {
        struct xfs_inobt_rec_incore     irec;
-       xfs_agino_t             agino;
-       uint16_t                holemask;
-       int                     has_record;
-       int                     i;
-       int                     error;
+       unsigned int                    ret = 0;
+       int                             has_record;
+       int                             error;
 
-       *exists = false;
        error = xfs_inobt_lookup(cur, low, XFS_LOOKUP_LE, &has_record);
-       while (error == 0 && has_record) {
+       if (error)
+               return error;
+
+       while (has_record) {
+               unsigned int            i, hole_idx;
+
                error = xfs_inobt_get_rec(cur, &irec, &has_record);
-               if (error || irec.ir_startino > high)
+               if (error)
+                       return error;
+               if (irec.ir_startino > high)
                        break;
 
-               agino = irec.ir_startino;
-               holemask = irec.ir_holemask;
-               for (i = 0; i < XFS_INOBT_HOLEMASK_BITS; holemask >>= 1,
-                               i++, agino += XFS_INODES_PER_HOLEMASK_BIT) {
-                       if (holemask & 1)
+               for (i = 0; i < XFS_INODES_PER_CHUNK; i++) {
+                       if (irec.ir_startino + i < low)
                                continue;
-                       if (agino + XFS_INODES_PER_HOLEMASK_BIT > low &&
-                                       agino <= high) {
-                               *exists = true;
-                               return 0;
-                       }
+                       if (irec.ir_startino + i > high)
+                               break;
+
+                       hole_idx = i / XFS_INODES_PER_HOLEMASK_BIT;
+                       if (!(irec.ir_holemask & (1U << hole_idx)))
+                               ret++;
                }
 
                error = xfs_btree_increment(cur, 0, &has_record);
+               if (error)
+                       return error;
        }
-       return error;
+
+       *allocated = ret;
+       return 0;
 }
 
 /* Is there an inode record covering a given extent? */
@@ -2689,15 +2693,27 @@ xfs_ialloc_has_inodes_at_extent(
        struct xfs_btree_cur    *cur,
        xfs_agblock_t           bno,
        xfs_extlen_t            len,
-       bool                    *exists)
+       enum xbtree_recpacking  *outcome)
 {
-       xfs_agino_t             low;
-       xfs_agino_t             high;
+       xfs_agino_t             agino;
+       xfs_agino_t             last_agino;
+       unsigned int            allocated;
+       int                     error;
 
-       low = XFS_AGB_TO_AGINO(cur->bc_mp, bno);
-       high = XFS_AGB_TO_AGINO(cur->bc_mp, bno + len) - 1;
+       agino = XFS_AGB_TO_AGINO(cur->bc_mp, bno);
+       last_agino = XFS_AGB_TO_AGINO(cur->bc_mp, bno + len) - 1;
 
-       return xfs_ialloc_has_inode_record(cur, low, high, exists);
+       error = xfs_ialloc_count_ondisk(cur, agino, last_agino, &allocated);
+       if (error)
+               return error;
+
+       if (allocated == 0)
+               *outcome = XBTREE_RECPACKING_EMPTY;
+       else if (allocated == last_agino - agino + 1)
+               *outcome = XBTREE_RECPACKING_FULL;
+       else
+               *outcome = XBTREE_RECPACKING_SPARSE;
+       return 0;
 }
 
 struct xfs_ialloc_count_inodes {
index 90b0e50793380d82b735ccd5884603941f846095..fe824bb04a091cf4781a4b595ff002f59b3a4f6c 100644 (file)
@@ -96,9 +96,8 @@ void xfs_inobt_btrec_to_irec(struct xfs_mount *mp,
 xfs_failaddr_t xfs_inobt_check_irec(struct xfs_btree_cur *cur,
                const struct xfs_inobt_rec_incore *irec);
 int xfs_ialloc_has_inodes_at_extent(struct xfs_btree_cur *cur,
-               xfs_agblock_t bno, xfs_extlen_t len, bool *exists);
-int xfs_ialloc_has_inode_record(struct xfs_btree_cur *cur, xfs_agino_t low,
-               xfs_agino_t high, bool *exists);
+               xfs_agblock_t bno, xfs_extlen_t len,
+               enum xbtree_recpacking *outcome);
 int xfs_ialloc_count_inodes(struct xfs_btree_cur *cur, xfs_agino_t *count,
                xfs_agino_t *freecount);
 int xfs_inobt_insert_rec(struct xfs_btree_cur *cur, uint16_t holemask,
index 6d08613db32f08bbb0cb13d3ec22def0d03972d0..fda96b5367301f09bf1d32007ea859f56e8707cf 100644 (file)
@@ -51,71 +51,234 @@ struct xchk_iallocbt {
 };
 
 /*
- * If we're checking the finobt, cross-reference with the inobt.
- * Otherwise we're checking the inobt; if there is an finobt, make sure
- * we have a record or not depending on freecount.
+ * Does the finobt have a record for this inode with the same hole/free state?
+ * This is a bit complicated because of the following:
+ *
+ * - The finobt need not have a record if all inodes in the inobt record are
+ *   allocated.
+ * - The finobt need not have a record if all inodes in the inobt record are
+ *   free.
+ * - The finobt need not have a record if the inobt record says this is a hole.
+ *   This likely doesn't happen in practice.
  */
-static inline void
-xchk_iallocbt_chunk_xref_other(
+STATIC int
+xchk_inobt_xref_finobt(
+       struct xfs_scrub        *sc,
+       struct xfs_inobt_rec_incore *irec,
+       xfs_agino_t             agino,
+       bool                    free,
+       bool                    hole)
+{
+       struct xfs_inobt_rec_incore frec;
+       struct xfs_btree_cur    *cur = sc->sa.fino_cur;
+       bool                    ffree, fhole;
+       unsigned int            frec_idx, fhole_idx;
+       int                     has_record;
+       int                     error;
+
+       ASSERT(cur->bc_btnum == XFS_BTNUM_FINO);
+
+       error = xfs_inobt_lookup(cur, agino, XFS_LOOKUP_LE, &has_record);
+       if (error)
+               return error;
+       if (!has_record)
+               goto no_record;
+
+       error = xfs_inobt_get_rec(cur, &frec, &has_record);
+       if (!has_record)
+               return -EFSCORRUPTED;
+
+       if (frec.ir_startino + XFS_INODES_PER_CHUNK <= agino)
+               goto no_record;
+
+       /* There's a finobt record; free and hole status must match. */
+       frec_idx = agino - frec.ir_startino;
+       ffree = frec.ir_free & (1ULL << frec_idx);
+       fhole_idx = frec_idx / XFS_INODES_PER_HOLEMASK_BIT;
+       fhole = frec.ir_holemask & (1U << fhole_idx);
+
+       if (ffree != free)
+               xchk_btree_xref_set_corrupt(sc, cur, 0);
+       if (fhole != hole)
+               xchk_btree_xref_set_corrupt(sc, cur, 0);
+       return 0;
+
+no_record:
+       /* inobt record is fully allocated */
+       if (irec->ir_free == 0)
+               return 0;
+
+       /* inobt record is totally unallocated */
+       if (irec->ir_free == XFS_INOBT_ALL_FREE)
+               return 0;
+
+       /* inobt record says this is a hole */
+       if (hole)
+               return 0;
+
+       /* finobt doesn't care about allocated inodes */
+       if (!free)
+               return 0;
+
+       xchk_btree_xref_set_corrupt(sc, cur, 0);
+       return 0;
+}
+
+/*
+ * Make sure that each inode of this part of an inobt record has the same
+ * sparse and free status as the finobt.
+ */
+STATIC void
+xchk_inobt_chunk_xref_finobt(
        struct xfs_scrub                *sc,
        struct xfs_inobt_rec_incore     *irec,
-       xfs_agino_t                     agino)
+       xfs_agino_t                     agino,
+       unsigned int                    nr_inodes)
 {
-       struct xfs_btree_cur            **pcur;
-       bool                            has_irec;
+       xfs_agino_t                     i;
+       unsigned int                    rec_idx;
        int                             error;
 
-       if (sc->sm->sm_type == XFS_SCRUB_TYPE_FINOBT)
-               pcur = &sc->sa.ino_cur;
-       else
-               pcur = &sc->sa.fino_cur;
-       if (!(*pcur))
-               return;
-       error = xfs_ialloc_has_inode_record(*pcur, agino, agino, &has_irec);
-       if (!xchk_should_check_xref(sc, &error, pcur))
+       ASSERT(sc->sm->sm_type == XFS_SCRUB_TYPE_INOBT);
+
+       if (!sc->sa.fino_cur || xchk_skip_xref(sc->sm))
                return;
-       if (((irec->ir_freecount > 0 && !has_irec) ||
-            (irec->ir_freecount == 0 && has_irec)))
-               xchk_btree_xref_set_corrupt(sc, *pcur, 0);
+
+       for (i = agino, rec_idx = agino - irec->ir_startino;
+            i < agino + nr_inodes;
+            i++, rec_idx++) {
+               bool                    free, hole;
+               unsigned int            hole_idx;
+
+               free = irec->ir_free & (1ULL << rec_idx);
+               hole_idx = rec_idx / XFS_INODES_PER_HOLEMASK_BIT;
+               hole = irec->ir_holemask & (1U << hole_idx);
+
+               error = xchk_inobt_xref_finobt(sc, irec, i, free, hole);
+               if (!xchk_should_check_xref(sc, &error, &sc->sa.fino_cur))
+                       return;
+       }
 }
 
-/* Cross-reference with the other btrees. */
+/*
+ * Does the inobt have a record for this inode with the same hole/free state?
+ * The inobt must always have a record if there's a finobt record.
+ */
+STATIC int
+xchk_finobt_xref_inobt(
+       struct xfs_scrub        *sc,
+       struct xfs_inobt_rec_incore *frec,
+       xfs_agino_t             agino,
+       bool                    ffree,
+       bool                    fhole)
+{
+       struct xfs_inobt_rec_incore irec;
+       struct xfs_btree_cur    *cur = sc->sa.ino_cur;
+       bool                    free, hole;
+       unsigned int            rec_idx, hole_idx;
+       int                     has_record;
+       int                     error;
+
+       ASSERT(cur->bc_btnum == XFS_BTNUM_INO);
+
+       error = xfs_inobt_lookup(cur, agino, XFS_LOOKUP_LE, &has_record);
+       if (error)
+               return error;
+       if (!has_record)
+               goto no_record;
+
+       error = xfs_inobt_get_rec(cur, &irec, &has_record);
+       if (!has_record)
+               return -EFSCORRUPTED;
+
+       if (irec.ir_startino + XFS_INODES_PER_CHUNK <= agino)
+               goto no_record;
+
+       /* There's an inobt record; free and hole status must match. */
+       rec_idx = agino - irec.ir_startino;
+       free = irec.ir_free & (1ULL << rec_idx);
+       hole_idx = rec_idx / XFS_INODES_PER_HOLEMASK_BIT;
+       hole = irec.ir_holemask & (1U << hole_idx);
+
+       if (ffree != free)
+               xchk_btree_xref_set_corrupt(sc, cur, 0);
+       if (fhole != hole)
+               xchk_btree_xref_set_corrupt(sc, cur, 0);
+       return 0;
+
+no_record:
+       /* finobt should never have a record for which the inobt does not */
+       xchk_btree_xref_set_corrupt(sc, cur, 0);
+       return 0;
+}
+
+/*
+ * Make sure that each inode of this part of an finobt record has the same
+ * sparse and free status as the inobt.
+ */
 STATIC void
-xchk_iallocbt_chunk_xref(
+xchk_finobt_chunk_xref_inobt(
        struct xfs_scrub                *sc,
-       struct xfs_inobt_rec_incore     *irec,
+       struct xfs_inobt_rec_incore     *frec,
        xfs_agino_t                     agino,
-       xfs_agblock_t                   agbno,
-       xfs_extlen_t                    len)
+       unsigned int                    nr_inodes)
 {
-       if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
+       xfs_agino_t                     i;
+       unsigned int                    rec_idx;
+       int                             error;
+
+       ASSERT(sc->sm->sm_type == XFS_SCRUB_TYPE_FINOBT);
+
+       if (!sc->sa.ino_cur || xchk_skip_xref(sc->sm))
                return;
 
-       xchk_xref_is_used_space(sc, agbno, len);
-       xchk_iallocbt_chunk_xref_other(sc, irec, agino);
-       xchk_xref_is_owned_by(sc, agbno, len, &XFS_RMAP_OINFO_INODES);
-       xchk_xref_is_not_shared(sc, agbno, len);
+       for (i = agino, rec_idx = agino - frec->ir_startino;
+            i < agino + nr_inodes;
+            i++, rec_idx++) {
+               bool                    ffree, fhole;
+               unsigned int            hole_idx;
+
+               ffree = frec->ir_free & (1ULL << rec_idx);
+               hole_idx = rec_idx / XFS_INODES_PER_HOLEMASK_BIT;
+               fhole = frec->ir_holemask & (1U << hole_idx);
+
+               error = xchk_finobt_xref_inobt(sc, frec, i, ffree, fhole);
+               if (!xchk_should_check_xref(sc, &error, &sc->sa.ino_cur))
+                       return;
+       }
 }
 
-/* Is this chunk worth checking? */
+/* Is this chunk worth checking and cross-referencing? */
 STATIC bool
 xchk_iallocbt_chunk(
        struct xchk_btree               *bs,
        struct xfs_inobt_rec_incore     *irec,
        xfs_agino_t                     agino,
-       xfs_extlen_t                    len)
+       unsigned int                    nr_inodes)
 {
+       struct xfs_scrub                *sc = bs->sc;
        struct xfs_mount                *mp = bs->cur->bc_mp;
        struct xfs_perag                *pag = bs->cur->bc_ag.pag;
-       xfs_agblock_t                   bno;
+       xfs_agblock_t                   agbno;
+       xfs_extlen_t                    len;
 
-       bno = XFS_AGINO_TO_AGBNO(mp, agino);
+       agbno = XFS_AGINO_TO_AGBNO(mp, agino);
+       len = XFS_B_TO_FSB(mp, nr_inodes * mp->m_sb.sb_inodesize);
 
-       if (!xfs_verify_agbext(pag, bno, len))
+       if (!xfs_verify_agbext(pag, agbno, len))
                xchk_btree_set_corrupt(bs->sc, bs->cur, 0);
 
-       xchk_iallocbt_chunk_xref(bs->sc, irec, agino, bno, len);
-       xchk_xref_is_not_cow_staging(bs->sc, bno, len);
+       if (bs->sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
+               return false;
+
+       xchk_xref_is_used_space(sc, agbno, len);
+       if (sc->sm->sm_type == XFS_SCRUB_TYPE_INOBT)
+               xchk_inobt_chunk_xref_finobt(sc, irec, agino, nr_inodes);
+       else
+               xchk_finobt_chunk_xref_inobt(sc, irec, agino, nr_inodes);
+       xchk_xref_is_owned_by(sc, agbno, len, &XFS_RMAP_OINFO_INODES);
+       xchk_xref_is_not_shared(sc, agbno, len);
+       xchk_xref_is_not_cow_staging(sc, agbno, len);
        return true;
 }
 
@@ -417,7 +580,6 @@ xchk_iallocbt_rec(
        struct xfs_inobt_rec_incore     irec;
        uint64_t                        holes;
        xfs_agino_t                     agino;
-       xfs_extlen_t                    len;
        int                             holecount;
        int                             i;
        int                             error = 0;
@@ -439,12 +601,11 @@ xchk_iallocbt_rec(
 
        /* Handle non-sparse inodes */
        if (!xfs_inobt_issparse(irec.ir_holemask)) {
-               len = XFS_B_TO_FSB(mp,
-                               XFS_INODES_PER_CHUNK * mp->m_sb.sb_inodesize);
                if (irec.ir_count != XFS_INODES_PER_CHUNK)
                        xchk_btree_set_corrupt(bs->sc, bs->cur, 0);
 
-               if (!xchk_iallocbt_chunk(bs, &irec, agino, len))
+               if (!xchk_iallocbt_chunk(bs, &irec, agino,
+                                       XFS_INODES_PER_CHUNK))
                        goto out;
                goto check_clusters;
        }
@@ -452,8 +613,6 @@ xchk_iallocbt_rec(
        /* Check each chunk of a sparse inode cluster. */
        holemask = irec.ir_holemask;
        holecount = 0;
-       len = XFS_B_TO_FSB(mp,
-                       XFS_INODES_PER_HOLEMASK_BIT * mp->m_sb.sb_inodesize);
        holes = ~xfs_inobt_irec_to_allocmask(&irec);
        if ((holes & irec.ir_free) != holes ||
            irec.ir_freecount > irec.ir_count)
@@ -462,8 +621,9 @@ xchk_iallocbt_rec(
        for (i = 0; i < XFS_INOBT_HOLEMASK_BITS; i++) {
                if (holemask & 1)
                        holecount += XFS_INODES_PER_HOLEMASK_BIT;
-               else if (!xchk_iallocbt_chunk(bs, &irec, agino, len))
-                       break;
+               else if (!xchk_iallocbt_chunk(bs, &irec, agino,
+                                       XFS_INODES_PER_HOLEMASK_BIT))
+                       goto out;
                holemask >>= 1;
                agino += XFS_INODES_PER_HOLEMASK_BIT;
        }
@@ -473,6 +633,9 @@ xchk_iallocbt_rec(
                xchk_btree_set_corrupt(bs->sc, bs->cur, 0);
 
 check_clusters:
+       if (bs->sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
+               goto out;
+
        error = xchk_iallocbt_check_clusters(bs, &irec);
        if (error)
                goto out;
@@ -602,18 +765,18 @@ xchk_xref_inode_check(
        xfs_agblock_t           agbno,
        xfs_extlen_t            len,
        struct xfs_btree_cur    **icur,
-       bool                    should_have_inodes)
+       enum xbtree_recpacking  expected)
 {
-       bool                    has_inodes;
+       enum xbtree_recpacking  outcome;
        int                     error;
 
        if (!(*icur) || xchk_skip_xref(sc->sm))
                return;
 
-       error = xfs_ialloc_has_inodes_at_extent(*icur, agbno, len, &has_inodes);
+       error = xfs_ialloc_has_inodes_at_extent(*icur, agbno, len, &outcome);
        if (!xchk_should_check_xref(sc, &error, icur))
                return;
-       if (has_inodes != should_have_inodes)
+       if (outcome != expected)
                xchk_btree_xref_set_corrupt(sc, *icur, 0);
 }
 
@@ -624,8 +787,10 @@ xchk_xref_is_not_inode_chunk(
        xfs_agblock_t           agbno,
        xfs_extlen_t            len)
 {
-       xchk_xref_inode_check(sc, agbno, len, &sc->sa.ino_cur, false);
-       xchk_xref_inode_check(sc, agbno, len, &sc->sa.fino_cur, false);
+       xchk_xref_inode_check(sc, agbno, len, &sc->sa.ino_cur,
+                       XBTREE_RECPACKING_EMPTY);
+       xchk_xref_inode_check(sc, agbno, len, &sc->sa.fino_cur,
+                       XBTREE_RECPACKING_EMPTY);
 }
 
 /* xref check that the extent is covered by inodes */
@@ -635,5 +800,6 @@ xchk_xref_is_inode_chunk(
        xfs_agblock_t           agbno,
        xfs_extlen_t            len)
 {
-       xchk_xref_inode_check(sc, agbno, len, &sc->sa.ino_cur, true);
+       xchk_xref_inode_check(sc, agbno, len, &sc->sa.ino_cur,
+                       XBTREE_RECPACKING_FULL);
 }