* [PATCH 2/2]Fix return value of splitting extents for dio write over fallocate @ 2009-10-07 17:23 Mingming 2009-10-07 18:50 ` Mingming 0 siblings, 1 reply; 3+ messages in thread From: Mingming @ 2009-10-07 17:23 UTC (permalink / raw) To: tytso; +Cc: ext4 development, Curt Wohlgemuth ext4: Fix return value of splitting extents for dio write over fallocate To prepare direct IO write, we need to split the unwritten extents before submit the IO. In case of there is no need to split at all, ext4_split_unwritten_extents() was incorrectly returns 0 instead of the size of uninitalized extents. This bug caused the buffer remains unmapped, thus mislead VFS direct IO falling back to buffered IO. Signed-off-by: Mingming Cao <cmm@us.ibm.com> --- fs/ext4/extents.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 @@ -2788,6 +2788,8 @@ fix_extent_len: * into three uninitialized extent(at most). After IO complete, the part * being filled will be convert to initialized by the end_io callback function * via ext4_convert_unwritten_extents(). + * + * Returns the size of unitialized extent to be writen, on success. */ static int ext4_split_unwritten_extents(handle_t *handle, struct inode *inode, @@ -2829,7 +2831,7 @@ static int ext4_split_unwritten_extents( * uninitialized extent */ if (allocated <= max_blocks) - return ret; + return allocated; err = ext4_ext_get_access(handle, inode, path + depth); if (err) ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2]Fix return value of splitting extents for dio write over fallocate 2009-10-07 17:23 [PATCH 2/2]Fix return value of splitting extents for dio write over fallocate Mingming @ 2009-10-07 18:50 ` Mingming 2009-10-08 23:08 ` Mingming 0 siblings, 1 reply; 3+ messages in thread From: Mingming @ 2009-10-07 18:50 UTC (permalink / raw) To: tytso; +Cc: ext4 development, Curt Wohlgemuth On Wed, 2009-10-07 at 10:23 -0700, Mingming wrote: > ext4: Fix return value of splitting extents for dio write over fallocate Please pick up this patch instead, after fixing compile warning. ext4: Fix return value of splitting extents for dio write over fallocate To prepare direct IO write, we need to split the unwritten extents before submit the IO. In case of no split needs at all, ext4_split_unwritten_extents() was incorrectly returns 0 instead of the size of uninitalized extents. This bug caused wrong return value sent back to VFS code when it gets called from async IO path, leads to falling back to buffered IO. Signed-off-by: Mingming Cao <cmm@us.ibm.com> --- fs/ext4/extents.c | 5 +++-- 1 file changed, 3 insertions(+), 2 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 @@ -2788,6 +2788,8 @@ fix_extent_len: * into three uninitialized extent(at most). After IO complete, the part * being filled will be convert to initialized by the end_io callback function * via ext4_convert_unwritten_extents(). + * + * Returns the size of unitialized extent to be writen, on success. */ static int ext4_split_unwritten_extents(handle_t *handle, struct inode *inode, @@ -2805,7 +2807,6 @@ static int ext4_split_unwritten_extents( unsigned int allocated, ee_len, depth; ext4_fsblk_t newblock; int err = 0; - int ret = 0; unsigned int newdepth; ext_debug("ext4_split_unwritten_extents: inode %lu," @@ -2829,7 +2830,7 @@ static int ext4_split_unwritten_extents( * uninitialized extent */ if (allocated <= max_blocks) - return ret; + return allocated; err = ext4_ext_get_access(handle, inode, path + depth); if (err) ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2]Fix return value of splitting extents for dio write over fallocate 2009-10-07 18:50 ` Mingming @ 2009-10-08 23:08 ` Mingming 0 siblings, 0 replies; 3+ messages in thread From: Mingming @ 2009-10-08 23:08 UTC (permalink / raw) To: tytso; +Cc: ext4 development, Curt Wohlgemuth On Wed, 2009-10-07 at 11:50 -0700, Mingming wrote: > On Wed, 2009-10-07 at 10:23 -0700, Mingming wrote: > > ext4: Fix return value of splitting extents for dio write over fallocate > > Please pick up this patch instead, after fixing compile warning. > > ext4: Fix return value of splitting extents for dio write over fallocate > > To prepare direct IO write, we need to split the unwritten extents before > submit the IO. In case of no split needs at all, ext4_split_unwritten_extents() > was incorrectly returns 0 instead of the size of uninitalized extents. This bug > caused wrong return value sent back to VFS code when it gets called from async > IO path, leads to falling back to buffered IO. > > Signed-off-by: Mingming Cao <cmm@us.ibm.com> Jiayang zhang pointed that there is another issue with avoid splitting the unintialized extent when it's going to be fullfilled. The start offset to write needs to be at the beginning of the uninitialized extent. The following is the updated patch. Please review and pick up this patch. Thanks. Mingming ext4: Fix direct IO return values over fallocate space To prepare direct IO write, we need to split the unwritten extents before submit the IO. In case of the full uninitialized extent is going to be written, there is no need to split the extenT at all. ext4_split_unwritten_extents() has bug to check whether it's full fill of the unintialized extent, and incorrectly returns 0 instead of the size of uninitalized extents. The later bug caused wrong return value sent back to VFS code when it gets called from async IO path, leads to falling back to buffered IO. Thanks to Jiaying Zhang who reported the bugs and tested the patch. Signed-off-by: Mingming Cao <cmm@us.ibm.com> --- fs/ext4/extents.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 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 @@ -2788,6 +2788,8 @@ fix_extent_len: * into three uninitialized extent(at most). After IO complete, the part * being filled will be convert to initialized by the end_io callback function * via ext4_convert_unwritten_extents(). + * + * Returns the size of unitialized extent to be writen, on success. */ static int ext4_split_unwritten_extents(handle_t *handle, struct inode *inode, @@ -2805,7 +2807,6 @@ static int ext4_split_unwritten_extents( unsigned int allocated, ee_len, depth; ext4_fsblk_t newblock; int err = 0; - int ret = 0; unsigned int newdepth; ext_debug("ext4_split_unwritten_extents: inode %lu," @@ -2828,8 +2829,8 @@ static int ext4_split_unwritten_extents( * the size of extent to write, there is no need to split * uninitialized extent */ - if (allocated <= max_blocks) - return ret; + if (iblock == ee_block && allocated <= max_blocks) + return allocated; err = ext4_ext_get_access(handle, inode, path + depth); if (err) ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-10-08 23:08 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-10-07 17:23 [PATCH 2/2]Fix return value of splitting extents for dio write over fallocate Mingming 2009-10-07 18:50 ` Mingming 2009-10-08 23:08 ` Mingming
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).