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: -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
