ocfs2: avoid direct write if we fall back to buffered
PATCH 1/1 ocfs2: avoid direct write if we fall back to buffered
when we fall back to buffered write from direct write, we call __generic_file_aio_write but that will end up doing direct write even we are only prepared to do buffered write because the file has O_DIRECT flag set. This is a fix for
https://bugzilla.novell.com/show_bug.cgi?id=591039
--- fs/ocfs2/file.c | 27 +++++++++++++++++---------- 1 files changed, 17 insertions(+), 10 deletions(-)
fs/ocfs2/file.c +17 -10
@@ -1973,18 +1973,24 @@ relock:/* communicate with ocfs2_dio_end_io */ ocfs2_iocb_set_rw_locked (iocb, rw_level); -if (direct_io) { - ret =generic_segment_checks (iov, &nr_segs, &ocount, -VERIFY_READ ); -if (ret) - goto out_dio; + ret =generic_segment_checks (iov, &nr_segs, &ocount, +VERIFY_READ ); +if (ret) + goto out_dio; - count = ocount; - ret =generic_write_checks (file, ppos, &count, + count = ocount; + ret =generic_write_checks (file, ppos, &count,S_ISBLK (inode->i_mode)); -if (ret) - goto out_dio; +if (ret) + goto out_dio; + + ret =file_remove_suid (file); +if (ret) + goto out_dio; +file_update_time (file); + +if (direct_io) { written =generic_file_direct_write (iocb, iov, &nr_segs, *ppos, ppos, count, ocount);if (written <0 ) {
@@ -1999,7 +2005,8 @@ relock: goto out_dio; } }else { - written =__generic_file_aio_write (iocb, iov, nr_segs, ppos); + written =generic_file_buffered_write (iocb, iov, nr_segs, + *ppos, ppos, count,0 ); } out_dio:
Signature · 1 line
1.7.0.3
PATCH 1/1 ocfs2: avoid direct write if we fall back to buffered
I cannot read the bugzilla. Now it maybe that that bz cannot be made public. That's ok. But if that's the case, can you explain the problem encountered. I am not qs the fix... rather trying to understand why this has not been reported before.
Quoted 61 lines · Li Dongyang
when we fall back to buffered write from direct write, we call
__generic_file_aio_writebut that will end up doing direct write even we are only prepared to do buffered write because the file hasO_DIRECTflag set. This is a fix for
https://bugzilla.novell.com/show_bug.cgi?id=591039Signed-off-by Li Dongyang <[email protected]>--- fs/ocfs2/file.c | 27 +++++++++++++++++---------- 1 files changed, 17 insertions(+), 10 deletions(-)fs/ocfs2/file.c +17
-10@@ -1973,18 +1973,24 @@ relock:/* communicate with ocfs2_dio_end_io */ ocfs2_iocb_set_rw_locked (iocb, rw_level); -if (direct_io) { - ret =generic_segment_checks (iov, &nr_segs, &ocount, -VERIFY_READ ); -if (ret) - goto out_dio; + ret =generic_segment_checks (iov, &nr_segs, &ocount, +VERIFY_READ ); +if (ret) + goto out_dio; - count = ocount; - ret =generic_write_checks (file, ppos, &count, + count = ocount; + ret =generic_write_checks (file, ppos, &count,S_ISBLK (inode->i_mode)); -if (ret) - goto out_dio; +if (ret) + goto out_dio; + + ret =file_remove_suid (file); +if (ret) + goto out_dio; +file_update_time (file); + +if (direct_io) { written =generic_file_direct_write (iocb, iov, &nr_segs, *ppos, ppos, count, ocount);if (written <0 ) {@@ -1999,7 +2005,8 @@ relock: goto out_dio; } }else { - written =__generic_file_aio_write (iocb, iov, nr_segs, ppos); + written =generic_file_buffered_write (iocb, iov, nr_segs, + *ppos, ppos, count,0 ); } out_dio:
PATCH 1/1 ocfs2: avoid direct write if we fall back to buffered
This is because ocfs2_file_aio_write calls ocfs2_prepare_inode_for_write which sets direct_io to 0 if it finds out that direct IO would extend the file. But later we call __generic_file_aio_write which end's up calling generic_file_direct_write because the file has O_DIRECT flag.So every time we do a direct write extending the file, the inode->i_size gets inconsistent with the i_size on disk because we call generic_file_direct_write, and if we do a truncate after this, we will meet a bug in ocfs2_truncate_file.
Quoted 68 lines · On Friday 09 April 2010 02:41:26 Sunil Mushran wrote:
I cannot read the bugzilla. Now it maybe that that bz cannot be made public. That's ok. But if that's the case, can you explain the problem encountered. I am not qs the fix... rather trying to understand why this has not been reported before.
Quoted 60 lines · Li Dongyang
when we fall back to buffered write from direct write, we call
__generic_file_aio_writebut that will end up doing direct write even we are only prepared to do buffered write because the file hasO_DIRECTflag set. This is a fix for
https://bugzilla.novell.com/show_bug.cgi?id=591039Signed-off-by Li Dongyang <[email protected]>--- fs/ocfs2/file.c | 27 +++++++++++++++++---------- 1 files changed, 17 insertions(+), 10 deletions(-)fs/ocfs2/file.c +17
-10@@ -1973,18 +1973,24 @@ relock:/* communicate with ocfs2_dio_end_io */ ocfs2_iocb_set_rw_locked (iocb, rw_level); -if (direct_io) { - ret =generic_segment_checks (iov, &nr_segs, &ocount, -VERIFY_READ ); -if (ret) - goto out_dio; + ret =generic_segment_checks (iov, &nr_segs, &ocount, +VERIFY_READ ); +if (ret) + goto out_dio; - count = ocount; - ret =generic_write_checks (file, ppos, &count, + count = ocount; + ret =generic_write_checks (file, ppos, &count,S_ISBLK (inode->i_mode)); -if (ret) - goto out_dio; +if (ret) + goto out_dio; + + ret =file_remove_suid (file); +if (ret) + goto out_dio; +file_update_time (file); + +if (direct_io) { written =generic_file_direct_write (iocb, iov, &nr_segs, *ppos, ppos, count, ocount);if (written <0 ) {@@ -1999,7 +2005,8 @@ relock: goto out_dio; } }else { - written =__generic_file_aio_write (iocb, iov, nr_segs, ppos); + written =generic_file_buffered_write (iocb, iov, nr_segs, + *ppos, ppos, count,0 ); } out_dio:
PATCH 1/1 ocfs2: avoid direct write if we fall back to buffered
Hi Dongyang,
Quoted 7 lines · Li Dongyang
This is because
ocfs2_file_aio_writecallsocfs2_prepare_inode_for_writewhich setsdirect_ioto 0 if it finds out that direct IO would extend the file. But later we call__generic_file_aio_writewhich end's up callinggeneric_file_direct_writebecause the file hasO_DIRECTflag.So every time we do a direct write extending the file, theinode->i_sizegets inconsistent with thei_sizeon disk because we callgeneric_file_direct_write, and if we do a truncate after this, we will meet a bug inocfs2_truncate_file.
yes we have O_DIRECT flag set and in __generic_file_aio_write it will call generic_file_direct_write first and then trigger to ocfs2_direct_IO. In this function we will check again and return 0. And _generic_file_aio_write will fall back to buffered write if the directIO can't write. Am I wrong somehow?
Regards,
Tao
Quoted 74 lines · On Friday 09 April 2010 02:41:26 Sunil Mushran wrote:
Quoted 66 lines
I cannot read the bugzilla. Now it maybe that that bz cannot be made public. That's ok. But if that's the case, can you explain the problem encountered. I am not qs the fix... rather trying to understand why this has not been reported before.
Quoted 59 lines · Li Dongyang
when we fall back to buffered write from direct write, we call
__generic_file_aio_writebut that will end up doing direct write even we are only prepared to do buffered write because the file hasO_DIRECTflag set. This is a fix for
https://bugzilla.novell.com/show_bug.cgi?id=591039Signed-off-by Li Dongyang <[email protected]>--- fs/ocfs2/file.c | 27 +++++++++++++++++---------- 1 files changed, 17 insertions(+), 10 deletions(-)fs/ocfs2/file.c +17
-10@@ -1973,18 +1973,24 @@ relock:/* communicate with ocfs2_dio_end_io */ ocfs2_iocb_set_rw_locked (iocb, rw_level); -if (direct_io) { - ret =generic_segment_checks (iov, &nr_segs, &ocount, -VERIFY_READ ); -if (ret) - goto out_dio; + ret =generic_segment_checks (iov, &nr_segs, &ocount, +VERIFY_READ ); +if (ret) + goto out_dio; - count = ocount; - ret =generic_write_checks (file, ppos, &count, + count = ocount; + ret =generic_write_checks (file, ppos, &count,S_ISBLK (inode->i_mode)); -if (ret) - goto out_dio; +if (ret) + goto out_dio; + + ret =file_remove_suid (file); +if (ret) + goto out_dio; +file_update_time (file); + +if (direct_io) { written =generic_file_direct_write (iocb, iov, &nr_segs, *ppos, ppos, count, ocount);if (written <0 ) {@@ -1999,7 +2005,8 @@ relock: goto out_dio; } }else { - written =__generic_file_aio_write (iocb, iov, nr_segs, ppos); + written =generic_file_buffered_write (iocb, iov, nr_segs, + *ppos, ppos, count,0 ); }Quoted 1 line
fs/ocfs2/file.c relock: · line 2012
Quoted 1 line
fs/ocfs2/file.c +0
-0out_dio:_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: avoid direct write if we fall back to buffered
Hi, Tao,
Quoted 12 lines · On Friday 09 April 2010 10:38:33 Tao Ma wrote:
Hi Dongyang,
Quoted 8 lines · Li Dongyang
This is because
ocfs2_file_aio_writecallsocfs2_prepare_inode_for_writewhich setsdirect_ioto 0 if it finds out that direct IO would extend the file. But later we call__generic_file_aio_writewhich end's up callinggeneric_file_direct_writebecause the file hasO_DIRECTflag.So every time we do a direct write extending the file, theinode->i_sizegets inconsistent with thei_sizeon disk because we callgeneric_file_direct_write, and if we do a truncate after this, we will meet a bug inocfs2_truncate_file.
Quoted 6 lines
yes we have
O_DIRECTflag set and in__generic_file_aio_writeit will callgeneric_file_direct_writefirst and then trigger toocfs2_direct_IO. In this function we will check again and return 0. And_generic_file_aio_writewill fall back to buffered write if the directIO can't write. Am I wrong somehow?
yes ocfs2_direct_IO has some check, but it just check if we are appending(the i_size <= offset), if the offset < i_size and offset + count > i_size, it will do direct io anyway. seems we also can fix this by adding a check to ocfs2_direct_IO.
Br,
Li Dongyang
Quoted 77 lines
Regards,
TaoQuoted 73 lines · On Friday 09 April 2010 02:41:26 Sunil Mushran wrote:
Quoted 66 lines
I cannot read the bugzilla. Now it maybe that that bz cannot be made public. That's ok. But if that's the case, can you explain the problem encountered. I am not qs the fix... rather trying to understand why this has not been reported before.
Quoted 59 lines · Li Dongyang
when we fall back to buffered write from direct write, we call
__generic_file_aio_writebut that will end up doing direct write even we are only prepared to do buffered write because the file hasO_DIRECTflag set. This is a fix for
https://bugzilla.novell.com/show_bug.cgi?id=591039Signed-off-by Li Dongyang <[email protected]>--- fs/ocfs2/file.c | 27 +++++++++++++++++---------- 1 files changed, 17 insertions(+), 10 deletions(-)fs/ocfs2/file.c +17
-10@@ -1973,18 +1973,24 @@ relock:/* communicate with ocfs2_dio_end_io */ ocfs2_iocb_set_rw_locked (iocb, rw_level); -if (direct_io) { - ret =generic_segment_checks (iov, &nr_segs, &ocount, -VERIFY_READ ); -if (ret) - goto out_dio; + ret =generic_segment_checks (iov, &nr_segs, &ocount, +VERIFY_READ ); +if (ret) + goto out_dio; - count = ocount; - ret =generic_write_checks (file, ppos, &count, + count = ocount; + ret =generic_write_checks (file, ppos, &count,S_ISBLK (inode->i_mode)); -if (ret) - goto out_dio; +if (ret) + goto out_dio; + + ret =file_remove_suid (file); +if (ret) + goto out_dio; +file_update_time (file); + +if (direct_io) { written =generic_file_direct_write (iocb, iov, &nr_segs, *ppos, ppos, count, ocount);if (written <0 ) {@@ -1999,7 +2005,8 @@ relock: goto out_dio; } }else { - written =__generic_file_aio_write (iocb, iov, nr_segs, ppos); + written =generic_file_buffered_write (iocb, iov, nr_segs, + *ppos, ppos, count,0 ); }Quoted 1 line
fs/ocfs2/file.c relock: · line 2012
Quoted 1 line
fs/ocfs2/file.c +0
-0out_dio:_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: avoid direct write if we fall back to buffered
Hi Dongyang,
Quoted 19 lines · Li Dongyang
Hi, Tao,
Quoted 11 lines · On Friday 09 April 2010 10:38:33 Tao Ma wrote:
Hi Dongyang,
Quoted 8 lines · Li Dongyang
This is because
ocfs2_file_aio_writecallsocfs2_prepare_inode_for_writewhich setsdirect_ioto 0 if it finds out that direct IO would extend the file. But later we call__generic_file_aio_writewhich end's up callinggeneric_file_direct_writebecause the file hasO_DIRECTflag.So every time we do a direct write extending the file, theinode->i_sizegets inconsistent with thei_sizeon disk because we callgeneric_file_direct_write, and if we do a truncate after this, we will meet a bug inocfs2_truncate_file.Quoted 6 lines
yes we have
O_DIRECTflag set and in__generic_file_aio_writeit will callgeneric_file_direct_writefirst and then trigger toocfs2_direct_IO. In this function we will check again and return 0. And_generic_file_aio_writewill fall back to buffered write if the directIO can't write. Am I wrong somehow?
Quoted 4 lines
yes
ocfs2_direct_IOhas some check, but it just check if we areappending(the i_size <= offset), if the offset <i_sizeand offset + count >i_size, it will do direct io anyway. seems we also can fix this by adding a check toocfs2_direct_IO.
It is done by ocfs2_direct_IO_get_blocks. Just debug the kernel and you will get what I mean. ;)
Regards,
Tao
Quoted 77 lines
Br,
Li DongyangQuoted 75 lines
Regards,
TaoQuoted 72 lines · On Friday 09 April 2010 02:41:26 Sunil Mushran wrote:
Quoted 66 lines
I cannot read the bugzilla. Now it maybe that that bz cannot be made public. That's ok. But if that's the case, can you explain the problem encountered. I am not qs the fix... rather trying to understand why this has not been reported before.
Quoted 59 lines · Li Dongyang
when we fall back to buffered write from direct write, we call
__generic_file_aio_writebut that will end up doing direct write even we are only prepared to do buffered write because the file hasO_DIRECTflag set. This is a fix for
https://bugzilla.novell.com/show_bug.cgi?id=591039Signed-off-by Li Dongyang <[email protected]>--- fs/ocfs2/file.c | 27 +++++++++++++++++---------- 1 files changed, 17 insertions(+), 10 deletions(-)fs/ocfs2/file.c +17
-10@@ -1973,18 +1973,24 @@ relock:/* communicate with ocfs2_dio_end_io */ ocfs2_iocb_set_rw_locked (iocb, rw_level); -if (direct_io) { - ret =generic_segment_checks (iov, &nr_segs, &ocount, -VERIFY_READ ); -if (ret) - goto out_dio; + ret =generic_segment_checks (iov, &nr_segs, &ocount, +VERIFY_READ ); +if (ret) + goto out_dio; - count = ocount; - ret =generic_write_checks (file, ppos, &count, + count = ocount; + ret =generic_write_checks (file, ppos, &count,S_ISBLK (inode->i_mode)); -if (ret) - goto out_dio; +if (ret) + goto out_dio; + + ret =file_remove_suid (file); +if (ret) + goto out_dio; +file_update_time (file); + +if (direct_io) { written =generic_file_direct_write (iocb, iov, &nr_segs, *ppos, ppos, count, ocount);if (written <0 ) {@@ -1999,7 +2005,8 @@ relock: goto out_dio; } }else { - written =__generic_file_aio_write (iocb, iov, nr_segs, ppos); + written =generic_file_buffered_write (iocb, iov, nr_segs, + *ppos, ppos, count,0 ); }Quoted 1 line
fs/ocfs2/file.c relock: · line 2012
Quoted 1 line
fs/ocfs2/file.c +0
-0out_dio:_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: avoid direct write if we fall back to buffered
Quoted 25 lines · On Friday 09 April 2010 11:32:10 Tao Ma wrote:
Hi Dongyang,
Quoted 22 lines · Li Dongyang
Hi, Tao,
Quoted 18 lines · On Friday 09 April 2010 10:38:33 Tao Ma wrote:
Hi Dongyang,
Quoted 9 lines · Li Dongyang
This is because
ocfs2_file_aio_writecallsocfs2_prepare_inode_for_writewhich setsdirect_ioto 0 if it finds out that direct IO would extend the file. But later we call__generic_file_aio_writewhich end's up callinggeneric_file_direct_writebecause the file hasO_DIRECTflag.So every time we do a direct write extending the file, theinode->i_sizegets inconsistent with thei_sizeon disk because we callgeneric_file_direct_write, and if we do a truncate after this, we will meet a bug inocfs2_truncate_file.yes we have
O_DIRECTflag set and in__generic_file_aio_writeit will callgeneric_file_direct_writefirst and then trigger toocfs2_direct_IO. In this function we will check again and return 0. And_generic_file_aio_writewill fall back to buffered write if the directIO can't write. Am I wrong somehow?
Quoted 8 lines
Quoted 4 lines
yes
ocfs2_direct_IOhas some check, but it just check if we areappending(the i_size <= offset), if the offset <i_sizeand offset + count >i_size, it will do direct io anyway. seems we also can fix this by adding a check toocfs2_direct_IO.It is done by
ocfs2_direct_IO_get_blocks. Just debug the kernel and you will get what I mean. ;)
Do you mean this section in ocfs2_direct_IO_get_blocks:? /*
* Any write past EOF is not allowed because we'd be extending. */
if (create && (iblock + max_blocks) > inode_blocks) {
ret = -EIO;
goto bail;
}I was using the linus treegit://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
and we don't have that check, but I can find this in thegit://git.kernel.org/pub/scm/linux/kernel/git/jlbec/ocfs2.git, introduced by commit 564f8a3228879d6962edb3432d01bcd7499a67ec
and now with this check I got what you mean, you are right, but I wonder why the linus tree doesn't have this check? and are we suppose to do with this? IMHO we can just push this commit to linus tree.
Br,
Li Dongyang
Quoted 83 lines
Regards,
TaoQuoted 79 lines
Br,
Li DongyangQuoted 76 lines
Regards,
TaoQuoted 73 lines · On Friday 09 April 2010 02:41:26 Sunil Mushran wrote:
Quoted 66 lines
I cannot read the bugzilla. Now it maybe that that bz cannot be made public. That's ok. But if that's the case, can you explain the problem encountered. I am not qs the fix... rather trying to understand why this has not been reported before.
Quoted 59 lines · Li Dongyang
when we fall back to buffered write from direct write, we call
__generic_file_aio_writebut that will end up doing direct write even we are only prepared to do buffered write because the file hasO_DIRECTflag set. This is a fix for
https://bugzilla.novell.com/show_bug.cgi?id=591039Signed-off-by Li Dongyang <[email protected]>--- fs/ocfs2/file.c | 27 +++++++++++++++++---------- 1 files changed, 17 insertions(+), 10 deletions(-)fs/ocfs2/file.c +17
-10@@ -1973,18 +1973,24 @@ relock:/* communicate with ocfs2_dio_end_io */ ocfs2_iocb_set_rw_locked (iocb, rw_level); -if (direct_io) { - ret =generic_segment_checks (iov, &nr_segs, &ocount, -VERIFY_READ ); -if (ret) - goto out_dio; + ret =generic_segment_checks (iov, &nr_segs, &ocount, +VERIFY_READ ); +if (ret) + goto out_dio; - count = ocount; - ret =generic_write_checks (file, ppos, &count, + count = ocount; + ret =generic_write_checks (file, ppos, &count,S_ISBLK (inode->i_mode)); -if (ret) - goto out_dio; +if (ret) + goto out_dio; + + ret =file_remove_suid (file); +if (ret) + goto out_dio; +file_update_time (file); + +if (direct_io) { written =generic_file_direct_write (iocb, iov, &nr_segs, *ppos, ppos, count, ocount);if (written <0 ) {@@ -1999,7 +2005,8 @@ relock: goto out_dio; } }else { - written =__generic_file_aio_write (iocb, iov, nr_segs, ppos); + written =generic_file_buffered_write (iocb, iov, nr_segs, + *ppos, ppos, count,0 ); }Quoted 1 line
fs/ocfs2/file.c relock: · line 2012
Quoted 1 line
fs/ocfs2/file.c +0
-0out_dio:_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: avoid direct write if we fall back to buffered
Quoted 45 lines · Li Dongyang
Quoted 23 lines · On Friday 09 April 2010 11:32:10 Tao Ma wrote:
Hi Dongyang,
Quoted 20 lines · Li Dongyang
Hi, Tao,
Quoted 17 lines · On Friday 09 April 2010 10:38:33 Tao Ma wrote:
Hi Dongyang,
Quoted 9 lines · Li Dongyang
This is because
ocfs2_file_aio_writecallsocfs2_prepare_inode_for_writewhich setsdirect_ioto 0 if it finds out that direct IO would extend the file. But later we call__generic_file_aio_writewhich end's up callinggeneric_file_direct_writebecause the file hasO_DIRECTflag.So every time we do a direct write extending the file, theinode->i_sizegets inconsistent with thei_sizeon disk because we callgeneric_file_direct_write, and if we do a truncate after this, we will meet a bug inocfs2_truncate_file.yes we have
O_DIRECTflag set and in__generic_file_aio_writeit will callgeneric_file_direct_writefirst and then trigger toocfs2_direct_IO. In this function we will check again and return 0. And_generic_file_aio_writewill fall back to buffered write if the directIO can't write. Am I wrong somehow?Quoted 6 lines
Quoted 4 lines
yes
ocfs2_direct_IOhas some check, but it just check if we areappending(the i_size <= offset), if the offset <i_sizeand offset + count >i_size, it will do direct io anyway. seems we also can fix this by adding a check toocfs2_direct_IO.It is done by
ocfs2_direct_IO_get_blocks. Just debug the kernel and you will get what I mean. ;)Do you mean this section in
ocfs2_direct_IO_get_blocks:? /*
* Any write past EOF is not allowed because we'd be extending. */if (create && (iblock + max_blocks) > inode_blocks) { ret = -EIO; goto bail; }I was using the linus tree
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
and we don't have that check, but I can find this in thegit://git.kernel.org/pub/scm/linux/kernel/git/jlbec/ocfs2.git, introduced by commit564f8a3228879d6962edb3432d01bcd7499a67ec
Quoted 3 lines
and now with this check I got what you mean, you are right, but I wonder why the linus tree doesn't have this check? and are we suppose to do with this? IMHO we can just push this commit to linus tree.
commit 5fe878ae7f82fbf0830dbfaee4c5ca18f3aee442
Author: Christoph Hellwig <[email protected]> Date: Tue Dec 15 16:47:50 2009 -0800
direct-io: cleanup blockdev_direct_IO locking
This check was removed recently by the above patch.
PATCH 1/1 ocfs2: avoid direct write if we fall back to buffered
In reply to Sunil MushranOn 04/09/2010 02:41 AM, Sunil Mushran Wrote:
Quoted 6 lines
I cannot read the bugzilla. Now it maybe that that bz cannot be made public. That's ok. But if that's the case, can you explain the problem encountered. I am not qs the fix... rather trying to understand why this has not been reported before.
Hi Sunil,
This issue was reported by Jiaju Zhang, another Novell ocfs2/dlm developer. When he did I/O pressure test (fsstress from ltp package), the following dmesg was observed,
Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717421] (11411,2):ocfs2_truncate_file:465 ERROR: bug expression: le64_to_cpu(fe->i_size) != i_size_read(inode) Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717437] (11411,2):ocfs2_truncate_file:465 ERROR: Inode 241893, inode i_size = 1540096 != di i_size = 1535498, i_flags = 0x1 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717462] ------------[ cut here]------------ Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717465] kernel BUG at /usr/src/packages/BUILD/ocfs2-1.4/xen/ocfs2/file.c:465! Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717468] invalid opcode: 0000 [#2] SMP Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717471] last sysfs file: /sys/kernel/uevent_seqnum Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717474] Modules linked in: ocfs2 jbd2 ocfs2_nodemanager quota_tree ocfs2_stack_user ocfs2_stackglue dlm configfs sg sd_mod crc_t10dif crc32c ib_iser rdma_cm ib_cm iw_cm ib_sa ib_mad ib_core ib_addr ipv6 iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi scsi_mod af_packet microcode softdog fuse loop dm_mod rtc_core rtc_lib joydev xennet ext3 mbcache jbd processor thermal_sys hwmon xenblk cdrom Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717516] Supported: Yes Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717518] Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717521] Pid: 11411, comm: fsstress Tainted: G D (2.6.32.9-0.5-xen #1) Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717525] EIP: 0061:[<d24701ba>] EFLAGS: 00010296 CPU: 2 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717538] EIP is at ocfs2_setattr+0xc1a/0x1d10 [ocfs2] Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717542] EAX: 00000089 EBX: cd8e25f0 ECX: c056c0ec EDX: 00000000 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717545] ESI: cc4c2000 EDI: cae4e908 EBP: 00068f02 ESP: c0a43e54 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717548] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717552] Process fsstress (pid: 11411, ti=c0a42000 task=cd8e25f0 task.ti=c0a42000) Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717555] Stack: Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717557] d24cfc30 00002c93 00000002 d24c809c 000001d1 0003b0e5 00000000 00178000 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717564] <0> 00000000 00176e0a 00000000 00000001 00110f02 00000000 00000000 00000000 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717572] <0> 00000000 00000000 00000000 00110f02 d24628e9 00008282 c0a43f44 ca5c4000 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717582] Call Trace: Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717606] [<c00d6191>] notify_change+0x141/0x320 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717614] [<c00bf1a8>] do_truncate+0x68/0xa0 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717619] [<c00bf547>] do_sys_truncate+0x177/0x220 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717624] [<c000666d>] syscall_call+0x7/0xb Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717629] [<f57fe424>] 0xf57fe424 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717631] Code: 69 e8 ed f6 05 82 9f d7 d1 80 75 09 f6 05 84 9f d7 d1 01 74 16 f6 05 8a 9f d7 d1 80 75 0d f6 05 8c 9f d7 d1 01 0f 84 48 06 00 00 <0f> 0b eb fe 66 90 8b 44 24 68 31 c9 e8 b5 2f c9 ed 31 c9 89 44 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717675] EIP: [<d24701ba>] ocfs2_setattr+0xc1a/0x1d10 [ocfs2] SS:ESP 0069:c0a43e54 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717688] ---[ end trace cce1004f6a64f124 ]---
The above error can be reproduced by Jiaju, Dongyang, and me. Dongyang also reproduced this issue on vanilla kernel. We find these steps is easier to reproduce the error: 1) fill the ocfs2 volume to 97%-98% full (dd a big file on ocfs2 volume) 2) then ran fsstress
Jan Kara also helps to review Dongyang's patch, no objection from him.
Hope the explanation is informative.
Signature · 2 lines
Coly Li SuSE Labs
PATCH 1/1 ocfs2: avoid direct write if we fall back to buffered
Hi coly,
Quoted 11 lines · Coly Li
On 04/09/2010 02:41 AM, Sunil Mushran Wrote:
Quoted 6 lines
I cannot read the bugzilla. Now it maybe that that bz cannot be made public. That's ok. But if that's the case, can you explain the problem encountered. I am not qs the fix... rather trying to understand why this has not been reported before.
Hi Sunil,
Quoted 7 lines
This issue was reported by Jiaju Zhang, another Novell ocfs2/dlm developer. When he did I/O pressure test (fsstress from ltp package), the following dmesg was observed,
Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717421] (11411,2):ocfs2_truncate_file:465 ERROR: bug expression: le64_to_cpu(fe->i_size) != i_size_read(inode) Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717437] (11411,2):ocfs2_truncate_file:465 ERROR: Inode 241893, inode i_size =1540096 != di
i_size= 1535498,i_flags=0x1
Why do you guys think this is caused by the directIO fall back? IMHO, we should update inode->i_size and fe->i_size simultaneously. So do you find a place where we don't sync them? I guess that should be the root cause.
Regards,
Tao
Quoted 43 lines
Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717462] ------------[ cut here]------------ Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717465] kernel BUG at /usr/src/packages/BUILD/ocfs2-1.4/xen/ocfs2/file.c:465! Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717468] invalid opcode: 0000 [#2] SMP Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717471] last sysfs file: /sys/kernel/uevent_seqnum Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717474] Modules linked in: ocfs2 jbd2 ocfs2_nodemanager quota_tree ocfs2_stack_user ocfs2_stackglue dlm configfs sg sd_mod crc_t10dif crc32c ib_iser rdma_cm ib_cm iw_cm ib_sa ib_mad ib_core ib_addr ipv6 iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi scsi_mod af_packet microcode softdog fuse loop dm_mod rtc_core rtc_lib joydev xennet ext3 mbcache jbd processor thermal_sys hwmon xenblk cdrom Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717516] Supported: Yes Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717518] Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717521] Pid: 11411, comm: fsstress Tainted: G D (2.6.32.9-0.5-xen #1) Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717525] EIP: 0061:[<d24701ba>] EFLAGS: 00010296 CPU: 2 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717538] EIP is at ocfs2_setattr+0xc1a/0x1d10 [ocfs2] Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717542] EAX: 00000089 EBX: cd8e25f0 ECX: c056c0ec EDX: 00000000 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717545] ESI: cc4c2000 EDI: cae4e908 EBP: 00068f02 ESP: c0a43e54 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717548] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717552] Process fsstress (pid: 11411, ti=c0a42000 task=cd8e25f0 task.ti=c0a42000) Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717555] Stack: Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717557] d24cfc30 00002c93 00000002 d24c809c 000001d1 0003b0e5 00000000 00178000 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717564] <0> 00000000 00176e0a 00000000 00000001 00110f02 00000000 00000000 00000000 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717572] <0> 00000000 00000000 00000000 00110f02 d24628e9 00008282 c0a43f44 ca5c4000 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717582] Call Trace: Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717606] [<c00d6191>] notify_change+0x141/0x320 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717614] [<c00bf1a8>] do_truncate+0x68/0xa0 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717619] [<c00bf547>] do_sys_truncate+0x177/0x220 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717624] [<c000666d>] syscall_call+0x7/0xb Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717629] [<f57fe424>] 0xf57fe424 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717631] Code: 69 e8 ed f6 05 82 9f d7 d1 80 75 09 f6 05 84 9f d7 d1 01 74 16 f6 05 8a 9f d7 d1 80 75 0d f6 05 8c 9f d7 d1 01 0f 84 48 06 00 00 <0f> 0b eb fe 66 90 8b 44 24 68 31 c9 e8 b5 2f c9 ed 31 c9 89 44 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717675] EIP: [<d24701ba>] ocfs2_setattr+0xc1a/0x1d10 [ocfs2] SS:ESP 0069:c0a43e54 Mar 24 15:59:07 xen-sp1-2 kernel: [ 2309.717688] ---[ end trace cce1004f6a64f124 ]---The above error can be reproduced by Jiaju, Dongyang, and me. Dongyang also reproduced this issue on vanilla kernel. We find these steps is easier to reproduce the error: 1) fill the ocfs2 volume to 97%-98% full (dd a big file on ocfs2 volume) 2) then ran fsstress
Jan Kara also helps to review Dongyang's patch, no objection from him.
Hope the explanation is informative.
PATCH 1/1 ocfs2: avoid direct write if we fall back to buffered
In reply to Li DongyangQuoted 5 lines · Li Dongyang
when we fall back to buffered write from direct write, we call
__generic_file_aio_writebut that will end up doing direct write even we are only prepared to do buffered write because the file hasO_DIRECTflag set. This is a fix for
https://bugzilla.novell.com/show_bug.cgi?id=591039
We need to evaluate what __g_f_a_w() is doing and make sure
we're matching it appropriately for ocfs2.
fs/ocfs2/file.c relock: · line 1989
Quoted 3 lines
fs/ocfs2/file.c +3
-0+ ret =file_remove_suid (file); +if (ret) + goto out_dio;
NAK. We do suid checks in ocfs2_prepare_inode_for_write().
Calling file_remove_suid() is outside of ocfs2's locking. It calls ->setattr() which has its own rules in ocfs2.
fs/ocfs2/file.c relock: · line 1991
Quoted 1 line
fs/ocfs2/file.c +1
-0+file_update_time (file);
We have special behaviors regarding time updates for ocfs2
direct I/O. This might want to live right next to the call to generic_file_buffered_write(). But maybe not. It needs to be checked.
Joel
Signature · 8 lines
To spot the expert, pick the one who predicts the job will take the longest and cost the most. Joel Becker Principal Software Developer Oracle E-mail: joel.becker at oracle.com Phone: (650) 506-8127
PATCH 1/1 ocfs2: avoid direct write if we fall back to buffered
Quoted 10 lines · On Wednesday 14 April 2010 09:58:19 Joel Becker wrote:
Quoted 5 lines · Li Dongyang
when we fall back to buffered write from direct write, we call
__generic_file_aio_writebut that will end up doing direct write even we are only prepared to do buffered write because the file hasO_DIRECTflag set. This is a fix for
https://bugzilla.novell.com/show_bug.cgi?id=591039We need to evaluate what
__g_f_a_w()is doing and make sure
we're matching it appropriately for ocfs2.
Thanks for reviewing, that's just the two uncertain points to me with the patch.
Quoted 7 lines
fs/ocfs2/file.c relock: · line 1989
Quoted 3 lines
fs/ocfs2/file.c +3
-0+ ret =file_remove_suid (file); +if (ret) + goto out_dio;NAK. We do suid checks in
ocfs2_prepare_inode_for_write().
Callingfile_remove_suid()is outside of ocfs2's locking. It calls ->setattr() which has its own rules in ocfs2.
you are right, I did that because we are calling __g_f_a_w() in o_f_a_w() which will call file_remove_suid() before.
Quoted 6 lines
fs/ocfs2/file.c relock: · line 1991
Quoted 2 lines
fs/ocfs2/file.c +1
-0+file_update_time (file);We have special behaviors regarding time updates for ocfs2
direct I/O. This might want to live right next to the call togeneric_file_buffered_write(). But maybe not. It needs to be checked.
I don't think we need that, as we will update the time in ocfs2_write_end_nolock() called by ocfs2_write_end().
and could I have more details on how we handle time updates for ocfs2 direct io? Br,
Li Dongyang
Quoted 3 lines
Joel