[PATCH] fix file system corruption [ was Re: mballoc errors ]

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

On Tue, Apr 01, 2008 at 01:48:19AM +0530, Aneesh Kumar K.V wrote:
> On Mon, Mar 31, 2008 at 04:58:06PM +0200, Valerie Clement wrote:
> 
> I actually added fallocate to fsstress and created the file filesystem
> as you suggested. I am able to reproduce the problem once. Currently
> doing a code audit. Will let you know if i make any progress.
> 

I found one case where it could fail. We are returning total extent size
in case of converting from uninitialized extent to initialized one. 
Now if we have the below layout

[p --------x--------]

where p is the start block and x is the location where we intent to
write. If converting the above uninit extent resulted in 

[p zeroed-out][uninit].

We should not be returning the size of
zeroed-out-extent. That is because in ext4_ext_get_blocks we cache
the extent information as below

out_new:
		....
/* Cache only when it is _not_ an uninitialized extent */
if (create != EXT4_CREATE_UNINITIALIZED_EXT)
	ext4_ext_put_in_cache(inode, iblock, allocated, newblock,
						EXT4_EXT_CACHE_EXTENT);
out:


That would mean we are caching an extent starting from newblock of
size allocated where allocated would be the size of zeroout extent,
which is actually wrong.

I am yet to test the patch. If you have time can you test the below
change

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 964d2c1..91f8f72 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2264,7 +2264,8 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 		ex->ee_len   = orig_ex.ee_len;
 		ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
 		ext4_ext_dirty(handle, inode, path + depth);
-		return le16_to_cpu(ex->ee_len);
+		/* zeroed the full extent */
+		return allocated;
 	}
 
 	/* ex1: ee_block to iblock - 1 : uninitialized */
@@ -2309,11 +2310,13 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 				ex->ee_len   = orig_ex.ee_len;
 				ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
 				ext4_ext_dirty(handle, inode, path + depth);
-				return le16_to_cpu(ex->ee_len);
+				/* zeroed the full extent */
+				return allocated;
 
 			} else if (err)
 				goto fix_extent_len;
 
+			/* zeroed the second half */
 			return allocated;
 		}
 		ex3 = &newex;
@@ -2331,7 +2334,8 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 			ex->ee_len   = orig_ex.ee_len;
 			ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
 			ext4_ext_dirty(handle, inode, path + depth);
-			return le16_to_cpu(ex->ee_len);
+			/* zeroed the full extent */
+			return allocated;
 
 		} else if (err)
 			goto fix_extent_len;
@@ -2379,7 +2383,8 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 			ex->ee_len   = orig_ex.ee_len;
 			ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
 			ext4_ext_dirty(handle, inode, path + depth);
-			return le16_to_cpu(ex->ee_len);
+			/* zero out the first half */
+			return allocated;
 		}
 	}
 	/*
@@ -2446,7 +2451,8 @@ insert:
 		ex->ee_len   = orig_ex.ee_len;
 		ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
 		ext4_ext_dirty(handle, inode, path + depth);
-		return le16_to_cpu(ex->ee_len);
+		/* zero out the first half */
+		return allocated;
 	} else if (err)
 		goto fix_extent_len;
 out: