Skip to contentSkip to conversation

repair: more fixes for 3.2.1

PATCH In review Dave Chinner 2 participants 5 messages Jul 8, 2014 – Jul 9, 2014

lore
Share & tools
lore
#0 lore raw

cover repair: more fixes for 3.2.1

Hi folks,

A couple more fixes for repair - the first being a change I missed during review of the last patch series, and a new one that removes a set of incorrect asserts when checking AG headers.

If nothing else pops up in the next couple of days, this is where I want to draw the line for a 3.2.1 release, so having people run some testing over the next couple of days woul dbe great....

-Dave.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#1 lore raw

PATCH 1/2 libxfs: clear the buffer error while the buffer is locked

From: Dave Chinner <[email protected]>

When releasing a buffer, the error shoul dbe cleared while the lock is still held on the buffer to avoid racing with a new user of the buffer.

This was pointed out in review of commit 6af7c1e ("libxfs: reused invalidated buffers leak state and data") but the version committed didn't have the fix. Thanks to Christoph Hellwig for checking and pointing out the oversight.

---
 libxfs/rdwr.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
libxfs/rdwr.c +6 -5
index 0294c98..9ee89d3 100644
@@ -648,6 +648,12 @@ libxfs_getbuf_map(struct xfs_buftarg *btp, struct xfs_buf_map *map,
 void
 libxfs_putbuf(xfs_buf_t *bp)
 {
+	/*
+	 * ensure that any errors on this use of the buffer don't carry
+	 * over to the next user.
+	 */
+	bp->b_error = 0;
+
 #ifdef XFS_BUF_TRACING
 	pthread_mutex_lock(&libxfs_bcache->c_mutex);
 	lock_buf_count--;
@@ -663,11 +669,6 @@ libxfs_putbuf(xfs_buf_t *bp)
 			pthread_mutex_unlock(&bp->b_lock);
 		}
 	}
-	/*
-	 * ensure that any errors on this use of the buffer don't carry
-	 * over to the next user.
-	 */
-	bp->b_error = 0;
 
 	cache_node_put(libxfs_bcache, (struct cache_node *)bp);
 }
Signature · 6 lines
2.0.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] libxfs: clear the buffer error while the buffer is locked

Quoted 10 lines · Dave Chinner

From: Dave Chinner <[email protected]>

When releasing a buffer, the error shoul dbe cleared while the lock is still held on the buffer to avoid racing with a new user of the buffer.

This was pointed out in review of commit 6af7c1e ("libxfs: reused invalidated buffers leak state and data") but the version committed didn't have the fix. Thanks to Christoph Hellwig for checking and pointing out the oversight.

Looks good,

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#3 lore raw

PATCH 2/2 repair: handle uncorrected corruptions in phase 2

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

Some of the AG header corruptions detected by the IO verifiers cannot be corrected in phase 2 when we do the initial scan of the AGs. Correcting some errors cannot be done until a full rebuild of the trees is done in phase 5.

Hence we can end up with a "clean" AGF/AGI buffer but have a EFSCORRUPTED error on the buffer. This results in an assert failing:

	ASSERT(agf_dirty || agfbuf->b_error != EFSCORRUPTED);

and repair not beign able to fix the problems it has tripped over. Hence the assert that we corrected all corruptions in the buffers is not valid and should be removed.

---
 repair/scan.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
repair/scan.c +5 -6
index f29ff8d..142d8d7 100644
@@ -1572,14 +1572,13 @@ scan_ag(
 
 	/*
 	 * Only pay attention to CRC/verifier errors if we can correct them.
-	 * While there, ensure that we corrected a corruption error if the
-	 * verifier detected one.
+	 * Note that we can get uncorrected EFSCORRUPTED errors here because
+	 * the verifier will flag on out of range values that we can't correct
+	 * until phase 5 when we have all the information necessary to rebuild
+	 * the freespace/inode btrees. We can correct bad CRC errors
+	 * immediately, though.
 	 */
 	if (!no_modify) {
-		ASSERT(agi_dirty || agibuf->b_error != EFSCORRUPTED);
-		ASSERT(agf_dirty || agfbuf->b_error != EFSCORRUPTED);
-		ASSERT(sb_dirty || sbbuf->b_error != EFSCORRUPTED);
-
 		agi_dirty += (agibuf->b_error == EFSBADCRC);
 		agf_dirty += (agfbuf->b_error == EFSBADCRC);
 		sb_dirty += (sbbuf->b_error == EFSBADCRC);
Signature · 6 lines
2.0.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs