[PATCH 0/4] xfsprogs/repair: sparse inode chunks cleanups

[PATCH 0/4] xfsprogs/repair: sparse inode chunks cleanups

[Brian Foster]

Hi all,

This is a small cleanup series for some of the repair code touched by
the sparse inode chunks feature, in response to review comments in the
following thread:

	http://oss.sgi.com/archives/xfs/2015-06/msg00053.html

It primarily creates a few helper functions for hunks of code executed
multiple times or code that is common between the increasingly similar
but separate inobt and finobt record scanning code. For much of the
latter code, the only difference between the separate functions is the
error message output to indicate which tree is affected by a problem.
Therefore, I abstracted out the inobt name and factored out some helpers
that can perform the appropriate checks regardless of the tree type.

This series is purely cleanup and should not alter functionality in any
way. Thoughts?

Brian

Brian Foster (4):
  repair: access helpers for on-disk inobt record freecount
  repair: helper for inode chunk alignment and start/end ino number
    verification
  repair: helper to import on-disk inobt records to in-core trees
  repair: helper to transition inode blocks to inode state

 repair/dino_chunks.c |  91 ++++++-------
 repair/incore.h      |  28 ++++
 repair/phase5.c      |  12 +-
 repair/scan.c        | 377 +++++++++++++++++++++++++--------------------------
 4 files changed, 259 insertions(+), 249 deletions(-)

-- 
1.9.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/4] repair: access helpers for on-disk inobt record freecount

[Brian Foster]

The on-disk inobt record has two formats depending on whether sparse
inode support is enabled or not. If so, the freecount field is a single
byte and does not require byte-conversion. Otherwise, it is a 4-byte
field and does.

Create the inorec_[get|set]_freecount() helpers to abstract this detail
away from the core repair code.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 repair/incore.h | 28 ++++++++++++++++++++++++++++
 repair/phase5.c | 12 +++++++-----
 repair/scan.c   | 15 +++------------
 3 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/repair/incore.h b/repair/incore.h
