From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sunil Mushran Date: Fri, 09 Apr 2010 10:36:23 -0700 Subject: [Ocfs2-devel] [PATCH] ocfs2: avoid direct write if we fall back to buffered In-Reply-To: <201004091720.45689.lidongyang@novell.com> References: <201004081547.24593.lidongyang@novell.com> <201004091100.28988.lidongyang@novell.com> <4BBE9FBA.9070004@oracle.com> <201004091720.45689.lidongyang@novell.com> Message-ID: <4BBF6597.6080709@oracle.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ocfs2-devel@oss.oracle.com Li Dongyang wrote: > On Friday 09 April 2010 11:32:10 Tao Ma wrote: >> Hi Dongyang, >> >> Li Dongyang wrote: >>> Hi, Tao, >>> >>> On Friday 09 April 2010 10:38:33 Tao Ma wrote: >>>> Hi Dongyang, >>>> >>>> Li Dongyang wrote: >>>>> 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. >>>> 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? >>> 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. >> 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 the > git://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. commit 5fe878ae7f82fbf0830dbfaee4c5ca18f3aee442 Author: Christoph Hellwig 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.