xfs
[Top] [All Lists]

[PATCH 2/4] repair: helper for inode chunk alignment and start/end ino n

To: xfs@xxxxxxxxxxx
Subject: [PATCH 2/4] repair: helper for inode chunk alignment and start/end ino number verification
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Mon, 8 Jun 2015 07:29:22 -0400
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1433762964-11502-1-git-send-email-bfoster@xxxxxxxxxx>
References: <1433762964-11502-1-git-send-email-bfoster@xxxxxxxxxx>
The inobt scan code executes different routines for processing inobt
records and finobt records. While some verification differs between the
trees, much of it is the same. One such example of this is the inode
record alignment and start/end inode number verification. The only
difference between the inobt and finobt verification is the error
message that is generated as a result of failure.

Factor out these alignment checks into a new helper that takes an enum
parameter that identifies which tree is undergoing the scan. Use a new
string array for this function and subsequent common inobt scan helpers
to convert the enum to the name of the tree for the purposes of
including in any resulting warning messages.

Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
---
 repair/scan.c | 167 ++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 93 insertions(+), 74 deletions(-)

diff --git a/repair/scan.c b/repair/scan.c
index 1a6f0c5..14a816d 100644
--- a/repair/scan.c
+++ b/repair/scan.c
@@ -764,77 +764,130 @@ ino_issparse(
        return xfs_inobt_is_sparse_disk(rp, offset);
 }
 
+/*
+ * The following helpers are to help process and validate individual on-disk
+ * inode btree records. We have two possible inode btrees with slightly
+ * different semantics. Many of the validations and actions are equivalent, 
such
+ * as record alignment constraints, etc. Other validations differ, such as the
+ * fact that the inode chunk block allocation state is set by the content of 
the
+ * core inobt and verified by the content of the finobt.
+ *
+ * The following structures are used to facilitate common validation routines
+ * where the only difference between validation of the inobt or finobt might be
+ * the error messages that results in the event of failure.
+ */
+
+enum inobt_type {
+       INOBT,
+       FINOBT
+};
+const char *inobt_names[] =    {
+       "inobt",
+       "finobt"
+};
+
 static int
-scan_single_ino_chunk(
+verify_single_ino_chunk_align(
        xfs_agnumber_t          agno,
-       xfs_inobt_rec_t         *rp,
-       int                     suspect)
+       enum inobt_type         type,
+       struct xfs_inobt_rec    *rp,
+       int                     suspect,
+       bool                    *skip)
 {
+       const char              *inobt_name = inobt_names[type];
        xfs_ino_t               lino;
        xfs_agino_t             ino;
        xfs_agblock_t           agbno;
-       int                     j;
-       int                     nfree;
-       int                     ninodes;
        int                     off;
-       int                     state;
-       ino_tree_node_t         *ino_rec = NULL;
-       ino_tree_node_t         *first_rec, *last_rec;
-       int                     freecount;
 
+       *skip = false;
        ino = be32_to_cpu(rp->ir_startino);
        off = XFS_AGINO_TO_OFFSET(mp, ino);
        agbno = XFS_AGINO_TO_AGBNO(mp, ino);
        lino = XFS_AGINO_TO_INO(mp, agno, ino);
-       freecount = inorec_get_freecount(mp, rp);
 
        /*
-        * on multi-block block chunks, all chunks start
-        * at the beginning of the block.  with multi-chunk
-        * blocks, all chunks must start on 64-inode boundaries
-        * since each block can hold N complete chunks. if
-        * fs has aligned inodes, all chunks must start
-        * at a fs_ino_alignment*N'th agbno.  skip recs
-        * with badly aligned starting inodes.
+        * on multi-block block chunks, all chunks start at the beginning of the
+        * block. with multi-chunk blocks, all chunks must start on 64-inode
+        * boundaries since each block can hold N complete chunks. if fs has
+        * aligned inodes, all chunks must start at a fs_ino_alignment*N'th
+        * agbno. skip recs with badly aligned starting inodes.
         */
        if (ino == 0 ||
            (inodes_per_block <= XFS_INODES_PER_CHUNK && off !=  0) ||
            (inodes_per_block > XFS_INODES_PER_CHUNK &&
             off % XFS_INODES_PER_CHUNK != 0) ||
            (fs_aligned_inodes && fs_ino_alignment &&
-            agbno % fs_ino_alignment != 0))  {
+            agbno % fs_ino_alignment != 0)) {
                do_warn(
-       _("badly aligned inode rec (starting inode = %" PRIu64 ")\n"),
-                       lino);
+       _("badly aligned %s rec (starting inode = %" PRIu64 ")\n"),
+                       inobt_name, lino);
                suspect++;
        }
 
        /*
-        * verify numeric validity of inode chunk first
-        * before inserting into a tree.  don't have to
-        * worry about the overflow case because the
-        * starting ino number of a chunk can only get
-        * within 255 inodes of max (NULLAGINO).  if it
-        * gets closer, the agino number will be illegal
-        * as the agbno will be too large.
+        * verify numeric validity of inode chunk first before inserting into a
+        * tree. don't have to worry about the overflow case because the
+        * starting ino number of a chunk can only get within 255 inodes of max
+        * (NULLAGINO). if it gets closer, the agino number will be illegal as
+        * the agbno will be too large.
         */
