ext4: code clean up for dio fallocate handling
In the non async IO direct IO case, the io_end structure could be NULL.
The ext4_debug() call in ext4_end_io_dio() (inode.c) should be moved
after checking the io_end structure to be a non NULL pointer.
The comment above ext4_get_block_dio_write() ("Maximum
number of blocks...") is a duplicate; the original and correct comment
is above the #define DIO_MAX_BLOCKS up above.
The check for allocated > max_blocks in ext4_split_unwritten_extents()
can be removed, since the code returns immediately once allocated blocks
is less or equals to the requested blocks to convert.
Based on review comments from Curt Wohlgemuth.
Signed-off-by: Mingming Cao <[email protected]>
---
fs/ext4/extents.c | 96
+++++++++++++++++++++++++-----------------------------
fs/ext4/inode.c | 9 ++---
2 files changed, 50 insertions(+), 55 deletions(-)
Index: linux-2.6.31-rc4/fs/ext4/extents.c
===================================================================
--- linux-2.6.31-rc4.orig/fs/ext4/extents.c
+++ linux-2.6.31-rc4/fs/ext4/extents.c
@@ -2806,6 +2806,7 @@ static int ext4_split_unwritten_extents(
ext4_fsblk_t newblock;
int err = 0;
int ret = 0;
+ unsigned int newdepth;
ext_debug("ext4_split_unwritten_extents: inode %lu,"
"iblock %llu, max_blocks %u\n", inode->i_ino,
@@ -2845,61 +2846,56 @@ static int ext4_split_unwritten_extents(
* we insert ex3, if ex1 is NULL. This is to avoid temporary
* overlap of blocks.
*/
- if (!ex1 && allocated > max_blocks)
+ if (!ex1)
ex2->ee_len = cpu_to_le16(max_blocks);
/* ex3: to ee_block + ee_len : uninitialised */
- if (allocated > max_blocks) {
- unsigned int newdepth;
- ex3 = &newex;
- ex3->ee_block = cpu_to_le32(iblock + max_blocks);
- ext4_ext_store_pblock(ex3, newblock + max_blocks);
- ex3->ee_len = cpu_to_le16(allocated - max_blocks);
- ext4_ext_mark_uninitialized(ex3);
- err = ext4_ext_insert_extent(handle, inode, path, ex3, flags);
- if (err == -ENOSPC) {
- err = ext4_ext_zeroout(inode, &orig_ex);
- if (err)
- goto fix_extent_len;
- /* update the extent length and mark as initialized */
- ex->ee_block = orig_ex.ee_block;
- 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)
- goto fix_extent_len;
- /*
- * The depth, and hence eh & ex might change
- * as part of the insert above.
- */
- newdepth = ext_depth(inode);
- /*
- * update the extent length after successful insert of the
- * split extent
- */
- orig_ex.ee_len = cpu_to_le16(ee_len -
- ext4_ext_get_actual_len(ex3));
- 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);
+ ex3 = &newex;
+ ex3->ee_block = cpu_to_le32(iblock + max_blocks);
+ ext4_ext_store_pblock(ex3, newblock + max_blocks);
+ ex3->ee_len = cpu_to_le16(allocated - max_blocks);
+ ext4_ext_mark_uninitialized(ex3);
+ err = ext4_ext_insert_extent(handle, inode, path, ex3, flags);
+ if (err == -ENOSPC) {
+ err = ext4_ext_zeroout(inode, &orig_ex);
if (err)
- goto out;
+ goto fix_extent_len;
+ /* update the extent length and mark as initialized */
+ ex->ee_block = orig_ex.ee_block;
+ ex->ee_len = orig_ex.ee_len;
+ ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
+ ext4_ext_dirty(handle, inode, path + depth);
+ return allocated;
- allocated = max_blocks;
+ } else if (err)
+ goto fix_extent_len;
+ /*
+ * The depth, and hence eh & ex might change
+ * as part of the insert above.
+ */
+ newdepth = ext_depth(inode);
+ /*
+ * update the extent length after successful insert of the
+ * split extent
+ */
+ orig_ex.ee_len = cpu_to_le16(ee_len -
+ ext4_ext_get_actual_len(ex3));
+ 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 there was a change of depth as part of the
* insertion of ex3 above, we need to update the length
Index: linux-2.6.31-rc4/fs/ext4/inode.c
===================================================================
--- linux-2.6.31-rc4.orig/fs/ext4/inode.c
+++ linux-2.6.31-rc4/fs/ext4/inode.c
@@ -3367,8 +3367,6 @@ out:
return ret;
}
-/* Maximum number of blocks we map for direct IO at once. */
On Wed, 2009-10-07 at 10:19 -0700, Mingming wrote:
> ext4: code clean up for dio fallocate handling
>
> In the non async IO direct IO case, the io_end structure could be NULL.
> The ext4_debug() call in ext4_end_io_dio() (inode.c) should be moved
> after checking the io_end structure to be a non NULL pointer.
>
> The comment above ext4_get_block_dio_write() ("Maximum
> number of blocks...") is a duplicate; the original and correct comment
> is above the #define DIO_MAX_BLOCKS up above.
>
> The check for allocated > max_blocks in ext4_split_unwritten_extents()
> can be removed, since the code returns immediately once allocated blocks
> is less or equals to the requested blocks to convert.
>
> Based on review comments from Curt Wohlgemuth.
>
> Signed-off-by: Mingming Cao <[email protected]>
Updated cleanup patch after [patch 2/2 Fix return value of splitting
extents for dio write over fallocate ]fixed another bug. We still need
the check for allocated > max_blocks in ext4_split_unwritten_extents().
I'll resubmit the updated patch series (including two other bug
fixes/clean ups sent earlier this week) to the list again to avoid
confusion.
Thanks,
Mingming
ext4: code clean up for dio fallocate handling
The ext4_debug() call in ext4_end_io_dio() (inode.c) has a
should be moved after checking the io_end structure to be not a NULL pointer.
The comment above ext4_get_block_dio_write() ("Maximum
number of blocks...") is a duplicate; the original and correct comment
is above the #define DIO_MAX_BLOCKS up above.
The check for allocated > max_blocks in ext4_split_unwritten_extents()
can be removed, since the code returns immediately once allocated blocks is
less or equals to the requested blocks to convert.
Based on review comments from Curt Wohlgemuth.
Signed-off-by: Mingming Cao <[email protected]>
---
fs/ext4/extents.c | 96 +++++++++++++++++++++++++-----------------------------
fs/ext4/inode.c | 9 ++---
2 files changed, 50 insertions(+), 55 deletions(-)
Index: linux-2.6.31-rc4/fs/ext4/inode.c
===================================================================
--- linux-2.6.31-rc4.orig/fs/ext4/inode.c
+++ linux-2.6.31-rc4/fs/ext4/inode.c
@@ -3367,8 +3367,6 @@ out:
return ret;
}
-/* Maximum number of blocks we map for direct IO at once. */