index 5a63e1e..c92475e 100644
--- a/repair/incore.h
+++ b/repair/incore.h
@@ -591,4 +591,32 @@ typedef struct bm_cursor  {
 
 void init_bm_cursor(bmap_cursor_t *cursor, int num_level);
 
+/*
+ * On-disk inobt record helpers. The sparse inode record format has a single
+ * byte freecount. The older format has a 32-bit freecount and thus byte
+ * conversion is necessary.
+ */
+
+static inline int
+inorec_get_freecount(
+	struct xfs_mount	*mp,
+	struct xfs_inobt_rec	*rp)
+{
+	if (xfs_sb_version_hassparseinodes(&mp->m_sb))
+		return rp->ir_u.sp.ir_freecount;
+	return be32_to_cpu(rp->ir_u.f.ir_freecount);
+}
+
+static inline void
+inorec_set_freecount(
+	struct xfs_mount	*mp,
+	struct xfs_inobt_rec	*rp,
+	int			freecount)
+{
+	if (xfs_sb_version_hassparseinodes(&mp->m_sb))
+		rp->ir_u.sp.ir_freecount = freecount;
+	else
+		rp->ir_u.f.ir_freecount = cpu_to_be32(freecount);
+}
+
 #endif /* XFS_REPAIR_INCORE_H */
diff --git a/repair/phase5.c b/repair/phase5.c
index 0601810..7372734 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -1258,11 +1258,14 @@ build_ino_tree(xfs_mount_t *mp, xfs_agnumber_t agno,
 				inocnt++;
 			}
 
-			if (!xfs_sb_version_hassparseinodes(&mp->m_sb)) {
-				bt_rec[j].ir_u.f.ir_freecount =
-							cpu_to_be32(finocnt);
+			/*
+			 * Set the freecount and check whether we need to update
+			 * the sparse format fields. Otherwise, skip to the next
+			 * record.
+			 */
+			inorec_set_freecount(mp, &bt_rec[j], finocnt);
+			if (!xfs_sb_version_hassparseinodes(&mp->m_sb))
 				goto nextrec;
-			}
 
 			/*
 			 * Convert the 64-bit in-core sparse inode state to the
@@ -1280,7 +1283,6 @@ build_ino_tree(xfs_mount_t *mp, xfs_agnumber_t agno,
 				sparse >>= XFS_INODES_PER_HOLEMASK_BIT;
 			}
 
-			bt_rec[j].ir_u.sp.ir_freecount = finocnt;
 			bt_rec[j].ir_u.sp.ir_count = inocnt;
 			bt_rec[j].ir_u.sp.ir_holemask = cpu_to_be16(holemask);
 
diff --git a/repair/scan.c b/repair/scan.c
index e3895c2..1a6f0c5 100644
--- a/repair/scan.c
+++ b/repair/scan.c
@@ -786,10 +786,7 @@ scan_single_ino_chunk(
 	off = XFS_AGINO_TO_OFFSET(mp, ino);
 	agbno = XFS_AGINO_TO_AGBNO(mp, ino);
 	lino = XFS_AGINO_TO_INO(mp, agno, ino);
-	if (xfs_sb_version_hassparseinodes(&mp->m_sb))
-		freecount = rp->ir_u.sp.ir_freecount;
-	else
-		freecount = be32_to_cpu(rp->ir_u.f.ir_freecount);
+	freecount = inorec_get_freecount(mp, rp);
 
 	/*
 	 * on multi-block block chunks, all chunks start
@@ -987,10 +984,7 @@ scan_single_finobt_chunk(
 	off = XFS_AGINO_TO_OFFSET(mp, ino);
 	agbno = XFS_AGINO_TO_AGBNO(mp, ino);
 	lino = XFS_AGINO_TO_INO(mp, agno, ino);
-	if (xfs_sb_version_hassparseinodes(&mp->m_sb))
-		freecount = rp->ir_u.sp.ir_freecount;
-	else
-		freecount = be32_to_cpu(rp->ir_u.f.ir_freecount);
+	freecount = inorec_get_freecount(mp, rp);
 
 	/*
 	 * on multi-block block chunks, all chunks start at the beginning of the
@@ -1331,10 +1325,7 @@ _("inode btree block claimed (state %d), agno %d, bno %d, suspect %d\n"),
 		 * the block.  skip processing of bogus records.
 		 */
 		for (i = 0; i < numrecs; i++) {
-			if (xfs_sb_version_hassparseinodes(&mp->m_sb))
-				freecount = rp[i].ir_u.sp.ir_freecount;
-			else
-				freecount = be32_to_cpu(rp[i].ir_u.f.ir_freecount);
+			freecount = inorec_get_freecount(mp, &rp[i]);
 
 			if (magic == XFS_IBT_MAGIC ||
 			    magic == XFS_IBT_CRC_MAGIC) {
-- 
1.9.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Brian Foster]

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@redhat.com>
---
 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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/4] repair: helper to import on-disk inobt records to in-core trees

[Brian Foster]

In the common case, the in-core inode state from the on-disk inobt
records is imported from the inobt and validated against the finobt (if
one exists). When both trees exist along with some form of corruption,
it's possible to find inodes in the finobt not tracked by the inobt.
While this is unexpected, we attempt to repair by importing the inodes
from the finobt.

The associated code in the finobt scan function mirrors the associated
code in the inobt scan function. Factor this into a separate helper that
can be called by either tree scan.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 repair/scan.c | 199 +++++++++++++++++++++++++++-------------------------------
 1 file changed, 91 insertions(+), 108 deletions(-)

diff --git a/repair/scan.c b/repair/scan.c
index 14a816d..8711b94 100644
--- a/repair/scan.c
+++ b/repair/scan.c
@@ -855,6 +855,81 @@ _("bad ending inode # (%" PRIu64 " (0x%x 0x%zx)) in %s rec, skipping rec\n"),
 	return suspect;
 }
 