-       if (verify_aginum(mp, agno, ino))  {
+       if (verify_aginum(mp, agno, ino)) {
                do_warn(
-_("bad starting inode # (%" PRIu64 " (0x%x 0x%x)) in ino rec, skipping rec\n"),
-                       lino, agno, ino);
+_("bad starting inode # (%" PRIu64 " (0x%x 0x%x)) in %s rec, skipping rec\n"),
+                       lino, agno, ino, inobt_name);
+               *skip = true;
                return ++suspect;
        }
 
        if (verify_aginum(mp, agno,
-                       ino + XFS_INODES_PER_CHUNK - 1))  {
+                       ino + XFS_INODES_PER_CHUNK - 1)) {
                do_warn(
-_("bad ending inode # (%" PRIu64 " (0x%x 0x%zx)) in ino rec, skipping rec\n"),
+_("bad ending inode # (%" PRIu64 " (0x%x 0x%zx)) in %s rec, skipping rec\n"),
                        lino + XFS_INODES_PER_CHUNK - 1,
                        agno,
-                       ino + XFS_INODES_PER_CHUNK - 1);
+                       ino + XFS_INODES_PER_CHUNK - 1,
+                       inobt_name);
+               *skip = true;
                return ++suspect;
        }
 
+       return suspect;
+}
+
+static int
+scan_single_ino_chunk(
+       xfs_agnumber_t          agno,
+       xfs_inobt_rec_t         *rp,
+       int                     suspect)
+{
+       xfs_ino_t               lino;
+       xfs_agino_t             ino;
+       xfs_agblock_t           agbno;
+       int                     j;
+       int                     nfree;
+       int                     ninodes;
+       int                     off;
+       int                     state;
+       ino_tree_node_t         *ino_rec = NULL;
+       ino_tree_node_t         *first_rec, *last_rec;
+       int                     freecount;
+       bool                    skip = false;
+
+       ino = be32_to_cpu(rp->ir_startino);
+       off = XFS_AGINO_TO_OFFSET(mp, ino);
+       agbno = XFS_AGINO_TO_AGBNO(mp, ino);
+       lino = XFS_AGINO_TO_INO(mp, agno, ino);
+       freecount = inorec_get_freecount(mp, rp);
+
+       /*
+        * Verify record alignment, start/end inode numbers, etc.
+        */
+       suspect = verify_single_ino_chunk_align(agno, INOBT, rp, suspect,
+                                               &skip);
+       if (skip)
+               return suspect;
+
        /*
         * set state of each block containing inodes
         */
@@ -979,6 +1032,7 @@ scan_single_finobt_chunk(
        ino_tree_node_t         *ino_rec = NULL;
        ino_tree_node_t         *first_rec, *last_rec;
        int                     freecount;
+       bool                    skip = false;
 
        ino = be32_to_cpu(rp->ir_startino);
        off = XFS_AGINO_TO_OFFSET(mp, ino);
@@ -987,47 +1041,12 @@ scan_single_finobt_chunk(
        freecount = inorec_get_freecount(mp, rp);
 
        /*
-        * on multi-block block chunks, all chunks start at the beginning of the
-        * block. with multi-chunk blocks, all chunks must start on 64-inode
-        * boundaries since each block can hold N complete chunks. if fs has
-        * aligned inodes, all chunks must start at a fs_ino_alignment*N'th
-        * agbno. skip recs with badly aligned starting inodes.
-        */
-       if (ino == 0 ||
-           (inodes_per_block <= XFS_INODES_PER_CHUNK && off !=  0) ||
-           (inodes_per_block > XFS_INODES_PER_CHUNK &&
-            off % XFS_INODES_PER_CHUNK != 0) ||
-           (fs_aligned_inodes && fs_ino_alignment &&
-            agbno % fs_ino_alignment != 0)) {
-               do_warn(
-       _("badly aligned finobt inode rec (starting inode = %" PRIu64 ")\n"),
-                       lino);
-               suspect++;
-       }
-
-       /*
-        * verify numeric validity of inode chunk first before inserting into a
-        * tree. don't have to worry about the overflow case because the
-        * starting ino number of a chunk can only get within 255 inodes of max
-        * (NULLAGINO). if it gets closer, the agino number will be illegal as
-        * the agbno will be too large.
+        * Verify record alignment, start/end inode numbers, etc.
         */
-       if (verify_aginum(mp, agno, ino)) {
-               do_warn(
-_("bad starting inode # (%" PRIu64 " (0x%x 0x%x)) in finobt rec, skipping 
rec\n"),
-                       lino, agno, ino);
-               return ++suspect;
-       }
-
-       if (verify_aginum(mp, agno,
-                       ino + XFS_INODES_PER_CHUNK - 1)) {
-               do_warn(
-_("bad ending inode # (%" PRIu64 " (0x%x 0x%zx)) in finobt rec, skipping 
rec\n"),
-                       lino + XFS_INODES_PER_CHUNK - 1,
-                       agno,
-                       ino + XFS_INODES_PER_CHUNK - 1);
-               return ++suspect;
-       }
+       suspect = verify_single_ino_chunk_align(agno, FINOBT, rp, suspect,
+                                               &skip);
+       if (skip)
+               return suspect;
 
        /*
         * cross check state of each block containing inodes referenced by the
-- 
1.9.3

<Prev in Thread] Current Thread [Next in Thread>