[PATCH 00/12] xfs_repair queue

[PATCH 00/12] xfs_repair queue

[Christoph Hellwig]

This is my current repair queue.  Many patches have been sent before,
either formally or in reply to buf reports.  Most of it are fixes
for arekm's corrupted filesystem from hell (and two fixes for it are
still missing, but I'm looking into it), two are for other bug reports
and the last three are cleanups for things I noticed while working on
the rest.

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

[PATCH 01/12] repair: do not walk the unlinked inode list

[Christoph Hellwig]

[-- Attachment #1: repair-skip-unlinked --]
[-- Type: text/plain, Size: 5034 bytes --]

Stefan Pfetzing reported a bug where xfs_repair got stuck eating 100% CPU in
phase3.  We track it down to a loop in the unlinked inode list, apparently
caused by memory corruption on an iSCSI target.

I looked into tracking if we already saw a given unlinked inode, but given
that we keep walking even for inodes where we can't find an allocation btree
record that seems infeasible.  On the other hand these inodes had their
final unlink and thus were dead even before the system went down.  There
really is no point in adding them to the uncertain list and looking for
references to them later.

So the simplest fix seems to be to simply remove the unlinked inode list
walk and just clear it - when we rebuild the inode allocation btrees these
will simply be marked free.

Reported-by: Stefan Pfetzing <stefan.pfetzing@1und1.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfsprogs-dev/repair/phase3.c
===================================================================
--- xfsprogs-dev.orig/repair/phase3.c	2011-11-09 08:59:35.313604606 +0100
+++ xfsprogs-dev/repair/phase3.c	2011-11-09 09:03:44.800605237 +0100
@@ -28,80 +28,15 @@
 #include "progress.h"
 #include "prefetch.h"
 
-/*
- * walks an unlinked list, returns 1 on an error (bogus pointer) or
- * I/O error
- */
-int
-walk_unlinked_list(xfs_mount_t *mp, xfs_agnumber_t agno, xfs_agino_t start_ino)
-{
-	xfs_buf_t *bp;
-	xfs_dinode_t *dip;
-	xfs_agino_t current_ino = start_ino;
-	xfs_agblock_t agbno;
-	int state;
-
-	while (current_ino != NULLAGINO)  {
-		if (verify_aginum(mp, agno, current_ino))
-			return(1);
-		if ((bp = get_agino_buf(mp, agno, current_ino, &dip)) == NULL)
-			return(1);
-		/*
-		 * if this looks like a decent inode, then continue
-		 * following the unlinked pointers.  If not, bail.
-		 */
-		if (verify_dinode(mp, dip, agno, current_ino) == 0)  {
-			/*
-			 * check if the unlinked list points to an unknown
-			 * inode.  if so, put it on the uncertain inode list
-			 * and set block map appropriately.
-			 */
-			if (find_inode_rec(mp, agno, current_ino) == NULL)  {
-				add_aginode_uncertain(agno, current_ino, 1);
-				agbno = XFS_AGINO_TO_AGBNO(mp, current_ino);
-
-				pthread_mutex_lock(&ag_locks[agno]);
-				state = get_bmap(agno, agbno);
-				switch (state) {
-				case XR_E_BAD_STATE:
-					do_error(_(
-						"bad state in block map %d\n"),
-						state);
-					break;
-				default:
-					/*
-					 * the block looks like inodes
-					 * so be conservative and try
-					 * to scavenge what's in there.
-					 * if what's there is completely
-					 * bogus, it'll show up later
-					 * and the inode will be trashed
-					 * anyway, hopefully without
-					 * losing too much other data
-					 */
-					set_bmap(agno, agbno, XR_E_INO);
-					break;
-				}
-				pthread_mutex_unlock(&ag_locks[agno]);
-			}
-			current_ino = be32_to_cpu(dip->di_next_unlinked);
-		} else  {
-			current_ino = NULLAGINO;;
-		}
-		libxfs_putbuf(bp);
-	}
-
-	return(0);
-}
-
-void
-process_agi_unlinked(xfs_mount_t *mp, xfs_agnumber_t agno)
+static void
+process_agi_unlinked(
+	struct xfs_mount	*mp,
+	xfs_agnumber_t		agno)
 {
-	xfs_agnumber_t i;
-	xfs_buf_t *bp;
-	xfs_agi_t *agip;
-	int err = 0;
-	int agi_dirty = 0;
+	struct xfs_buf		*bp;
+	struct xfs_agi		*agip;
+	xfs_agnumber_t		i;
+	int			agi_dirty = 0;
 
 	bp = libxfs_readbuf(mp->m_dev,
 			XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
@@ -112,28 +47,16 @@ process_agi_unlinked(xfs_mount_t *mp, xf
 
 	agip = XFS_BUF_TO_AGI(bp);
 
-	ASSERT(no_modify || be32_to_cpu(agip->agi_seqno) == agno);
+	ASSERT(be32_to_cpu(agip->agi_seqno) == agno);
 
 	for (i = 0; i < XFS_AGI_UNLINKED_BUCKETS; i++)  {
-		if (be32_to_cpu(agip->agi_unlinked[i]) != NULLAGINO)  {
-			err += walk_unlinked_list(mp, agno,
-					be32_to_cpu(agip->agi_unlinked[i]));
-			/*
-			 * clear the list
-			 */
-			if (!no_modify)  {
-				agip->agi_unlinked[i] = cpu_to_be32(NULLAGINO);
-				agi_dirty = 1;
-			}
+		if (agip->agi_unlinked[i] != cpu_to_be32(NULLAGINO)) {
+			agip->agi_unlinked[i] = cpu_to_be32(NULLAGINO);
+			agi_dirty = 1;
 		}
 	}
 
-	if (err)
-		do_warn(_("error following ag %d unlinked list\n"), agno);
-
-	ASSERT(agi_dirty == 0 || (agi_dirty && !no_modify));
-
-	if (agi_dirty && !no_modify)
+	if (agi_dirty)
 		libxfs_writebuf(bp, 0);
 	else
 		libxfs_putbuf(bp);
@@ -209,14 +132,14 @@ phase3(xfs_mount_t *mp)
 
 	set_progress_msg(PROG_FMT_AGI_UNLINKED, (__uint64_t) glob_agcount);
 
-	/*
-	 * first, let's look at the possibly bogus inodes
-	 */
+	/* first clear the agi unlinked AGI list */
+	if (!no_modify) {
+		for (i = 0; i < mp->m_sb.sb_agcount; i++)
+			process_agi_unlinked(mp, i);
+	}
+
+	/* now look at possibly bogus inodes */
 	for (i = 0; i < mp->m_sb.sb_agcount; i++)  {
-		/*
-		 * walk unlinked list to add more potential inodes to list
-		 */
-		process_agi_unlinked(mp, i);
 		check_uncertain_aginodes(mp, i);
 		PROG_RPT_INC(prog_rpt_done[i], 1);
 	}

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

Re: [PATCH 01/12] repair: do not walk the unlinked inode list

[Dave Chinner]

On Fri, Dec 02, 2011 at 12:46:20PM -0500, Christoph Hellwig wrote:
> Stefan Pfetzing reported a bug where xfs_repair got stuck eating 100% CPU in
> phase3.  We track it down to a loop in the unlinked inode list, apparently
> caused by memory corruption on an iSCSI target.
> 
> I looked into tracking if we already saw a given unlinked inode, but given
> that we keep walking even for inodes where we can't find an allocation btree
> record that seems infeasible.  On the other hand these inodes had their
> final unlink and thus were dead even before the system went down.  There
> really is no point in adding them to the uncertain list and looking for
> references to them later.
> 
> So the simplest fix seems to be to simply remove the unlinked inode list
> walk and just clear it - when we rebuild the inode allocation btrees these
> will simply be marked free.
> 
> Reported-by: Stefan Pfetzing <stefan.pfetzing@1und1.de>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

After further consideration, this is probably fine as all the
unlinked inodes do get picked up elsewhere.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH 01/12] repair: do not walk the unlinked inode list

[Mark Tinguely]

On 01/-10/63 13:59, Christoph Hellwig wrote:
> Stefan Pfetzing reported a bug where xfs_repair got stuck eating 100% CPU in
> phase3.  We track it down to a loop in the unlinked inode list, apparently
> caused by memory corruption on an iSCSI target.
>
> I looked into tracking if we already saw a given unlinked inode, but given
> that we keep walking even for inodes where we can't find an allocation btree
> record that seems infeasible.  On the other hand these inodes had their
> final unlink and thus were dead even before the system went down.  There
> really is no point in adding them to the uncertain list and looking for
> references to them later.
>
> So the simplest fix seems to be to simply remove the unlinked inode list
> walk and just clear it - when we rebuild the inode allocation btrees these
> will simply be marked free.


Makes sense to me.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

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

[PATCH 02/12] repair: allocate and free inode records individually

[Christoph Hellwig]

[-- Attachment #1: repair-free-inode-chunk-node --]
[-- Type: text/plain, Size: 8962 bytes --]

Instead of allocating inode records in chunks and keeping a freelist of them
which never gets released to the system memory allocator use plain malloc
and free for them.  The freelist just means adding a global lock instead
of relying on malloc and free which could be implemented lockless, and the
freelist is almost completely worthless as we are done allocating new
inode records once we start freeing them in major quantities.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfsprogs-dev/repair/incore_ino.c
===================================================================
--- xfsprogs-dev.orig/repair/incore_ino.c	2011-11-09 18:52:15.041861085 +0000
+++ xfsprogs-dev/repair/incore_ino.c	2011-11-09 19:36:39.389806408 +0000
@@ -25,7 +25,6 @@
 #include "threads.h"
 #include "err_protos.h"
 
-static pthread_mutex_t	ino_flist_lock;
 extern avlnode_t	*avl_firstino(avlnode_t *root);
 
 /*
@@ -38,18 +37,6 @@
  */
 static avltree_desc_t	**inode_uncertain_tree_ptrs;
 
-#define ALLOC_NUM_INOS		100
-
-/* free lists -- inode nodes and extent nodes */
-
-typedef struct ino_flist_s  {
-	ino_tree_node_t		*list;
-	ino_tree_node_t		*last;
-	long long		cnt;
-} ino_flist_t;
-
-static ino_flist_t ino_flist;	/* free list must be initialized before use */
-
 /* memory optimised nlink counting for all inodes */
 
 static void nlink_grow_8_to_16(ino_tree_node_t *irec);
@@ -238,102 +225,63 @@
 }
 
 /*
- * next is the uncertain inode list -- a sorted (in ascending order)
+ * Next is the uncertain inode list -- a sorted (in ascending order)
  * list of inode records sorted on the starting inode number.  There
  * is one list per ag.
  */
 
 /*
- * common code for creating inode records for use by trees and lists.
+ * Common code for creating inode records for use by trees and lists.
  * called only from add_inodes and add_inodes_uncertain
  *
  * IMPORTANT:  all inodes (inode records) start off as free and
  *		unconfirmed.
  */
-/* ARGSUSED */
-static ino_tree_node_t *
-mk_ino_tree_nodes(
+static struct ino_tree_node *
+alloc_ino_node(
 	xfs_agino_t		starting_ino)
 {
-	int 			i;
-	ino_tree_node_t 	*ino_rec;
-	avlnode_t 		*node;
-
-	pthread_mutex_lock(&ino_flist_lock);
-	if (ino_flist.cnt == 0)  {
-		ASSERT(ino_flist.list == NULL);
-
-		if ((ino_rec = malloc(sizeof(ino_tree_node_t[ALLOC_NUM_INOS])))
-					== NULL)
-			do_error(_("inode map malloc failed\n"));
-
-		for (i = 0; i < ALLOC_NUM_INOS; i++)  {
-			ino_rec->avl_node.avl_nextino =
-				(avlnode_t *) ino_flist.list;
-			ino_flist.list = ino_rec;
-			ino_flist.cnt++;
-			ino_rec++;
-		}
-	}
+	struct ino_tree_node 	*irec;
 
-	ASSERT(ino_flist.list != NULL);
-
-	ino_rec = ino_flist.list;
-	ino_flist.list = (ino_tree_node_t *) ino_rec->avl_node.avl_nextino;
-	ino_flist.cnt--;
-	node = &ino_rec->avl_node;
-	node->avl_nextino = node->avl_forw = node->avl_back = NULL;
-	pthread_mutex_unlock(&ino_flist_lock);
-
-	/* initialize node */
-
-	ino_rec->ino_startnum = 0;
-	ino_rec->ino_confirmed = 0;
-	ino_rec->ino_isa_dir = 0;
-	ino_rec->ir_free = (xfs_inofree_t) - 1;
-	ino_rec->ino_un.ex_data = NULL;
-	ino_rec->nlinkops = &nlinkops[0];
-	ino_rec->disk_nlinks = calloc(1, nlinkops[0].nlink_size);
-	if (ino_rec->disk_nlinks == NULL)
+	irec = malloc(sizeof(*irec));
+	if (!irec)
+		do_error(_("inode map malloc failed\n"));
+
+	irec->avl_node.avl_nextino = NULL;
+	irec->avl_node.avl_forw = NULL;
+	irec->avl_node.avl_back = NULL;
+
+	irec->ino_startnum = starting_ino;
+	irec->ino_confirmed = 0;
+	irec->ino_isa_dir = 0;
+	irec->ir_free = (xfs_inofree_t) - 1;
+	irec->ino_un.ex_data = NULL;
+	irec->nlinkops = &nlinkops[0];
+	irec->disk_nlinks = calloc(1, nlinkops[0].nlink_size);
+	if (!irec->disk_nlinks)
 		do_error(_("could not allocate nlink array\n"));
-
-	return(ino_rec);
+	return irec;
 }
 
-/*
- * return inode record to free list, will be initialized when
- * it gets pulled off list
- */
 static void
-free_ino_tree_node(ino_tree_node_t *ino_rec)
+free_ino_tree_node(
+	struct ino_tree_node	*irec)
 {
-	ino_rec->avl_node.avl_nextino = NULL;
-	ino_rec->avl_node.avl_forw = NULL;
-	ino_rec->avl_node.avl_back = NULL;
-
-	pthread_mutex_lock(&ino_flist_lock);
-	if (ino_flist.list != NULL)  {
-		ASSERT(ino_flist.cnt > 0);
-		ino_rec->avl_node.avl_nextino = (avlnode_t *) ino_flist.list;
-	} else  {
-		ASSERT(ino_flist.cnt == 0);
-		ino_rec->avl_node.avl_nextino = NULL;
-	}
+	irec->avl_node.avl_nextino = NULL;
+	irec->avl_node.avl_forw = NULL;
+	irec->avl_node.avl_back = NULL;
 
-	ino_flist.list = ino_rec;
-	ino_flist.cnt++;
-
-	free(ino_rec->disk_nlinks);
-
-	if (ino_rec->ino_un.ex_data != NULL)  {
+	free(irec->disk_nlinks);
+	if (irec->ino_un.ex_data != NULL)  {
 		if (full_ino_ex_data) {
-			free(ino_rec->ino_un.ex_data->parents);
-			free(ino_rec->ino_un.ex_data->counted_nlinks);
+			free(irec->ino_un.ex_data->parents);
+			free(irec->ino_un.ex_data->counted_nlinks);
 		}
-		free(ino_rec->ino_un.ex_data);
+		free(irec->ino_un.ex_data);
 
 	}
-	pthread_mutex_unlock(&ino_flist_lock);
+
+	free(irec);
 }
 
 /*
@@ -379,17 +327,15 @@
 	 * check to see if record containing inode is already in the tree.
 	 * if not, add it
 	 */
-	if ((ino_rec = (ino_tree_node_t *)
-			avl_findrange(inode_uncertain_tree_ptrs[agno],
-				s_ino)) == NULL)  {
-		ino_rec = mk_ino_tree_nodes(s_ino);
-		ino_rec->ino_startnum = s_ino;
-
-		if (avl_insert(inode_uncertain_tree_ptrs[agno],
-				(avlnode_t *) ino_rec) == NULL)  {
-			do_error(_("add_aginode_uncertain - "
-				   "duplicate inode range\n"));
-		}
+	ino_rec = (ino_tree_node_t *)
+		avl_findrange(inode_uncertain_tree_ptrs[agno], s_ino);
+	if (!ino_rec) {
+		ino_rec = alloc_ino_node(s_ino);
+
+		if (!avl_insert(inode_uncertain_tree_ptrs[agno],
+				&ino_rec->avl_node))
+			do_error(
+	_("add_aginode_uncertain - duplicate inode range\n"));
 	}
 
 	if (free)
@@ -454,43 +400,38 @@
 
 
 /*
- * next comes the inode trees.  One per ag.  AVL trees
- * of inode records, each inode record tracking 64 inodes
+ * Next comes the inode trees.  One per AG,  AVL trees of inode records, each
+ * inode record tracking 64 inodes
  */
+
 /*
- * set up an inode tree record for a group of inodes that will
- * include the requested inode.
- *
- * does NOT error-check for duplicate records.  Caller is
- * responsible for checking that.
+ * Set up an inode tree record for a group of inodes that will include the
+ * requested inode.
  *
- * ino must be the start of an XFS_INODES_PER_CHUNK (64) inode chunk
+ * This does NOT do error-check for duplicate records.  The caller is
+ * responsible for checking that. Ino must be the start of an
+ * XFS_INODES_PER_CHUNK (64) inode chunk
  *
- * Each inode resides in a 64-inode chunk which can be part
- * one or more chunks (MAX(64, inodes-per-block).  The fs allocates
- * in chunks (as opposed to 1 chunk) when a block can hold more than
- * one chunk (inodes per block > 64).  Allocating in one chunk pieces
- * causes us problems when it takes more than one fs block to contain
- * an inode chunk because the chunks can start on *any* block boundary.
- * So we assume that the caller has a clue because at this level, we
- * don't.
- */
-static ino_tree_node_t *
-add_inode(struct xfs_mount *mp, xfs_agnumber_t agno, xfs_agino_t ino)
+ * Each inode resides in a 64-inode chunk which can be part one or more chunks
+ * (MAX(64, inodes-per-block).  The fs allocates in chunks (as opposed to 1
+ * chunk) when a block can hold more than one chunk (inodes per block > 64).
+ * Allocating in one chunk pieces causes us problems when it takes more than
+ * one fs block to contain an inode chunk because the chunks can start on
+ * *any* block boundary. So we assume that the caller has a clue because at
+ * this level, we don't.
+ */
+static struct ino_tree_node *
+add_inode(
+	struct xfs_mount	*mp,
+	xfs_agnumber_t		agno,
+	xfs_agino_t		agino)
 {
-	ino_tree_node_t *ino_rec;
-
-	/* no record exists, make some and put them into the tree */
-
-	ino_rec = mk_ino_tree_nodes(ino);
-	ino_rec->ino_startnum = ino;
+	struct ino_tree_node	*irec;
 
-	if (avl_insert(inode_tree_ptrs[agno],
-			(avlnode_t *) ino_rec) == NULL)  {
+	irec = alloc_ino_node(agino);
+	if (!avl_insert(inode_tree_ptrs[agno],	&irec->avl_node))
 		do_warn(_("add_inode - duplicate inode range\n"));
-	}
-
-	return(ino_rec);
+	return irec;
 }
 
 /*
@@ -816,7 +757,6 @@
 	int i;
 	int agcount = mp->m_sb.sb_agcount;
 
-	pthread_mutex_init(&ino_flist_lock, NULL);
 	if ((inode_tree_ptrs = malloc(agcount *
 					sizeof(avltree_desc_t *))) == NULL)
 		do_error(_("couldn't malloc inode tree descriptor table\n"));
@@ -839,9 +779,6 @@
 		avl_init_tree(inode_uncertain_tree_ptrs[i], &avl_ino_tree_ops);
 	}
 
-	ino_flist.cnt = 0;
-	ino_flist.list = NULL;
-
 	if ((last_rec = malloc(sizeof(ino_tree_node_t *) * agcount)) == NULL)
 		do_error(_("couldn't malloc uncertain inode cache area\n"));
 

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

Re: [PATCH 02/12] repair: allocate and free inode records individually

[Dave Chinner]

On Fri, Dec 02, 2011 at 12:46:21PM -0500, Christoph Hellwig wrote:
> Instead of allocating inode records in chunks and keeping a freelist of them
> which never gets released to the system memory allocator use plain malloc
> and free for them.  The freelist just means adding a global lock instead
> of relying on malloc and free which could be implemented lockless, and the
> freelist is almost completely worthless as we are done allocating new
> inode records once we start freeing them in major quantities.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

OK, the freelist has been there since at least before the start of
the git tree history, but the locking was added as part of the
threading of repair back in 2007....

It looks to me like the allocation strategy is similar to the way
log tickets used to be allocated - a roll-your-own slab cache to
work around an inefficient memory allocator....

.....
> -	/* initialize node */
> -
> -	ino_rec->ino_startnum = 0;
> -	ino_rec->ino_confirmed = 0;
> -	ino_rec->ino_isa_dir = 0;
> -	ino_rec->ir_free = (xfs_inofree_t) - 1;
> -	ino_rec->ino_un.ex_data = NULL;
> -	ino_rec->nlinkops = &nlinkops[0];
> -	ino_rec->disk_nlinks = calloc(1, nlinkops[0].nlink_size);
> -	if (ino_rec->disk_nlinks == NULL)
> +	irec = malloc(sizeof(*irec));
> +	if (!irec)
> +		do_error(_("inode map malloc failed\n"));
> +
> +	irec->avl_node.avl_nextino = NULL;
> +	irec->avl_node.avl_forw = NULL;
> +	irec->avl_node.avl_back = NULL;
> +
> +	irec->ino_startnum = starting_ino;

OK, so you moved this initialisation into the init function instead
of doing it externally. Seems OK - both callers do post-init of
this, so no reason why not to do it here.

Everything else looks fine. the renaming of mk_ino_tree_nodes()
makes it kind of obvious now what that function is doing, too, so
that is good...

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH 02/12] repair: allocate and free inode records individually

[Mark Tinguely]

On 01/-10/63 13:59, Christoph Hellwig wrote:
> Instead of allocating inode records in chunks and keeping a freelist of them
> which never gets released to the system memory allocator use plain malloc
> and free for them.  The freelist just means adding a global lock instead
> of relying on malloc and free which could be implemented lockless, and the
> freelist is almost completely worthless as we are done allocating new
> inode records once we start freeing them in major quantities.
>

Looks good. Has been running without issues.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

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

[PATCH 03/12] repair: allocate and free extent records individually

[Christoph Hellwig]

[-- Attachment #1: repair-free-extent-chunk-node --]
[-- Type: text/plain, Size: 6532 bytes --]

Instead of allocating inode records in chunks and keeping a freelist of them
which gets released to the system memory allocator in one go use plain malloc
and free for them.  The freelist just means adding a global lock instead
of relying on malloc and free which could be implemented lockless.  In
addition smart allocators like tcmalloc have far less overhead than our
chunk and linked list.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfsprogs-dev/repair/incore_ext.c
===================================================================
--- xfsprogs-dev.orig/repair/incore_ext.c	2011-11-10 14:01:04.905470023 +0000
+++ xfsprogs-dev/repair/incore_ext.c	2011-11-14 10:14:57.696692843 +0000
@@ -26,20 +26,6 @@
 #include "err_protos.h"
 #include "avl64.h"
 #include "threads.h"
-#define ALLOC_NUM_EXTS		100
-
-/*
- * paranoia -- account for any weird padding, 64/32-bit alignment, etc.
- */
-typedef struct extent_alloc_rec  {
-	struct list_head	list;
-	extent_tree_node_t	extents[ALLOC_NUM_EXTS];
-} extent_alloc_rec_t;
-
-typedef struct rt_extent_alloc_rec  {
-	struct list_head	list;
-	rt_extent_tree_node_t	extents[ALLOC_NUM_EXTS];
-} rt_extent_alloc_rec_t;
 
 /*
  * note:  there are 4 sets of incore things handled here:
@@ -57,21 +43,9 @@
  * phase 5.  The uncertain inode list goes away at the end of
  * phase 3.  The inode tree and bno/bnct trees go away after phase 5.
  */
-typedef struct ext_flist_s  {
-	extent_tree_node_t	*list;
-	int			cnt;
-} ext_flist_t;
-
-static ext_flist_t ext_flist;
-
-typedef struct rt_ext_flist_s  {
-	rt_extent_tree_node_t	*list;
-	int			cnt;
-} rt_ext_flist_t;
-
-static rt_ext_flist_t rt_ext_flist;
 
 static avl64tree_desc_t	*rt_ext_tree_ptr;	/* dup extent tree for rt */
+static pthread_mutex_t	rt_ext_tree_lock;
 
 static struct btree_root **dup_extent_trees;	/* per ag dup extent trees */
 static pthread_mutex_t *dup_extent_tree_locks;
@@ -89,19 +63,6 @@
 						 */
 
 /*
- * list of allocated "blocks" for easy freeing later
- */
-static struct list_head	ba_list;
-static struct list_head	rt_ba_list;
-
-/*
- * locks.
- */
-static pthread_mutex_t	ext_flist_lock;
-static pthread_mutex_t	rt_ext_tree_lock;
-static pthread_mutex_t	rt_ext_flist_lock;
-
-/*
  * duplicate extent tree functions
  */
 
@@ -167,60 +128,26 @@
 mk_extent_tree_nodes(xfs_agblock_t new_startblock,
 	xfs_extlen_t new_blockcount, extent_state_t new_state)
 {
-	int i;
 	extent_tree_node_t *new;
-	extent_alloc_rec_t *rec;
-
-	pthread_mutex_lock(&ext_flist_lock);
-	if (ext_flist.cnt == 0)  {
-		ASSERT(ext_flist.list == NULL);
-
-		if ((rec = malloc(sizeof(extent_alloc_rec_t))) == NULL)
-			do_error(
-			_("couldn't allocate new extent descriptors.\n"));
-
-		list_add(&rec->list, &ba_list);
-
-		new = &rec->extents[0];
 
-		for (i = 0; i < ALLOC_NUM_EXTS; i++)  {
-			new->avl_node.avl_nextino = (avlnode_t *)
-							ext_flist.list;
-			ext_flist.list = new;
-			ext_flist.cnt++;
-			new++;
-		}
-	}
-
-	ASSERT(ext_flist.list != NULL);
+	new = malloc(sizeof(*new));
+	if (!new)
+		do_error(_("couldn't allocate new extent descriptor.\n"));
 
-	new = ext_flist.list;
-	ext_flist.list = (extent_tree_node_t *) new->avl_node.avl_nextino;
-	ext_flist.cnt--;
 	new->avl_node.avl_nextino = NULL;
-	pthread_mutex_unlock(&ext_flist_lock);
-
-	/* initialize node */
-
 	new->ex_startblock = new_startblock;
 	new->ex_blockcount = new_blockcount;
 	new->ex_state = new_state;
 	new->next = NULL;
 	new->last = NULL;
 
-	return(new);
+	return new;
 }
 
 void
 release_extent_tree_node(extent_tree_node_t *node)
 {
-	pthread_mutex_lock(&ext_flist_lock);
-	node->avl_node.avl_nextino = (avlnode_t *) ext_flist.list;
-	ext_flist.list = node;
-	ext_flist.cnt++;
-	pthread_mutex_unlock(&ext_flist_lock);
-
-	return;
+	free(node);
 }
 
 /*
@@ -630,57 +557,24 @@
 mk_rt_extent_tree_nodes(xfs_drtbno_t new_startblock,
 	xfs_extlen_t new_blockcount, extent_state_t new_state)
 {
-	int i;
 	rt_extent_tree_node_t *new;
-	rt_extent_alloc_rec_t *rec;
 
-	pthread_mutex_lock(&rt_ext_flist_lock);
-	if (rt_ext_flist.cnt == 0)  {
-		ASSERT(rt_ext_flist.list == NULL);
-
-		if ((rec = malloc(sizeof(rt_extent_alloc_rec_t))) == NULL)
-			do_error(
-			_("couldn't allocate new extent descriptors.\n"));
+	new = malloc(sizeof(*new));
+	if (!new)
+		do_error(_("couldn't allocate new extent descriptor.\n"));
 
-		list_add(&rec->list, &rt_ba_list);
-
-		new = &rec->extents[0];
-
-		for (i = 0; i < ALLOC_NUM_EXTS; i++)  {
-			new->avl_node.avl_nextino = (avlnode_t *)
-							rt_ext_flist.list;
-			rt_ext_flist.list = new;
-			rt_ext_flist.cnt++;
-			new++;
-		}
-	}
-
-	ASSERT(rt_ext_flist.list != NULL);
-
-	new = rt_ext_flist.list;
-	rt_ext_flist.list = (rt_extent_tree_node_t *) new->avl_node.avl_nextino;
-	rt_ext_flist.cnt--;
 	new->avl_node.avl_nextino = NULL;
-	pthread_mutex_unlock(&rt_ext_flist_lock);
-
-	/* initialize node */
-
 	new->rt_startblock = new_startblock;
 	new->rt_blockcount = new_blockcount;
 	new->rt_state = new_state;
-
-	return(new);
+	return new;
 }
 
 #if 0
 void
 release_rt_extent_tree_node(rt_extent_tree_node_t *node)
 {
-	node->avl_node.avl_nextino = (avlnode_t *) rt_ext_flist.list;
-	rt_ext_flist.list = node;
-	rt_ext_flist.cnt++;
-
-	return;
+	free(node);
 }
 
 void
@@ -719,18 +613,9 @@
 void
 free_rt_dup_extent_tree(xfs_mount_t *mp)
 {
-	rt_extent_alloc_rec_t *cur, *tmp;
-
 	ASSERT(mp->m_sb.sb_rblocks != 0);
-
-	list_for_each_entry_safe(cur, tmp, &rt_ba_list, list)
-		free(cur);
-
 	free(rt_ext_tree_ptr);
-
 	rt_ext_tree_ptr = NULL;
-
-	return;
 }
 
 /*
@@ -862,11 +747,7 @@
 	int i;
 	xfs_agnumber_t agcount = mp->m_sb.sb_agcount;
 
-	list_head_init(&ba_list);
-	list_head_init(&rt_ba_list);
-	pthread_mutex_init(&ext_flist_lock, NULL);
 	pthread_mutex_init(&rt_ext_tree_lock, NULL);
-	pthread_mutex_init(&rt_ext_flist_lock, NULL);
 
 	dup_extent_trees = calloc(agcount, sizeof(struct btree_root *));
 	if (!dup_extent_trees)
@@ -908,11 +789,6 @@
 		do_error(_("couldn't malloc dup rt extent tree descriptor\n"));
 
 	avl64_init_tree(rt_ext_tree_ptr, &avl64_extent_tree_ops);
-
-	ext_flist.cnt = 0;
-	ext_flist.list = NULL;
-
-	return;
 }
 
 /*
@@ -921,12 +797,8 @@
 void
 incore_ext_teardown(xfs_mount_t *mp)
 {
-	extent_alloc_rec_t *cur, *tmp;
 	xfs_agnumber_t i;
 
-	list_for_each_entry_safe(cur, tmp, &ba_list, list)
-		free(cur);
-
 	for (i = 0; i < mp->m_sb.sb_agcount; i++)  {
 		btree_destroy(dup_extent_trees[i]);
 		free(extent_bno_ptrs[i]);

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

Re: [PATCH 03/12] repair: allocate and free extent records individually

[Dave Chinner]

On Fri, Dec 02, 2011 at 12:46:22PM -0500, Christoph Hellwig wrote:
> Instead of allocating inode records in chunks and keeping a freelist of them
> which gets released to the system memory allocator in one go use plain malloc
> and free for them.  The freelist just means adding a global lock instead
> of relying on malloc and free which could be implemented lockless.  In
> addition smart allocators like tcmalloc have far less overhead than our
> chunk and linked list.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Same context as the last patch - seems much better to rely on malloc
to get this right.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH 03/12] repair: allocate and free extent records individually

[Mark Tinguely]

On 01/-10/63 13:59, Christoph Hellwig wrote:
> Instead of allocating inode records in chunks and keeping a freelist of them
> which gets released to the system memory allocator in one go use plain malloc
> and free for them.  The freelist just means adding a global lock instead
> of relying on malloc and free which could be implemented lockless.  In
> addition smart allocators like tcmalloc have far less overhead than our
> chunk and linked list.

Looks good. Has been running without issues.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

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

[PATCH 04/12] xfsprogs: allow linking against libtcmalloc

[Christoph Hellwig]

[-- Attachment #1: xfsprogs-use-tcmalloc --]
[-- Type: text/plain, Size: 1965 bytes --]

Allow linking against the libtcmalloc library from Google's performance
tools, which at least for repair reduces the memory usage dramatically.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfsprogs-dev/configure.in
===================================================================
--- xfsprogs-dev.orig/configure.in	2011-11-14 13:54:28.000000000 +0000
+++ xfsprogs-dev/configure.in	2011-11-20 19:21:26.000000000 +0000
@@ -31,6 +31,26 @@ AC_ARG_ENABLE(editline,
 AC_SUBST(libeditline)
 AC_SUBST(enable_editline)
 
+AC_ARG_ENABLE(tcmalloc,
+[ --enable-tcmalloc=[yes/no] Enable tcmalloc [default=no]],,
+	enable_tcmalloc=check)
+
+if test x$enable_tcmalloc != xno; then
+    saved_CPPFLAGS="$CPPFLAGS"
+    CPPFLAGS="$CPPFLAGS -fno-builtin-malloc"
+    AC_CHECK_LIB([tcmalloc_minimal], [malloc], [libtcmalloc="-ltcmalloc_minimal"],
+        [AC_CHECK_LIB([tcmalloc], [malloc], [libtcmalloc="-ltcmalloc"], [
+         if test x$enable_tcmalloc = xyes; then
+            AC_MSG_ERROR([libtcmalloc_minimal or libtcmalloc library not found], 1)
+         fi]
+        )]
+    )
+    if test x$libtcmalloc = x; then
+        CPPFLAGS="$saved_CPPFLAGS"
+    fi
+fi
+AC_SUBST(libtcmalloc)
+
 AC_ARG_ENABLE(termcap,
 [ --enable-termcap=[yes/no] Enable terminal capabilities library [default=no]],
 	test $enable_termcap = yes && libtermcap="-ltermcap",)
Index: xfsprogs-dev/include/builddefs.in
===================================================================
--- xfsprogs-dev.orig/include/builddefs.in	2011-11-14 13:54:28.000000000 +0000
+++ xfsprogs-dev/include/builddefs.in	2011-11-14 13:57:55.000000000 +0000
@@ -22,7 +22,7 @@ _BUILDDEFS_INCLUDED_ = 1
 
 DEBUG = @debug_build@
 OPTIMIZER = @opt_build@
-MALLOCLIB = @malloc_lib@
+MALLOCLIB = @malloc_lib@ @libtcmalloc@
 LOADERFLAGS = @LDFLAGS@
 LTLDFLAGS = @LDFLAGS@
 CFLAGS = @CFLAGS@

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

Re: [PATCH 04/12] xfsprogs: allow linking against libtcmalloc

[Dave Chinner]

On Fri, Dec 02, 2011 at 12:46:23PM -0500, Christoph Hellwig wrote:
> Allow linking against the libtcmalloc library from Google's performance
> tools, which at least for repair reduces the memory usage dramatically.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: xfsprogs-dev/configure.in
> ===================================================================
> --- xfsprogs-dev.orig/configure.in	2011-11-14 13:54:28.000000000 +0000
> +++ xfsprogs-dev/configure.in	2011-11-20 19:21:26.000000000 +0000
> @@ -31,6 +31,26 @@ AC_ARG_ENABLE(editline,
>  AC_SUBST(libeditline)
>  AC_SUBST(enable_editline)
>  
> +AC_ARG_ENABLE(tcmalloc,
> +[ --enable-tcmalloc=[yes/no] Enable tcmalloc [default=no]],,
> +	enable_tcmalloc=check)
> +
> +if test x$enable_tcmalloc != xno; then

Firstly, I don't think this branch of detection code belongs here in
configure.in - m4/package_globals.m4 is where malloc libraries and
compiler flags are set. Indeed, I doubt electric fence is compatible
with tcmalloc, so those options need to be made exclusive....

> +    saved_CPPFLAGS="$CPPFLAGS"
> +    CPPFLAGS="$CPPFLAGS -fno-builtin-malloc"

This needs a comment explaining why this is necessary. Something
like:

"gcc assumes that malloc is implemented by glibc and makes
assumptions and optimisations that break glibc-external
optimisations and malloc-hook debugging. Hence we have to tell it
not to make those optimisations when using tcmalloc."

I also don't see how this "-fno-builtin-malloc" is being passed to
the build environment. There needs to be another variable exported
to add this to the CFLAGS of the build....

> +    AC_CHECK_LIB([tcmalloc_minimal], [malloc], [libtcmalloc="-ltcmalloc_minimal"],
> +        [AC_CHECK_LIB([tcmalloc], [malloc], [libtcmalloc="-ltcmalloc"], [
> +         if test x$enable_tcmalloc = xyes; then
> +            AC_MSG_ERROR([libtcmalloc_minimal or libtcmalloc library not found], 1)
> +         fi]
> +        )]
> +    )

I can't say this is my favourite way of encoding all the options.
I'd prefer something more verbose and explicit that keeps all the
malloc library options in one set of logic. That is, I'm pretty sure
electric fence won't work with tcmalloc, so those options are
multually exclusive. So perhaps putting this in
m4/package_globals.m4:

	tcmalloc=false
	saved_CPPFLAGS="$CPPFLAGS"
	if test $enable_tcmalloc != no ; then
		CPPFLAGS="$CPPFLAGS -fno-builtin-malloc"
		AC_CHECK_LIB(tcmalloc_minimal, malloc, tcmalloc=minimum, tcmalloc=false)
		AC_CHECK_LIB(tcmalloc, malloc, tcmalloc=full,)
	fi

	if test $tcmalloc = minimum ; then
		malloc_lib="-ltcmalloc_minimum"
		malloc_cflags="-fno-builtin-malloc"
	fi
	if test $tcmalloc = full ; then
		malloc_lib="-ltcmalloc"
		malloc_cflags="-fno-builtin-malloc"
	fi
	if test $tcmalloc = false ; then
		CPPFLAGS="$saved_CPPFLAGS"
		malloc_cflags=""

		# electric fence malloc debugging might be enabled
		MALLOCLIB=${MALLOCLIB:-''}	# /usr/lib/libefence.a
		malloc_lib="$MALLOCLIB"
	fi

	AC_SUBST(malloc_lib)
	AC_SUBST(malloc_cflags)

And then adding this to include/builddefs.in:

-CFLAGS = @CFLAGS@
+CFLAGS = @CFLAGS@ @malloc_cflags@

would appear to be a better way to do this....

> +    if test x$libtcmalloc = x; then
> +        CPPFLAGS="$saved_CPPFLAGS"
> +    fi
> +fi
> +AC_SUBST(libtcmalloc)
> +

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH 04/12] xfsprogs: allow linking against libtcmalloc

[Christoph Hellwig]

I'll look at all your suggestions.  It's probably going to take a while
to sort it out, and I have some more urgent repair bits pending.

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

[PATCH 05/12] repair: update extent count after zapping duplicate blocks

[Christoph Hellwig]

[-- Attachment #1: repair-fix-duplicate-block-handling --]
[-- Type: text/plain, Size: 5550 bytes --]

When we find a duplicate extent in an extern format inode we do not zap
the whole inode, but just truncate it to the point where the duplicate
extent was found.  But the current code only updates di_nblocks for the
new size, but no di_nextents/di_anextents.  In most cases this isn't noticed,
but when moving such an inode to the lost+found directoy the consistency
check in xfs_iformat trips over it.  Fix this by updating the on-disk
extent count as part of the inode repair.

Note that we zap btree format inodes with duplicate block completely
at this point, so this fix doesn't apply to them.

Reported-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
Tested-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfsprogs-dev/repair/dinode.c
===================================================================
--- xfsprogs-dev.orig/repair/dinode.c	2011-11-08 12:15:40.000000000 +0000
+++ xfsprogs-dev/repair/dinode.c	2011-11-14 12:09:54.000000000 +0000
@@ -606,7 +606,7 @@ int
 process_bmbt_reclist_int(
 	xfs_mount_t		*mp,
 	xfs_bmbt_rec_t		*rp,
-	int			numrecs,
+	int			*numrecs,
 	int			type,
 	xfs_ino_t		ino,
 	xfs_drfsbno_t		*tot,
@@ -642,7 +642,7 @@ process_bmbt_reclist_int(
 	else
 		ftype = _("regular");
 
-	for (i = 0; i < numrecs; i++) {
+	for (i = 0; i < *numrecs; i++) {
 		libxfs_bmbt_disk_get_all(rp + i, &irec);
 		if (i == 0)
 			*last_key = *first_key = irec.br_startoff;
@@ -831,6 +831,13 @@ _("illegal state %d in block map %" PRIu
 done:
 	if (locked_agno != -1)
 		pthread_mutex_unlock(&ag_locks[locked_agno]);
+
+	if (i != *numrecs) {
+		ASSERT(i < *numrecs);
+		do_warn(_("correcting nextents for inode %" PRIu64 "\n"), ino);
+		*numrecs = i;
+	}
+
 	return error;
 }
 
@@ -842,7 +849,7 @@ int
 process_bmbt_reclist(
 	xfs_mount_t		*mp,
 	xfs_bmbt_rec_t		*rp,
-	int			numrecs,
+	int			*numrecs,
 	int			type,
 	xfs_ino_t		ino,
 	xfs_drfsbno_t		*tot,
@@ -863,7 +870,7 @@ int
 scan_bmbt_reclist(
 	xfs_mount_t		*mp,
 	xfs_bmbt_rec_t		*rp,
-	int			numrecs,
+	int			*numrecs,
 	int			type,
 	xfs_ino_t		ino,
 	xfs_drfsbno_t		*tot,
@@ -1356,23 +1363,29 @@ process_exinode(
 	xfs_bmbt_rec_t		*rp;
 	xfs_dfiloff_t		first_key;
 	xfs_dfiloff_t		last_key;
+	int			numrecs;
+	int			ret;
 
 	lino = XFS_AGINO_TO_INO(mp, agno, ino);
 	rp = (xfs_bmbt_rec_t *)XFS_DFORK_PTR(dip, whichfork);
 	*tot = 0;
-	*nex = XFS_DFORK_NEXTENTS(dip, whichfork);
+	numrecs = XFS_DFORK_NEXTENTS(dip, whichfork);
+
 	/*
 	 * XXX - if we were going to fix up the btree record,
 	 * we'd do it right here.  For now, if there's a problem,
 	 * we'll bail out and presumably clear the inode.
 	 */
 	if (check_dups == 0)
-		return(process_bmbt_reclist(mp, rp, *nex, type, lino,
+		ret = process_bmbt_reclist(mp, rp, &numrecs, type, lino,
 					tot, blkmapp, &first_key, &last_key,
-					whichfork));
+					whichfork);
 	else
-		return(scan_bmbt_reclist(mp, rp, *nex, type, lino, tot,
-					whichfork));
+		ret = scan_bmbt_reclist(mp, rp, &numrecs, type, lino, tot,
+					whichfork);
+
+	*nex = numrecs;
+	return ret;
 }
 
 /*
@@ -2003,6 +2016,12 @@ process_inode_blocks_and_extents(
 	xfs_ino_t	lino,
 	int		*dirty)
 {
+	if (nblocks < nextents + anextents) {
+		do_warn(
+_("nblocks (%" PRIu64 ") smaller than nextents for inode %" PRIu64 "\n"), nblocks, lino);
+		return 1;
+	}
+
 	if (nblocks != be64_to_cpu(dino->di_nblocks))  {
 		if (!no_modify)  {
 			do_warn(
@@ -2063,6 +2082,7 @@ _("bad anextents %d for inode %" PRIu64
 				lino, anextents);
 		}
 	}
+
 	return 0;
 }
 
Index: xfsprogs-dev/repair/dinode.h
===================================================================
--- xfsprogs-dev.orig/repair/dinode.h	2010-05-10 18:16:35.000000000 +0000
+++ xfsprogs-dev/repair/dinode.h	2011-11-14 12:09:54.000000000 +0000
@@ -42,7 +42,7 @@ convert_extent(
 int
 process_bmbt_reclist(xfs_mount_t	*mp,
 		xfs_bmbt_rec_t		*rp,
-		int			numrecs,
+		int			*numrecs,
 		int			type,
 		xfs_ino_t		ino,
 		xfs_drfsbno_t		*tot,
@@ -55,7 +55,7 @@ int
 scan_bmbt_reclist(
 	xfs_mount_t		*mp,
 	xfs_bmbt_rec_t		*rp,
-	int			numrecs,
+	int			*numrecs,
 	int			type,
 	xfs_ino_t		ino,
 	xfs_drfsbno_t		*tot,
Index: xfsprogs-dev/repair/scan.c
===================================================================
--- xfsprogs-dev.orig/repair/scan.c	2011-11-10 11:23:47.000000000 +0000
+++ xfsprogs-dev/repair/scan.c	2011-11-14 12:09:54.000000000 +0000
@@ -351,12 +351,12 @@ _("inode %" PRIu64 " bad # of bmap recor
 		 * we'll bail out and presumably clear the inode.
 		 */
 		if (check_dups == 0)  {
-			err = process_bmbt_reclist(mp, rp, numrecs,
-					type, ino, tot, blkmapp,
-					&first_key, &last_key,
-					whichfork);
+			err = process_bmbt_reclist(mp, rp, &numrecs, type, ino,
+						   tot, blkmapp, &first_key,
+						   &last_key, whichfork);
 			if (err)
-				return(1);
+				return 1;
+
 			/*
 			 * check that key ordering is monotonically increasing.
 			 * if the last_key value in the cursor is set to
@@ -380,10 +380,11 @@ _("out-of-order bmap key (file offset) i
 			bm_cursor->level[level].first_key = first_key;
 			bm_cursor->level[level].last_key = last_key;
 
-			return(0);
-		} else
-			return(scan_bmbt_reclist(mp, rp, numrecs,
-						type, ino, tot, whichfork));
+			return 0;
+		} else {
+			return scan_bmbt_reclist(mp, rp, &numrecs, type, ino,
+						 tot, whichfork);
+		}
 	}
 	if (numrecs > mp->m_bmap_dmxr[1] || (isroot == 0 && numrecs <
 							mp->m_bmap_dmnr[1])) {

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

Re: [PATCH 05/12] repair: update extent count after zapping duplicate blocks

[Dave Chinner]

On Fri, Dec 02, 2011 at 12:46:24PM -0500, Christoph Hellwig wrote:
> When we find a duplicate extent in an extern format inode we do not zap
> the whole inode, but just truncate it to the point where the duplicate
> extent was found.  But the current code only updates di_nblocks for the
> new size, but no di_nextents/di_anextents.  In most cases this isn't noticed,
> but when moving such an inode to the lost+found directoy the consistency
> check in xfs_iformat trips over it.  Fix this by updating the on-disk
> extent count as part of the inode repair.
> 
> Note that we zap btree format inodes with duplicate block completely
> at this point, so this fix doesn't apply to them.
> 
> Reported-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
> Tested-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

.....

> @@ -2003,6 +2016,12 @@ process_inode_blocks_and_extents(
>  	xfs_ino_t	lino,
>  	int		*dirty)
>  {
> +	if (nblocks < nextents + anextents) {
> +		do_warn(
> +_("nblocks (%" PRIu64 ") smaller than nextents for inode %" PRIu64 "\n"), nblocks, lino);
> +		return 1;
> +	}
> +

I thikn that should be done after validating then block/extent
counts as otherwise it doesn't handle overflows.

It also took me a moment to realise the comparison was valid
- the confusion came from the fact you are comparing different units
(block count vs extent count). It didn't occur to me immediately
that this check is valid because an extent must contain one or
more blocks, so perhaps a comment is in order.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

[PATCH v2] repair: update extent count after zapping duplicate blocks

[Christoph Hellwig]

When we find a duplicate extent in an extern format inode we do not zap
the whole inode, but just truncate it to the point where the duplicate
extent was found.  But the current code only updates di_nblocks for the
new size, but no di_nextents/di_anextents.  In most cases this isn't noticed,
but when moving such an inode to the lost+found directoy the consistency
check in xfs_iformat trips over it.  Fix this by updating the on-disk
extent count as part of the inode repair.

Note that we zap btree format inodes with duplicate block completely
at this point, so this fix doesn't apply to them.

Reported-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
Tested-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
Signed-off-by: Christoph Hellwig <hch@lst.de>

---
updates since v2:
  - address review comments from Dave

Index: xfsprogs-dev/repair/dinode.c
===================================================================
--- xfsprogs-dev.orig/repair/dinode.c	2012-01-11 11:23:30.000000000 +0000
+++ xfsprogs-dev/repair/dinode.c	2012-02-02 11:19:43.000000000 +0000
@@ -606,7 +606,7 @@ int
 process_bmbt_reclist_int(
 	xfs_mount_t		*mp,
 	xfs_bmbt_rec_t		*rp,
-	int			numrecs,
+	int			*numrecs,
 	int			type,
 	xfs_ino_t		ino,
 	xfs_drfsbno_t		*tot,
@@ -642,7 +642,7 @@ process_bmbt_reclist_int(
 	else
 		ftype = _("regular");
 
-	for (i = 0; i < numrecs; i++) {
+	for (i = 0; i < *numrecs; i++) {
 		libxfs_bmbt_disk_get_all(rp + i, &irec);
 		if (i == 0)
 			*last_key = *first_key = irec.br_startoff;
@@ -831,6 +831,13 @@ _("illegal state %d in block map %" PRIu
 done:
 	if (locked_agno != -1)
 		pthread_mutex_unlock(&ag_locks[locked_agno]);
+
+	if (i != *numrecs) {
+		ASSERT(i < *numrecs);
+		do_warn(_("correcting nextents for inode %" PRIu64 "\n"), ino);
+		*numrecs = i;
+	}
+
 	return error;
 }
 
@@ -842,7 +849,7 @@ int
 process_bmbt_reclist(
 	xfs_mount_t		*mp,
 	xfs_bmbt_rec_t		*rp,
-	int			numrecs,
+	int			*numrecs,
 	int			type,
 	xfs_ino_t		ino,
 	xfs_drfsbno_t		*tot,
@@ -863,7 +870,7 @@ int
 scan_bmbt_reclist(
 	xfs_mount_t		*mp,
 	xfs_bmbt_rec_t		*rp,
-	int			numrecs,
+	int			*numrecs,
 	int			type,
 	xfs_ino_t		ino,
 	xfs_drfsbno_t		*tot,
@@ -1356,23 +1363,29 @@ process_exinode(
 	xfs_bmbt_rec_t		*rp;
 	xfs_dfiloff_t		first_key;
 	xfs_dfiloff_t		last_key;
+	int			numrecs;
+	int			ret;
 
 	lino = XFS_AGINO_TO_INO(mp, agno, ino);
 	rp = (xfs_bmbt_rec_t *)XFS_DFORK_PTR(dip, whichfork);
 	*tot = 0;
-	*nex = XFS_DFORK_NEXTENTS(dip, whichfork);
+	numrecs = XFS_DFORK_NEXTENTS(dip, whichfork);
+
 	/*
 	 * XXX - if we were going to fix up the btree record,
 	 * we'd do it right here.  For now, if there's a problem,
 	 * we'll bail out and presumably clear the inode.
 	 */
 	if (check_dups == 0)
-		return(process_bmbt_reclist(mp, rp, *nex, type, lino,
+		ret = process_bmbt_reclist(mp, rp, &numrecs, type, lino,
 					tot, blkmapp, &first_key, &last_key,
-					whichfork));
+					whichfork);
 	else
-		return(scan_bmbt_reclist(mp, rp, *nex, type, lino, tot,
-					whichfork));
+		ret = scan_bmbt_reclist(mp, rp, &numrecs, type, lino, tot,
+					whichfork);
+
+	*nex = numrecs;
+	return ret;
 }
 
 /*
@@ -2063,6 +2076,17 @@ _("bad anextents %d for inode %" PRIu64
 				lino, anextents);
 		}
 	}
+
+	/*
+	 * We are comparing different units here, but that's fine given that
+	 * an extent has to have at least a block in it.
+	 */
+	if (nblocks < nextents + anextents) {
+		do_warn(
+_("nblocks (%" PRIu64 ") smaller than nextents for inode %" PRIu64 "\n"), nblocks, lino);
+		return 1;
+	}
+
 	return 0;
 }
 
Index: xfsprogs-dev/repair/dinode.h
===================================================================
--- xfsprogs-dev.orig/repair/dinode.h	2012-01-11 11:23:30.000000000 +0000
+++ xfsprogs-dev/repair/dinode.h	2012-02-02 11:17:38.000000000 +0000
@@ -42,7 +42,7 @@ convert_extent(
 int
 process_bmbt_reclist(xfs_mount_t	*mp,
 		xfs_bmbt_rec_t		*rp,
-		int			numrecs,
+		int			*numrecs,
 		int			type,
 		xfs_ino_t		ino,
 		xfs_drfsbno_t		*tot,
@@ -55,7 +55,7 @@ int
 scan_bmbt_reclist(
 	xfs_mount_t		*mp,
 	xfs_bmbt_rec_t		*rp,
-	int			numrecs,
+	int			*numrecs,
 	int			type,
 	xfs_ino_t		ino,
 	xfs_drfsbno_t		*tot,
Index: xfsprogs-dev/repair/scan.c
===================================================================
--- xfsprogs-dev.orig/repair/scan.c	2012-01-11 11:23:30.000000000 +0000
+++ xfsprogs-dev/repair/scan.c	2012-02-02 11:17:38.000000000 +0000
@@ -351,12 +351,12 @@ _("inode %" PRIu64 " bad # of bmap recor
 		 * we'll bail out and presumably clear the inode.
 		 */
 		if (check_dups == 0)  {
-			err = process_bmbt_reclist(mp, rp, numrecs,
-					type, ino, tot, blkmapp,
-					&first_key, &last_key,
-					whichfork);
+			err = process_bmbt_reclist(mp, rp, &numrecs, type, ino,
+						   tot, blkmapp, &first_key,
+						   &last_key, whichfork);
 			if (err)
-				return(1);
+				return 1;
+
 			/*
 			 * check that key ordering is monotonically increasing.
 			 * if the last_key value in the cursor is set to
@@ -380,10 +380,11 @@ _("out-of-order bmap key (file offset) i
 			bm_cursor->level[level].first_key = first_key;
 			bm_cursor->level[level].last_key = last_key;
 
-			return(0);
-		} else
-			return(scan_bmbt_reclist(mp, rp, numrecs,
-						type, ino, tot, whichfork));
+			return 0;
+		} else {
+			return scan_bmbt_reclist(mp, rp, &numrecs, type, ino,
+						 tot, whichfork);
+		}
 	}
 	if (numrecs > mp->m_bmap_dmxr[1] || (isroot == 0 && numrecs <
 							mp->m_bmap_dmnr[1])) {

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

Re: [PATCH v2] repair: update extent count after zapping duplicate blocks

[Mark Tinguely]

On 02/02/12 06:39, Christoph Hellwig wrote:
> When we find a duplicate extent in an extern format inode we do not zap
> the whole inode, but just truncate it to the point where the duplicate
> extent was found.  But the current code only updates di_nblocks for the
> new size, but no di_nextents/di_anextents.  In most cases this isn't noticed,
> but when moving such an inode to the lost+found directoy the consistency
> check in xfs_iformat trips over it.  Fix this by updating the on-disk
> extent count as part of the inode repair.


Looks good.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

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

Re: [PATCH 05/12] repair: update extent count after zapping duplicate blocks

[Mark Tinguely]

On 01/-10/63 13:59, Christoph Hellwig wrote:
> When we find a duplicate extent in an extern format inode we do not zap
> the whole inode, but just truncate it to the point where the duplicate
> extent was found.  But the current code only updates di_nblocks for the
> new size, but no di_nextents/di_anextents.  In most cases this isn't noticed,
> but when moving such an inode to the lost+found directoy the consistency
> check in xfs_iformat trips over it.  Fix this by updating the on-disk
> extent count as part of the inode repair.
>
> Note that we zap btree format inodes with duplicate block completely
> at this point, so this fix doesn't apply to them.
>
> Reported-by: Arkadiusz Mi??kiewicz<arekm@maven.pl>
> Tested-by: Arkadiusz Mi??kiewicz<arekm@maven.pl>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
>
> Index: xfsprogs-dev/repair/dinode.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/dinode.c	2011-11-08 12:15:40.000000000 +0000
> +++ xfsprogs-dev/repair/dinode.c	2011-11-14 12:09:54.000000000 +0000

> @@ -2003,6 +2016,12 @@ process_inode_blocks_and_extents(
>   	xfs_ino_t	lino,
>   	int		*dirty)
>   {
> +	if (nblocks<  nextents + anextents) {
> +		do_warn(
> +_("nblocks (%" PRIu64 ") smaller than nextents for inode %" PRIu64 "\n"), nblocks, lino);
> +		return 1;
> +	}
> +

I agree with David on an inserted comment and relocation. I would not 
have figured out this test without David's observation.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

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

[PATCH 06/12] repair: use recursive buffer locking

[Christoph Hellwig]

[-- Attachment #1: repair-fix-recursion-deadlocks --]
[-- Type: text/plain, Size: 2995 bytes --]

On a sufficiently corrupt filesystem walking the btree nodes might hit the
same node node again, which currently will deadlock.  Use a recursion
counter to avoid the direct deadlock and let them normal loop detection
(two bad nodes and out) do its work.  This is how repair behaved before
we added the lock when implementing buffer prefetching.

Reported-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
Tested-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfsprogs-dev/include/libxfs.h
===================================================================
--- xfsprogs-dev.orig/include/libxfs.h	2011-11-22 22:28:23.000000000 +0000
+++ xfsprogs-dev/include/libxfs.h	2011-11-22 22:34:27.000000000 +0000
@@ -226,6 +226,8 @@ typedef struct xfs_buf {
 	unsigned		b_bcount;
 	dev_t			b_dev;
 	pthread_mutex_t		b_lock;
+	pthread_t		b_holder;
+	unsigned int		b_recur;
 	void			*b_fsprivate;
 	void			*b_fsprivate2;
 	void			*b_fsprivate3;
Index: xfsprogs-dev/libxfs/rdwr.c
===================================================================
--- xfsprogs-dev.orig/libxfs/rdwr.c	2011-11-22 22:28:23.000000000 +0000
+++ xfsprogs-dev/libxfs/rdwr.c	2011-11-22 22:40:01.000000000 +0000
@@ -342,6 +342,8 @@ libxfs_initbuf(xfs_buf_t *bp, dev_t devi
 	list_head_init(&bp->b_lock_list);
 #endif
 	pthread_mutex_init(&bp->b_lock, NULL);
+	bp->b_holder = 0;
+	bp->b_recur = 0;
 }
 
 xfs_buf_t *
@@ -410,18 +412,24 @@ libxfs_getbuf_flags(dev_t device, xfs_da
 		return NULL;
 
 	if (use_xfs_buf_lock) {
-		if (flags & LIBXFS_GETBUF_TRYLOCK) {
-			int ret;
+		int ret;
 
-			ret = pthread_mutex_trylock(&bp->b_lock);
-			if (ret) {
-				ASSERT(ret == EAGAIN);
-				cache_node_put(libxfs_bcache, (struct cache_node *)bp);
-				return NULL;
+		ret = pthread_mutex_trylock(&bp->b_lock);
+		if (ret) {
+			ASSERT(ret == EAGAIN);
+			if (flags & LIBXFS_GETBUF_TRYLOCK)
+				goto out_put;
+
+			if (pthread_equal(bp->b_holder, pthread_self())) {
+				fprintf(stderr,
+	_("recursive buffer locking detected\n"));
+				bp->b_recur++;
+			} else {
+				pthread_mutex_lock(&bp->b_lock);
 			}
-		} else {
-			pthread_mutex_lock(&bp->b_lock);
 		}
+
+		bp->b_holder = pthread_self();
 	}
 
 	cache_node_set_priority(libxfs_bcache, (struct cache_node *)bp,
@@ -440,6 +448,9 @@ libxfs_getbuf_flags(dev_t device, xfs_da
 #endif
 
 	return bp;
+out_put:
+	cache_node_put(libxfs_bcache, (struct cache_node *)bp);
+	return NULL;
 }
 
 struct xfs_buf *
@@ -458,8 +469,14 @@ libxfs_putbuf(xfs_buf_t *bp)
 	list_del_init(&bp->b_lock_list);
 	pthread_mutex_unlock(&libxfs_bcache->c_mutex);
 #endif
-	if (use_xfs_buf_lock)
-		pthread_mutex_unlock(&bp->b_lock);
+	if (use_xfs_buf_lock) {
+		if (bp->b_recur) {
+			bp->b_recur--;
+		} else {
+			bp->b_holder = 0;
+			pthread_mutex_unlock(&bp->b_lock);
+		}
+	}
 	cache_node_put(libxfs_bcache, (struct cache_node *)bp);
 }
 

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

Re: [PATCH 06/12] repair: use recursive buffer locking

[Dave Chinner]

On Fri, Dec 02, 2011 at 12:46:25PM -0500, Christoph Hellwig wrote:
> On a sufficiently corrupt filesystem walking the btree nodes might hit the
> same node node again, which currently will deadlock.  Use a recursion
> counter to avoid the direct deadlock and let them normal loop detection
> (two bad nodes and out) do its work.  This is how repair behaved before
> we added the lock when implementing buffer prefetching.
> 
> Reported-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
> Tested-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: xfsprogs-dev/include/libxfs.h
> ===================================================================
> --- xfsprogs-dev.orig/include/libxfs.h	2011-11-22 22:28:23.000000000 +0000
> +++ xfsprogs-dev/include/libxfs.h	2011-11-22 22:34:27.000000000 +0000
> @@ -226,6 +226,8 @@ typedef struct xfs_buf {
>  	unsigned		b_bcount;
>  	dev_t			b_dev;
>  	pthread_mutex_t		b_lock;
> +	pthread_t		b_holder;
> +	unsigned int		b_recur;
>  	void			*b_fsprivate;
>  	void			*b_fsprivate2;
>  	void			*b_fsprivate3;
> Index: xfsprogs-dev/libxfs/rdwr.c
> ===================================================================
> --- xfsprogs-dev.orig/libxfs/rdwr.c	2011-11-22 22:28:23.000000000 +0000
> +++ xfsprogs-dev/libxfs/rdwr.c	2011-11-22 22:40:01.000000000 +0000
> @@ -342,6 +342,8 @@ libxfs_initbuf(xfs_buf_t *bp, dev_t devi
>  	list_head_init(&bp->b_lock_list);
>  #endif
>  	pthread_mutex_init(&bp->b_lock, NULL);
> +	bp->b_holder = 0;
> +	bp->b_recur = 0;
>  }
>  
>  xfs_buf_t *
> @@ -410,18 +412,24 @@ libxfs_getbuf_flags(dev_t device, xfs_da
>  		return NULL;
>  
>  	if (use_xfs_buf_lock) {
> -		if (flags & LIBXFS_GETBUF_TRYLOCK) {
> -			int ret;
> +		int ret;
>  
> -			ret = pthread_mutex_trylock(&bp->b_lock);
> -			if (ret) {
> -				ASSERT(ret == EAGAIN);
> -				cache_node_put(libxfs_bcache, (struct cache_node *)bp);
> -				return NULL;
> +		ret = pthread_mutex_trylock(&bp->b_lock);
> +		if (ret) {
> +			ASSERT(ret == EAGAIN);
> +			if (flags & LIBXFS_GETBUF_TRYLOCK)
> +				goto out_put;
> +
> +			if (pthread_equal(bp->b_holder, pthread_self())) {
> +				fprintf(stderr,
> +	_("recursive buffer locking detected\n"));

"Warning: recursive buffer locking @ bno %lld detected"

might be more informative, especially to do with the severity of the
issue.

> +				bp->b_recur++;
> +			} else {
> +				pthread_mutex_lock(&bp->b_lock);
>  			}
> -		} else {
> -			pthread_mutex_lock(&bp->b_lock);
>  		}
> +
> +		bp->b_holder = pthread_self();

That should probably only be written in the branch where the lock is
taken not every time through here.

Also, it might be worth commenting that the only reason there isn't
a race checking bp->b_holder without holding a lock is that the
holder is initialised to zero and cleared before the buffer lock is
dropped so that when a concurrent lookup fails the value of b_holder
will never match the failed thread ID.

Otherwise, looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH 06/12] repair: use recursive buffer locking

[Christoph Hellwig]

On Tue, Dec 13, 2011 at 01:22:08PM +1100, Dave Chinner wrote:
> >  	if (use_xfs_buf_lock) {
> > -		if (flags & LIBXFS_GETBUF_TRYLOCK) {
> > -			int ret;
> > +		int ret;
> >  
> > -			ret = pthread_mutex_trylock(&bp->b_lock);
> > -			if (ret) {
> > -				ASSERT(ret == EAGAIN);
> > -				cache_node_put(libxfs_bcache, (struct cache_node *)bp);
> > -				return NULL;
> > +		ret = pthread_mutex_trylock(&bp->b_lock);
> > +		if (ret) {
> > +			ASSERT(ret == EAGAIN);
> > +			if (flags & LIBXFS_GETBUF_TRYLOCK)
> > +				goto out_put;
> > +
> > +			if (pthread_equal(bp->b_holder, pthread_self())) {
> > +				fprintf(stderr,
> > +	_("recursive buffer locking detected\n"));
> 
> "Warning: recursive buffer locking @ bno %lld detected"
> 
> might be more informative, especially to do with the severity of the
> issue.

Ok, I'll make it print the block number.

> 
> > +				bp->b_recur++;
> > +			} else {
> > +				pthread_mutex_lock(&bp->b_lock);
> >  			}
> > -		} else {
> > -			pthread_mutex_lock(&bp->b_lock);
> >  		}
> > +
> > +		bp->b_holder = pthread_self();
> 
> That should probably only be written in the branch where the lock is
> taken not every time through here.

We actually should return the buffer just after incrementing the
recursion count, else we might add it to the global list of buffers
twice.

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

Re: Re: [PATCH 06/12] repair: use recursive buffer locking

[Mark Tinguely]

On 01/-10/63 13:59, Christoph Hellwig wrote:
> On Tue, Dec 13, 2011 at 01:22:08PM +1100, Dave Chinner wrote:
>>>   	if (use_xfs_buf_lock) {
>>> -		if (flags&  LIBXFS_GETBUF_TRYLOCK) {
>>> -			int ret;
>>> +		int ret;
>>>
>>> -			ret = pthread_mutex_trylock(&bp->b_lock);
>>> -			if (ret) {
>>> -				ASSERT(ret == EAGAIN);
>>> -				cache_node_put(libxfs_bcache, (struct cache_node *)bp);
>>> -				return NULL;
>>> +		ret = pthread_mutex_trylock(&bp->b_lock);
>>> +		if (ret) {
>>> +			ASSERT(ret == EAGAIN);
>>> +			if (flags&  LIBXFS_GETBUF_TRYLOCK)
>>> +				goto out_put;
>>> +
>>> +			if (pthread_equal(bp->b_holder, pthread_self())) {
>>> +				fprintf(stderr,
>>> +	_("recursive buffer locking detected\n"));
>>
>> "Warning: recursive buffer locking @ bno %lld detected"
>>
>> might be more informative, especially to do with the severity of the
>> issue.
>
> Ok, I'll make it print the block number.
>
>>
>>> +				bp->b_recur++;
>>> +			} else {
>>> +				pthread_mutex_lock(&bp->b_lock);
>>>   			}
>>> -		} else {
>>> -			pthread_mutex_lock(&bp->b_lock);
>>>   		}
>>> +
>>> +		bp->b_holder = pthread_self();
>>
>> That should probably only be written in the branch where the lock is
>> taken not every time through here.
>
> We actually should return the buffer just after incrementing the
> recursion count, else we might add it to the global list of buffers
> twice.
>
>

I liked the patch the way it was.

I thought the placement of the setting of b_holder is correct because it 
covers the pthread_mutex_trylock and the pthread_mutex_lock attempts.

I did not see anything in cache_node_get_priority() nor
cache_node_set_priority() that would add this to a list.


Reviewed-by: Mark Tinguely <tinguely@sgi.com>



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

[PATCH 07/12] repair: fix another ABBA deadlock in inode prefetching

[Christoph Hellwig]

[-- Attachment #1: repair-fix-prefetch-deadlock --]
[-- Type: text/plain, Size: 1523 bytes --]

The inode prefetching code has a fixed limit of inodes that might are
submitted at a time.  Unfortunately the buffers for them get locked
once the prefetching starts.  That way the threads processing the inode
might get stuck on buffer locked, but not submitted for reading yet.

Fix this by kicking the queue as soon as we would have to wait on the
ra_count semaphore.

Reported-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
Tested-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfsprogs-dev/repair/prefetch.c
===================================================================
--- xfsprogs-dev.orig/repair/prefetch.c	2011-11-25 13:46:47.195999430 +0100
+++ xfsprogs-dev/repair/prefetch.c	2011-11-25 13:50:41.264731371 +0100
@@ -641,7 +641,18 @@ pf_queuing_worker(
 		pftrace("queuing irec %p in AG %d, sem count = %d",
 			irec, args->agno, i);
 #endif
-		sem_wait(&args->ra_count);
+		err = sem_trywait(&args->ra_count);
+		if (err == EAGAIN) {
+			/*
+			 * Kick the queue once we have reached the limit;
+			 * without this the threads processing the inodes
+			 * might get stuck on a buffer that has been locked
+			 * and added to the I/O queue but is waiting for
+			 * the thread to be woken.
+			 */
+			pf_start_io_workers(args);
+			sem_wait(&args->ra_count);
+		}
 
 		num_inos = 0;
 		bno = XFS_AGINO_TO_AGBNO(mp, cur_irec->ino_startnum);

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

Re: [PATCH 07/12] repair: fix another ABBA deadlock in inode prefetching

[Dave Chinner]

On Fri, Dec 02, 2011 at 12:46:26PM -0500, Christoph Hellwig wrote:
> The inode prefetching code has a fixed limit of inodes that might are
> submitted at a time.  Unfortunately the buffers for them get locked
> once the prefetching starts.  That way the threads processing the inode
> might get stuck on buffer locked, but not submitted for reading yet.
> 
> Fix this by kicking the queue as soon as we would have to wait on the
> ra_count semaphore.
> 
> Reported-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
> Tested-by: Arkadiusz Mi??kiewicz <arekm@maven.pl>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks safe - I can't think up a scenario that would deadlock on
the second sem_wait call...

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH 07/12] repair: fix another ABBA deadlock in inode prefetching

[Mark Tinguely]

> Index: xfsprogs-dev/repair/prefetch.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/prefetch.c	2011-11-25 13:46:47.195999430 +0100
> +++ xfsprogs-dev/repair/prefetch.c	2011-11-25 13:50:41.264731371 +0100
> @@ -641,7 +641,18 @@ pf_queuing_worker(
>   		pftrace("queuing irec %p in AG %d, sem count = %d",
>   			irec, args->agno, i);
>   #endif
> -		sem_wait(&args->ra_count);
> +		err = sem_trywait(&args->ra_count);
> +		if (err == EAGAIN) {
> +			/*
> +			 * Kick the queue once we have reached the limit;
> +			 * without this the threads processing the inodes
> +			 * might get stuck on a buffer that has been locked
> +			 * and added to the I/O queue but is waiting for
> +			 * the thread to be woken.
> +			 */
> +			pf_start_io_workers(args);
> +			sem_wait(&args->ra_count);
> +		}
>
>   		num_inos = 0;
>   		bno = XFS_AGINO_TO_AGBNO(mp, cur_irec->ino_startnum);
>

Could the second sem_wait() also fail with EAGAIN?

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

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

[PATCH 08/12] repair: handle filesystems with the log in allocation group 0

[Christoph Hellwig]

[-- Attachment #1: repair-fix-geomety-detection --]
[-- Type: text/plain, Size: 1731 bytes --]

Sindre Skogen reported that repair chokes on a very small filesystem created
by mkfs.xfs from xfsprogs 2.9.4.  It turned out that for some reason this
filesystem had the log in allocation group 0 and thus repairs validation
of the root inode number was off.  Fix this by adding the log blocks if
the log is allocated in allocation group 0.

Reported-by: Sindre Skogen <sindre@workzone.no>
Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfsprogs-dev/repair/xfs_repair.c
===================================================================
--- xfsprogs-dev.orig/repair/xfs_repair.c	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/xfs_repair.c	2011-12-01 16:17:58.000000000 +0000
@@ -417,12 +417,16 @@ calc_mkfs(xfs_mount_t *mp)
 	fino_bno = inobt_root + XFS_MIN_FREELIST_RAW(1, 1, mp) + 1;
 
 	/*
-	 * If we only have a single allocation group the log is also allocated
-	 * in the first allocation group and we need to add the number of
-	 * blocks used by the log to the above calculation.
-	 * All this of course doesn't apply if we have an external log.
+	 * If the log is allocated in the first allocation group we need to
+	 * add the number of blocks used by the log to the above calculation.
+	 *
+	 * This can happens with filesystems that only have a single
+	 * allocation group, or very odd geometries created by old mkfs
+	 * versions on very small filesystems.
 	 */
-	if (mp->m_sb.sb_agcount == 1 && mp->m_sb.sb_logstart) {
+	if (mp->m_sb.sb_logstart &&
+	    XFS_FSB_TO_AGNO(mp, mp->m_sb.sb_logstart) == 0) {
+
 		/*
 		 * XXX(hch): verify that sb_logstart makes sense?
 		 */

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

Re: [PATCH 08/12] repair: handle filesystems with the log in allocation group 0

[Dave Chinner]

On Fri, Dec 02, 2011 at 12:46:27PM -0500, Christoph Hellwig wrote:
> Sindre Skogen reported that repair chokes on a very small filesystem created
> by mkfs.xfs from xfsprogs 2.9.4.  It turned out that for some reason this
> filesystem had the log in allocation group 0 and thus repairs validation
> of the root inode number was off.  Fix this by adding the log blocks if
> the log is allocated in allocation group 0.
> 
> Reported-by: Sindre Skogen <sindre@workzone.no>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good - handling all occurrences of the log in AG 0 is much
nicer.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH 08/12] repair: handle filesystems with the log in allocation group 0

[Mark Tinguely]

On 01/-10/63 13:59, Christoph Hellwig wrote:
> Sindre Skogen reported that repair chokes on a very small filesystem created
> by mkfs.xfs from xfsprogs 2.9.4.  It turned out that for some reason this
> filesystem had the log in allocation group 0 and thus repairs validation
> of the root inode number was off.  Fix this by adding the log blocks if
> the log is allocated in allocation group 0.
>


Reviewed-by: Mark Tinguely <tinguely@sgi.com>

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

[PATCH 09/12] repair: kill check_inode_block

[Christoph Hellwig]

[-- Attachment #1: repair-kill-check_inode_block --]
[-- Type: text/plain, Size: 1260 bytes --]

It's a wrapper around check_aginode_block, but given that the only caller
already has the agno and agbno at hand it isn't overly useful.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfsprogs-dev/repair/dino_chunks.c
===================================================================
--- xfsprogs-dev.orig/repair/dino_chunks.c	2011-11-14 20:04:08.847867904 +0100
+++ xfsprogs-dev/repair/dino_chunks.c	2011-11-14 20:04:59.487867601 +0100
@@ -72,14 +72,6 @@ check_aginode_block(xfs_mount_t	*mp,
 	return(cnt);
 }
 
-int
-check_inode_block(xfs_mount_t		*mp,
-			xfs_ino_t	ino)
-{
-	return(check_aginode_block(mp, XFS_INO_TO_AGNO(mp, ino),
-					XFS_INO_TO_AGBNO(mp, ino)));
-}
-
 /*
  * tries to establish if the inode really exists in a valid
  * inode chunk.  returns number of new inodes if things are good
@@ -145,10 +137,9 @@ verify_inode_chunk(xfs_mount_t		*mp,
 	 */
 	if (XFS_IALLOC_BLOCKS(mp) == 1)  {
 		if (agbno > max_agbno)
-			return(0);
-
-		if (check_inode_block(mp, ino) == 0)
-			return(0);
+			return 0;
+		if (check_aginode_block(mp, agno, agino) == 0)
+			return 0;
 
 		pthread_mutex_lock(&ag_locks[agno]);
 

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

Re: [PATCH 09/12] repair: kill check_inode_block

[Christoph Hellwig]

ping?

On Fri, Dec 02, 2011 at 12:46:28PM -0500, Christoph Hellwig wrote:
> It's a wrapper around check_aginode_block, but given that the only caller
> already has the agno and agbno at hand it isn't overly useful.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: xfsprogs-dev/repair/dino_chunks.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/dino_chunks.c	2011-11-14 20:04:08.847867904 +0100
> +++ xfsprogs-dev/repair/dino_chunks.c	2011-11-14 20:04:59.487867601 +0100
> @@ -72,14 +72,6 @@ check_aginode_block(xfs_mount_t	*mp,
>  	return(cnt);
>  }
>  
> -int
> -check_inode_block(xfs_mount_t		*mp,
> -			xfs_ino_t	ino)
> -{
> -	return(check_aginode_block(mp, XFS_INO_TO_AGNO(mp, ino),
> -					XFS_INO_TO_AGBNO(mp, ino)));
> -}
> -
>  /*
>   * tries to establish if the inode really exists in a valid
>   * inode chunk.  returns number of new inodes if things are good
> @@ -145,10 +137,9 @@ verify_inode_chunk(xfs_mount_t		*mp,
>  	 */
>  	if (XFS_IALLOC_BLOCKS(mp) == 1)  {
>  		if (agbno > max_agbno)
> -			return(0);
> -
> -		if (check_inode_block(mp, ino) == 0)
> -			return(0);
> +			return 0;
> +		if (check_aginode_block(mp, agno, agino) == 0)
> +			return 0;
>  
>  		pthread_mutex_lock(&ag_locks[agno]);
>  
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---

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

Re: [PATCH 09/12] repair: kill check_inode_block

[Mark Tinguely]

On 01/11/12 05:29, Christoph Hellwig wrote:
> ping?

Sorry.

This replacement makes sense.

I have been running the entire patch set without problems.


Reviewed-by: Mark Tinguely <tinguely@sgi.com>

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

[PATCH 10/12] repair: mark local functions static

[Christoph Hellwig]

[-- Attachment #1: repair-mark-static --]
[-- Type: text/plain, Size: 28891 bytes --]

Also remove unused function, remove useless ARGSUSED annotations and
similar tiny cleanups.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfsprogs-dev/repair/agheader.c
===================================================================
--- xfsprogs-dev.orig/repair/agheader.c	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/agheader.c	2011-12-01 16:18:23.000000000 +0000
@@ -22,7 +22,7 @@
 #include "protos.h"
 #include "err_protos.h"
 
-int
+static int
 verify_set_agf(xfs_mount_t *mp, xfs_agf_t *agf, xfs_agnumber_t i)
 {
 	xfs_drfsbno_t agblocks;
@@ -107,7 +107,7 @@ verify_set_agf(xfs_mount_t *mp, xfs_agf_
 	return(retval);
 }
 
-int
+static int
 verify_set_agi(xfs_mount_t *mp, xfs_agi_t *agi, xfs_agnumber_t agno)
 {
 	xfs_drfsbno_t agblocks;
@@ -177,14 +177,13 @@ verify_set_agi(xfs_mount_t *mp, xfs_agi_
  *			filesystem mount-point superblock
  *
  * the verified fields include id and geometry.
-
+ *
  * the inprogress fields, version numbers, and counters
  * are allowed to differ as well as all fields after the
  * counters to cope with the pre-6.5 mkfs non-zeroed
  * secondary superblock sectors.
  */
-
-int
+static int
 compare_sb(xfs_mount_t *mp, xfs_sb_t *sb)
 {
 	fs_geometry_t fs_geo, sb_geo;
@@ -213,7 +212,7 @@ compare_sb(xfs_mount_t *mp, xfs_sb_t *sb
  * Note: contrary to the name, this routine is called for all
  * superblocks, not just the secondary superblocks.
  */
-int
+static int
 secondary_sb_wack(xfs_mount_t *mp, xfs_buf_t *sbuf, xfs_sb_t *sb,
 	xfs_agnumber_t i)
 {
Index: xfsprogs-dev/repair/attr_repair.c
===================================================================
--- xfsprogs-dev.orig/repair/attr_repair.c	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/attr_repair.c	2011-12-01 16:18:23.000000000 +0000
@@ -80,8 +80,7 @@ static int xfs_mac_valid(xfs_mac_label_t
  * in user attribute land without a conflict.
  * If value is non-zero, then a remote attribute is being passed in
  */
-
-int
+static int
 valuecheck(char *namevalue, char *value, int namelen, int valuelen)
 {
 	/* for proper alignment issues, get the structs and memmove the values */
@@ -146,7 +145,7 @@ valuecheck(char *namevalue, char *value,
  * if you cannot modify the structures. repair is set to 1, if anything
  * was fixed.
  */
-int
+static int
 process_shortform_attr(
 	xfs_ino_t	ino,
 	xfs_dinode_t	*dip,
@@ -490,7 +489,7 @@ bad_free_out:
 	return -1;
 }
 
-int
+static int
 process_leaf_attr_block(
 	xfs_mount_t	*mp,
 	xfs_attr_leafblock_t *leaf,
@@ -643,7 +642,7 @@ process_leaf_attr_block(
 /*
  * returns 0 if the attribute fork is ok, 1 if it has to be junked.
  */
-int
+static int
 process_leaf_attr_level(xfs_mount_t	*mp,
 			da_bt_cursor_t	*da_cursor)
 {
@@ -775,7 +774,7 @@ error_out:
  * returns 0 if things are ok, 1 if bad
  * Note this code has been based off process_node_dir.
  */
-int
+static int
 process_node_attr(
 	xfs_mount_t	*mp,
 	xfs_ino_t	ino,
@@ -825,8 +824,7 @@ process_node_attr(
  * returns 0 if things are ok, 1 if bad (attributes needs to be junked)
  * repair is set, if anything was changed, but attributes can live thru it
  */
-
-int
+static int
 process_longform_attr(
 	xfs_mount_t	*mp,
 	xfs_ino_t	ino,
Index: xfsprogs-dev/repair/dino_chunks.c
===================================================================
--- xfsprogs-dev.orig/repair/dino_chunks.c	2011-12-01 16:18:23.000000000 +0000
+++ xfsprogs-dev/repair/dino_chunks.c	2011-12-01 16:18:23.000000000 +0000
@@ -34,8 +34,7 @@
  * the dinodes are verified using verify_uncertain_dinode() which
  * means only the basic inode info is checked, no fork checks.
  */
-
-int
+static int
 check_aginode_block(xfs_mount_t	*mp,
 			xfs_agnumber_t	agno,
 			xfs_agblock_t	agbno)
@@ -85,7 +84,7 @@ check_aginode_block(xfs_mount_t	*mp,
  * routines called by check_uncertain_aginodes() and
  * process_uncertain_aginodes().
  */
-int
+static int
 verify_inode_chunk(xfs_mount_t		*mp,
 			xfs_ino_t	ino,
 			xfs_ino_t	*start_ino)
@@ -513,7 +512,7 @@ verify_inode_chunk(xfs_mount_t		*mp,
 /*
  * same as above only for ag inode chunks
  */
-int
+static int
 verify_aginode_chunk(xfs_mount_t	*mp,
 			xfs_agnumber_t	agno,
 			xfs_agino_t	agino,
@@ -536,7 +535,7 @@ verify_aginode_chunk(xfs_mount_t	*mp,
  * this does the same as the two above only it returns a pointer
  * to the inode record in the good inode tree
  */
-ino_tree_node_t *
+static ino_tree_node_t *
 verify_aginode_chunk_irec(xfs_mount_t	*mp,
 			xfs_agnumber_t	agno,
 			xfs_agino_t	agino)
Index: xfsprogs-dev/repair/dinode.c
===================================================================
--- xfsprogs-dev.orig/repair/dinode.c	2011-12-01 16:17:32.000000000 +0000
+++ xfsprogs-dev/repair/dinode.c	2011-12-01 16:18:23.000000000 +0000
@@ -36,47 +36,7 @@
  * inode clearing routines
  */
 
-/*
- * return the offset into the inode where the attribute fork starts
- */
-/* ARGSUSED */
-int
-calc_attr_offset(xfs_mount_t *mp, xfs_dinode_t *dino)
-{
-	int	offset = (__psint_t)XFS_DFORK_DPTR(dino) - (__psint_t)dino;
-	xfs_bmdr_block_t        *dfp;
-
-	/*
-	 * don't worry about alignment when calculating offset
-	 * because the data fork is already 8-byte aligned
-	 */
-	switch (dino->di_format)  {
-	case XFS_DINODE_FMT_DEV:
-		offset += sizeof(xfs_dev_t);
-		break;
-	case XFS_DINODE_FMT_LOCAL:
-		offset += be64_to_cpu(dino->di_size);
-		break;
-	case XFS_DINODE_FMT_EXTENTS:
-		offset += be32_to_cpu(dino->di_nextents) *
-						sizeof(xfs_bmbt_rec_t);
-		break;
-	case XFS_DINODE_FMT_BTREE:
-		dfp = (xfs_bmdr_block_t *)XFS_DFORK_DPTR(dino);
-		offset += be16_to_cpu(dfp->bb_numrecs) *
-						sizeof(xfs_bmbt_rec_t);
-		break;
-	default:
-		do_error(_("Unknown inode format.\n"));
-		abort();
-		break;
-	}
-
-	return(offset);
-}
-
-/* ARGSUSED */
-int
+static int
 clear_dinode_attr(xfs_mount_t *mp, xfs_dinode_t *dino, xfs_ino_t ino_num)
 {
 	ASSERT(dino->di_forkoff != 0);
@@ -125,8 +85,7 @@ _("would have cleared inode %" PRIu64 "
 	return(1);
 }
 
-/* ARGSUSED */
-int
+static int
 clear_dinode_core(xfs_dinode_t *dinoc, xfs_ino_t ino_num)
 {
 	int dirty = 0;
@@ -262,8 +221,7 @@ clear_dinode_core(xfs_dinode_t *dinoc, x
 	return(dirty);
 }
 
-/* ARGSUSED */
-int
+static int
 clear_dinode_unlinked(xfs_mount_t *mp, xfs_dinode_t *dino)
 {
 
@@ -281,7 +239,7 @@ clear_dinode_unlinked(xfs_mount_t *mp, x
  * until after the agi unlinked lists are walked in phase 3.
  * returns > zero if the inode has been altered while being cleared
  */
-int
+static int
 clear_dinode(xfs_mount_t *mp, xfs_dinode_t *dino, xfs_ino_t ino_num)
 {
 	int dirty;
@@ -445,31 +403,6 @@ verify_agbno(xfs_mount_t	*mp,
 	return verify_ag_bno(sbp, agno, agbno) == 0;
 }
 
-/*
- * return address of block fblock if it's within the range described
- * by the extent list.  Otherwise, returns a null address.
- */
-/* ARGSUSED */
-xfs_dfsbno_t
-get_bmbt_reclist(
-	xfs_mount_t		*mp,
-	xfs_bmbt_rec_t		*rp,
-	int			numrecs,
-	xfs_dfiloff_t		fblock)
-{
-	int			i;
-	xfs_bmbt_irec_t 	irec;
-
-	for (i = 0; i < numrecs; i++) {
-		libxfs_bmbt_disk_get_all(rp + i, &irec);
-		if (irec.br_startoff >= fblock &&
-				irec.br_startoff + irec.br_blockcount < fblock)
-			return (irec.br_startblock + fblock - irec.br_startoff);
-	}
-	return(NULLDFSBNO);
-}
-
-
 static int
 process_rt_rec(
 	xfs_mount_t		*mp,
@@ -601,8 +534,7 @@ _("illegal state %d in rt block map %" P
  * file overlaps with any duplicate extents (in the
  * duplicate extent list).
  */
-/* ARGSUSED */
-int
+static int
 process_bmbt_reclist_int(
 	xfs_mount_t		*mp,
 	xfs_bmbt_rec_t		*rp,
@@ -931,7 +863,7 @@ get_agino_buf(xfs_mount_t	 *mp,
  *
  * NOTE: getfunc_extlist only used by dirv1 checking code
  */
-xfs_dfsbno_t
+static xfs_dfsbno_t
 getfunc_extlist(xfs_mount_t		*mp,
 		xfs_ino_t		ino,
 		xfs_dinode_t		*dip,
@@ -960,7 +892,7 @@ getfunc_extlist(xfs_mount_t		*mp,
 /*
  * NOTE: getfunc_btree only used by dirv1 checking code... 
  */
-xfs_dfsbno_t
+static xfs_dfsbno_t
 getfunc_btree(xfs_mount_t		*mp,
 		xfs_ino_t		ino,
 		xfs_dinode_t		*dip,
@@ -1168,8 +1100,7 @@ get_bmapi(xfs_mount_t *mp, xfs_dinode_t
 /*
  * return 1 if inode should be cleared, 0 otherwise
  */
-/* ARGSUSED */
-int
+static int
 process_btinode(
 	xfs_mount_t		*mp,
 	xfs_agnumber_t		agno,
@@ -1344,8 +1275,7 @@ _("bad numrecs 0 in inode %" PRIu64 " bm
 /*
  * return 1 if inode should be cleared, 0 otherwise
  */
-/* ARGSUSED */
-int
+static int
 process_exinode(
 	xfs_mount_t		*mp,
 	xfs_agnumber_t		agno,
@@ -1431,7 +1361,7 @@ process_lclinode(
 	return(0);
 }
 
-int
+static int
 process_symlink_extlist(xfs_mount_t *mp, xfs_ino_t lino, xfs_dinode_t *dino)
 {
 	xfs_dfiloff_t		expected_offset;
@@ -1502,7 +1432,7 @@ _("bad extent #%d count (%" PRIu64 ") in
  * takes a name and length and returns 1 if the name contains
  * a \0, returns 0 otherwise
  */
-int
+static int
 null_check(char *name, int length)
 {
 	int i;
@@ -1521,7 +1451,7 @@ null_check(char *name, int length)
  * like usual, returns 0 if everything's ok and 1 if something's
  * bogus
  */
-int
+static int
 process_symlink(
 	xfs_mount_t	*mp,
 	xfs_ino_t	lino,
@@ -2459,8 +2389,7 @@ _("would clear obsolete nlink field in v
  *
  * for detailed, info, look at process_dinode() comments.
  */
-/* ARGSUSED */
-int
+static int
 process_dinode_int(xfs_mount_t *mp,
 		xfs_dinode_t *dino,
 		xfs_agnumber_t agno,
Index: xfsprogs-dev/repair/dinode.h
===================================================================
--- xfsprogs-dev.orig/repair/dinode.h	2011-12-01 16:17:32.000000000 +0000
+++ xfsprogs-dev/repair/dinode.h	2011-12-01 16:18:23.000000000 +0000
@@ -61,19 +61,6 @@ scan_bmbt_reclist(
 	xfs_drfsbno_t		*tot,
 	int			whichfork);
 
-int
-verify_inode_chunk(xfs_mount_t		*mp,
-			xfs_ino_t	ino,
-			xfs_ino_t	*start_ino);
-
-int	verify_aginode_chunk(xfs_mount_t	*mp,
-				xfs_agnumber_t	agno,
-				xfs_agino_t	agino,
-				xfs_agino_t	*agino_start);
-
-int
-clear_dinode(xfs_mount_t *mp, xfs_dinode_t *dino, xfs_ino_t ino_num);
-
 void
 update_rootino(xfs_mount_t *mp);
 
Index: xfsprogs-dev/repair/dir.c
===================================================================
--- xfsprogs-dev.orig/repair/dir.c	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/dir.c	2011-12-01 16:18:23.000000000 +0000
@@ -72,8 +72,7 @@ namecheck(char *name, int length)
  * entries.  a non-zero return value means the directory is bogus
  * and should be blasted.
  */
-/* ARGSUSED */
-int
+static int
 process_shortform_dir(
 	xfs_mount_t	*mp,
 	xfs_ino_t	ino,
@@ -553,7 +552,7 @@ set_da_freemap(xfs_mount_t *mp, da_freem
  * returns 0 if holemap is consistent with reality (as expressed by
  * the da_freemap_t).  returns 1 if there's a conflict.
  */
-int
+static int
 verify_da_freemap(xfs_mount_t *mp, da_freemap_t *map, da_hole_map_t *holes,
 			xfs_ino_t ino, xfs_dablk_t da_bno)
 {
@@ -591,7 +590,7 @@ verify_da_freemap(xfs_mount_t *mp, da_fr
 	return(0);
 }
 
-void
+static void
 process_da_freemap(xfs_mount_t *mp, da_freemap_t *map, da_hole_map_t *holes)
 {
 	int i, j, in_hole, start, length, smallest, num_holes;
@@ -678,8 +677,7 @@ process_da_freemap(xfs_mount_t *mp, da_f
 /*
  * returns 1 if the hole info doesn't match, 0 if it does
  */
-/* ARGSUSED */
-int
+static int
 compare_da_freemaps(xfs_mount_t *mp, da_hole_map_t *holemap,
 			da_hole_map_t *block_hmap, int entries,
 			xfs_ino_t ino, xfs_dablk_t da_bno)
@@ -879,7 +877,7 @@ error_out:
  * buffers (e.g. if we do, it's a mistake).  if error == 1, we're
  * in an error-handling case so unreleased buffers may exist.
  */
-void
+static void
 release_da_cursor_int(xfs_mount_t	*mp,
 			da_bt_cursor_t	*cursor,
 			int		prev_level,
@@ -922,91 +920,6 @@ err_release_da_cursor(xfs_mount_t	*mp,
 }
 
 /*
- * like traverse_int_dablock only it does far less checking
- * and doesn't maintain the cursor.  Just gets you to the
- * leftmost block in the directory.  returns the fsbno
- * of that block if successful, NULLDFSBNO if not.
- */
-xfs_dfsbno_t
-get_first_dblock_fsbno(xfs_mount_t	*mp,
-			xfs_ino_t	ino,
-			xfs_dinode_t	*dino)
-{
-	xfs_dablk_t		bno;
-	int			i;
-	xfs_da_intnode_t	*node;
-	xfs_dfsbno_t		fsbno;
-	xfs_buf_t		*bp;
-
-	/*
-	 * traverse down left-side of tree until we hit the
-	 * left-most leaf block setting up the btree cursor along
-	 * the way.
-	 */
-	bno = 0;
-	i = -1;
-	node = NULL;
-
-	fsbno = get_bmapi(mp, dino, ino, bno, XFS_DATA_FORK);
-
-	if (fsbno == NULLDFSBNO)  {
-		do_warn(_("bmap of block #%u of inode %" PRIu64 " failed\n"),
-			bno, ino);
-		return(fsbno);
-	}
-
-	if (be64_to_cpu(dino->di_size) <= XFS_LBSIZE(mp))
-		return(fsbno);
-
-	do {
-		/*
-		 * walk down left side of btree, release buffers as you
-		 * go.  if the root block is a leaf (single-level btree),
-		 * just return it.
-		 *
-		 */
-
-		bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
-				XFS_FSB_TO_BB(mp, 1), 0);
-		if (!bp) {
-			do_warn(
-	_("can't read block %u (fsbno %" PRIu64 ") for directory inode %" PRIu64 "\n"),
-				bno, fsbno, ino);
-			return(NULLDFSBNO);
-		}
-
-		node = (xfs_da_intnode_t *)XFS_BUF_PTR(bp);
-
-		if (XFS_DA_NODE_MAGIC !=
-		    be16_to_cpu(node->hdr.info.magic))  {
-			do_warn(
-	_("bad dir/attr magic number in inode %" PRIu64 ", file bno = %u, fsbno = %" PRIu64 "\n"),
-				ino, bno, fsbno);
-			libxfs_putbuf(bp);
-			return(NULLDFSBNO);
-		}
-
-		if (i == -1)
-			i = be16_to_cpu(node->hdr.level);
-		bno = be32_to_cpu(node->btree[0].before);
-
-		libxfs_putbuf(bp);
-
-		fsbno = get_bmapi(mp, dino, ino, bno, XFS_DATA_FORK);
-
-		if (fsbno == NULLDFSBNO)  {
-			do_warn(_("bmap of block #%u of inode %" PRIu64 " failed\n"),
-				bno, ino);
-			return(NULLDFSBNO);
-		}
-
-		i--;
-	} while(i > 0);
-
-	return(fsbno);
-}
-
-/*
  * make sure that all entries in all blocks along the right side of
  * of the tree are used and hashval's are consistent.  level is the
  * level of the descendent block.  returns 0 if good (even if it had
@@ -1401,8 +1314,7 @@ size_t ts_dirbuf_size = 64*1024;
  * bad entry name index pointers), we lose the directory.  We could
  * try harder to fix this but it'll do for now.
  */
-/* ARGSUSED */
-int
+static int
 process_leaf_dir_block(
 	xfs_mount_t		*mp,
 	xfs_dir_leafblock_t	*leaf,
@@ -2311,7 +2223,7 @@ _("- existing hole info for block %d, di
 /*
  * returns 0 if the directory is ok, 1 if it has to be junked.
  */
-int
+static int
 process_leaf_dir_level(xfs_mount_t	*mp,
 			da_bt_cursor_t	*da_cursor,
 			int		ino_discovery,
@@ -2489,8 +2401,7 @@ error_out:
  *
  * returns 0 if things are ok, 1 if bad (directory needs to be junked)
  */
-/* ARGSUSED */
-int
+static int
 process_node_dir(
 	xfs_mount_t	*mp,
 	xfs_ino_t	ino,
@@ -2588,8 +2499,7 @@ _("setting directory inode (%" PRIu64 ")
  *
  * returns 0 if things are ok, 1 if bad (directory needs to be junked)
  */
-/* ARGSUSED */
-int
+static int
 process_leaf_dir(
 	xfs_mount_t	*mp,
 	xfs_ino_t	ino,
Index: xfsprogs-dev/repair/dir.h
===================================================================
--- xfsprogs-dev.orig/repair/dir.h	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/dir.h	2011-12-01 16:18:23.000000000 +0000
@@ -75,12 +75,6 @@ err_release_da_cursor(
 	da_bt_cursor_t	*cursor,
 	int		prev_level);
 
-xfs_dfsbno_t
-get_first_dblock_fsbno(
-	xfs_mount_t	*mp,
-	xfs_ino_t	ino,
-	xfs_dinode_t	*dino);
-
 void
 init_da_freemap(
 	da_freemap_t *dir_freemap);
@@ -91,17 +85,6 @@ namecheck(
 	int		length);
 
 int
-process_shortform_dir(
-	xfs_mount_t	*mp,
-	xfs_ino_t	ino,
-	xfs_dinode_t	*dip,
-	int		ino_discovery,
-	int		*dino_dirty,	/* is dinode buffer dirty? */
-	xfs_ino_t	*parent,	/* out - NULLFSINO if entry doesn't exist */
-	char		*dirname,	/* directory pathname */
-	int		*repair);	/* out - 1 if dir was fixed up */
-
-int
 process_dir(
 	xfs_mount_t	*mp,
 	xfs_ino_t	ino,
Index: xfsprogs-dev/repair/dir2.c
===================================================================
--- xfsprogs-dev.orig/repair/dir2.c	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/dir2.c	2011-12-01 16:18:23.000000000 +0000
@@ -43,9 +43,10 @@ typedef struct dir2_bad {
 	xfs_ino_t	ino;
 	struct dir2_bad	*next;
 } dir2_bad_t;
-dir2_bad_t *dir2_bad_list;
 
-void
+static dir2_bad_t *dir2_bad_list;
+
+static void
 dir2_add_badlist(
 	xfs_ino_t	ino)
 {
@@ -78,7 +79,7 @@ dir2_is_badino(
  * Multibuffer handling.
  * V2 directory blocks can be noncontiguous, needing multiple buffers.
  */
-xfs_dabuf_t *
+static xfs_dabuf_t *
 da_read_buf(
 	xfs_mount_t	*mp,
 	int		nex,
@@ -186,7 +187,7 @@ da_buf_done(
 	free(dabuf);
 }
 
-int
+static int
 da_bwrite(
 	xfs_mount_t	*mp,
 	xfs_dabuf_t	*dabuf)
@@ -226,7 +227,7 @@ da_bwrite(
 	return error;
 }
 
-void
+static void
 da_brelse(
 	xfs_dabuf_t	*dabuf)
 {
@@ -262,7 +263,7 @@ da_brelse(
  * left-most leaf block if successful (bno).  returns 1 if successful,
  * 0 if unsuccessful.
  */
-int
+static int
 traverse_int_dir2block(xfs_mount_t	*mp,
 		dir2_bt_cursor_t	*da_cursor,
 		xfs_dablk_t		*rbno)
@@ -392,7 +393,7 @@ error_out:
  * buffers (e.g. if we do, it's a mistake).  if error == 1, we're
  * in an error-handling case so unreleased buffers may exist.
  */
-void
+static void
 release_dir2_cursor_int(xfs_mount_t		*mp,
 			dir2_bt_cursor_t	*cursor,
 			int			prev_level,
@@ -418,7 +419,7 @@ release_dir2_cursor_int(xfs_mount_t		*mp
 	return;
 }
 
-void
+static void
 release_dir2_cursor(xfs_mount_t		*mp,
 		dir2_bt_cursor_t	*cursor,
 		int			prev_level)
@@ -426,7 +427,7 @@ release_dir2_cursor(xfs_mount_t		*mp,
 	release_dir2_cursor_int(mp, cursor, prev_level, 0);
 }
 
-void
+static void
 err_release_dir2_cursor(xfs_mount_t		*mp,
 			dir2_bt_cursor_t	*cursor,
 			int			prev_level)
@@ -442,7 +443,7 @@ err_release_dir2_cursor(xfs_mount_t		*mp
  * technically a block boundary.  This routine should be used then
  * instead of verify_dir2_path().
  */
-int
+static int
 verify_final_dir2_path(xfs_mount_t	*mp,
 		dir2_bt_cursor_t	*cursor,
 		const int		p_level)
@@ -589,7 +590,7 @@ _("would correct bad hashval in non-leaf
  * since they have to be set so we can get a buffer for the
  * block.
  */
-int
+static int
 verify_dir2_path(xfs_mount_t	*mp,
 	dir2_bt_cursor_t	*cursor,
 	const int		p_level)
Index: xfsprogs-dev/repair/dir2.h
===================================================================
--- xfsprogs-dev.orig/repair/dir2.h	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/dir2.h	2011-12-01 16:18:23.000000000 +0000
@@ -59,30 +59,6 @@ typedef struct dir2_bt_cursor  {
 	struct blkmap		*blkmap;
 } dir2_bt_cursor_t;
 
-
-/* ROUTINES */
-
-void
-err_release_dir2_cursor(
-	xfs_mount_t		*mp,
-	dir2_bt_cursor_t	*cursor,
-	int			prev_level);
-
-xfs_dabuf_t *
-da_read_buf(
-	xfs_mount_t	*mp,
-	int		nex,
-	struct bmap_ext	*bmp);
-
-int
-da_bwrite(
-	xfs_mount_t	*mp,
-	xfs_dabuf_t	*bp);
-
-void
-da_brelse(
-	xfs_dabuf_t	*bp);
-
 int
 process_dir2(
 	xfs_mount_t	*mp,
@@ -99,10 +75,6 @@ process_sf_dir2_fixi8(
 	xfs_dir2_sf_t		*sfp,
 	xfs_dir2_sf_entry_t	**next_sfep);
 
-void
-dir2_add_badlist(
-	xfs_ino_t	ino);
-
 int
 dir2_is_badino(
 	xfs_ino_t	ino);
Index: xfsprogs-dev/repair/phase1.c
===================================================================
--- xfsprogs-dev.orig/repair/phase1.c	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/phase1.c	2011-12-01 16:18:23.000000000 +0000
@@ -22,7 +22,7 @@
 #include "protos.h"
 #include "err_protos.h"
 
-void
+static void
 no_sb(void)
 {
 	do_warn(_("Sorry, could not find valid secondary superblock\n"));
Index: xfsprogs-dev/repair/phase4.c
===================================================================
--- xfsprogs-dev.orig/repair/phase4.c	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/phase4.c	2011-12-01 16:18:23.000000000 +0000
@@ -40,7 +40,7 @@
  * free in which case they'd never be cleared so the fields wouldn't
  * be cleared by process_dinode().
  */
-void
+static void
 quotino_check(xfs_mount_t *mp)
 {
 	ino_tree_node_t *irec;
@@ -81,7 +81,7 @@ quotino_check(xfs_mount_t *mp)
 	}
 }
 
-void
+static void
 quota_sb_check(xfs_mount_t *mp)
 {
 	/*
Index: xfsprogs-dev/repair/phase5.c
===================================================================
--- xfsprogs-dev.orig/repair/phase5.c	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/phase5.c	2011-12-01 16:18:23.000000000 +0000
@@ -78,7 +78,7 @@ static __uint64_t	*sb_icount_ag;		/* all
 static __uint64_t	*sb_ifree_ag;		/* free inodes per ag */
 static __uint64_t	*sb_fdblocks_ag;	/* free data blocks per ag */
 
-int
+static int
 mk_incore_fstree(xfs_mount_t *mp, xfs_agnumber_t agno)
 {
 	int			in_extent;
@@ -165,8 +165,7 @@ mk_incore_fstree(xfs_mount_t *mp, xfs_ag
 	return(num_extents);
 }
 
-/* ARGSUSED */
-xfs_agblock_t
+static xfs_agblock_t
 get_next_blockaddr(xfs_agnumber_t agno, int level, bt_status_t *curs)
 {
 	ASSERT(curs->free_btree_blocks < curs->btree_blocks +
@@ -185,8 +184,7 @@ get_next_blockaddr(xfs_agnumber_t agno,
  * cursor pointer to the btree root.   called by init_freespace_cursor()
  * and init_ino_cursor()
  */
-/* ARGSUSED */
-void
+static void
 setup_cursor(xfs_mount_t *mp, xfs_agnumber_t agno, bt_status_t *curs)
 {
 	int			j;
@@ -301,7 +299,7 @@ setup_cursor(xfs_mount_t *mp, xfs_agnumb
 #endif
 }
 
-void
+static void
 write_cursor(bt_status_t *curs)
 {
 	int i;
@@ -322,7 +320,7 @@ write_cursor(bt_status_t *curs)
 	}
 }
 
-void
+static void
 finish_cursor(bt_status_t *curs)
 {
 	ASSERT(curs->num_free_blocks == 0);
@@ -341,8 +339,7 @@ finish_cursor(bt_status_t *curs)
  * btree_curs is an in/out.  returns the number of
  * blocks that will show up in the AGFL.
  */
-
-int
+static int
 calculate_freespace_cursor(xfs_mount_t *mp, xfs_agnumber_t agno,
 			xfs_agblock_t *extents, bt_status_t *btree_curs)
 {
@@ -595,7 +592,7 @@ calculate_freespace_cursor(xfs_mount_t *
 	return(extra_blocks);
 }
 
-void
+static void
 prop_freespace_cursor(xfs_mount_t *mp, xfs_agnumber_t agno,
 		bt_status_t *btree_curs, xfs_agblock_t startblock,
 		xfs_extlen_t blockcount, int level, __uint32_t magic)
@@ -689,7 +686,7 @@ prop_freespace_cursor(xfs_mount_t *mp, x
  * of tree to build (bno or bcnt).  returns the number of free blocks
  * represented by the tree.
  */
-xfs_extlen_t
+static xfs_extlen_t
 build_freespace_tree(xfs_mount_t *mp, xfs_agnumber_t agno,
 		bt_status_t *btree_curs, __uint32_t magic)
 {
@@ -854,7 +851,7 @@ build_freespace_tree(xfs_mount_t *mp, xf
  * may perturb things because inode tree building happens before
  * freespace tree building.
  */
-void
+static void
 init_ino_cursor(xfs_mount_t *mp, xfs_agnumber_t agno, bt_status_t *btree_curs,
 		__uint64_t *num_inos, __uint64_t *num_free_inos)
 {
@@ -942,7 +939,7 @@ init_ino_cursor(xfs_mount_t *mp, xfs_agn
 	return;
 }
 
-void
+static void
 prop_ino_cursor(xfs_mount_t *mp, xfs_agnumber_t agno, bt_status_t *btree_curs,
 	xfs_agino_t startino, int level)
 {
@@ -1027,7 +1024,7 @@ prop_ino_cursor(xfs_mount_t *mp, xfs_agn
 	*bt_ptr = cpu_to_be32(btree_curs->level[level-1].agbno);
 }
 
-void
+static void
 build_agi(xfs_mount_t *mp, xfs_agnumber_t agno,
 		bt_status_t *btree_curs, xfs_agino_t first_agino,
 		xfs_agino_t count, xfs_agino_t freecount)
@@ -1067,7 +1064,7 @@ build_agi(xfs_mount_t *mp, xfs_agnumber_
  * rebuilds an inode tree given a cursor.  We're lazy here and call
  * the routine that builds the agi
  */
-void
+static void
 build_ino_tree(xfs_mount_t *mp, xfs_agnumber_t agno,
 		bt_status_t *btree_curs)
 {
@@ -1197,7 +1194,7 @@ build_ino_tree(xfs_mount_t *mp, xfs_agnu
  * build both the agf and the agfl for an agno given both
  * btree cursors
  */
-void
+static void
 build_agf_agfl(xfs_mount_t	*mp,
 		xfs_agnumber_t	agno,
 		bt_status_t	*bno_bt,
@@ -1353,7 +1350,7 @@ build_agf_agfl(xfs_mount_t	*mp,
  * feature bits to the filesystem, and sync up the on-disk superblock
  * to match the incore superblock.
  */
-void
+static void
 sync_sb(xfs_mount_t *mp)
 {
 	xfs_buf_t	*bp;
@@ -1377,7 +1374,7 @@ sync_sb(xfs_mount_t *mp)
  * make sure the root and realtime inodes show up allocated
  * even if they've been freed.  they get reinitialized in phase6.
  */
-void
+static void
 keep_fsinos(xfs_mount_t *mp)
 {
 	ino_tree_node_t		*irec;
Index: xfsprogs-dev/repair/phase6.c
===================================================================
--- xfsprogs-dev.orig/repair/phase6.c	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/phase6.c	2011-12-01 16:18:23.000000000 +0000
@@ -509,7 +509,7 @@ mk_rbmino(xfs_mount_t *mp)
 	libxfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_SYNC);
 }
 
-int
+static int
 fill_rbmino(xfs_mount_t *mp)
 {
 	xfs_buf_t	*bp;
@@ -576,7 +576,7 @@ _("can't access block %" PRIu64 " (fsbno
 	return(0);
 }
 
-int
+static int
 fill_rsumino(xfs_mount_t *mp)
 {
 	xfs_buf_t	*bp;
@@ -645,7 +645,7 @@ _("can't access block %" PRIu64 " (fsbno
 	return(0);
 }
 
-void
+static void
 mk_rsumino(xfs_mount_t *mp)
 {
 	xfs_trans_t	*tp;
@@ -751,7 +751,7 @@ mk_rsumino(xfs_mount_t *mp)
 /*
  * makes a new root directory.
  */
-void
+static void
 mk_root_dir(xfs_mount_t *mp)
 {
 	xfs_trans_t	*tp;
@@ -815,7 +815,7 @@ mk_root_dir(xfs_mount_t *mp)
 /*
  * orphanage name == lost+found
  */
-xfs_ino_t
+static xfs_ino_t
 mk_orphanage(xfs_mount_t *mp)
 {
 	xfs_ino_t	ino;
@@ -1130,9 +1130,6 @@ mv_orphanage(
 }
 
 /*
- * like get_first_dblock_fsbno only it uses the simulation code instead
- * of raw I/O.
- *
  * Returns the fsbno of the first (leftmost) block in the directory leaf.
  * sets *bno to the directory block # corresponding to the returned fsbno.
  */
@@ -3519,7 +3516,7 @@ out:
  * mark realtime bitmap and summary inodes as reached.
  * quota inode will be marked here as well
  */
-void
+static void
 mark_standalone_inodes(xfs_mount_t *mp)
 {
 	ino_tree_node_t		*irec;
Index: xfsprogs-dev/repair/progress.c
===================================================================
--- xfsprogs-dev.orig/repair/progress.c	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/progress.c	2011-12-01 16:18:23.000000000 +0000
@@ -360,7 +360,7 @@ print_final_rpt(void)
 	return(sum);
 }
 
-void
+static void
 timediff(int phase)
 {
 	phase_times[phase].duration =
Index: xfsprogs-dev/repair/sb.c
===================================================================
--- xfsprogs-dev.orig/repair/sb.c	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/sb.c	2011-12-01 16:18:23.000000000 +0000
@@ -32,7 +32,7 @@
  * copy the fields of a superblock that are present in primary and
  * secondaries -- preserve fields that are different in the primary.
  */
-void
+static void
 copy_sb(xfs_sb_t *source, xfs_sb_t *dest)
 {
 	xfs_ino_t	rootino;
@@ -169,7 +169,7 @@ find_secondary_sb(xfs_sb_t *rsb)
  * calculate what inode alignment field ought to be
  * based on internal superblock info
  */
-int
+static int
 calc_ino_align(xfs_sb_t *sb)
 {
 	xfs_extlen_t align;
@@ -516,8 +516,7 @@ get_sb(xfs_sb_t *sbp, xfs_off_t off, int
 }
 
 /* returns element on list with highest reference count */
-
-fs_geo_list_t *
+static fs_geo_list_t *
 get_best_geo(fs_geo_list_t *list)
 {
 	int cnt = 0;
@@ -537,8 +536,7 @@ get_best_geo(fs_geo_list_t *list)
 }
 
 /* adds geometry info to linked list.  returns (sometimes new) head of list */
-
-fs_geo_list_t *
+static fs_geo_list_t *
 add_geo(fs_geo_list_t *list, fs_geometry_t *geo_p, int index)
 {
 	fs_geo_list_t	*current = list;
@@ -565,7 +563,7 @@ add_geo(fs_geo_list_t *list, fs_geometry
 	return(current);
 }
 
-void
+static void
 free_geo(fs_geo_list_t *list)
 {
 	fs_geo_list_t	*next;
Index: xfsprogs-dev/repair/scan.c
===================================================================
--- xfsprogs-dev.orig/repair/scan.c	2011-12-01 16:17:32.000000000 +0000
+++ xfsprogs-dev/repair/scan.c	2011-12-01 16:18:23.000000000 +0000
@@ -69,7 +69,7 @@ set_mp(xfs_mount_t *mpp)
 	mp = mpp;
 }
 
-void
+static void
 scan_sbtree(
 	xfs_agblock_t	root,
 	int		nlevels,
Index: xfsprogs-dev/repair/xfs_repair.c
===================================================================
--- xfsprogs-dev.orig/repair/xfs_repair.c	2011-12-01 16:17:58.000000000 +0000
+++ xfsprogs-dev/repair/xfs_repair.c	2011-12-01 16:18:23.000000000 +0000
@@ -49,8 +49,7 @@ extern void	phase7(xfs_mount_t *);
 /*
  * -o: user-supplied override options
  */
-
-char *o_opts[] = {
+static char *o_opts[] = {
 #define ASSUME_XFS	0
 	"assume_xfs",
 #define PRE_65_BETA	1
@@ -71,8 +70,7 @@ char *o_opts[] = {
 /*
  * -c: conversion options
  */
-
-char *c_opts[] = {
+static char *c_opts[] = {
 #define CONVERT_LAZY_COUNT	0
 	"lazycount",
 	NULL
@@ -183,7 +181,7 @@ unknown(char opt, char *s)
 /*
  * sets only the global argument flags and variables
  */
-void
+static void
 process_args(int argc, char **argv)
 {
 	char *p;
@@ -398,7 +396,7 @@ do_log(char const *msg, ...)
 	va_end(args);
 }
 
-void
+static void
 calc_mkfs(xfs_mount_t *mp)
 {
 	xfs_agblock_t	fino_bno;

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

Re: [PATCH 10/12] repair: mark local functions static

[Christoph Hellwig]

ping?

On Fri, Dec 02, 2011 at 12:46:29PM -0500, Christoph Hellwig wrote:
> Also remove unused function, remove useless ARGSUSED annotations and
> similar tiny cleanups.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: xfsprogs-dev/repair/agheader.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/agheader.c	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/agheader.c	2011-12-01 16:18:23.000000000 +0000
> @@ -22,7 +22,7 @@
>  #include "protos.h"
>  #include "err_protos.h"
>  
> -int
> +static int
>  verify_set_agf(xfs_mount_t *mp, xfs_agf_t *agf, xfs_agnumber_t i)
>  {
>  	xfs_drfsbno_t agblocks;
> @@ -107,7 +107,7 @@ verify_set_agf(xfs_mount_t *mp, xfs_agf_
>  	return(retval);
>  }
>  
> -int
> +static int
>  verify_set_agi(xfs_mount_t *mp, xfs_agi_t *agi, xfs_agnumber_t agno)
>  {
>  	xfs_drfsbno_t agblocks;
> @@ -177,14 +177,13 @@ verify_set_agi(xfs_mount_t *mp, xfs_agi_
>   *			filesystem mount-point superblock
>   *
>   * the verified fields include id and geometry.
> -
> + *
>   * the inprogress fields, version numbers, and counters
>   * are allowed to differ as well as all fields after the
>   * counters to cope with the pre-6.5 mkfs non-zeroed
>   * secondary superblock sectors.
>   */
> -
> -int
> +static int
>  compare_sb(xfs_mount_t *mp, xfs_sb_t *sb)
>  {
>  	fs_geometry_t fs_geo, sb_geo;
> @@ -213,7 +212,7 @@ compare_sb(xfs_mount_t *mp, xfs_sb_t *sb
>   * Note: contrary to the name, this routine is called for all
>   * superblocks, not just the secondary superblocks.
>   */
> -int
> +static int
>  secondary_sb_wack(xfs_mount_t *mp, xfs_buf_t *sbuf, xfs_sb_t *sb,
>  	xfs_agnumber_t i)
>  {
> Index: xfsprogs-dev/repair/attr_repair.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/attr_repair.c	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/attr_repair.c	2011-12-01 16:18:23.000000000 +0000
> @@ -80,8 +80,7 @@ static int xfs_mac_valid(xfs_mac_label_t
>   * in user attribute land without a conflict.
>   * If value is non-zero, then a remote attribute is being passed in
>   */
> -
> -int
> +static int
>  valuecheck(char *namevalue, char *value, int namelen, int valuelen)
>  {
>  	/* for proper alignment issues, get the structs and memmove the values */
> @@ -146,7 +145,7 @@ valuecheck(char *namevalue, char *value,
>   * if you cannot modify the structures. repair is set to 1, if anything
>   * was fixed.
>   */
> -int
> +static int
>  process_shortform_attr(
>  	xfs_ino_t	ino,
>  	xfs_dinode_t	*dip,
> @@ -490,7 +489,7 @@ bad_free_out:
>  	return -1;
>  }
>  
> -int
> +static int
>  process_leaf_attr_block(
>  	xfs_mount_t	*mp,
>  	xfs_attr_leafblock_t *leaf,
> @@ -643,7 +642,7 @@ process_leaf_attr_block(
>  /*
>   * returns 0 if the attribute fork is ok, 1 if it has to be junked.
>   */
> -int
> +static int
>  process_leaf_attr_level(xfs_mount_t	*mp,
>  			da_bt_cursor_t	*da_cursor)
>  {
> @@ -775,7 +774,7 @@ error_out:
>   * returns 0 if things are ok, 1 if bad
>   * Note this code has been based off process_node_dir.
>   */
> -int
> +static int
>  process_node_attr(
>  	xfs_mount_t	*mp,
>  	xfs_ino_t	ino,
> @@ -825,8 +824,7 @@ process_node_attr(
>   * returns 0 if things are ok, 1 if bad (attributes needs to be junked)
>   * repair is set, if anything was changed, but attributes can live thru it
>   */
> -
> -int
> +static int
>  process_longform_attr(
>  	xfs_mount_t	*mp,
>  	xfs_ino_t	ino,
> Index: xfsprogs-dev/repair/dino_chunks.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/dino_chunks.c	2011-12-01 16:18:23.000000000 +0000
> +++ xfsprogs-dev/repair/dino_chunks.c	2011-12-01 16:18:23.000000000 +0000
> @@ -34,8 +34,7 @@
>   * the dinodes are verified using verify_uncertain_dinode() which
>   * means only the basic inode info is checked, no fork checks.
>   */
> -
> -int
> +static int
>  check_aginode_block(xfs_mount_t	*mp,
>  			xfs_agnumber_t	agno,
>  			xfs_agblock_t	agbno)
> @@ -85,7 +84,7 @@ check_aginode_block(xfs_mount_t	*mp,
>   * routines called by check_uncertain_aginodes() and
>   * process_uncertain_aginodes().
>   */
> -int
> +static int
>  verify_inode_chunk(xfs_mount_t		*mp,
>  			xfs_ino_t	ino,
>  			xfs_ino_t	*start_ino)
> @@ -513,7 +512,7 @@ verify_inode_chunk(xfs_mount_t		*mp,
>  /*
>   * same as above only for ag inode chunks
>   */
> -int
> +static int
>  verify_aginode_chunk(xfs_mount_t	*mp,
>  			xfs_agnumber_t	agno,
>  			xfs_agino_t	agino,
> @@ -536,7 +535,7 @@ verify_aginode_chunk(xfs_mount_t	*mp,
>   * this does the same as the two above only it returns a pointer
>   * to the inode record in the good inode tree
>   */
> -ino_tree_node_t *
> +static ino_tree_node_t *
>  verify_aginode_chunk_irec(xfs_mount_t	*mp,
>  			xfs_agnumber_t	agno,
>  			xfs_agino_t	agino)
> Index: xfsprogs-dev/repair/dinode.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/dinode.c	2011-12-01 16:17:32.000000000 +0000
> +++ xfsprogs-dev/repair/dinode.c	2011-12-01 16:18:23.000000000 +0000
> @@ -36,47 +36,7 @@
>   * inode clearing routines
>   */
>  
> -/*
> - * return the offset into the inode where the attribute fork starts
> - */
> -/* ARGSUSED */
> -int
> -calc_attr_offset(xfs_mount_t *mp, xfs_dinode_t *dino)
> -{
> -	int	offset = (__psint_t)XFS_DFORK_DPTR(dino) - (__psint_t)dino;
> -	xfs_bmdr_block_t        *dfp;
> -
> -	/*
> -	 * don't worry about alignment when calculating offset
> -	 * because the data fork is already 8-byte aligned
> -	 */
> -	switch (dino->di_format)  {
> -	case XFS_DINODE_FMT_DEV:
> -		offset += sizeof(xfs_dev_t);
> -		break;
> -	case XFS_DINODE_FMT_LOCAL:
> -		offset += be64_to_cpu(dino->di_size);
> -		break;
> -	case XFS_DINODE_FMT_EXTENTS:
> -		offset += be32_to_cpu(dino->di_nextents) *
> -						sizeof(xfs_bmbt_rec_t);
> -		break;
> -	case XFS_DINODE_FMT_BTREE:
> -		dfp = (xfs_bmdr_block_t *)XFS_DFORK_DPTR(dino);
> -		offset += be16_to_cpu(dfp->bb_numrecs) *
> -						sizeof(xfs_bmbt_rec_t);
> -		break;
> -	default:
> -		do_error(_("Unknown inode format.\n"));
> -		abort();
> -		break;
> -	}
> -
> -	return(offset);
> -}
> -
> -/* ARGSUSED */
> -int
> +static int
>  clear_dinode_attr(xfs_mount_t *mp, xfs_dinode_t *dino, xfs_ino_t ino_num)
>  {
>  	ASSERT(dino->di_forkoff != 0);
> @@ -125,8 +85,7 @@ _("would have cleared inode %" PRIu64 "
>  	return(1);
>  }
>  
> -/* ARGSUSED */
> -int
> +static int
>  clear_dinode_core(xfs_dinode_t *dinoc, xfs_ino_t ino_num)
>  {
>  	int dirty = 0;
> @@ -262,8 +221,7 @@ clear_dinode_core(xfs_dinode_t *dinoc, x
>  	return(dirty);
>  }
>  
> -/* ARGSUSED */
> -int
> +static int
>  clear_dinode_unlinked(xfs_mount_t *mp, xfs_dinode_t *dino)
>  {
>  
> @@ -281,7 +239,7 @@ clear_dinode_unlinked(xfs_mount_t *mp, x
>   * until after the agi unlinked lists are walked in phase 3.
>   * returns > zero if the inode has been altered while being cleared
>   */
> -int
> +static int
>  clear_dinode(xfs_mount_t *mp, xfs_dinode_t *dino, xfs_ino_t ino_num)
>  {
>  	int dirty;
> @@ -445,31 +403,6 @@ verify_agbno(xfs_mount_t	*mp,
>  	return verify_ag_bno(sbp, agno, agbno) == 0;
>  }
>  
> -/*
> - * return address of block fblock if it's within the range described
> - * by the extent list.  Otherwise, returns a null address.
> - */
> -/* ARGSUSED */
> -xfs_dfsbno_t
> -get_bmbt_reclist(
> -	xfs_mount_t		*mp,
> -	xfs_bmbt_rec_t		*rp,
> -	int			numrecs,
> -	xfs_dfiloff_t		fblock)
> -{
> -	int			i;
> -	xfs_bmbt_irec_t 	irec;
> -
> -	for (i = 0; i < numrecs; i++) {
> -		libxfs_bmbt_disk_get_all(rp + i, &irec);
> -		if (irec.br_startoff >= fblock &&
> -				irec.br_startoff + irec.br_blockcount < fblock)
> -			return (irec.br_startblock + fblock - irec.br_startoff);
> -	}
> -	return(NULLDFSBNO);
> -}
> -
> -
>  static int
>  process_rt_rec(
>  	xfs_mount_t		*mp,
> @@ -601,8 +534,7 @@ _("illegal state %d in rt block map %" P
>   * file overlaps with any duplicate extents (in the
>   * duplicate extent list).
>   */
> -/* ARGSUSED */
> -int
> +static int
>  process_bmbt_reclist_int(
>  	xfs_mount_t		*mp,
>  	xfs_bmbt_rec_t		*rp,
> @@ -931,7 +863,7 @@ get_agino_buf(xfs_mount_t	 *mp,
>   *
>   * NOTE: getfunc_extlist only used by dirv1 checking code
>   */
> -xfs_dfsbno_t
> +static xfs_dfsbno_t
>  getfunc_extlist(xfs_mount_t		*mp,
>  		xfs_ino_t		ino,
>  		xfs_dinode_t		*dip,
> @@ -960,7 +892,7 @@ getfunc_extlist(xfs_mount_t		*mp,
>  /*
>   * NOTE: getfunc_btree only used by dirv1 checking code... 
>   */
> -xfs_dfsbno_t
> +static xfs_dfsbno_t
>  getfunc_btree(xfs_mount_t		*mp,
>  		xfs_ino_t		ino,
>  		xfs_dinode_t		*dip,
> @@ -1168,8 +1100,7 @@ get_bmapi(xfs_mount_t *mp, xfs_dinode_t
>  /*
>   * return 1 if inode should be cleared, 0 otherwise
>   */
> -/* ARGSUSED */
> -int
> +static int
>  process_btinode(
>  	xfs_mount_t		*mp,
>  	xfs_agnumber_t		agno,
> @@ -1344,8 +1275,7 @@ _("bad numrecs 0 in inode %" PRIu64 " bm
>  /*
>   * return 1 if inode should be cleared, 0 otherwise
>   */
> -/* ARGSUSED */
> -int
> +static int
>  process_exinode(
>  	xfs_mount_t		*mp,
>  	xfs_agnumber_t		agno,
> @@ -1431,7 +1361,7 @@ process_lclinode(
>  	return(0);
>  }
>  
> -int
> +static int
>  process_symlink_extlist(xfs_mount_t *mp, xfs_ino_t lino, xfs_dinode_t *dino)
>  {
>  	xfs_dfiloff_t		expected_offset;
> @@ -1502,7 +1432,7 @@ _("bad extent #%d count (%" PRIu64 ") in
>   * takes a name and length and returns 1 if the name contains
>   * a \0, returns 0 otherwise
>   */
> -int
> +static int
>  null_check(char *name, int length)
>  {
>  	int i;
> @@ -1521,7 +1451,7 @@ null_check(char *name, int length)
>   * like usual, returns 0 if everything's ok and 1 if something's
>   * bogus
>   */
> -int
> +static int
>  process_symlink(
>  	xfs_mount_t	*mp,
>  	xfs_ino_t	lino,
> @@ -2459,8 +2389,7 @@ _("would clear obsolete nlink field in v
>   *
>   * for detailed, info, look at process_dinode() comments.
>   */
> -/* ARGSUSED */
> -int
> +static int
>  process_dinode_int(xfs_mount_t *mp,
>  		xfs_dinode_t *dino,
>  		xfs_agnumber_t agno,
> Index: xfsprogs-dev/repair/dinode.h
> ===================================================================
> --- xfsprogs-dev.orig/repair/dinode.h	2011-12-01 16:17:32.000000000 +0000
> +++ xfsprogs-dev/repair/dinode.h	2011-12-01 16:18:23.000000000 +0000
> @@ -61,19 +61,6 @@ scan_bmbt_reclist(
>  	xfs_drfsbno_t		*tot,
>  	int			whichfork);
>  
> -int
> -verify_inode_chunk(xfs_mount_t		*mp,
> -			xfs_ino_t	ino,
> -			xfs_ino_t	*start_ino);
> -
> -int	verify_aginode_chunk(xfs_mount_t	*mp,
> -				xfs_agnumber_t	agno,
> -				xfs_agino_t	agino,
> -				xfs_agino_t	*agino_start);
> -
> -int
> -clear_dinode(xfs_mount_t *mp, xfs_dinode_t *dino, xfs_ino_t ino_num);
> -
>  void
>  update_rootino(xfs_mount_t *mp);
>  
> Index: xfsprogs-dev/repair/dir.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/dir.c	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/dir.c	2011-12-01 16:18:23.000000000 +0000
> @@ -72,8 +72,7 @@ namecheck(char *name, int length)
>   * entries.  a non-zero return value means the directory is bogus
>   * and should be blasted.
>   */
> -/* ARGSUSED */
> -int
> +static int
>  process_shortform_dir(
>  	xfs_mount_t	*mp,
>  	xfs_ino_t	ino,
> @@ -553,7 +552,7 @@ set_da_freemap(xfs_mount_t *mp, da_freem
>   * returns 0 if holemap is consistent with reality (as expressed by
>   * the da_freemap_t).  returns 1 if there's a conflict.
>   */
> -int
> +static int
>  verify_da_freemap(xfs_mount_t *mp, da_freemap_t *map, da_hole_map_t *holes,
>  			xfs_ino_t ino, xfs_dablk_t da_bno)
>  {
> @@ -591,7 +590,7 @@ verify_da_freemap(xfs_mount_t *mp, da_fr
>  	return(0);
>  }
>  
> -void
> +static void
>  process_da_freemap(xfs_mount_t *mp, da_freemap_t *map, da_hole_map_t *holes)
>  {
>  	int i, j, in_hole, start, length, smallest, num_holes;
> @@ -678,8 +677,7 @@ process_da_freemap(xfs_mount_t *mp, da_f
>  /*
>   * returns 1 if the hole info doesn't match, 0 if it does
>   */
> -/* ARGSUSED */
> -int
> +static int
>  compare_da_freemaps(xfs_mount_t *mp, da_hole_map_t *holemap,
>  			da_hole_map_t *block_hmap, int entries,
>  			xfs_ino_t ino, xfs_dablk_t da_bno)
> @@ -879,7 +877,7 @@ error_out:
>   * buffers (e.g. if we do, it's a mistake).  if error == 1, we're
>   * in an error-handling case so unreleased buffers may exist.
>   */
> -void
> +static void
>  release_da_cursor_int(xfs_mount_t	*mp,
>  			da_bt_cursor_t	*cursor,
>  			int		prev_level,
> @@ -922,91 +920,6 @@ err_release_da_cursor(xfs_mount_t	*mp,
>  }
>  
>  /*
> - * like traverse_int_dablock only it does far less checking
> - * and doesn't maintain the cursor.  Just gets you to the
> - * leftmost block in the directory.  returns the fsbno
> - * of that block if successful, NULLDFSBNO if not.
> - */
> -xfs_dfsbno_t
> -get_first_dblock_fsbno(xfs_mount_t	*mp,
> -			xfs_ino_t	ino,
> -			xfs_dinode_t	*dino)
> -{
> -	xfs_dablk_t		bno;
> -	int			i;
> -	xfs_da_intnode_t	*node;
> -	xfs_dfsbno_t		fsbno;
> -	xfs_buf_t		*bp;
> -
> -	/*
> -	 * traverse down left-side of tree until we hit the
> -	 * left-most leaf block setting up the btree cursor along
> -	 * the way.
> -	 */
> -	bno = 0;
> -	i = -1;
> -	node = NULL;
> -
> -	fsbno = get_bmapi(mp, dino, ino, bno, XFS_DATA_FORK);
> -
> -	if (fsbno == NULLDFSBNO)  {
> -		do_warn(_("bmap of block #%u of inode %" PRIu64 " failed\n"),
> -			bno, ino);
> -		return(fsbno);
> -	}
> -
> -	if (be64_to_cpu(dino->di_size) <= XFS_LBSIZE(mp))
> -		return(fsbno);
> -
> -	do {
> -		/*
> -		 * walk down left side of btree, release buffers as you
> -		 * go.  if the root block is a leaf (single-level btree),
> -		 * just return it.
> -		 *
> -		 */
> -
> -		bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
> -				XFS_FSB_TO_BB(mp, 1), 0);
> -		if (!bp) {
> -			do_warn(
> -	_("can't read block %u (fsbno %" PRIu64 ") for directory inode %" PRIu64 "\n"),
> -				bno, fsbno, ino);
> -			return(NULLDFSBNO);
> -		}
> -
> -		node = (xfs_da_intnode_t *)XFS_BUF_PTR(bp);
> -
> -		if (XFS_DA_NODE_MAGIC !=
> -		    be16_to_cpu(node->hdr.info.magic))  {
> -			do_warn(
> -	_("bad dir/attr magic number in inode %" PRIu64 ", file bno = %u, fsbno = %" PRIu64 "\n"),
> -				ino, bno, fsbno);
> -			libxfs_putbuf(bp);
> -			return(NULLDFSBNO);
> -		}
> -
> -		if (i == -1)
> -			i = be16_to_cpu(node->hdr.level);
> -		bno = be32_to_cpu(node->btree[0].before);
> -
> -		libxfs_putbuf(bp);
> -
> -		fsbno = get_bmapi(mp, dino, ino, bno, XFS_DATA_FORK);
> -
> -		if (fsbno == NULLDFSBNO)  {
> -			do_warn(_("bmap of block #%u of inode %" PRIu64 " failed\n"),
> -				bno, ino);
> -			return(NULLDFSBNO);
> -		}
> -
> -		i--;
> -	} while(i > 0);
> -
> -	return(fsbno);
> -}
> -
> -/*
>   * make sure that all entries in all blocks along the right side of
>   * of the tree are used and hashval's are consistent.  level is the
>   * level of the descendent block.  returns 0 if good (even if it had
> @@ -1401,8 +1314,7 @@ size_t ts_dirbuf_size = 64*1024;
>   * bad entry name index pointers), we lose the directory.  We could
>   * try harder to fix this but it'll do for now.
>   */
> -/* ARGSUSED */
> -int
> +static int
>  process_leaf_dir_block(
>  	xfs_mount_t		*mp,
>  	xfs_dir_leafblock_t	*leaf,
> @@ -2311,7 +2223,7 @@ _("- existing hole info for block %d, di
>  /*
>   * returns 0 if the directory is ok, 1 if it has to be junked.
>   */
> -int
> +static int
>  process_leaf_dir_level(xfs_mount_t	*mp,
>  			da_bt_cursor_t	*da_cursor,
>  			int		ino_discovery,
> @@ -2489,8 +2401,7 @@ error_out:
>   *
>   * returns 0 if things are ok, 1 if bad (directory needs to be junked)
>   */
> -/* ARGSUSED */
> -int
> +static int
>  process_node_dir(
>  	xfs_mount_t	*mp,
>  	xfs_ino_t	ino,
> @@ -2588,8 +2499,7 @@ _("setting directory inode (%" PRIu64 ")
>   *
>   * returns 0 if things are ok, 1 if bad (directory needs to be junked)
>   */
> -/* ARGSUSED */
> -int
> +static int
>  process_leaf_dir(
>  	xfs_mount_t	*mp,
>  	xfs_ino_t	ino,
> Index: xfsprogs-dev/repair/dir.h
> ===================================================================
> --- xfsprogs-dev.orig/repair/dir.h	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/dir.h	2011-12-01 16:18:23.000000000 +0000
> @@ -75,12 +75,6 @@ err_release_da_cursor(
>  	da_bt_cursor_t	*cursor,
>  	int		prev_level);
>  
> -xfs_dfsbno_t
> -get_first_dblock_fsbno(
> -	xfs_mount_t	*mp,
> -	xfs_ino_t	ino,
> -	xfs_dinode_t	*dino);
> -
>  void
>  init_da_freemap(
>  	da_freemap_t *dir_freemap);
> @@ -91,17 +85,6 @@ namecheck(
>  	int		length);
>  
>  int
> -process_shortform_dir(
> -	xfs_mount_t	*mp,
> -	xfs_ino_t	ino,
> -	xfs_dinode_t	*dip,
> -	int		ino_discovery,
> -	int		*dino_dirty,	/* is dinode buffer dirty? */
> -	xfs_ino_t	*parent,	/* out - NULLFSINO if entry doesn't exist */
> -	char		*dirname,	/* directory pathname */
> -	int		*repair);	/* out - 1 if dir was fixed up */
> -
> -int
>  process_dir(
>  	xfs_mount_t	*mp,
>  	xfs_ino_t	ino,
> Index: xfsprogs-dev/repair/dir2.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/dir2.c	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/dir2.c	2011-12-01 16:18:23.000000000 +0000
> @@ -43,9 +43,10 @@ typedef struct dir2_bad {
>  	xfs_ino_t	ino;
>  	struct dir2_bad	*next;
>  } dir2_bad_t;
> -dir2_bad_t *dir2_bad_list;
>  
> -void
> +static dir2_bad_t *dir2_bad_list;
> +
> +static void
>  dir2_add_badlist(
>  	xfs_ino_t	ino)
>  {
> @@ -78,7 +79,7 @@ dir2_is_badino(
>   * Multibuffer handling.
>   * V2 directory blocks can be noncontiguous, needing multiple buffers.
>   */
> -xfs_dabuf_t *
> +static xfs_dabuf_t *
>  da_read_buf(
>  	xfs_mount_t	*mp,
>  	int		nex,
> @@ -186,7 +187,7 @@ da_buf_done(
>  	free(dabuf);
>  }
>  
> -int
> +static int
>  da_bwrite(
>  	xfs_mount_t	*mp,
>  	xfs_dabuf_t	*dabuf)
> @@ -226,7 +227,7 @@ da_bwrite(
>  	return error;
>  }
>  
> -void
> +static void
>  da_brelse(
>  	xfs_dabuf_t	*dabuf)
>  {
> @@ -262,7 +263,7 @@ da_brelse(
>   * left-most leaf block if successful (bno).  returns 1 if successful,
>   * 0 if unsuccessful.
>   */
> -int
> +static int
>  traverse_int_dir2block(xfs_mount_t	*mp,
>  		dir2_bt_cursor_t	*da_cursor,
>  		xfs_dablk_t		*rbno)
> @@ -392,7 +393,7 @@ error_out:
>   * buffers (e.g. if we do, it's a mistake).  if error == 1, we're
>   * in an error-handling case so unreleased buffers may exist.
>   */
> -void
> +static void
>  release_dir2_cursor_int(xfs_mount_t		*mp,
>  			dir2_bt_cursor_t	*cursor,
>  			int			prev_level,
> @@ -418,7 +419,7 @@ release_dir2_cursor_int(xfs_mount_t		*mp
>  	return;
>  }
>  
> -void
> +static void
>  release_dir2_cursor(xfs_mount_t		*mp,
>  		dir2_bt_cursor_t	*cursor,
>  		int			prev_level)
> @@ -426,7 +427,7 @@ release_dir2_cursor(xfs_mount_t		*mp,
>  	release_dir2_cursor_int(mp, cursor, prev_level, 0);
>  }
>  
> -void
> +static void
>  err_release_dir2_cursor(xfs_mount_t		*mp,
>  			dir2_bt_cursor_t	*cursor,
>  			int			prev_level)
> @@ -442,7 +443,7 @@ err_release_dir2_cursor(xfs_mount_t		*mp
>   * technically a block boundary.  This routine should be used then
>   * instead of verify_dir2_path().
>   */
> -int
> +static int
>  verify_final_dir2_path(xfs_mount_t	*mp,
>  		dir2_bt_cursor_t	*cursor,
>  		const int		p_level)
> @@ -589,7 +590,7 @@ _("would correct bad hashval in non-leaf
>   * since they have to be set so we can get a buffer for the
>   * block.
>   */
> -int
> +static int
>  verify_dir2_path(xfs_mount_t	*mp,
>  	dir2_bt_cursor_t	*cursor,
>  	const int		p_level)
> Index: xfsprogs-dev/repair/dir2.h
> ===================================================================
> --- xfsprogs-dev.orig/repair/dir2.h	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/dir2.h	2011-12-01 16:18:23.000000000 +0000
> @@ -59,30 +59,6 @@ typedef struct dir2_bt_cursor  {
>  	struct blkmap		*blkmap;
>  } dir2_bt_cursor_t;
>  
> -
> -/* ROUTINES */
> -
> -void
> -err_release_dir2_cursor(
> -	xfs_mount_t		*mp,
> -	dir2_bt_cursor_t	*cursor,
> -	int			prev_level);
> -
> -xfs_dabuf_t *
> -da_read_buf(
> -	xfs_mount_t	*mp,
> -	int		nex,
> -	struct bmap_ext	*bmp);
> -
> -int
> -da_bwrite(
> -	xfs_mount_t	*mp,
> -	xfs_dabuf_t	*bp);
> -
> -void
> -da_brelse(
> -	xfs_dabuf_t	*bp);
> -
>  int
>  process_dir2(
>  	xfs_mount_t	*mp,
> @@ -99,10 +75,6 @@ process_sf_dir2_fixi8(
>  	xfs_dir2_sf_t		*sfp,
>  	xfs_dir2_sf_entry_t	**next_sfep);
>  
> -void
> -dir2_add_badlist(
> -	xfs_ino_t	ino);
> -
>  int
>  dir2_is_badino(
>  	xfs_ino_t	ino);
> Index: xfsprogs-dev/repair/phase1.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/phase1.c	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/phase1.c	2011-12-01 16:18:23.000000000 +0000
> @@ -22,7 +22,7 @@
>  #include "protos.h"
>  #include "err_protos.h"
>  
> -void
> +static void
>  no_sb(void)
>  {
>  	do_warn(_("Sorry, could not find valid secondary superblock\n"));
> Index: xfsprogs-dev/repair/phase4.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/phase4.c	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/phase4.c	2011-12-01 16:18:23.000000000 +0000
> @@ -40,7 +40,7 @@
>   * free in which case they'd never be cleared so the fields wouldn't
>   * be cleared by process_dinode().
>   */
> -void
> +static void
>  quotino_check(xfs_mount_t *mp)
>  {
>  	ino_tree_node_t *irec;
> @@ -81,7 +81,7 @@ quotino_check(xfs_mount_t *mp)
>  	}
>  }
>  
> -void
> +static void
>  quota_sb_check(xfs_mount_t *mp)
>  {
>  	/*
> Index: xfsprogs-dev/repair/phase5.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/phase5.c	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/phase5.c	2011-12-01 16:18:23.000000000 +0000
> @@ -78,7 +78,7 @@ static __uint64_t	*sb_icount_ag;		/* all
>  static __uint64_t	*sb_ifree_ag;		/* free inodes per ag */
>  static __uint64_t	*sb_fdblocks_ag;	/* free data blocks per ag */
>  
> -int
> +static int
>  mk_incore_fstree(xfs_mount_t *mp, xfs_agnumber_t agno)
>  {
>  	int			in_extent;
> @@ -165,8 +165,7 @@ mk_incore_fstree(xfs_mount_t *mp, xfs_ag
>  	return(num_extents);
>  }
>  
> -/* ARGSUSED */
> -xfs_agblock_t
> +static xfs_agblock_t
>  get_next_blockaddr(xfs_agnumber_t agno, int level, bt_status_t *curs)
>  {
>  	ASSERT(curs->free_btree_blocks < curs->btree_blocks +
> @@ -185,8 +184,7 @@ get_next_blockaddr(xfs_agnumber_t agno,
>   * cursor pointer to the btree root.   called by init_freespace_cursor()
>   * and init_ino_cursor()
>   */
> -/* ARGSUSED */
> -void
> +static void
>  setup_cursor(xfs_mount_t *mp, xfs_agnumber_t agno, bt_status_t *curs)
>  {
>  	int			j;
> @@ -301,7 +299,7 @@ setup_cursor(xfs_mount_t *mp, xfs_agnumb
>  #endif
>  }
>  
> -void
> +static void
>  write_cursor(bt_status_t *curs)
>  {
>  	int i;
> @@ -322,7 +320,7 @@ write_cursor(bt_status_t *curs)
>  	}
>  }
>  
> -void
> +static void
>  finish_cursor(bt_status_t *curs)
>  {
>  	ASSERT(curs->num_free_blocks == 0);
> @@ -341,8 +339,7 @@ finish_cursor(bt_status_t *curs)
>   * btree_curs is an in/out.  returns the number of
>   * blocks that will show up in the AGFL.
>   */
> -
> -int
> +static int
>  calculate_freespace_cursor(xfs_mount_t *mp, xfs_agnumber_t agno,
>  			xfs_agblock_t *extents, bt_status_t *btree_curs)
>  {
> @@ -595,7 +592,7 @@ calculate_freespace_cursor(xfs_mount_t *
>  	return(extra_blocks);
>  }
>  
> -void
> +static void
>  prop_freespace_cursor(xfs_mount_t *mp, xfs_agnumber_t agno,
>  		bt_status_t *btree_curs, xfs_agblock_t startblock,
>  		xfs_extlen_t blockcount, int level, __uint32_t magic)
> @@ -689,7 +686,7 @@ prop_freespace_cursor(xfs_mount_t *mp, x
>   * of tree to build (bno or bcnt).  returns the number of free blocks
>   * represented by the tree.
>   */
> -xfs_extlen_t
> +static xfs_extlen_t
>  build_freespace_tree(xfs_mount_t *mp, xfs_agnumber_t agno,
>  		bt_status_t *btree_curs, __uint32_t magic)
>  {
> @@ -854,7 +851,7 @@ build_freespace_tree(xfs_mount_t *mp, xf
>   * may perturb things because inode tree building happens before
>   * freespace tree building.
>   */
> -void
> +static void
>  init_ino_cursor(xfs_mount_t *mp, xfs_agnumber_t agno, bt_status_t *btree_curs,
>  		__uint64_t *num_inos, __uint64_t *num_free_inos)
>  {
> @@ -942,7 +939,7 @@ init_ino_cursor(xfs_mount_t *mp, xfs_agn
>  	return;
>  }
>  
> -void
> +static void
>  prop_ino_cursor(xfs_mount_t *mp, xfs_agnumber_t agno, bt_status_t *btree_curs,
>  	xfs_agino_t startino, int level)
>  {
> @@ -1027,7 +1024,7 @@ prop_ino_cursor(xfs_mount_t *mp, xfs_agn
>  	*bt_ptr = cpu_to_be32(btree_curs->level[level-1].agbno);
>  }
>  
> -void
> +static void
>  build_agi(xfs_mount_t *mp, xfs_agnumber_t agno,
>  		bt_status_t *btree_curs, xfs_agino_t first_agino,
>  		xfs_agino_t count, xfs_agino_t freecount)
> @@ -1067,7 +1064,7 @@ build_agi(xfs_mount_t *mp, xfs_agnumber_
>   * rebuilds an inode tree given a cursor.  We're lazy here and call
>   * the routine that builds the agi
>   */
> -void
> +static void
>  build_ino_tree(xfs_mount_t *mp, xfs_agnumber_t agno,
>  		bt_status_t *btree_curs)
>  {
> @@ -1197,7 +1194,7 @@ build_ino_tree(xfs_mount_t *mp, xfs_agnu
>   * build both the agf and the agfl for an agno given both
>   * btree cursors
>   */
> -void
> +static void
>  build_agf_agfl(xfs_mount_t	*mp,
>  		xfs_agnumber_t	agno,
>  		bt_status_t	*bno_bt,
> @@ -1353,7 +1350,7 @@ build_agf_agfl(xfs_mount_t	*mp,
>   * feature bits to the filesystem, and sync up the on-disk superblock
>   * to match the incore superblock.
>   */
> -void
> +static void
>  sync_sb(xfs_mount_t *mp)
>  {
>  	xfs_buf_t	*bp;
> @@ -1377,7 +1374,7 @@ sync_sb(xfs_mount_t *mp)
>   * make sure the root and realtime inodes show up allocated
>   * even if they've been freed.  they get reinitialized in phase6.
>   */
> -void
> +static void
>  keep_fsinos(xfs_mount_t *mp)
>  {
>  	ino_tree_node_t		*irec;
> Index: xfsprogs-dev/repair/phase6.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/phase6.c	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/phase6.c	2011-12-01 16:18:23.000000000 +0000
> @@ -509,7 +509,7 @@ mk_rbmino(xfs_mount_t *mp)
>  	libxfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_SYNC);
>  }
>  
> -int
> +static int
>  fill_rbmino(xfs_mount_t *mp)
>  {
>  	xfs_buf_t	*bp;
> @@ -576,7 +576,7 @@ _("can't access block %" PRIu64 " (fsbno
>  	return(0);
>  }
>  
> -int
> +static int
>  fill_rsumino(xfs_mount_t *mp)
>  {
>  	xfs_buf_t	*bp;
> @@ -645,7 +645,7 @@ _("can't access block %" PRIu64 " (fsbno
>  	return(0);
>  }
>  
> -void
> +static void
>  mk_rsumino(xfs_mount_t *mp)
>  {
>  	xfs_trans_t	*tp;
> @@ -751,7 +751,7 @@ mk_rsumino(xfs_mount_t *mp)
>  /*
>   * makes a new root directory.
>   */
> -void
> +static void
>  mk_root_dir(xfs_mount_t *mp)
>  {
>  	xfs_trans_t	*tp;
> @@ -815,7 +815,7 @@ mk_root_dir(xfs_mount_t *mp)
>  /*
>   * orphanage name == lost+found
>   */
> -xfs_ino_t
> +static xfs_ino_t
>  mk_orphanage(xfs_mount_t *mp)
>  {
>  	xfs_ino_t	ino;
> @@ -1130,9 +1130,6 @@ mv_orphanage(
>  }
>  
>  /*
> - * like get_first_dblock_fsbno only it uses the simulation code instead
> - * of raw I/O.
> - *
>   * Returns the fsbno of the first (leftmost) block in the directory leaf.
>   * sets *bno to the directory block # corresponding to the returned fsbno.
>   */
> @@ -3519,7 +3516,7 @@ out:
>   * mark realtime bitmap and summary inodes as reached.
>   * quota inode will be marked here as well
>   */
> -void
> +static void
>  mark_standalone_inodes(xfs_mount_t *mp)
>  {
>  	ino_tree_node_t		*irec;
> Index: xfsprogs-dev/repair/progress.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/progress.c	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/progress.c	2011-12-01 16:18:23.000000000 +0000
> @@ -360,7 +360,7 @@ print_final_rpt(void)
>  	return(sum);
>  }
>  
> -void
> +static void
>  timediff(int phase)
>  {
>  	phase_times[phase].duration =
> Index: xfsprogs-dev/repair/sb.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/sb.c	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/sb.c	2011-12-01 16:18:23.000000000 +0000
> @@ -32,7 +32,7 @@
>   * copy the fields of a superblock that are present in primary and
>   * secondaries -- preserve fields that are different in the primary.
>   */
> -void
> +static void
>  copy_sb(xfs_sb_t *source, xfs_sb_t *dest)
>  {
>  	xfs_ino_t	rootino;
> @@ -169,7 +169,7 @@ find_secondary_sb(xfs_sb_t *rsb)
>   * calculate what inode alignment field ought to be
>   * based on internal superblock info
>   */
> -int
> +static int
>  calc_ino_align(xfs_sb_t *sb)
>  {
>  	xfs_extlen_t align;
> @@ -516,8 +516,7 @@ get_sb(xfs_sb_t *sbp, xfs_off_t off, int
>  }
>  
>  /* returns element on list with highest reference count */
> -
> -fs_geo_list_t *
> +static fs_geo_list_t *
>  get_best_geo(fs_geo_list_t *list)
>  {
>  	int cnt = 0;
> @@ -537,8 +536,7 @@ get_best_geo(fs_geo_list_t *list)
>  }
>  
>  /* adds geometry info to linked list.  returns (sometimes new) head of list */
> -
> -fs_geo_list_t *
> +static fs_geo_list_t *
>  add_geo(fs_geo_list_t *list, fs_geometry_t *geo_p, int index)
>  {
>  	fs_geo_list_t	*current = list;
> @@ -565,7 +563,7 @@ add_geo(fs_geo_list_t *list, fs_geometry
>  	return(current);
>  }
>  
> -void
> +static void
>  free_geo(fs_geo_list_t *list)
>  {
>  	fs_geo_list_t	*next;
> Index: xfsprogs-dev/repair/scan.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/scan.c	2011-12-01 16:17:32.000000000 +0000
> +++ xfsprogs-dev/repair/scan.c	2011-12-01 16:18:23.000000000 +0000
> @@ -69,7 +69,7 @@ set_mp(xfs_mount_t *mpp)
>  	mp = mpp;
>  }
>  
> -void
> +static void
>  scan_sbtree(
>  	xfs_agblock_t	root,
>  	int		nlevels,
> Index: xfsprogs-dev/repair/xfs_repair.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/xfs_repair.c	2011-12-01 16:17:58.000000000 +0000
> +++ xfsprogs-dev/repair/xfs_repair.c	2011-12-01 16:18:23.000000000 +0000
> @@ -49,8 +49,7 @@ extern void	phase7(xfs_mount_t *);
>  /*
>   * -o: user-supplied override options
>   */
> -
> -char *o_opts[] = {
> +static char *o_opts[] = {
>  #define ASSUME_XFS	0
>  	"assume_xfs",
>  #define PRE_65_BETA	1
> @@ -71,8 +70,7 @@ char *o_opts[] = {
>  /*
>   * -c: conversion options
>   */
> -
> -char *c_opts[] = {
> +static char *c_opts[] = {
>  #define CONVERT_LAZY_COUNT	0
>  	"lazycount",
>  	NULL
> @@ -183,7 +181,7 @@ unknown(char opt, char *s)
>  /*
>   * sets only the global argument flags and variables
>   */
> -void
> +static void
>  process_args(int argc, char **argv)
>  {
>  	char *p;
> @@ -398,7 +396,7 @@ do_log(char const *msg, ...)
>  	va_end(args);
>  }
>  
> -void
> +static void
>  calc_mkfs(xfs_mount_t *mp)
>  {
>  	xfs_agblock_t	fino_bno;
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---

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

Re: [PATCH 10/12] repair: mark local functions static

[Mark Tinguely]

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

Changes look appropriate. Compiles and runs without errors.

--Mark Tinguely.

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

[PATCH 11/12] repair: move extern declarations to headers

[Christoph Hellwig]

[-- Attachment #1: repair-cleanup-externs --]
[-- Type: text/plain, Size: 6581 bytes --]

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfsprogs-dev/repair/avl.h
===================================================================
--- xfsprogs-dev.orig/repair/avl.h	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/avl.h	2011-12-01 16:18:33.000000000 +0000
@@ -134,6 +134,15 @@ avl_findranges(
 	avlnode_t	        **startp,
 	avlnode_t		**endp);
 
+avlnode_t *
+avl_firstino(
+	avlnode_t		*root);
+
+avlnode_t *
+avl_lastino(
+	avlnode_t		*root);
+
+
 #define AVL_PRECEED	0x1
 #define AVL_SUCCEED	0x2
 
Index: xfsprogs-dev/repair/bmap.h
===================================================================
--- xfsprogs-dev.orig/repair/bmap.h	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/bmap.h	2011-12-01 16:18:33.000000000 +0000
@@ -53,6 +53,9 @@ typedef	struct blkmap {
 #define BLKMAP_NEXTS_MAX	INT_MAX
 #endif
 
+extern pthread_key_t dblkmap_key;
+extern pthread_key_t ablkmap_key;
+
 blkmap_t	*blkmap_alloc(xfs_extnum_t nex, int whichfork);
 void		blkmap_free(blkmap_t *blkmap);
 
Index: xfsprogs-dev/repair/incore.h
===================================================================
--- xfsprogs-dev.orig/repair/incore.h	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/incore.h	2011-12-01 16:18:33.000000000 +0000
@@ -209,9 +209,12 @@ void		incore_ext_init(xfs_mount_t *);
  * the nodes.
  */
 void		incore_ext_teardown(xfs_mount_t *mp);
-
 void		incore_ino_init(xfs_mount_t *);
 
+int		count_bno_extents(xfs_agnumber_t);
+int		count_bno_extents_blocks(xfs_agnumber_t, uint *);
+int		count_bcnt_extents(xfs_agnumber_t);
+
 /*
  * inode definitions
  */
Index: xfsprogs-dev/repair/incore_ext.c
===================================================================
--- xfsprogs-dev.orig/repair/incore_ext.c	2011-12-01 16:17:31.000000000 +0000
+++ xfsprogs-dev/repair/incore_ext.c	2011-12-01 16:18:33.000000000 +0000
@@ -386,8 +386,6 @@ findfirst_bcnt_extent(xfs_agnumber_t agn
 extent_tree_node_t *
 findbiggest_bcnt_extent(xfs_agnumber_t agno)
 {
-	extern avlnode_t *avl_lastino(avlnode_t *root);
-
 	ASSERT(extent_bcnt_ptrs != NULL);
 	ASSERT(extent_bcnt_ptrs[agno] != NULL);
 
Index: xfsprogs-dev/repair/incore_ino.c
===================================================================
--- xfsprogs-dev.orig/repair/incore_ino.c	2011-12-01 16:17:31.000000000 +0000
+++ xfsprogs-dev/repair/incore_ino.c	2011-12-01 16:18:51.000000000 +0000
@@ -25,8 +25,6 @@
 #include "threads.h"
 #include "err_protos.h"
 
-extern avlnode_t	*avl_firstino(avlnode_t *root);
-
 /*
  * array of inode tree ptrs, one per ag
  */
Index: xfsprogs-dev/repair/init.c
===================================================================
--- xfsprogs-dev.orig/repair/init.c	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/init.c	2011-12-01 16:18:33.000000000 +0000
@@ -34,9 +34,6 @@ static pthread_key_t dirbuf_key;
 static pthread_key_t dir_freemap_key;
 static pthread_key_t attr_freemap_key;
 
-extern pthread_key_t dblkmap_key;
-extern pthread_key_t ablkmap_key;
-
 static void
 ts_alloc(pthread_key_t key, unsigned n, size_t size)
 {
Index: xfsprogs-dev/repair/phase4.c
===================================================================
--- xfsprogs-dev.orig/repair/phase4.c	2011-12-01 16:18:23.000000000 +0000
+++ xfsprogs-dev/repair/phase4.c	2011-12-01 16:18:33.000000000 +0000
@@ -196,8 +196,6 @@ phase4(xfs_mount_t *mp)
 	int			ag_hdr_len = 4 * mp->m_sb.sb_sectsize;
 	int			ag_hdr_block;
 	int			bstate;
-	int			count_bcnt_extents(xfs_agnumber_t agno);
-	int			count_bno_extents(xfs_agnumber_t agno);
 
 	ag_hdr_block = howmany(ag_hdr_len, mp->m_sb.sb_blocksize);
 
Index: xfsprogs-dev/repair/phase5.c
===================================================================
--- xfsprogs-dev.orig/repair/phase5.c	2011-12-01 16:18:23.000000000 +0000
+++ xfsprogs-dev/repair/phase5.c	2011-12-01 16:18:33.000000000 +0000
@@ -1404,11 +1404,6 @@ phase5_func(
 	xfs_extlen_t	freeblks2;
 #endif
 	xfs_agblock_t	num_extents;
-	extern int	count_bno_extents(xfs_agnumber_t);
-	extern int	count_bno_extents_blocks(xfs_agnumber_t, uint *);
-#ifdef XR_BLD_FREE_TRACE
-	extern int	count_bcnt_extents(xfs_agnumber_t);
-#endif
 
 	if (verbose)
 		do_log(_("        - agno = %d\n"), agno);
Index: xfsprogs-dev/repair/protos.h
===================================================================
--- xfsprogs-dev.orig/repair/protos.h	2011-12-01 16:17:07.000000000 +0000
+++ xfsprogs-dev/repair/protos.h	2011-12-01 16:18:33.000000000 +0000
@@ -39,11 +39,23 @@ void	get_sb_geometry(struct fs_geometry
 char	*alloc_ag_buf(int size);
 
 void	print_inode_list(xfs_agnumber_t i);
-char *	err_string(int err_code);
+char	*err_string(int err_code);
 
-extern void *ts_attr_freemap(void);
-extern void *ts_dir_freemap(void);
-extern void *ts_dirbuf(void);
-extern void ts_init(void);
-extern void thread_init(void);
+void	*ts_attr_freemap(void);
+void	*ts_dir_freemap(void);
+void	*ts_dirbuf(void);
+void	ts_init(void);
+void	thread_init(void);
+
+void	phase1(struct xfs_mount *);
+void	phase2(struct xfs_mount *, int);
+void	phase3(struct xfs_mount *);
+void	phase4(struct xfs_mount *);
+void	phase5(struct xfs_mount *);
+void	phase6(struct xfs_mount *);
+void	phase7(struct xfs_mount *);
+
+int	verify_set_agheader(struct xfs_mount *, struct xfs_buf *,
+		struct xfs_sb *, struct xfs_agf *, struct xfs_agi *,
+		xfs_agnumber_t);
 
Index: xfsprogs-dev/repair/scan.c
===================================================================
--- xfsprogs-dev.orig/repair/scan.c	2011-12-01 16:18:23.000000000 +0000
+++ xfsprogs-dev/repair/scan.c	2011-12-01 16:18:33.000000000 +0000
@@ -30,9 +30,6 @@
 #include "progress.h"
 #include "threads.h"
 
-extern int verify_set_agheader(xfs_mount_t *mp, xfs_buf_t *sbuf, xfs_sb_t *sb,
-		xfs_agf_t *agf, xfs_agi_t *agi, xfs_agnumber_t i);
-
 static xfs_mount_t	*mp = NULL;
 
 /*
Index: xfsprogs-dev/repair/xfs_repair.c
===================================================================
--- xfsprogs-dev.orig/repair/xfs_repair.c	2011-12-01 16:18:23.000000000 +0000
+++ xfsprogs-dev/repair/xfs_repair.c	2011-12-01 16:18:33.000000000 +0000
@@ -32,14 +32,6 @@
 
 #define	rounddown(x, y)	(((x)/(y))*(y))
 
-extern void	phase1(xfs_mount_t *);
-extern void	phase2(xfs_mount_t *, int);
-extern void	phase3(xfs_mount_t *);
-extern void	phase4(xfs_mount_t *);
-extern void	phase5(xfs_mount_t *);
-extern void	phase6(xfs_mount_t *);
-extern void	phase7(xfs_mount_t *);
-
 #define		XR_MAX_SECT_SIZE	(64 * 1024)
 
 /*

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

Re: [PATCH 11/12] repair: move extern declarations to headers

[Christoph Hellwig]

ping?

On Fri, Dec 02, 2011 at 12:46:30PM -0500, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: xfsprogs-dev/repair/avl.h
> ===================================================================
> --- xfsprogs-dev.orig/repair/avl.h	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/avl.h	2011-12-01 16:18:33.000000000 +0000
> @@ -134,6 +134,15 @@ avl_findranges(
>  	avlnode_t	        **startp,
>  	avlnode_t		**endp);
>  
> +avlnode_t *
> +avl_firstino(
> +	avlnode_t		*root);
> +
> +avlnode_t *
> +avl_lastino(
> +	avlnode_t		*root);
> +
> +
>  #define AVL_PRECEED	0x1
>  #define AVL_SUCCEED	0x2
>  
> Index: xfsprogs-dev/repair/bmap.h
> ===================================================================
> --- xfsprogs-dev.orig/repair/bmap.h	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/bmap.h	2011-12-01 16:18:33.000000000 +0000
> @@ -53,6 +53,9 @@ typedef	struct blkmap {
>  #define BLKMAP_NEXTS_MAX	INT_MAX
>  #endif
>  
> +extern pthread_key_t dblkmap_key;
> +extern pthread_key_t ablkmap_key;
> +
>  blkmap_t	*blkmap_alloc(xfs_extnum_t nex, int whichfork);
>  void		blkmap_free(blkmap_t *blkmap);
>  
> Index: xfsprogs-dev/repair/incore.h
> ===================================================================
> --- xfsprogs-dev.orig/repair/incore.h	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/incore.h	2011-12-01 16:18:33.000000000 +0000
> @@ -209,9 +209,12 @@ void		incore_ext_init(xfs_mount_t *);
>   * the nodes.
>   */
>  void		incore_ext_teardown(xfs_mount_t *mp);
> -
>  void		incore_ino_init(xfs_mount_t *);
>  
> +int		count_bno_extents(xfs_agnumber_t);
> +int		count_bno_extents_blocks(xfs_agnumber_t, uint *);
> +int		count_bcnt_extents(xfs_agnumber_t);
> +
>  /*
>   * inode definitions
>   */
> Index: xfsprogs-dev/repair/incore_ext.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/incore_ext.c	2011-12-01 16:17:31.000000000 +0000
> +++ xfsprogs-dev/repair/incore_ext.c	2011-12-01 16:18:33.000000000 +0000
> @@ -386,8 +386,6 @@ findfirst_bcnt_extent(xfs_agnumber_t agn
>  extent_tree_node_t *
>  findbiggest_bcnt_extent(xfs_agnumber_t agno)
>  {
> -	extern avlnode_t *avl_lastino(avlnode_t *root);
> -
>  	ASSERT(extent_bcnt_ptrs != NULL);
>  	ASSERT(extent_bcnt_ptrs[agno] != NULL);
>  
> Index: xfsprogs-dev/repair/incore_ino.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/incore_ino.c	2011-12-01 16:17:31.000000000 +0000
> +++ xfsprogs-dev/repair/incore_ino.c	2011-12-01 16:18:51.000000000 +0000
> @@ -25,8 +25,6 @@
>  #include "threads.h"
>  #include "err_protos.h"
>  
> -extern avlnode_t	*avl_firstino(avlnode_t *root);
> -
>  /*
>   * array of inode tree ptrs, one per ag
>   */
> Index: xfsprogs-dev/repair/init.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/init.c	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/init.c	2011-12-01 16:18:33.000000000 +0000
> @@ -34,9 +34,6 @@ static pthread_key_t dirbuf_key;
>  static pthread_key_t dir_freemap_key;
>  static pthread_key_t attr_freemap_key;
>  
> -extern pthread_key_t dblkmap_key;
> -extern pthread_key_t ablkmap_key;
> -
>  static void
>  ts_alloc(pthread_key_t key, unsigned n, size_t size)
>  {
> Index: xfsprogs-dev/repair/phase4.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/phase4.c	2011-12-01 16:18:23.000000000 +0000
> +++ xfsprogs-dev/repair/phase4.c	2011-12-01 16:18:33.000000000 +0000
> @@ -196,8 +196,6 @@ phase4(xfs_mount_t *mp)
>  	int			ag_hdr_len = 4 * mp->m_sb.sb_sectsize;
>  	int			ag_hdr_block;
>  	int			bstate;
> -	int			count_bcnt_extents(xfs_agnumber_t agno);
> -	int			count_bno_extents(xfs_agnumber_t agno);
>  
>  	ag_hdr_block = howmany(ag_hdr_len, mp->m_sb.sb_blocksize);
>  
> Index: xfsprogs-dev/repair/phase5.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/phase5.c	2011-12-01 16:18:23.000000000 +0000
> +++ xfsprogs-dev/repair/phase5.c	2011-12-01 16:18:33.000000000 +0000
> @@ -1404,11 +1404,6 @@ phase5_func(
>  	xfs_extlen_t	freeblks2;
>  #endif
>  	xfs_agblock_t	num_extents;
> -	extern int	count_bno_extents(xfs_agnumber_t);
> -	extern int	count_bno_extents_blocks(xfs_agnumber_t, uint *);
> -#ifdef XR_BLD_FREE_TRACE
> -	extern int	count_bcnt_extents(xfs_agnumber_t);
> -#endif
>  
>  	if (verbose)
>  		do_log(_("        - agno = %d\n"), agno);
> Index: xfsprogs-dev/repair/protos.h
> ===================================================================
> --- xfsprogs-dev.orig/repair/protos.h	2011-12-01 16:17:07.000000000 +0000
> +++ xfsprogs-dev/repair/protos.h	2011-12-01 16:18:33.000000000 +0000
> @@ -39,11 +39,23 @@ void	get_sb_geometry(struct fs_geometry
>  char	*alloc_ag_buf(int size);
>  
>  void	print_inode_list(xfs_agnumber_t i);
> -char *	err_string(int err_code);
> +char	*err_string(int err_code);
>  
> -extern void *ts_attr_freemap(void);
> -extern void *ts_dir_freemap(void);
> -extern void *ts_dirbuf(void);
> -extern void ts_init(void);
> -extern void thread_init(void);
> +void	*ts_attr_freemap(void);
> +void	*ts_dir_freemap(void);
> +void	*ts_dirbuf(void);
> +void	ts_init(void);
> +void	thread_init(void);
> +
> +void	phase1(struct xfs_mount *);
> +void	phase2(struct xfs_mount *, int);
> +void	phase3(struct xfs_mount *);
> +void	phase4(struct xfs_mount *);
> +void	phase5(struct xfs_mount *);
> +void	phase6(struct xfs_mount *);
> +void	phase7(struct xfs_mount *);
> +
> +int	verify_set_agheader(struct xfs_mount *, struct xfs_buf *,
> +		struct xfs_sb *, struct xfs_agf *, struct xfs_agi *,
> +		xfs_agnumber_t);
>  
> Index: xfsprogs-dev/repair/scan.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/scan.c	2011-12-01 16:18:23.000000000 +0000
> +++ xfsprogs-dev/repair/scan.c	2011-12-01 16:18:33.000000000 +0000
> @@ -30,9 +30,6 @@
>  #include "progress.h"
>  #include "threads.h"
>  
> -extern int verify_set_agheader(xfs_mount_t *mp, xfs_buf_t *sbuf, xfs_sb_t *sb,
> -		xfs_agf_t *agf, xfs_agi_t *agi, xfs_agnumber_t i);
> -
>  static xfs_mount_t	*mp = NULL;
>  
>  /*
> Index: xfsprogs-dev/repair/xfs_repair.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/xfs_repair.c	2011-12-01 16:18:23.000000000 +0000
> +++ xfsprogs-dev/repair/xfs_repair.c	2011-12-01 16:18:33.000000000 +0000
> @@ -32,14 +32,6 @@
>  
>  #define	rounddown(x, y)	(((x)/(y))*(y))
>  
> -extern void	phase1(xfs_mount_t *);
> -extern void	phase2(xfs_mount_t *, int);
> -extern void	phase3(xfs_mount_t *);
> -extern void	phase4(xfs_mount_t *);
> -extern void	phase5(xfs_mount_t *);
> -extern void	phase6(xfs_mount_t *);
> -extern void	phase7(xfs_mount_t *);
> -
>  #define		XR_MAX_SECT_SIZE	(64 * 1024)
>  
>  /*
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---

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

Re: [PATCH 11/12] repair: move extern declarations to headers

[Mark Tinguely]

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

Looks good. compiles and runs the same.

--Mark Tinguely

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

[PATCH 12/12] repair: cleanup inode record macros

[Christoph Hellwig]

[-- Attachment #1: repair-cleanup-irec-macros --]
[-- Type: text/plain, Size: 12746 bytes --]

Remove indirections in the inode record bit manipulation macros and flatten
them to a single level of inlines.  Also use a common IREC_MASK define
instead of duplicating it for every bitmask.


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfsprogs-dev/repair/incore.h
===================================================================
--- xfsprogs-dev.orig/repair/incore.h	2011-12-02 11:04:59.000000000 +0000
+++ xfsprogs-dev/repair/incore.h	2011-12-02 11:17:31.000000000 +0000
@@ -289,6 +289,9 @@ typedef struct ino_tree_node  {
 	} ino_un;
 } ino_tree_node_t;
 
+#define INOS_PER_IREC	(sizeof(__uint64_t) * NBBY)
+#define	IREC_MASK(i)	((__uint64_t)1 << (i))
+
 typedef struct nlink_ops {
 	const int	nlink_size;
 	void		(*disk_nlink_set)(ino_tree_node_t *, int, __uint32_t);
@@ -299,7 +302,6 @@ typedef struct nlink_ops {
 } nlink_ops_t;
 
 
-#define INOS_PER_IREC		(sizeof(__uint64_t) * NBBY)
 void		add_ino_ex_data(xfs_mount_t *mp);
 
 /*
@@ -381,92 +383,73 @@ void			clear_uncertain_ino_cache(xfs_agn
 		((ino_tree_node_t *) ((ino_node_ptr)->avl_node.avl_forw))
 
 /*
- * Bit manipulations for processed field
+ * Has an inode been processed for phase 6 (reference count checking)?
+ *
+ * add_inode_refchecked() is set on an inode when it gets traversed
+ * during the reference count phase (6).  It's set so that if the inode
+ * is a directory, it's traversed (and it's links counted) only once.
  */
-#define	XFS_INOPROC_MASK(i)	((__uint64_t)1 << (i))
-#define	XFS_INOPROC_MASKN(i,n)	((__uint64_t)((1 << (n)) - 1) << (i))
+static inline void add_inode_refchecked(struct ino_tree_node *irec, int offset)
+{
+	irec->ino_un.ex_data->ino_processed |= IREC_MASK(offset);
+}
 
-#define	XFS_INOPROC_IS_PROC(rp, i) \
-	(((rp)->ino_un.ex_data->ino_processed & XFS_INOPROC_MASK((i))) == 0LL \
-		? 0 : 1)
-#define	XFS_INOPROC_SET_PROC(rp, i) \
-	((rp)->ino_un.ex_data->ino_processed |= XFS_INOPROC_MASK((i)))
-/*
-#define	XFS_INOPROC_CLR_PROC(rp, i) \
-	((rp)->ino_un.ex_data->ino_processed &= ~XFS_INOPROC_MASK((i)))
-*/
+static inline int is_inode_refchecked(struct ino_tree_node *irec, int offset)
+{
+	return (irec->ino_un.ex_data->ino_processed & IREC_MASK(offset)) != 0;
+}
 
 /*
- * same for ir_confirmed.
+ * set/test is inode known to be valid (although perhaps corrupt)
  */
-#define	XFS_INOCF_MASK(i)	((__uint64_t)1 << (i))
-#define	XFS_INOCF_MASKN(i,n)	((__uint64_t)((1 << (n)) - 1) << (i))
-
-#define	XFS_INOCF_IS_CF(rp, i) \
-		(((rp)->ino_confirmed & XFS_INOCF_MASK((i))) == 0LL \
-			? 0 : 1)
-#define	XFS_INOCF_SET_CF(rp, i) \
-			((rp)->ino_confirmed |= XFS_INOCF_MASK((i)))
-#define	XFS_INOCF_CLR_CF(rp, i) \
-			((rp)->ino_confirmed &= ~XFS_INOCF_MASK((i)))
+static inline void set_inode_confirmed(struct ino_tree_node *irec, int offset)
+{
+	irec->ino_confirmed |= IREC_MASK(offset);
+}
 
-/*
- * same for backptr->ino_reached
- */
-#define	XFS_INO_RCHD_MASK(i)	((__uint64_t)1 << (i))
+static inline int is_inode_confirmed(struct ino_tree_node *irec, int offset)
+{
+	return (irec->ino_confirmed & IREC_MASK(offset)) != 0;
+}
 
-#define	XFS_INO_RCHD_IS_RCHD(rp, i) \
-	(((rp)->ino_un.ex_data->ino_reached & XFS_INO_RCHD_MASK((i))) == 0LL \
-		? 0 : 1)
-#define	XFS_INO_RCHD_SET_RCHD(rp, i) \
-		((rp)->ino_un.ex_data->ino_reached |= XFS_INO_RCHD_MASK((i)))
-#define	XFS_INO_RCHD_CLR_RCHD(rp, i) \
-		((rp)->ino_un.ex_data->ino_reached &= ~XFS_INO_RCHD_MASK((i)))
 /*
  * set/clear/test is inode a directory inode
  */
-#define	XFS_INO_ISADIR_MASK(i)	((__uint64_t)1 << (i))
-
-#define inode_isadir(ino_rec, ino_offset) \
-	(((ino_rec)->ino_isa_dir & XFS_INO_ISADIR_MASK((ino_offset))) == 0LL \
-		? 0 : 1)
-#define set_inode_isadir(ino_rec, ino_offset) \
-		((ino_rec)->ino_isa_dir |= XFS_INO_ISADIR_MASK((ino_offset)))
-#define clear_inode_isadir(ino_rec, ino_offset) \
-		((ino_rec)->ino_isa_dir &= ~XFS_INO_ISADIR_MASK((ino_offset)))
-
-
-/*
- * set/clear/test is inode known to be valid (although perhaps corrupt)
- */
-#define clear_inode_confirmed(ino_rec, ino_offset) \
-			XFS_INOCF_CLR_CF((ino_rec), (ino_offset))
+static inline void set_inode_isadir(struct ino_tree_node *irec, int offset)
+{
+	irec->ino_isa_dir |= IREC_MASK(offset);
+}
 
-#define set_inode_confirmed(ino_rec, ino_offset) \
-			XFS_INOCF_SET_CF((ino_rec), (ino_offset))
+static inline void clear_inode_isadir(struct ino_tree_node *irec, int offset)
+{
+	irec->ino_isa_dir &= ~IREC_MASK(offset);
+}
 
-#define is_inode_confirmed(ino_rec, ino_offset) \
-			XFS_INOCF_IS_CF(ino_rec, ino_offset)
+static inline int inode_isadir(struct ino_tree_node *irec, int offset)
+{
+	return (irec->ino_isa_dir & IREC_MASK(offset)) != 0;
+}
 
 /*
  * set/clear/test is inode free or used
  */
-#define set_inode_free(ino_rec, ino_offset) \
-	XFS_INOCF_SET_CF((ino_rec), (ino_offset)), \
-	XFS_INOBT_SET_FREE((ino_rec), (ino_offset))
-
-#define set_inode_used(ino_rec, ino_offset) \
-	XFS_INOCF_SET_CF((ino_rec), (ino_offset)), \
-	XFS_INOBT_CLR_FREE((ino_rec), (ino_offset))
+static inline void set_inode_free(struct ino_tree_node *irec, int offset)
+{
+	set_inode_confirmed(irec, offset);
+	irec->ir_free |= XFS_INOBT_MASK(offset);
 
-#define XFS_INOBT_IS_FREE(ino_rec, ino_offset) \
-	(((ino_rec)->ir_free & XFS_INOBT_MASK(ino_offset)) != 0)
+}
 
-#define is_inode_used(ino_rec, ino_offset)	\
-	!XFS_INOBT_IS_FREE((ino_rec), (ino_offset))
+static inline void set_inode_used(struct ino_tree_node *irec, int offset)
+{
+	set_inode_confirmed(irec, offset);
+	irec->ir_free &= ~XFS_INOBT_MASK(offset);
+}
 
-#define is_inode_free(ino_rec, ino_offset)	\
-	XFS_INOBT_IS_FREE((ino_rec), (ino_offset))
+static inline int is_inode_free(struct ino_tree_node *irec, int offset)
+{
+	return (irec->ir_free & XFS_INOBT_MASK(offset)) != 0;
+}
 
 /*
  * add_inode_reached() is set on inode I only if I has been reached
@@ -478,88 +461,54 @@ void			clear_uncertain_ino_cache(xfs_agn
  * an inode that we've counted is removed.
  */
 
-static inline int
-is_inode_reached(ino_tree_node_t *ino_rec, int ino_offset)
-{
-	ASSERT(ino_rec->ino_un.ex_data != NULL);
-	return(XFS_INO_RCHD_IS_RCHD(ino_rec, ino_offset));
-}
-
-static inline void
-add_inode_reached(ino_tree_node_t *ino_rec, int ino_offset)
+static inline void add_inode_ref(struct ino_tree_node *irec, int offset)
 {
-	ASSERT(ino_rec->ino_un.ex_data != NULL);
-
-	(*ino_rec->nlinkops->counted_nlink_inc)(ino_rec, ino_offset);
-	XFS_INO_RCHD_SET_RCHD(ino_rec, ino_offset);
+	ASSERT(irec->ino_un.ex_data != NULL);
 
-	ASSERT(is_inode_reached(ino_rec, ino_offset));
+	irec->nlinkops->counted_nlink_inc(irec, offset);
 }
 
-static inline void
-add_inode_ref(ino_tree_node_t *ino_rec, int ino_offset)
+static inline void drop_inode_ref(struct ino_tree_node *irec, int offset)
 {
-	ASSERT(ino_rec->ino_un.ex_data != NULL);
+	ASSERT(irec->ino_un.ex_data != NULL);
 
-	(*ino_rec->nlinkops->counted_nlink_inc)(ino_rec, ino_offset);
+	if (irec->nlinkops->counted_nlink_dec(irec, offset) == 0)
+		irec->ino_un.ex_data->ino_reached &= ~IREC_MASK(offset);
 }
 
-static inline void
-drop_inode_ref(ino_tree_node_t *ino_rec, int ino_offset)
+static inline __uint32_t num_inode_references(struct ino_tree_node *irec,
+		int offset)
 {
-	ASSERT(ino_rec->ino_un.ex_data != NULL);
+	ASSERT(irec->ino_un.ex_data != NULL);
 
-	if ((*ino_rec->nlinkops->counted_nlink_dec)(ino_rec, ino_offset) == 0)
-		XFS_INO_RCHD_CLR_RCHD(ino_rec, ino_offset);
+	return irec->nlinkops->counted_nlink_get(irec, offset);
 }
 
-static inline int
-is_inode_referenced(ino_tree_node_t *ino_rec, int ino_offset)
+static inline int is_inode_reached(struct ino_tree_node *irec, int offset)
 {
-	ASSERT(ino_rec->ino_un.ex_data != NULL);
-
-	return (*ino_rec->nlinkops->counted_nlink_get)(ino_rec, ino_offset) > 0;
+	ASSERT(irec->ino_un.ex_data != NULL);
+	return (irec->ino_un.ex_data->ino_reached & IREC_MASK(offset)) != 0;
 }
 
-static inline __uint32_t
-num_inode_references(ino_tree_node_t *ino_rec, int ino_offset)
+static inline void add_inode_reached(struct ino_tree_node *irec, int offset)
 {
-	ASSERT(ino_rec->ino_un.ex_data != NULL);
-
-	return (*ino_rec->nlinkops->counted_nlink_get)(ino_rec, ino_offset);
+	add_inode_ref(irec, offset);
+	irec->ino_un.ex_data->ino_reached |= IREC_MASK(offset);
 }
 
-static inline void
-set_inode_disk_nlinks(ino_tree_node_t *ino_rec, int ino_offset, __uint32_t nlinks)
+static inline void set_inode_disk_nlinks(struct ino_tree_node *irec, int offset,
+		__uint32_t nlinks)
 {
-	(*ino_rec->nlinkops->disk_nlink_set)(ino_rec, ino_offset, nlinks);
+	irec->nlinkops->disk_nlink_set(irec, offset, nlinks);
 }
 
-static inline __uint32_t
-get_inode_disk_nlinks(ino_tree_node_t *ino_rec, int ino_offset)
+static inline __uint32_t get_inode_disk_nlinks(struct ino_tree_node *irec,
+		int offset)
 {
-	return (*ino_rec->nlinkops->disk_nlink_get)(ino_rec, ino_offset);
+	return irec->nlinkops->disk_nlink_get(irec, offset);
 }
 
 /*
- * has an inode been processed for phase 6 (reference count checking)?
- * add_inode_refchecked() is set on an inode when it gets traversed
- * during the reference count phase (6).  It's set so that if the inode
- * is a directory, it's traversed (and it's links counted) only once.
- */
-#ifndef XR_INO_REF_DEBUG
-#define add_inode_refchecked(ino, ino_rec, ino_offset) \
-		XFS_INOPROC_SET_PROC((ino_rec), (ino_offset))
-#define is_inode_refchecked(ino, ino_rec, ino_offset) \
-		(XFS_INOPROC_IS_PROC(ino_rec, ino_offset) != 0LL)
-#else
-void add_inode_refchecked(xfs_ino_t ino,
-			ino_tree_node_t *ino_rec, int ino_offset);
-int is_inode_refchecked(xfs_ino_t ino,
-			ino_tree_node_t *ino_rec, int ino_offset);
-#endif /* XR_INO_REF_DEBUG */
-
-/*
  * set/get inode number of parent -- works for directory inodes only
  */
 void		set_inode_parent(ino_tree_node_t *irec, int ino_offset,
Index: xfsprogs-dev/repair/incore_ino.c
===================================================================
--- xfsprogs-dev.orig/repair/incore_ino.c	2011-12-02 11:04:59.000000000 +0000
+++ xfsprogs-dev/repair/incore_ino.c	2011-12-02 11:05:19.000000000 +0000
@@ -784,19 +784,3 @@ incore_ino_init(xfs_mount_t *mp)
 
 	full_ino_ex_data = 0;
 }
-
-#ifdef XR_INO_REF_DEBUG
-void
-add_inode_refchecked(xfs_ino_t ino, ino_tree_node_t *ino_rec, int ino_offset)
-{
-	XFS_INOPROC_SET_PROC((ino_rec), (ino_offset));
-
-	ASSERT(is_inode_refchecked(ino, ino_rec, ino_offset));
-}
-
-int
-is_inode_refchecked(xfs_ino_t ino, ino_tree_node_t *ino_rec, int ino_offset)
-{
-	return(XFS_INOPROC_IS_PROC(ino_rec, ino_offset) == 0LL ? 0 : 1);
-}
-#endif /* XR_INO_REF_DEBUG */
Index: xfsprogs-dev/include/libxfs.h
===================================================================
--- xfsprogs-dev.orig/include/libxfs.h	2011-12-02 11:04:59.000000000 +0000
+++ xfsprogs-dev/include/libxfs.h	2011-12-02 11:05:19.000000000 +0000
@@ -503,8 +503,6 @@ extern unsigned long	libxfs_physmem(void
 #include <xfs/xfs_log.h>
 #include <xfs/xfs_log_priv.h>
 
-#define XFS_INOBT_CLR_FREE(rp,i)	((rp)->ir_free &= ~XFS_INOBT_MASK(i))
-#define XFS_INOBT_SET_FREE(rp,i)	((rp)->ir_free |= XFS_INOBT_MASK(i))
 #define XFS_INOBT_IS_FREE_DISK(rp,i)		\
 			((be64_to_cpu((rp)->ir_free) & XFS_INOBT_MASK(i)) != 0)
 
Index: xfsprogs-dev/repair/phase6.c
===================================================================
--- xfsprogs-dev.orig/repair/phase6.c	2011-12-02 11:04:59.000000000 +0000
+++ xfsprogs-dev/repair/phase6.c	2011-12-02 11:05:19.000000000 +0000
@@ -3258,7 +3258,7 @@ process_dir_inode(
 	 * remaining illegal directory entries.
 	 */
 
-	ASSERT(!is_inode_refchecked(ino, irec, ino_offset) || dotdot_update);
+	ASSERT(!is_inode_refchecked(irec, ino_offset) || dotdot_update);
 
 	error = libxfs_iget(mp, NULL, ino, 0, &ip, 0);
 	if (error) {
@@ -3282,7 +3282,7 @@ process_dir_inode(
 			}
 		}
 
-		add_inode_refchecked(ino, irec, 0);
+		add_inode_refchecked(irec, 0);
 		return;
 	}
 
@@ -3300,7 +3300,7 @@ process_dir_inode(
 		add_inode_reached(irec, ino_offset);
 	}
 
-	add_inode_refchecked(ino, irec, ino_offset);
+	add_inode_refchecked(irec, ino_offset);
 
 	hashtab = dir_hash_init(ip->i_d.di_size);
 
Index: xfsprogs-dev/repair/phase7.c
===================================================================
--- xfsprogs-dev.orig/repair/phase7.c	2011-12-02 11:16:58.000000000 +0000
+++ xfsprogs-dev/repair/phase7.c	2011-12-02 11:17:27.000000000 +0000
@@ -143,10 +143,9 @@ phase7(xfs_mount_t *mp)
 					continue;
 
 				ASSERT(no_modify || is_inode_reached(irec, j));
-				ASSERT(no_modify ||
-						is_inode_referenced(irec, j));
 
 				nrefs = num_inode_references(irec, j);
+				ASSERT(no_modify || nrefs > 0);
 
 				if (get_inode_disk_nlinks(irec, j) != nrefs)
 					update_inode_nlinks(mp,

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

Re: [PATCH 12/12] repair: cleanup inode record macros

[Christoph Hellwig]

ping?

On Fri, Dec 02, 2011 at 12:46:31PM -0500, Christoph Hellwig wrote:
> Remove indirections in the inode record bit manipulation macros and flatten
> them to a single level of inlines.  Also use a common IREC_MASK define
> instead of duplicating it for every bitmask.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: xfsprogs-dev/repair/incore.h
> ===================================================================
> --- xfsprogs-dev.orig/repair/incore.h	2011-12-02 11:04:59.000000000 +0000
> +++ xfsprogs-dev/repair/incore.h	2011-12-02 11:17:31.000000000 +0000
> @@ -289,6 +289,9 @@ typedef struct ino_tree_node  {
>  	} ino_un;
>  } ino_tree_node_t;
>  
> +#define INOS_PER_IREC	(sizeof(__uint64_t) * NBBY)
> +#define	IREC_MASK(i)	((__uint64_t)1 << (i))
> +
>  typedef struct nlink_ops {
>  	const int	nlink_size;
>  	void		(*disk_nlink_set)(ino_tree_node_t *, int, __uint32_t);
> @@ -299,7 +302,6 @@ typedef struct nlink_ops {
>  } nlink_ops_t;
>  
>  
> -#define INOS_PER_IREC		(sizeof(__uint64_t) * NBBY)
>  void		add_ino_ex_data(xfs_mount_t *mp);
>  
>  /*
> @@ -381,92 +383,73 @@ void			clear_uncertain_ino_cache(xfs_agn
>  		((ino_tree_node_t *) ((ino_node_ptr)->avl_node.avl_forw))
>  
>  /*
> - * Bit manipulations for processed field
> + * Has an inode been processed for phase 6 (reference count checking)?
> + *
> + * add_inode_refchecked() is set on an inode when it gets traversed
> + * during the reference count phase (6).  It's set so that if the inode
> + * is a directory, it's traversed (and it's links counted) only once.
>   */
> -#define	XFS_INOPROC_MASK(i)	((__uint64_t)1 << (i))
> -#define	XFS_INOPROC_MASKN(i,n)	((__uint64_t)((1 << (n)) - 1) << (i))
> +static inline void add_inode_refchecked(struct ino_tree_node *irec, int offset)
> +{
> +	irec->ino_un.ex_data->ino_processed |= IREC_MASK(offset);
> +}
>  
> -#define	XFS_INOPROC_IS_PROC(rp, i) \
> -	(((rp)->ino_un.ex_data->ino_processed & XFS_INOPROC_MASK((i))) == 0LL \
> -		? 0 : 1)
> -#define	XFS_INOPROC_SET_PROC(rp, i) \
> -	((rp)->ino_un.ex_data->ino_processed |= XFS_INOPROC_MASK((i)))
> -/*
> -#define	XFS_INOPROC_CLR_PROC(rp, i) \
> -	((rp)->ino_un.ex_data->ino_processed &= ~XFS_INOPROC_MASK((i)))
> -*/
> +static inline int is_inode_refchecked(struct ino_tree_node *irec, int offset)
> +{
> +	return (irec->ino_un.ex_data->ino_processed & IREC_MASK(offset)) != 0;
> +}
>  
>  /*
> - * same for ir_confirmed.
> + * set/test is inode known to be valid (although perhaps corrupt)
>   */
> -#define	XFS_INOCF_MASK(i)	((__uint64_t)1 << (i))
> -#define	XFS_INOCF_MASKN(i,n)	((__uint64_t)((1 << (n)) - 1) << (i))
> -
> -#define	XFS_INOCF_IS_CF(rp, i) \
> -		(((rp)->ino_confirmed & XFS_INOCF_MASK((i))) == 0LL \
> -			? 0 : 1)
> -#define	XFS_INOCF_SET_CF(rp, i) \
> -			((rp)->ino_confirmed |= XFS_INOCF_MASK((i)))
> -#define	XFS_INOCF_CLR_CF(rp, i) \
> -			((rp)->ino_confirmed &= ~XFS_INOCF_MASK((i)))
> +static inline void set_inode_confirmed(struct ino_tree_node *irec, int offset)
> +{
> +	irec->ino_confirmed |= IREC_MASK(offset);
> +}
>  
> -/*
> - * same for backptr->ino_reached
> - */
> -#define	XFS_INO_RCHD_MASK(i)	((__uint64_t)1 << (i))
> +static inline int is_inode_confirmed(struct ino_tree_node *irec, int offset)
> +{
> +	return (irec->ino_confirmed & IREC_MASK(offset)) != 0;
> +}
>  
> -#define	XFS_INO_RCHD_IS_RCHD(rp, i) \
> -	(((rp)->ino_un.ex_data->ino_reached & XFS_INO_RCHD_MASK((i))) == 0LL \
> -		? 0 : 1)
> -#define	XFS_INO_RCHD_SET_RCHD(rp, i) \
> -		((rp)->ino_un.ex_data->ino_reached |= XFS_INO_RCHD_MASK((i)))
> -#define	XFS_INO_RCHD_CLR_RCHD(rp, i) \
> -		((rp)->ino_un.ex_data->ino_reached &= ~XFS_INO_RCHD_MASK((i)))
>  /*
>   * set/clear/test is inode a directory inode
>   */
> -#define	XFS_INO_ISADIR_MASK(i)	((__uint64_t)1 << (i))
> -
> -#define inode_isadir(ino_rec, ino_offset) \
> -	(((ino_rec)->ino_isa_dir & XFS_INO_ISADIR_MASK((ino_offset))) == 0LL \
> -		? 0 : 1)
> -#define set_inode_isadir(ino_rec, ino_offset) \
> -		((ino_rec)->ino_isa_dir |= XFS_INO_ISADIR_MASK((ino_offset)))
> -#define clear_inode_isadir(ino_rec, ino_offset) \
> -		((ino_rec)->ino_isa_dir &= ~XFS_INO_ISADIR_MASK((ino_offset)))
> -
> -
> -/*
> - * set/clear/test is inode known to be valid (although perhaps corrupt)
> - */
> -#define clear_inode_confirmed(ino_rec, ino_offset) \
> -			XFS_INOCF_CLR_CF((ino_rec), (ino_offset))
> +static inline void set_inode_isadir(struct ino_tree_node *irec, int offset)
> +{
> +	irec->ino_isa_dir |= IREC_MASK(offset);
> +}
>  
> -#define set_inode_confirmed(ino_rec, ino_offset) \
> -			XFS_INOCF_SET_CF((ino_rec), (ino_offset))
> +static inline void clear_inode_isadir(struct ino_tree_node *irec, int offset)
> +{
> +	irec->ino_isa_dir &= ~IREC_MASK(offset);
> +}
>  
> -#define is_inode_confirmed(ino_rec, ino_offset) \
> -			XFS_INOCF_IS_CF(ino_rec, ino_offset)
> +static inline int inode_isadir(struct ino_tree_node *irec, int offset)
> +{
> +	return (irec->ino_isa_dir & IREC_MASK(offset)) != 0;
> +}
>  
>  /*
>   * set/clear/test is inode free or used
>   */
> -#define set_inode_free(ino_rec, ino_offset) \
> -	XFS_INOCF_SET_CF((ino_rec), (ino_offset)), \
> -	XFS_INOBT_SET_FREE((ino_rec), (ino_offset))
> -
> -#define set_inode_used(ino_rec, ino_offset) \
> -	XFS_INOCF_SET_CF((ino_rec), (ino_offset)), \
> -	XFS_INOBT_CLR_FREE((ino_rec), (ino_offset))
> +static inline void set_inode_free(struct ino_tree_node *irec, int offset)
> +{
> +	set_inode_confirmed(irec, offset);
> +	irec->ir_free |= XFS_INOBT_MASK(offset);
>  
> -#define XFS_INOBT_IS_FREE(ino_rec, ino_offset) \
> -	(((ino_rec)->ir_free & XFS_INOBT_MASK(ino_offset)) != 0)
> +}
>  
> -#define is_inode_used(ino_rec, ino_offset)	\
> -	!XFS_INOBT_IS_FREE((ino_rec), (ino_offset))
> +static inline void set_inode_used(struct ino_tree_node *irec, int offset)
> +{
> +	set_inode_confirmed(irec, offset);
> +	irec->ir_free &= ~XFS_INOBT_MASK(offset);
> +}
>  
> -#define is_inode_free(ino_rec, ino_offset)	\
> -	XFS_INOBT_IS_FREE((ino_rec), (ino_offset))
> +static inline int is_inode_free(struct ino_tree_node *irec, int offset)
> +{
> +	return (irec->ir_free & XFS_INOBT_MASK(offset)) != 0;
> +}
>  
>  /*
>   * add_inode_reached() is set on inode I only if I has been reached
> @@ -478,88 +461,54 @@ void			clear_uncertain_ino_cache(xfs_agn
>   * an inode that we've counted is removed.
>   */
>  
> -static inline int
> -is_inode_reached(ino_tree_node_t *ino_rec, int ino_offset)
> -{
> -	ASSERT(ino_rec->ino_un.ex_data != NULL);
> -	return(XFS_INO_RCHD_IS_RCHD(ino_rec, ino_offset));
> -}
> -
> -static inline void
> -add_inode_reached(ino_tree_node_t *ino_rec, int ino_offset)
> +static inline void add_inode_ref(struct ino_tree_node *irec, int offset)
>  {
> -	ASSERT(ino_rec->ino_un.ex_data != NULL);
> -
> -	(*ino_rec->nlinkops->counted_nlink_inc)(ino_rec, ino_offset);
> -	XFS_INO_RCHD_SET_RCHD(ino_rec, ino_offset);
> +	ASSERT(irec->ino_un.ex_data != NULL);
>  
> -	ASSERT(is_inode_reached(ino_rec, ino_offset));
> +	irec->nlinkops->counted_nlink_inc(irec, offset);
>  }
>  
> -static inline void
> -add_inode_ref(ino_tree_node_t *ino_rec, int ino_offset)
> +static inline void drop_inode_ref(struct ino_tree_node *irec, int offset)
>  {
> -	ASSERT(ino_rec->ino_un.ex_data != NULL);
> +	ASSERT(irec->ino_un.ex_data != NULL);
>  
> -	(*ino_rec->nlinkops->counted_nlink_inc)(ino_rec, ino_offset);
> +	if (irec->nlinkops->counted_nlink_dec(irec, offset) == 0)
> +		irec->ino_un.ex_data->ino_reached &= ~IREC_MASK(offset);
>  }
>  
> -static inline void
> -drop_inode_ref(ino_tree_node_t *ino_rec, int ino_offset)
> +static inline __uint32_t num_inode_references(struct ino_tree_node *irec,
> +		int offset)
>  {
> -	ASSERT(ino_rec->ino_un.ex_data != NULL);
> +	ASSERT(irec->ino_un.ex_data != NULL);
>  
> -	if ((*ino_rec->nlinkops->counted_nlink_dec)(ino_rec, ino_offset) == 0)
> -		XFS_INO_RCHD_CLR_RCHD(ino_rec, ino_offset);
> +	return irec->nlinkops->counted_nlink_get(irec, offset);
>  }
>  
> -static inline int
> -is_inode_referenced(ino_tree_node_t *ino_rec, int ino_offset)
> +static inline int is_inode_reached(struct ino_tree_node *irec, int offset)
>  {
> -	ASSERT(ino_rec->ino_un.ex_data != NULL);
> -
> -	return (*ino_rec->nlinkops->counted_nlink_get)(ino_rec, ino_offset) > 0;
> +	ASSERT(irec->ino_un.ex_data != NULL);
> +	return (irec->ino_un.ex_data->ino_reached & IREC_MASK(offset)) != 0;
>  }
>  
> -static inline __uint32_t
> -num_inode_references(ino_tree_node_t *ino_rec, int ino_offset)
> +static inline void add_inode_reached(struct ino_tree_node *irec, int offset)
>  {
> -	ASSERT(ino_rec->ino_un.ex_data != NULL);
> -
> -	return (*ino_rec->nlinkops->counted_nlink_get)(ino_rec, ino_offset);
> +	add_inode_ref(irec, offset);
> +	irec->ino_un.ex_data->ino_reached |= IREC_MASK(offset);
>  }
>  
> -static inline void
> -set_inode_disk_nlinks(ino_tree_node_t *ino_rec, int ino_offset, __uint32_t nlinks)
> +static inline void set_inode_disk_nlinks(struct ino_tree_node *irec, int offset,
> +		__uint32_t nlinks)
>  {
> -	(*ino_rec->nlinkops->disk_nlink_set)(ino_rec, ino_offset, nlinks);
> +	irec->nlinkops->disk_nlink_set(irec, offset, nlinks);
>  }
>  
> -static inline __uint32_t
> -get_inode_disk_nlinks(ino_tree_node_t *ino_rec, int ino_offset)
> +static inline __uint32_t get_inode_disk_nlinks(struct ino_tree_node *irec,
> +		int offset)
>  {
> -	return (*ino_rec->nlinkops->disk_nlink_get)(ino_rec, ino_offset);
> +	return irec->nlinkops->disk_nlink_get(irec, offset);
>  }
>  
>  /*
> - * has an inode been processed for phase 6 (reference count checking)?
> - * add_inode_refchecked() is set on an inode when it gets traversed
> - * during the reference count phase (6).  It's set so that if the inode
> - * is a directory, it's traversed (and it's links counted) only once.
> - */
> -#ifndef XR_INO_REF_DEBUG
> -#define add_inode_refchecked(ino, ino_rec, ino_offset) \
> -		XFS_INOPROC_SET_PROC((ino_rec), (ino_offset))
> -#define is_inode_refchecked(ino, ino_rec, ino_offset) \
> -		(XFS_INOPROC_IS_PROC(ino_rec, ino_offset) != 0LL)
> -#else
> -void add_inode_refchecked(xfs_ino_t ino,
> -			ino_tree_node_t *ino_rec, int ino_offset);
> -int is_inode_refchecked(xfs_ino_t ino,
> -			ino_tree_node_t *ino_rec, int ino_offset);
> -#endif /* XR_INO_REF_DEBUG */
> -
> -/*
>   * set/get inode number of parent -- works for directory inodes only
>   */
>  void		set_inode_parent(ino_tree_node_t *irec, int ino_offset,
> Index: xfsprogs-dev/repair/incore_ino.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/incore_ino.c	2011-12-02 11:04:59.000000000 +0000
> +++ xfsprogs-dev/repair/incore_ino.c	2011-12-02 11:05:19.000000000 +0000
> @@ -784,19 +784,3 @@ incore_ino_init(xfs_mount_t *mp)
>  
>  	full_ino_ex_data = 0;
>  }
> -
> -#ifdef XR_INO_REF_DEBUG
> -void
> -add_inode_refchecked(xfs_ino_t ino, ino_tree_node_t *ino_rec, int ino_offset)
> -{
> -	XFS_INOPROC_SET_PROC((ino_rec), (ino_offset));
> -
> -	ASSERT(is_inode_refchecked(ino, ino_rec, ino_offset));
> -}
> -
> -int
> -is_inode_refchecked(xfs_ino_t ino, ino_tree_node_t *ino_rec, int ino_offset)
> -{
> -	return(XFS_INOPROC_IS_PROC(ino_rec, ino_offset) == 0LL ? 0 : 1);
> -}
> -#endif /* XR_INO_REF_DEBUG */
> Index: xfsprogs-dev/include/libxfs.h
> ===================================================================
> --- xfsprogs-dev.orig/include/libxfs.h	2011-12-02 11:04:59.000000000 +0000
> +++ xfsprogs-dev/include/libxfs.h	2011-12-02 11:05:19.000000000 +0000
> @@ -503,8 +503,6 @@ extern unsigned long	libxfs_physmem(void
>  #include <xfs/xfs_log.h>
>  #include <xfs/xfs_log_priv.h>
>  
> -#define XFS_INOBT_CLR_FREE(rp,i)	((rp)->ir_free &= ~XFS_INOBT_MASK(i))
> -#define XFS_INOBT_SET_FREE(rp,i)	((rp)->ir_free |= XFS_INOBT_MASK(i))
>  #define XFS_INOBT_IS_FREE_DISK(rp,i)		\
>  			((be64_to_cpu((rp)->ir_free) & XFS_INOBT_MASK(i)) != 0)
>  
> Index: xfsprogs-dev/repair/phase6.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/phase6.c	2011-12-02 11:04:59.000000000 +0000
> +++ xfsprogs-dev/repair/phase6.c	2011-12-02 11:05:19.000000000 +0000
> @@ -3258,7 +3258,7 @@ process_dir_inode(
>  	 * remaining illegal directory entries.
>  	 */
>  
> -	ASSERT(!is_inode_refchecked(ino, irec, ino_offset) || dotdot_update);
> +	ASSERT(!is_inode_refchecked(irec, ino_offset) || dotdot_update);
>  
>  	error = libxfs_iget(mp, NULL, ino, 0, &ip, 0);
>  	if (error) {
> @@ -3282,7 +3282,7 @@ process_dir_inode(
>  			}
>  		}
>  
> -		add_inode_refchecked(ino, irec, 0);
> +		add_inode_refchecked(irec, 0);
>  		return;
>  	}
>  
> @@ -3300,7 +3300,7 @@ process_dir_inode(
>  		add_inode_reached(irec, ino_offset);
>  	}
>  
> -	add_inode_refchecked(ino, irec, ino_offset);
> +	add_inode_refchecked(irec, ino_offset);
>  
>  	hashtab = dir_hash_init(ip->i_d.di_size);
>  
> Index: xfsprogs-dev/repair/phase7.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/phase7.c	2011-12-02 11:16:58.000000000 +0000
> +++ xfsprogs-dev/repair/phase7.c	2011-12-02 11:17:27.000000000 +0000
> @@ -143,10 +143,9 @@ phase7(xfs_mount_t *mp)
>  					continue;
>  
>  				ASSERT(no_modify || is_inode_reached(irec, j));
> -				ASSERT(no_modify ||
> -						is_inode_referenced(irec, j));
>  
>  				nrefs = num_inode_references(irec, j);
> +				ASSERT(no_modify || nrefs > 0);
>  
>  				if (get_inode_disk_nlinks(irec, j) != nrefs)
>  					update_inode_nlinks(mp,
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---

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

Re: [PATCH 12/12] repair: cleanup inode record macros

[Mark Tinguely]

On 01/11/12 05:30, Christoph Hellwig wrote:
> ping?
>
> On Fri, Dec 02, 2011 at 12:46:31PM -0500, Christoph Hellwig wrote:
>> Remove indirections in the inode record bit manipulation macros and flatten
>> them to a single level of inlines.  Also use a common IREC_MASK define
>> instead of duplicating it for every bitmask.
>>
>>
>> Signed-off-by: Christoph Hellwig<hch@lst.de>
>>

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

The clean up looks good. I like the removal of the side effect that was 
in one of the ASSERT statement.

--Mark Tinguely

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