Re: [PATCH] fix file system corruption [ was Re: mballoc errors ]
From: Valerie Clement <hidden>
Date: 2008-04-01 08:38:31
Aneesh Kumar K.V wrote:
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
Hi Aneesh, I tested your patch but unfortunately I still reproduced the problem with it: EXT4-fs error (device sdc1): ext4_valid_block_bitmap: Invalid block bitmap - block_group = 6233, block = 51060737 Is there another thing I could do to help debugging the problem? Valerie
quoted hunk ↗ jump to hunk
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: