We need to recalucate the path information on extent insert
even if depth doesn't change.
Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
fs/ext4/extents.c | 41 +++++++++++++++++++++++------------------
1 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 7bdaeec..9c8541e 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2462,7 +2462,10 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
unsigned int newdepth;
/* If extent has less than EXT4_EXT_ZERO_LEN zerout directly */
if (allocated <= EXT4_EXT_ZERO_LEN) {
- /* Mark first half uninitialized.
+ /*
+ * iblock == ee_block is handled by the zerouout
+ * at the beginning.
+ * Mark first half uninitialized.
* Mark second half initialized and zero out the
* initialized extent
*/
@@ -2485,7 +2488,7 @@ 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);
- /* zeroed the full extent */
+ /* blocks available from iblock */
return allocated;
} else if (err)
@@ -2513,6 +2516,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
err = PTR_ERR(path);
return err;
}
+ /* get the second half extent details */
ex = path[depth].p_ext;
err = ext4_ext_get_access(handle, inode,
path + depth);
@@ -2542,6 +2546,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
ext4_ext_dirty(handle, inode, path + depth);
/* zeroed the full extent */
+ /* blocks available from iblock */
return allocated;
} else if (err)
@@ -2557,23 +2562,22 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
*/
orig_ex.ee_len = cpu_to_le16(ee_len -
ext4_ext_get_actual_len(ex3));
- if (newdepth != depth) {
- depth = newdepth;
- ext4_ext_drop_refs(path);
- path = ext4_ext_find_extent(inode, iblock, path);
- if (IS_ERR(path)) {
- err = PTR_ERR(path);
- goto out;
- }
- eh = path[depth].p_hdr;
- ex = path[depth].p_ext;
- if (ex2 != &newex)
- ex2 = ex;
-
- err = ext4_ext_get_access(handle, inode, path + depth);
- if (err)
- goto out;
+ depth = newdepth;
+ ext4_ext_drop_refs(path);
+ path = ext4_ext_find_extent(inode, iblock, path);
+ if (IS_ERR(path)) {
+ err = PTR_ERR(path);
+ goto out;
}
+ eh = path[depth].p_hdr;
+ ex = path[depth].p_ext;
+ if (ex2 != &newex)
+ ex2 = ex;
+
+ err = ext4_ext_get_access(handle, inode, path + depth);
+ if (err)
+ goto out;
+
allocated = max_blocks;
/* If extent has less than EXT4_EXT_ZERO_LEN and we are trying
@@ -2591,6 +2595,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
ext4_ext_dirty(handle, inode, path + depth);
/* zero out the first half */
+ /* blocks available from iblock */
return allocated;
}
}
--
1.5.6.3.439.g1e10.dirty
Aneesh Kumar K.V wrote:
> We need to recalucate the path information on extent insert
> even if depth doesn't change.
Aneesh, do you have a compact test case for this?
Thanks,
-Eric
> Signed-off-by: Aneesh Kumar K.V <[email protected]>
> ---
> fs/ext4/extents.c | 41 +++++++++++++++++++++++------------------
> 1 files changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 7bdaeec..9c8541e 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2462,7 +2462,10 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> unsigned int newdepth;
> /* If extent has less than EXT4_EXT_ZERO_LEN zerout directly */
> if (allocated <= EXT4_EXT_ZERO_LEN) {
> - /* Mark first half uninitialized.
> + /*
> + * iblock == ee_block is handled by the zerouout
> + * at the beginning.
> + * Mark first half uninitialized.
> * Mark second half initialized and zero out the
> * initialized extent
> */
> @@ -2485,7 +2488,7 @@ 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);
> - /* zeroed the full extent */
> + /* blocks available from iblock */
> return allocated;
>
> } else if (err)
> @@ -2513,6 +2516,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> err = PTR_ERR(path);
> return err;
> }
> + /* get the second half extent details */
> ex = path[depth].p_ext;
> err = ext4_ext_get_access(handle, inode,
> path + depth);
> @@ -2542,6 +2546,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
> ext4_ext_dirty(handle, inode, path + depth);
> /* zeroed the full extent */
> + /* blocks available from iblock */
> return allocated;
>
> } else if (err)
> @@ -2557,23 +2562,22 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> */
> orig_ex.ee_len = cpu_to_le16(ee_len -
> ext4_ext_get_actual_len(ex3));
> - if (newdepth != depth) {
> - depth = newdepth;
> - ext4_ext_drop_refs(path);
> - path = ext4_ext_find_extent(inode, iblock, path);
> - if (IS_ERR(path)) {
> - err = PTR_ERR(path);
> - goto out;
> - }
> - eh = path[depth].p_hdr;
> - ex = path[depth].p_ext;
> - if (ex2 != &newex)
> - ex2 = ex;
> -
> - err = ext4_ext_get_access(handle, inode, path + depth);
> - if (err)
> - goto out;
> + depth = newdepth;
> + ext4_ext_drop_refs(path);
> + path = ext4_ext_find_extent(inode, iblock, path);
> + if (IS_ERR(path)) {
> + err = PTR_ERR(path);
> + goto out;
> }
> + eh = path[depth].p_hdr;
> + ex = path[depth].p_ext;
> + if (ex2 != &newex)
> + ex2 = ex;
> +
> + err = ext4_ext_get_access(handle, inode, path + depth);
> + if (err)
> + goto out;
> +
> allocated = max_blocks;
>
> /* If extent has less than EXT4_EXT_ZERO_LEN and we are trying
> @@ -2591,6 +2595,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
> ext4_ext_dirty(handle, inode, path + depth);
> /* zero out the first half */
> + /* blocks available from iblock */
> return allocated;
> }
> }
On Fri, Jul 18, 2008 at 10:12:46AM -0500, Eric Sandeen wrote:
> Aneesh Kumar K.V wrote:
> > We need to recalucate the path information on extent insert
> > even if depth doesn't change.
>
> Aneesh, do you have a compact test case for this?
>
git://github.com/shehjart/tools.git
writefallocate/test.sh
-aneesh
I tried this patch, with nodelalloc, the kernel did not crash! Previous
it crashes in a few minutes.
but the test did not stop after fill the file by specified 2G size
I noticed the write() to write to the last fallocate extent(up to 2G)
returns junk value.
The testcase I got from Shehjart (who originally reported the problem)
is
http://www.gelato.unsw.edu.au/~shehjart/writefallocate.c
command I use to reproduce the problem is
./writefallocate --filesize 2147483648 --wrblksize 65536 --fallocate
1048576 --filename /mnt/ext4/a --nounlink
.............
offset: 2147024896, written: 65536^M
offset: 2147090432, written: 65536^M
offset: 2147155968, written: 65536^M
offset: 2147221504, written: 65536^M
offset: 2147287040, written: 65536^M
offset: 2147352576, written: 65536^M
offset: 2147418112, written: 65535^M
offset: 2147483647, written: 18446744073709551615^M
fallocate offset: 2147483647^M
offset: 2147483646, written: 18446744073709551615^M
offset: 2147483645, written: 18446744073709551615^M
offset: 2147483644, written: 18446744073709551615^M
offset: 2147483643, written: 18446744073709551615^M
offset: 2147483642, written: 18446744073709551615^M
offset: 2147483641, written: 18446744073709551615^M
offset: 2147483640, written: 18446744073709551615^M
Appearently the test has fallocate/fill the 2G file, but since the last
write() returns 18446744073709551615, the offset is decreased, cause the
test continue with a break, and writes to wrong offset
在 2008-07-18五的 18:47 +0530，Aneesh Kumar K.V写道：
> We need to recalucate the path information on extent insert
> even if depth doesn't change.
>
> Signed-off-by: Aneesh Kumar K.V <[email protected]>
> ---
> fs/ext4/extents.c | 41 +++++++++++++++++++++++------------------
> 1 files changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 7bdaeec..9c8541e 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2462,7 +2462,10 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> unsigned int newdepth;
> /* If extent has less than EXT4_EXT_ZERO_LEN zerout directly */
> if (allocated <= EXT4_EXT_ZERO_LEN) {
> - /* Mark first half uninitialized.
> + /*
> + * iblock == ee_block is handled by the zerouout
> + * at the beginning.
> + * Mark first half uninitialized.
> * Mark second half initialized and zero out the
> * initialized extent
> */
> @@ -2485,7 +2488,7 @@ 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);
> - /* zeroed the full extent */
> + /* blocks available from iblock */
> return allocated;
>
> } else if (err)
> @@ -2513,6 +2516,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> err = PTR_ERR(path);
> return err;
> }
> + /* get the second half extent details */
> ex = path[depth].p_ext;
> err = ext4_ext_get_access(handle, inode,
> path + depth);
> @@ -2542,6 +2546,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
> ext4_ext_dirty(handle, inode, path + depth);
> /* zeroed the full extent */
> + /* blocks available from iblock */
> return allocated;
>
> } else if (err)
> @@ -2557,23 +2562,22 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> */
> orig_ex.ee_len = cpu_to_le16(ee_len -
> ext4_ext_get_actual_len(ex3));
> - if (newdepth != depth) {
> - depth = newdepth;
> - ext4_ext_drop_refs(path);
> - path = ext4_ext_find_extent(inode, iblock, path);
> - if (IS_ERR(path)) {
> - err = PTR_ERR(path);
> - goto out;
> - }
> - eh = path[depth].p_hdr;
> - ex = path[depth].p_ext;
> - if (ex2 != &newex)
> - ex2 = ex;
> -
> - err = ext4_ext_get_access(handle, inode, path + depth);
> - if (err)
> - goto out;
> + depth = newdepth;
> + ext4_ext_drop_refs(path);
> + path = ext4_ext_find_extent(inode, iblock, path);
> + if (IS_ERR(path)) {
> + err = PTR_ERR(path);
> + goto out;
> }
> + eh = path[depth].p_hdr;
> + ex = path[depth].p_ext;
> + if (ex2 != &newex)
> + ex2 = ex;
> +
> + err = ext4_ext_get_access(handle, inode, path + depth);
> + if (err)
> + goto out;
> +
> allocated = max_blocks;
>
> /* If extent has less than EXT4_EXT_ZERO_LEN and we are trying
> @@ -2591,6 +2595,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
> ext4_ext_dirty(handle, inode, path + depth);
> /* zero out the first half */
> + /* blocks available from iblock */
> return allocated;
> }
> }
在 2008-07-18五的 10:02 -0700，Mingming Cao写道：
> I tried this patch, with nodelalloc, the kernel did not crash! Previous
> it crashes in a few minutes.
>
> but the test did not stop after fill the file by specified 2G size
>
>
> I noticed the write() to write to the last fallocate extent(up to 2G)
> returns junk value.
>
>
> The testcase I got from Shehjart (who originally reported the problem)
> is
>
> http://www.gelato.unsw.edu.au/~shehjart/writefallocate.c
>
I forget to mention that I modify the test slight to call fallocate()
with default mode (expnd size) instead of keep size.
> command I use to reproduce the problem is
>
> ./writefallocate --filesize 2147483648 --wrblksize 65536 --fallocate
> 1048576 --filename /mnt/ext4/a --nounlink
>
> .............
> offset: 2147024896, written: 65536^M
> offset: 2147090432, written: 65536^M
> offset: 2147155968, written: 65536^M
> offset: 2147221504, written: 65536^M
> offset: 2147287040, written: 65536^M
> offset: 2147352576, written: 65536^M
> offset: 2147418112, written: 65535^M
> offset: 2147483647, written: 18446744073709551615^M
> fallocate offset: 2147483647^M
> offset: 2147483646, written: 18446744073709551615^M
> offset: 2147483645, written: 18446744073709551615^M
> offset: 2147483644, written: 18446744073709551615^M
> offset: 2147483643, written: 18446744073709551615^M
> offset: 2147483642, written: 18446744073709551615^M
> offset: 2147483641, written: 18446744073709551615^M
> offset: 2147483640, written: 18446744073709551615^M
>
>
> Appearently the test has fallocate/fill the 2G file, but since the last
> write() returns 18446744073709551615, the offset is decreased, cause the
> test continue with a break, and writes to wrong offset
>
>
>
>
> 在 2008-07-18五的 18:47 +0530，Aneesh Kumar K.V写道：
> > We need to recalucate the path information on extent insert
> > even if depth doesn't change.
> >
> > Signed-off-by: Aneesh Kumar K.V <[email protected]>
> > ---
> > fs/ext4/extents.c | 41 +++++++++++++++++++++++------------------
> > 1 files changed, 23 insertions(+), 18 deletions(-)
> >
> > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> > index 7bdaeec..9c8541e 100644
> > --- a/fs/ext4/extents.c
> > +++ b/fs/ext4/extents.c
> > @@ -2462,7 +2462,10 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> > unsigned int newdepth;
> > /* If extent has less than EXT4_EXT_ZERO_LEN zerout directly */
> > if (allocated <= EXT4_EXT_ZERO_LEN) {
> > - /* Mark first half uninitialized.
> > + /*
> > + * iblock == ee_block is handled by the zerouout
> > + * at the beginning.
> > + * Mark first half uninitialized.
> > * Mark second half initialized and zero out the
> > * initialized extent
> > */
> > @@ -2485,7 +2488,7 @@ 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);
> > - /* zeroed the full extent */
> > + /* blocks available from iblock */
> > return allocated;
> >
> > } else if (err)
> > @@ -2513,6 +2516,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> > err = PTR_ERR(path);
> > return err;
> > }
> > + /* get the second half extent details */
> > ex = path[depth].p_ext;
> > err = ext4_ext_get_access(handle, inode,
> > path + depth);
> > @@ -2542,6 +2546,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> > ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
> > ext4_ext_dirty(handle, inode, path + depth);
> > /* zeroed the full extent */
> > + /* blocks available from iblock */
> > return allocated;
> >
> > } else if (err)
> > @@ -2557,23 +2562,22 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> > */
> > orig_ex.ee_len = cpu_to_le16(ee_len -
> > ext4_ext_get_actual_len(ex3));
> > - if (newdepth != depth) {
> > - depth = newdepth;
> > - ext4_ext_drop_refs(path);
> > - path = ext4_ext_find_extent(inode, iblock, path);
> > - if (IS_ERR(path)) {
> > - err = PTR_ERR(path);
> > - goto out;
> > - }
> > - eh = path[depth].p_hdr;
> > - ex = path[depth].p_ext;
> > - if (ex2 != &newex)
> > - ex2 = ex;
> > -
> > - err = ext4_ext_get_access(handle, inode, path + depth);
> > - if (err)
> > - goto out;
> > + depth = newdepth;
> > + ext4_ext_drop_refs(path);
> > + path = ext4_ext_find_extent(inode, iblock, path);
> > + if (IS_ERR(path)) {
> > + err = PTR_ERR(path);
> > + goto out;
> > }
> > + eh = path[depth].p_hdr;
> > + ex = path[depth].p_ext;
> > + if (ex2 != &newex)
> > + ex2 = ex;
> > +
> > + err = ext4_ext_get_access(handle, inode, path + depth);
> > + if (err)
> > + goto out;
> > +
> > allocated = max_blocks;
> >
> > /* If extent has less than EXT4_EXT_ZERO_LEN and we are trying
> > @@ -2591,6 +2595,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> > ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
> > ext4_ext_dirty(handle, inode, path + depth);
> > /* zero out the first half */
> > + /* blocks available from iblock */
> > return allocated;
> > }
> > }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
在 2008-07-18五的 10:22 -0700，Mingming Cao写道：
> 在 2008-07-18五的 10:02 -0700，Mingming Cao写道：
> > I tried this patch, with nodelalloc, the kernel did not crash! Previous
> > it crashes in a few minutes.
> >
> > but the test did not stop after fill the file by specified 2G size
> >
> >
> > I noticed the write() to write to the last fallocate extent(up to 2G)
> > returns junk value.
> >
> >
> > The testcase I got from Shehjart (who originally reported the problem)
> > is
> >
> > http://www.gelato.unsw.edu.au/~shehjart/writefallocate.c
> >
>
> I forget to mention that I modify the test slight to call fallocate()
> with default mode (expnd size) instead of keep size.
>
Oh, it's the file size problem for the 2 GB barrier, the latest test
source git tree has fixed this,. With the patch ext4 runs fine with
both KEEP_SIZE mode and ALLOCATE mode.
Mingming
在 2008-07-18五的 18:47 +0530，Aneesh Kumar K.V写道：
> We need to recalucate the path information on extent insert
> even if depth doesn't change.
>
Patch has been added to ext4 patch queue
Acked-by: Mingming Cao <[email protected]>
> Signed-off-by: Aneesh Kumar K.V <[email protected]>
> ---
> fs/ext4/extents.c | 41 +++++++++++++++++++++++------------------
> 1 files changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 7bdaeec..9c8541e 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2462,7 +2462,10 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> unsigned int newdepth;
> /* If extent has less than EXT4_EXT_ZERO_LEN zerout directly */
> if (allocated <= EXT4_EXT_ZERO_LEN) {
> - /* Mark first half uninitialized.
> + /*
> + * iblock == ee_block is handled by the zerouout
> + * at the beginning.
> + * Mark first half uninitialized.
> * Mark second half initialized and zero out the
> * initialized extent
> */
> @@ -2485,7 +2488,7 @@ 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);
> - /* zeroed the full extent */
> + /* blocks available from iblock */
> return allocated;
>
> } else if (err)
> @@ -2513,6 +2516,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> err = PTR_ERR(path);
> return err;
> }
> + /* get the second half extent details */
> ex = path[depth].p_ext;
> err = ext4_ext_get_access(handle, inode,
> path + depth);
> @@ -2542,6 +2546,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
> ext4_ext_dirty(handle, inode, path + depth);
> /* zeroed the full extent */
> + /* blocks available from iblock */
> return allocated;
>
> } else if (err)
> @@ -2557,23 +2562,22 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> */
> orig_ex.ee_len = cpu_to_le16(ee_len -
> ext4_ext_get_actual_len(ex3));
> - if (newdepth != depth) {
> - depth = newdepth;
> - ext4_ext_drop_refs(path);
> - path = ext4_ext_find_extent(inode, iblock, path);
> - if (IS_ERR(path)) {
> - err = PTR_ERR(path);
> - goto out;
> - }
> - eh = path[depth].p_hdr;
> - ex = path[depth].p_ext;
> - if (ex2 != &newex)
> - ex2 = ex;
> -
> - err = ext4_ext_get_access(handle, inode, path + depth);
> - if (err)
> - goto out;
> + depth = newdepth;
> + ext4_ext_drop_refs(path);
> + path = ext4_ext_find_extent(inode, iblock, path);
> + if (IS_ERR(path)) {
> + err = PTR_ERR(path);
> + goto out;
> }
> + eh = path[depth].p_hdr;
> + ex = path[depth].p_ext;
> + if (ex2 != &newex)
> + ex2 = ex;
> +
> + err = ext4_ext_get_access(handle, inode, path + depth);
> + if (err)
> + goto out;
> +
> allocated = max_blocks;
>
> /* If extent has less than EXT4_EXT_ZERO_LEN and we are trying
> @@ -2591,6 +2595,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
> ext4_ext_dirty(handle, inode, path + depth);
> /* zero out the first half */
> + /* blocks available from iblock */
> return allocated;
> }
> }