* [PATCH 00/12] xfs_repair queue
@ 2011-12-02 17:46 Christoph Hellwig
2011-12-02 17:46 ` [PATCH 01/12] repair: do not walk the unlinked inode list Christoph Hellwig
` (11 more replies)
0 siblings, 12 replies; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 01/12] repair: do not walk the unlinked inode list
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
@ 2011-12-02 17:46 ` Christoph Hellwig
2011-12-12 22:55 ` Dave Chinner
2012-01-12 19:30 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 02/12] repair: allocate and free inode records individually Christoph Hellwig
` (10 subsequent siblings)
11 siblings, 2 replies; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 02/12] repair: allocate and free inode records individually
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
2011-12-02 17:46 ` [PATCH 01/12] repair: do not walk the unlinked inode list Christoph Hellwig
@ 2011-12-02 17:46 ` Christoph Hellwig
2011-12-12 23:16 ` Dave Chinner
2012-01-12 22:38 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 03/12] repair: allocate and free extent " Christoph Hellwig
` (9 subsequent siblings)
11 siblings, 2 replies; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 03/12] repair: allocate and free extent records individually
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
2011-12-02 17:46 ` [PATCH 01/12] repair: do not walk the unlinked inode list Christoph Hellwig
2011-12-02 17:46 ` [PATCH 02/12] repair: allocate and free inode records individually Christoph Hellwig
@ 2011-12-02 17:46 ` Christoph Hellwig
2011-12-12 23:21 ` Dave Chinner
2012-01-12 22:39 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 04/12] xfsprogs: allow linking against libtcmalloc Christoph Hellwig
` (8 subsequent siblings)
11 siblings, 2 replies; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 04/12] xfsprogs: allow linking against libtcmalloc
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
` (2 preceding siblings ...)
2011-12-02 17:46 ` [PATCH 03/12] repair: allocate and free extent " Christoph Hellwig
@ 2011-12-02 17:46 ` Christoph Hellwig
2011-12-13 0:05 ` Dave Chinner
2011-12-02 17:46 ` [PATCH 05/12] repair: update extent count after zapping duplicate blocks Christoph Hellwig
` (7 subsequent siblings)
11 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 05/12] repair: update extent count after zapping duplicate blocks
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
` (3 preceding siblings ...)
2011-12-02 17:46 ` [PATCH 04/12] xfsprogs: allow linking against libtcmalloc Christoph Hellwig
@ 2011-12-02 17:46 ` Christoph Hellwig
2011-12-13 2:12 ` Dave Chinner
2012-01-13 17:18 ` [PATCH 05/12] " Mark Tinguely
2011-12-02 17:46 ` [PATCH 06/12] repair: use recursive buffer locking Christoph Hellwig
` (6 subsequent siblings)
11 siblings, 2 replies; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 06/12] repair: use recursive buffer locking
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
` (4 preceding siblings ...)
2011-12-02 17:46 ` [PATCH 05/12] repair: update extent count after zapping duplicate blocks Christoph Hellwig
@ 2011-12-02 17:46 ` Christoph Hellwig
2011-12-13 2:22 ` Dave Chinner
2011-12-02 17:46 ` [PATCH 07/12] repair: fix another ABBA deadlock in inode prefetching Christoph Hellwig
` (5 subsequent siblings)
11 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 07/12] repair: fix another ABBA deadlock in inode prefetching
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
` (5 preceding siblings ...)
2011-12-02 17:46 ` [PATCH 06/12] repair: use recursive buffer locking Christoph Hellwig
@ 2011-12-02 17:46 ` Christoph Hellwig
2011-12-13 2:35 ` Dave Chinner
2012-01-13 18:51 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 08/12] repair: handle filesystems with the log in allocation group 0 Christoph Hellwig
` (4 subsequent siblings)
11 siblings, 2 replies; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 08/12] repair: handle filesystems with the log in allocation group 0
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
` (6 preceding siblings ...)
2011-12-02 17:46 ` [PATCH 07/12] repair: fix another ABBA deadlock in inode prefetching Christoph Hellwig
@ 2011-12-02 17:46 ` Christoph Hellwig
2011-12-13 2:36 ` Dave Chinner
2012-01-13 15:18 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 09/12] repair: kill check_inode_block Christoph Hellwig
` (3 subsequent siblings)
11 siblings, 2 replies; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 09/12] repair: kill check_inode_block
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
` (7 preceding siblings ...)
2011-12-02 17:46 ` [PATCH 08/12] repair: handle filesystems with the log in allocation group 0 Christoph Hellwig
@ 2011-12-02 17:46 ` Christoph Hellwig
2012-01-11 11:29 ` Christoph Hellwig
2011-12-02 17:46 ` [PATCH 10/12] repair: mark local functions static Christoph Hellwig
` (2 subsequent siblings)
11 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 10/12] repair: mark local functions static
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
` (8 preceding siblings ...)
2011-12-02 17:46 ` [PATCH 09/12] repair: kill check_inode_block Christoph Hellwig
@ 2011-12-02 17:46 ` Christoph Hellwig
2012-01-11 11:30 ` Christoph Hellwig
2011-12-02 17:46 ` [PATCH 11/12] repair: move extern declarations to headers Christoph Hellwig
2011-12-02 17:46 ` [PATCH 12/12] repair: cleanup inode record macros Christoph Hellwig
11 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 11/12] repair: move extern declarations to headers
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
` (9 preceding siblings ...)
2011-12-02 17:46 ` [PATCH 10/12] repair: mark local functions static Christoph Hellwig
@ 2011-12-02 17:46 ` Christoph Hellwig
2012-01-11 11:30 ` Christoph Hellwig
2011-12-02 17:46 ` [PATCH 12/12] repair: cleanup inode record macros Christoph Hellwig
11 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 12/12] repair: cleanup inode record macros
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
` (10 preceding siblings ...)
2011-12-02 17:46 ` [PATCH 11/12] repair: move extern declarations to headers Christoph Hellwig
@ 2011-12-02 17:46 ` Christoph Hellwig
2012-01-11 11:30 ` Christoph Hellwig
11 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-02 17:46 UTC (permalink / raw)
To: xfs
[-- 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 01/12] repair: do not walk the unlinked inode list
2011-12-02 17:46 ` [PATCH 01/12] repair: do not walk the unlinked inode list Christoph Hellwig
@ 2011-12-12 22:55 ` Dave Chinner
2012-01-12 19:30 ` Mark Tinguely
1 sibling, 0 replies; 40+ messages in thread
From: Dave Chinner @ 2011-12-12 22:55 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 02/12] repair: allocate and free inode records individually
2011-12-02 17:46 ` [PATCH 02/12] repair: allocate and free inode records individually Christoph Hellwig
@ 2011-12-12 23:16 ` Dave Chinner
2012-01-12 22:38 ` Mark Tinguely
1 sibling, 0 replies; 40+ messages in thread
From: Dave Chinner @ 2011-12-12 23:16 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 03/12] repair: allocate and free extent records individually
2011-12-02 17:46 ` [PATCH 03/12] repair: allocate and free extent " Christoph Hellwig
@ 2011-12-12 23:21 ` Dave Chinner
2012-01-12 22:39 ` Mark Tinguely
1 sibling, 0 replies; 40+ messages in thread
From: Dave Chinner @ 2011-12-12 23:21 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 04/12] xfsprogs: allow linking against libtcmalloc
2011-12-02 17:46 ` [PATCH 04/12] xfsprogs: allow linking against libtcmalloc Christoph Hellwig
@ 2011-12-13 0:05 ` Dave Chinner
2011-12-18 22:47 ` Christoph Hellwig
0 siblings, 1 reply; 40+ messages in thread
From: Dave Chinner @ 2011-12-13 0:05 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 05/12] repair: update extent count after zapping duplicate blocks
2011-12-02 17:46 ` [PATCH 05/12] repair: update extent count after zapping duplicate blocks Christoph Hellwig
@ 2011-12-13 2:12 ` Dave Chinner
2012-02-02 12:39 ` [PATCH v2] " Christoph Hellwig
2012-01-13 17:18 ` [PATCH 05/12] " Mark Tinguely
1 sibling, 1 reply; 40+ messages in thread
From: Dave Chinner @ 2011-12-13 2:12 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 06/12] repair: use recursive buffer locking
2011-12-02 17:46 ` [PATCH 06/12] repair: use recursive buffer locking Christoph Hellwig
@ 2011-12-13 2:22 ` Dave Chinner
2011-12-18 22:54 ` Christoph Hellwig
0 siblings, 1 reply; 40+ messages in thread
From: Dave Chinner @ 2011-12-13 2:22 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 07/12] repair: fix another ABBA deadlock in inode prefetching
2011-12-02 17:46 ` [PATCH 07/12] repair: fix another ABBA deadlock in inode prefetching Christoph Hellwig
@ 2011-12-13 2:35 ` Dave Chinner
2012-01-13 18:51 ` Mark Tinguely
1 sibling, 0 replies; 40+ messages in thread
From: Dave Chinner @ 2011-12-13 2:35 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 08/12] repair: handle filesystems with the log in allocation group 0
2011-12-02 17:46 ` [PATCH 08/12] repair: handle filesystems with the log in allocation group 0 Christoph Hellwig
@ 2011-12-13 2:36 ` Dave Chinner
2012-01-13 15:18 ` Mark Tinguely
1 sibling, 0 replies; 40+ messages in thread
From: Dave Chinner @ 2011-12-13 2:36 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 04/12] xfsprogs: allow linking against libtcmalloc
2011-12-13 0:05 ` Dave Chinner
@ 2011-12-18 22:47 ` Christoph Hellwig
0 siblings, 0 replies; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-18 22:47 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 06/12] repair: use recursive buffer locking
2011-12-13 2:22 ` Dave Chinner
@ 2011-12-18 22:54 ` Christoph Hellwig
2012-01-13 20:10 ` Mark Tinguely
0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2011-12-18 22:54 UTC (permalink / raw)
To: Dave Chinner; +Cc: Christoph Hellwig, xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 09/12] repair: kill check_inode_block
2011-12-02 17:46 ` [PATCH 09/12] repair: kill check_inode_block Christoph Hellwig
@ 2012-01-11 11:29 ` Christoph Hellwig
2012-01-11 21:28 ` Mark Tinguely
0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2012-01-11 11:29 UTC (permalink / raw)
To: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 10/12] repair: mark local functions static
2011-12-02 17:46 ` [PATCH 10/12] repair: mark local functions static Christoph Hellwig
@ 2012-01-11 11:30 ` Christoph Hellwig
2012-01-11 21:37 ` Mark Tinguely
0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2012-01-11 11:30 UTC (permalink / raw)
To: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 11/12] repair: move extern declarations to headers
2011-12-02 17:46 ` [PATCH 11/12] repair: move extern declarations to headers Christoph Hellwig
@ 2012-01-11 11:30 ` Christoph Hellwig
2012-01-11 21:40 ` Mark Tinguely
0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2012-01-11 11:30 UTC (permalink / raw)
To: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 12/12] repair: cleanup inode record macros
2011-12-02 17:46 ` [PATCH 12/12] repair: cleanup inode record macros Christoph Hellwig
@ 2012-01-11 11:30 ` Christoph Hellwig
2012-01-12 17:05 ` Mark Tinguely
0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2012-01-11 11:30 UTC (permalink / raw)
To: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 09/12] repair: kill check_inode_block
2012-01-11 11:29 ` Christoph Hellwig
@ 2012-01-11 21:28 ` Mark Tinguely
0 siblings, 0 replies; 40+ messages in thread
From: Mark Tinguely @ 2012-01-11 21:28 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 10/12] repair: mark local functions static
2012-01-11 11:30 ` Christoph Hellwig
@ 2012-01-11 21:37 ` Mark Tinguely
0 siblings, 0 replies; 40+ messages in thread
From: Mark Tinguely @ 2012-01-11 21:37 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 11/12] repair: move extern declarations to headers
2012-01-11 11:30 ` Christoph Hellwig
@ 2012-01-11 21:40 ` Mark Tinguely
0 siblings, 0 replies; 40+ messages in thread
From: Mark Tinguely @ 2012-01-11 21:40 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 12/12] repair: cleanup inode record macros
2012-01-11 11:30 ` Christoph Hellwig
@ 2012-01-12 17:05 ` Mark Tinguely
0 siblings, 0 replies; 40+ messages in thread
From: Mark Tinguely @ 2012-01-12 17:05 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 01/12] repair: do not walk the unlinked inode list
2011-12-02 17:46 ` [PATCH 01/12] repair: do not walk the unlinked inode list Christoph Hellwig
2011-12-12 22:55 ` Dave Chinner
@ 2012-01-12 19:30 ` Mark Tinguely
1 sibling, 0 replies; 40+ messages in thread
From: Mark Tinguely @ 2012-01-12 19:30 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 02/12] repair: allocate and free inode records individually
2011-12-02 17:46 ` [PATCH 02/12] repair: allocate and free inode records individually Christoph Hellwig
2011-12-12 23:16 ` Dave Chinner
@ 2012-01-12 22:38 ` Mark Tinguely
1 sibling, 0 replies; 40+ messages in thread
From: Mark Tinguely @ 2012-01-12 22:38 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 03/12] repair: allocate and free extent records individually
2011-12-02 17:46 ` [PATCH 03/12] repair: allocate and free extent " Christoph Hellwig
2011-12-12 23:21 ` Dave Chinner
@ 2012-01-12 22:39 ` Mark Tinguely
1 sibling, 0 replies; 40+ messages in thread
From: Mark Tinguely @ 2012-01-12 22:39 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 08/12] repair: handle filesystems with the log in allocation group 0
2011-12-02 17:46 ` [PATCH 08/12] repair: handle filesystems with the log in allocation group 0 Christoph Hellwig
2011-12-13 2:36 ` Dave Chinner
@ 2012-01-13 15:18 ` Mark Tinguely
1 sibling, 0 replies; 40+ messages in thread
From: Mark Tinguely @ 2012-01-13 15:18 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 05/12] repair: update extent count after zapping duplicate blocks
2011-12-02 17:46 ` [PATCH 05/12] repair: update extent count after zapping duplicate blocks Christoph Hellwig
2011-12-13 2:12 ` Dave Chinner
@ 2012-01-13 17:18 ` Mark Tinguely
1 sibling, 0 replies; 40+ messages in thread
From: Mark Tinguely @ 2012-01-13 17:18 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 07/12] repair: fix another ABBA deadlock in inode prefetching
2011-12-02 17:46 ` [PATCH 07/12] repair: fix another ABBA deadlock in inode prefetching Christoph Hellwig
2011-12-13 2:35 ` Dave Chinner
@ 2012-01-13 18:51 ` Mark Tinguely
1 sibling, 0 replies; 40+ messages in thread
From: Mark Tinguely @ 2012-01-13 18:51 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
> 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Re: [PATCH 06/12] repair: use recursive buffer locking
2011-12-18 22:54 ` Christoph Hellwig
@ 2012-01-13 20:10 ` Mark Tinguely
0 siblings, 0 replies; 40+ messages in thread
From: Mark Tinguely @ 2012-01-13 20:10 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v2] repair: update extent count after zapping duplicate blocks
2011-12-13 2:12 ` Dave Chinner
@ 2012-02-02 12:39 ` Christoph Hellwig
2012-02-02 18:19 ` Mark Tinguely
0 siblings, 1 reply; 40+ messages in thread
From: Christoph Hellwig @ 2012-02-02 12:39 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] repair: update extent count after zapping duplicate blocks
2012-02-02 12:39 ` [PATCH v2] " Christoph Hellwig
@ 2012-02-02 18:19 ` Mark Tinguely
0 siblings, 0 replies; 40+ messages in thread
From: Mark Tinguely @ 2012-02-02 18:19 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2012-02-02 18:19 UTC | newest]
Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-02 17:46 [PATCH 00/12] xfs_repair queue Christoph Hellwig
2011-12-02 17:46 ` [PATCH 01/12] repair: do not walk the unlinked inode list Christoph Hellwig
2011-12-12 22:55 ` Dave Chinner
2012-01-12 19:30 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 02/12] repair: allocate and free inode records individually Christoph Hellwig
2011-12-12 23:16 ` Dave Chinner
2012-01-12 22:38 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 03/12] repair: allocate and free extent " Christoph Hellwig
2011-12-12 23:21 ` Dave Chinner
2012-01-12 22:39 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 04/12] xfsprogs: allow linking against libtcmalloc Christoph Hellwig
2011-12-13 0:05 ` Dave Chinner
2011-12-18 22:47 ` Christoph Hellwig
2011-12-02 17:46 ` [PATCH 05/12] repair: update extent count after zapping duplicate blocks Christoph Hellwig
2011-12-13 2:12 ` Dave Chinner
2012-02-02 12:39 ` [PATCH v2] " Christoph Hellwig
2012-02-02 18:19 ` Mark Tinguely
2012-01-13 17:18 ` [PATCH 05/12] " Mark Tinguely
2011-12-02 17:46 ` [PATCH 06/12] repair: use recursive buffer locking Christoph Hellwig
2011-12-13 2:22 ` Dave Chinner
2011-12-18 22:54 ` Christoph Hellwig
2012-01-13 20:10 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 07/12] repair: fix another ABBA deadlock in inode prefetching Christoph Hellwig
2011-12-13 2:35 ` Dave Chinner
2012-01-13 18:51 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 08/12] repair: handle filesystems with the log in allocation group 0 Christoph Hellwig
2011-12-13 2:36 ` Dave Chinner
2012-01-13 15:18 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 09/12] repair: kill check_inode_block Christoph Hellwig
2012-01-11 11:29 ` Christoph Hellwig
2012-01-11 21:28 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 10/12] repair: mark local functions static Christoph Hellwig
2012-01-11 11:30 ` Christoph Hellwig
2012-01-11 21:37 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 11/12] repair: move extern declarations to headers Christoph Hellwig
2012-01-11 11:30 ` Christoph Hellwig
2012-01-11 21:40 ` Mark Tinguely
2011-12-02 17:46 ` [PATCH 12/12] repair: cleanup inode record macros Christoph Hellwig
2012-01-11 11:30 ` Christoph Hellwig
2012-01-12 17:05 ` Mark Tinguely
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.