On 5/11/2011 6:15 PM, Mingming Cao wrote:
On Wed, 2011-05-11 at 09:47 +0800, Yongqiang Yang wrote:
On Wed, May 11, 2011 at 1:56 AM, Allison Henderson
<achender@xxxxxxxxxxxxxxxxxx> wrote:
Hi All,
We've been trying to get punch hole through some extended fsx tests, and I ran across some other tests that were failing because the test file contained zeros where it shouldn't. I made this fix to the ext4_ext_convert_to_initialized
What do you mean zeros here?
Some useful data is zeroed?
and the test has been running smooth for about an hour now.
Yongqiang, this one looks like it may have been associated with the
split extents clean up patch. Would you mind taking a look at this
fix and giving it your ok if it looks good? Thx!
Signed-off-by: Allison Henderson<achender@xxxxxxxxxx>
---
:100644 100644 e363f21... ce69450... M fs/ext4/extents.c
fs/ext4/extents.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index e363f21..ce69450 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2819,7 +2819,8 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
/* case 3 */
zero_ex.ee_block =
cpu_to_le32(map->m_lblk + map->m_len);
- zero_ex.ee_len = cpu_to_le16(allocated - map->m_len);
+ zero_ex.ee_len = cpu_to_le16(ee_len -
+ allocated - map->m_len);
The logic is that we splits [ee_block, ee_block + ee_len) into
[ee_block, map->m_blk) that is uninitialized and [map->m_blk, ee_block
+ ee_len) that is initialized. We need to zero [map->m_lblk +
map->m_len, ee_block + ee_len).
and [map->m_lblk, map->m_lblk + map->m_len) is zeroed by upper layer
because of MAP_NEW flag.
Right logic?
Hmm, the logic in case 3 is-- if ex2[map->m_blk, map->m_blk+m_len] and
ex3 together[map->mblk+m_len+1, map->m_blk+allocated] total length
(allocated)is< than 7 blocks, then we zero out the entire ex2 and ext3,
there is no need to do split.
I think zero_ex.ee_len should be "allocated". Look at the original code
(before the extents splits cleanup patches), it will zero out entire
[map->mblk, map->m_blk+allocated] and don't do split anymore.
something like this, not a patch, but show what I think the right fix.
if (allocated> map->m_len) {
if (allocated<= EXT4_EXT_ZERO_LEN&&
(EXT4_EXT_MAY_ZEROOUT& split_flag)) {
/* case 3 */
zero_ex.ee_block =
cpu_to_le32(map->m_lblk + map->m_len);
- zero_ex.ee_len = cpu_to_le16(allocated - map->m_len);
zero_ex.ee_len = cpu_to_le16(allocated);
ext4_ext_store_pblock(&zero_ex,
ext4_ext_pblock(ex) + map->m_lblk - ee_block);
err = ext4_ext_zeroout(inode,&zero_ex);
if (err)
goto out;
- split_map.m_lblk = map->m_lblk;
- split_map.m_len = allocated;
+ ext4_ext_mark_initialized(ex);
+ ext4_ext_try_to_merge(inode, path, ex);
+ err = ext4_ext_dirty(handle, inode, path + depth);
+ goto out;
}
Mingming
I can not see the error and the meaning of ee_len - allocated - map->m_len.
Hi Yongqiang,
Sorry I didnt see your extra question down here. Initially I had read
"allocated" to be the length of ex1, but now I see that it is the length
of ex2+ex3. So ee_len - allocated - map->m_len was supposed to be ex3,
but I think Mingming has the right idea now with zeroing out all of
"allocated".
Allison Henderson
Thanks,
Yongqiang.
ext4_ext_store_pblock(&zero_ex,
ext4_ext_pblock(ex) + map->m_lblk - ee_block);
err = ext4_ext_zeroout(inode,&zero_ex);
--
1.7.1
--
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