+/*
+ * Process the state of individual inodes in an on-disk inobt record and import
+ * into the appropriate in-core tree based on whether the on-disk tree is
+ * suspect. Return the total and free inode counts based on the record free and
+ * hole masks.
+ */
+static int
+import_single_ino_chunk(
+	xfs_agnumber_t		agno,
+	enum inobt_type		type,
+	struct xfs_inobt_rec	*rp,
+	int			suspect,
+	int			*p_nfree,
+	int			*p_ninodes)
+{
+	struct ino_tree_node	*ino_rec = NULL;
+	const char		*inobt_name = inobt_names[type];
+	xfs_agino_t		ino;
+	int			j;
+	int			nfree;
+	int			ninodes;
+
+	ino = be32_to_cpu(rp->ir_startino);
+
+	if (!suspect) {
+		if (XFS_INOBT_IS_FREE_DISK(rp, 0))
+			ino_rec = set_inode_free_alloc(mp, agno, ino);
+		else
+			ino_rec = set_inode_used_alloc(mp, agno, ino);
+		for (j = 1; j < XFS_INODES_PER_CHUNK; j++) {
+			if (XFS_INOBT_IS_FREE_DISK(rp, j))
+				set_inode_free(ino_rec, j);
+			else
+				set_inode_used(ino_rec, j);
+		}
+	} else {
+		for (j = 0; j < XFS_INODES_PER_CHUNK; j++) {
+			if (XFS_INOBT_IS_FREE_DISK(rp, j))
+				add_aginode_uncertain(mp, agno, ino + j, 1);
+			else
+				add_aginode_uncertain(mp, agno, ino + j, 0);
+		}
+	}
+
+	/*
+	 * Mark sparse inodes as such in the in-core tree. Verify that sparse
+	 * inodes are free and that freecount is consistent with the free mask.
+	 */
+	nfree = ninodes = 0;
+	for (j = 0; j < XFS_INODES_PER_CHUNK; j++) {
+		if (ino_issparse(rp, j)) {
+			if (!suspect && !XFS_INOBT_IS_FREE_DISK(rp, j)) {
+				do_warn(
+_("ir_holemask/ir_free mismatch, %s chunk %d/%u, holemask 0x%x free 0x%llx\n"),
+					inobt_name, agno, ino,
+					be16_to_cpu(rp->ir_u.sp.ir_holemask),
+					be64_to_cpu(rp->ir_free));
+				suspect++;
+			}
+			if (!suspect && ino_rec)
+				set_inode_sparse(ino_rec, j);
+		} else {
+			/* count fields track non-sparse inos */
+			if (XFS_INOBT_IS_FREE_DISK(rp, j))
+				nfree++;
+			ninodes++;
+		}
+	}
+
+	*p_nfree = nfree;
+	*p_ninodes = ninodes;
+
+	return suspect;
+}
+
 static int
 scan_single_ino_chunk(
 	xfs_agnumber_t		agno,
@@ -869,7 +944,6 @@ scan_single_ino_chunk(
 	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;
@@ -946,57 +1020,13 @@ _("inode rec for ino %" PRIu64 " (%d/%d) overlaps existing rec (start %d/%d)\n")
 	}
 
 	/*
-	 * now mark all the inodes as existing and free or used.
-	 * if the tree is suspect, put them into the uncertain
-	 * inode tree.
-	 */
-	if (!suspect)  {
-		if (XFS_INOBT_IS_FREE_DISK(rp, 0)) {
-			ino_rec = set_inode_free_alloc(mp, agno, ino);
-		} else  {
-			ino_rec = set_inode_used_alloc(mp, agno, ino);
-		}
-		for (j = 1; j < XFS_INODES_PER_CHUNK; j++) {
-			if (XFS_INOBT_IS_FREE_DISK(rp, j)) {
-				set_inode_free(ino_rec, j);
-			} else  {
-				set_inode_used(ino_rec, j);
-			}
-		}
-	} else  {
-		for (j = 0; j < XFS_INODES_PER_CHUNK; j++) {
-			if (XFS_INOBT_IS_FREE_DISK(rp, j)) {
-				add_aginode_uncertain(mp, agno, ino + j, 1);
-			} else  {
-				add_aginode_uncertain(mp, agno, ino + j, 0);
-			}
-		}
-	}
-
-	/*
-	 * Mark sparse inodes as such in the in-core tree. Verify that sparse
-	 * inodes are free and that freecount is consistent with the free mask.
+	 * Import the state of individual inodes into the appropriate in-core
+	 * trees, mark them free or used, and get the resulting total and free
+	 * inode counts.
 	 */
 	nfree = ninodes = 0;
-	for (j = 0; j < XFS_INODES_PER_CHUNK; j++) {
-		if (ino_issparse(rp, j)) {
-			if (!suspect && !XFS_INOBT_IS_FREE_DISK(rp, j)) {
-				do_warn(
-_("ir_holemask/ir_free mismatch, inode chunk %d/%u, holemask 0x%x free 0x%llx\n"),
-					agno, ino,
-					be16_to_cpu(rp->ir_u.sp.ir_holemask),
-					be64_to_cpu(rp->ir_free));
-				suspect++;
-			}
-			if (!suspect && ino_rec)
-				set_inode_sparse(ino_rec, j);
-		} else {
-			/* count fields track non-sparse inos */
-			if (XFS_INOBT_IS_FREE_DISK(rp, j))
-				nfree++;
-			ninodes++;
-		}
-	}
+	suspect = import_single_ino_chunk(agno, INOBT, rp, suspect, &nfree,
+					 &ninodes);
 
 	if (nfree != freecount) {
 		do_warn(
@@ -1029,7 +1059,6 @@ scan_single_finobt_chunk(
 	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;
@@ -1138,68 +1167,22 @@ _("finobt rec for ino %" PRIu64 " (%d/%u) does not match existing rec (%d/%d)\n"
 	}
 
 	/*
-	 * the finobt contains a record that the previous alloc inobt scan never
-	 * found. insert the inodes into the appropriate tree.
+	 * The finobt contains a record that the previous inobt scan never
+	 * found. Warn about it and import the inodes into the appropriate
+	 * trees.
+	 *
+	 * Note that this should do the right thing if the previous inobt scan
+	 * had added these inodes to the uncertain tree. If the finobt is not
+	 * suspect, these inodes should supercede the uncertain ones. Otherwise,
+	 * the uncertain tree helpers handle the case where uncertain inodes
+	 * already exist.
 	 */
 	do_warn(_("undiscovered finobt record, ino %" PRIu64 " (%d/%u)\n"),
 		lino, agno, ino);
 
-	if (!suspect) {
-		/*
-		 * inodes previously inserted into the uncertain tree should be
-		 * superceded by these when the uncertain tree is processed
-		 */
-		if (XFS_INOBT_IS_FREE_DISK(rp, 0)) {
-			ino_rec = set_inode_free_alloc(mp, agno, ino);
-		} else  {
-			ino_rec = set_inode_used_alloc(mp, agno, ino);
-		}
-		for (j = 1; j < XFS_INODES_PER_CHUNK; j++) {
-			if (XFS_INOBT_IS_FREE_DISK(rp, j)) {
-				set_inode_free(ino_rec, j);
-			} else  {
-				set_inode_used(ino_rec, j);
-			}
-		}
-	} else {
-		/*
-		 * this should handle the case where the inobt scan may have
-		 * already added uncertain inodes
-		 */
-		for (j = 0; j < XFS_INODES_PER_CHUNK; j++) {
-			if (XFS_INOBT_IS_FREE_DISK(rp, j)) {
-				add_aginode_uncertain(mp, agno, ino + j, 1);
-			} else {
-				add_aginode_uncertain(mp, agno, ino + j, 0);
-			}
-		}
-	}
-
-	/*
-	 * Mark sparse inodes as such in the in-core tree. Verify that sparse
-	 * inodes are free and that freecount is consistent with the free mask.
-	 */
 	nfree = ninodes = 0;
-	for (j = 0; j < XFS_INODES_PER_CHUNK; j++) {
-		if (ino_issparse(rp, j)) {
-			if (!suspect && !XFS_INOBT_IS_FREE_DISK(rp, j)) {
-				do_warn(
-_("finobt ir_holemask/ir_free mismatch, inode chunk %d/%u, holemask 0x%x free 0x%llx\n"),
-					agno, ino,
-					be16_to_cpu(rp->ir_u.sp.ir_holemask),
-					be64_to_cpu(rp->ir_free));
-				suspect++;
-			}
-			if (!suspect && ino_rec)
-				set_inode_sparse(ino_rec, j);
-		} else {
-			/* count fields track non-sparse inos */
-			if (XFS_INOBT_IS_FREE_DISK(rp, j))
-				nfree++;
-			ninodes++;
-		}
-
-	}
+	suspect = import_single_ino_chunk(agno, FINOBT, rp, suspect, &nfree,
+					 &ninodes);
 
 check_freecount:
 
-- 
1.9.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/4] repair: helper to transition inode blocks to inode state

[Brian Foster]

The state of each block in an inode chunk transitions from free state to
inode state as we process physical inodes on disk. We take care to
detect invalid transitions and warn the user if multiply claimed blocks
are detected.

This block of code is a largish switch statement that is executed twice
due to the implementation details of the inode processing loop. Factor
it into a new helper.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 repair/dino_chunks.c | 91 ++++++++++++++++++++++------------------------------
 1 file changed, 39 insertions(+), 52 deletions(-)

diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
index 9b7d017..834227b 100644
--- a/repair/dino_chunks.c
+++ b/repair/dino_chunks.c
@@ -550,7 +550,42 @@ verify_aginode_chunk_irec(xfs_mount_t	*mp,
 	return(irec);
 }
 
+/*
+ * Set the state of an inode block during inode chunk processing. The block is
+ * expected to be in the free or inode state. If free, it transitions to the
+ * inode state. Warn if the block is in neither expected state as this indicates
+ * multiply claimed blocks.
+ */
+static void
+process_inode_agbno_state(
+	struct xfs_mount	*mp,
+	xfs_agnumber_t		agno,
+	xfs_agblock_t		agbno)
+{
+	int state;
 
+	pthread_mutex_lock(&ag_locks[agno].lock);
+	state = get_bmap(agno, agbno);
+	switch (state) {
+	case XR_E_INO:	/* already marked */
+		break;
+	case XR_E_UNKNOWN:
+	case XR_E_FREE:
+	case XR_E_FREE1:
+		set_bmap(agno, agbno, XR_E_INO);
+		break;
+	case XR_E_BAD_STATE:
+		do_error(_("bad state in block map %d\n"), state);
+		break;
+	default:
+		set_bmap(agno, agbno, XR_E_MULT);
+		do_warn(
+	_("inode block %" PRIu64 " multiply claimed, state was %d\n"),
+			XFS_AGB_TO_FSB(mp, agno, agbno), state);
+		break;
+	}
+	pthread_mutex_unlock(&ag_locks[agno].lock);
+}
 
 /*
  * processes an inode allocation chunk/block, returns 1 on I/O errors,
@@ -558,8 +593,6 @@ verify_aginode_chunk_irec(xfs_mount_t	*mp,
  *
  * *bogus is set to 1 if the entire set of inodes is bad.
  */
-
-/* ARGSUSED */
 static int
 process_inode_chunk(
 	xfs_mount_t 		*mp,
@@ -578,7 +611,6 @@ process_inode_chunk(
 	int			icnt;
 	int			status;
 	int			is_used;
-	int			state;
 	int			ino_dirty;
 	int			irec_offset;
 	int			ibuf_offset;
@@ -757,29 +789,8 @@ next_readbuf:
 	/*
 	 * mark block as an inode block in the incore bitmap
 	 */
-	if (!is_inode_sparse(ino_rec, irec_offset)) {
-		pthread_mutex_lock(&ag_locks[agno].lock);
-		state = get_bmap(agno, agbno);
-		switch (state) {
-		case XR_E_INO:	/* already marked */
-			break;
-		case XR_E_UNKNOWN:
-		case XR_E_FREE:
-		case XR_E_FREE1:
-			set_bmap(agno, agbno, XR_E_INO);
-			break;
-		case XR_E_BAD_STATE:
-			do_error(_("bad state in block map %d\n"), state);
-			break;
-		default:
-			set_bmap(agno, agbno, XR_E_MULT);
-			do_warn(
-		_("inode block %" PRIu64 " multiply claimed, state was %d\n"),
-				XFS_AGB_TO_FSB(mp, agno, agbno), state);
-			break;
-		}
-		pthread_mutex_unlock(&ag_locks[agno].lock);
-	}
+	if (!is_inode_sparse(ino_rec, irec_offset))
+		process_inode_agbno_state(mp, agno, agbno);
 
 	for (;;) {
 		agino = irec_offset + ino_rec->ino_startnum;
@@ -956,32 +967,8 @@ process_next:
 			ibuf_offset = 0;
 			agbno++;
 
-			if (!is_inode_sparse(ino_rec, irec_offset)) {
-				pthread_mutex_lock(&ag_locks[agno].lock);
-				state = get_bmap(agno, agbno);
-				switch (state) {
-				case XR_E_INO:	/* already marked */
-					break;
-				case XR_E_UNKNOWN:
-				case XR_E_FREE:
-				case XR_E_FREE1:
-					set_bmap(agno, agbno, XR_E_INO);
-					break;
-				case XR_E_BAD_STATE:
-					do_error(
-					_("bad state in block map %d\n"),
-						state);
-					break;
-				default:
-					set_bmap(agno, agbno, XR_E_MULT);
-					do_warn(
-		_("inode block %" PRIu64 " multiply claimed, state was %d\n"),
-						XFS_AGB_TO_FSB(mp, agno, agbno),
-						state);
-					break;
-				}
-				pthread_mutex_unlock(&ag_locks[agno].lock);
-			}
+			if (!is_inode_sparse(ino_rec, irec_offset))
+				process_inode_agbno_state(mp, agno, agbno);
 		} else if (irec_offset == XFS_INODES_PER_CHUNK)  {
 			/*
 			 * get new irec (multiple chunks per block fs)
-- 
1.9.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs