public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: [PATCH v2] repair: update extent count after zapping duplicate blocks
Date: Thu, 2 Feb 2012 07:39:10 -0500	[thread overview]
Message-ID: <20120202123910.GA8708@infradead.org> (raw)
In-Reply-To: <20111213021207.GY14273@dastard>

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

  reply	other threads:[~2012-02-02 12:39 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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     ` Christoph Hellwig [this message]
2012-02-02 18:19       ` [PATCH v2] " 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120202123910.GA8708@infradead.org \
    --to=hch@infradead.org \
    --cc=david@fromorbit.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox