Re: ext3: bogus i_mode errors with 2.6.18.1

[Andre Noll <maan@systemlinux.org>]

On 14:27, Andreas Dilger wrote:

> > +		j = find_next_usable_block(-1, gdp, EXT3_BLOCKS_PER_GROUP(sb));
> 
> I'm not sure why the "find_next_usable_block()" part is in here?  At this
> point we KNOW that ret_block is not a block we should be allocating, yet
> it is marked free in the bitmap.  So we should just mark the block(s) in-use
> in the bitmap and look for a different block(s).

Are you saying that ext3_set_bit() should simply be called with
"ret_block" as its first argument? If yes, that is what the revised
patch below does.

Thanks
Andre

diff --git a/fs/ext3/balloc.c b/fs/ext3/balloc.c
index 063d994..3cca317 100644
--- a/fs/ext3/balloc.c
+++ b/fs/ext3/balloc.c
@@ -359,17 +359,6 @@ do_more:
 	if (!desc)
 		goto error_return;
 
-	if (in_range (le32_to_cpu(desc->bg_block_bitmap), block, count) ||
-	    in_range (le32_to_cpu(desc->bg_inode_bitmap), block, count) ||
-	    in_range (block, le32_to_cpu(desc->bg_inode_table),
-		      sbi->s_itb_per_group) ||
-	    in_range (block + count - 1, le32_to_cpu(desc->bg_inode_table),
-		      sbi->s_itb_per_group))
-		ext3_error (sb, "ext3_free_blocks",
-			    "Freeing blocks in system zones - "
-			    "Block = "E3FSBLK", count = %lu",
-			    block, count);
-
 	/*
 	 * We are about to start releasing blocks in the bitmap,
 	 * so we need undo access.
@@ -392,7 +381,17 @@ do_more:
 
 	jbd_lock_bh_state(bitmap_bh);
 
-	for (i = 0, group_freed = 0; i < count; i++) {
+	for (i = 0, group_freed = 0; i < count; i++, block++) {
+		struct ext3_group_desc *gdp = ext3_get_group_desc(sb, i, NULL);
+		if (block == le32_to_cpu(gdp->bg_block_bitmap) ||
+			block == le32_to_cpu(gdp->bg_inode_bitmap) ||
+			in_range(block, le32_to_cpu(gdp->bg_inode_table),
+				EXT3_SB(sb)->s_itb_per_group)) {
+			ext3_error(sb, __FUNCTION__,
+				"Freeing block in system zone - block = %lu",
+				block);
+			continue;
+		}
 		/*
 		 * An HJ special.  This is expensive...
 		 */
@@ -400,7 +399,7 @@ #ifdef CONFIG_JBD_DEBUG
 		jbd_unlock_bh_state(bitmap_bh);
 		{
 			struct buffer_head *debug_bh;
-			debug_bh = sb_find_get_block(sb, block + i);
+			debug_bh = sb_find_get_block(sb, block);
 			if (debug_bh) {
 				BUFFER_TRACE(debug_bh, "Deleted!");
 				if (!bh2jh(bitmap_bh)->b_committed_data)
@@ -452,7 +451,7 @@ #endif
 			jbd_unlock_bh_state(bitmap_bh);
 			ext3_error(sb, __FUNCTION__,
 				"bit already cleared for block "E3FSBLK,
-				 block + i);
+				block);
 			jbd_lock_bh_state(bitmap_bh);
 			BUFFER_TRACE(bitmap_bh, "bit already cleared");
 		} else {
@@ -479,7 +478,6 @@ #endif
 	*pdquot_freed_blocks += group_freed;
 
 	if (overflow && !err) {
-		block += count;
 		count = overflow;
 		goto do_more;
 	}
@@ -1260,7 +1258,7 @@ #endif
 		*errp = -ENOSPC;
 		goto out;
 	}
-
+repeat:
 	/*
 	 * First, test whether the goal block is free.
 	 */
@@ -1372,12 +1370,21 @@ allocated:
 	    in_range(ret_block, le32_to_cpu(gdp->bg_inode_table),
 		      EXT3_SB(sb)->s_itb_per_group) ||
 	    in_range(ret_block + num - 1, le32_to_cpu(gdp->bg_inode_table),
-		      EXT3_SB(sb)->s_itb_per_group))
-		ext3_error(sb, "ext3_new_block",
+		      EXT3_SB(sb)->s_itb_per_group)) {
+		ext3_error(sb, __FUNCTION__,
 			    "Allocating block in system zone - "
 			    "blocks from "E3FSBLK", length %lu",
 			     ret_block, num);
-
+		/* Note: This will potentially use up one of the handle's
+		 * buffer credits.  Normally we have way too many credits,
+		 * so that is OK.  In _very_ rare cases it might not be OK.
+		 * We will trigger an assertion if we run out of credits,
+		 * and we will have to do a full fsck of the filesystem -
+		 * better than randomly corrupting filesystem metadata.
+		 */
+		ext3_set_bit(ret_block, gdp_bh->b_data);
+		goto repeat;
+	}
 	performed_allocation = 1;
 
 #ifdef CONFIG_JBD_DEBUG
-- 
The only person who always got his work done by Friday was Robinson Crusoe

Attachment: signature.asc
Description: Digital signature