[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20080331205030.GB30646@skywalker>
Date: Tue, 1 Apr 2008 02:20:30 +0530
From: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To: Valerie Clement <valerie.clement@...l.net>
Cc: linux-ext4 <linux-ext4@...r.kernel.org>
Subject: [PATCH] fix file system corruption [ was Re: mballoc errors ]
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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists