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
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
