[Ocfs2-devel] [PATCH] ocfs2: avoid direct write if we fall back to buffered v3

[Ocfs2-devel] [PATCH] ocfs2: avoid direct write if we fall back to buffered v3

[Li Dongyang]

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
revised with Joel's comments.

Signed-off-by: Li Dongyang <lidongyang@novell.com>
---
 fs/ocfs2/file.c |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index de059f4..0c15be9 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1973,18 +1973,18 @@ 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,
-					   S_ISBLK(inode->i_mode));
-		if (ret)
-			goto out_dio;
+	count = ocount;
+	ret = generic_write_checks(file, ppos, &count,
+				   S_ISBLK(inode->i_mode));
+	if (ret)
+		goto out_dio;
 
+	if (direct_io) {
 		written = generic_file_direct_write(iocb, iov, &nr_segs, *ppos,
 						    ppos, count, ocount);
 		if (written < 0) {
@@ -1999,7 +1999,10 @@ relock:
 			goto out_dio;
 		}
 	} else {
-		written = __generic_file_aio_write(iocb, iov, nr_segs, ppos);
+		current->backing_dev_info = file->f_mapping->backing_dev_info;
+		written = generic_file_buffered_write(iocb, iov, nr_segs, *ppos
+						      ppos, count, 0);
+		current->backing_dev_info = NULL;
 	}
 
 out_dio:
-- 
1.6.4.2

[Ocfs2-devel] [PATCH] ocfs2: avoid direct write if we fall back to buffered v3

[Li Dongyang]

On Saturday 17 April 2010 17:49:10 Li Dongyang wrote:
> 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
> revised with Joel's comments.
> 
> Signed-off-by: Li Dongyang <lidongyang@novell.com>
> ---
>  fs/ocfs2/file.c |   25 ++++++++++++++-----------
>  1 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index de059f4..0c15be9 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1973,18 +1973,18 @@ 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,
> -					   S_ISBLK(inode->i_mode));
> -		if (ret)
> -			goto out_dio;
> +	count = ocount;
> +	ret = generic_write_checks(file, ppos, &count,
> +				   S_ISBLK(inode->i_mode));
> +	if (ret)
> +		goto out_dio;
> 
> +	if (direct_io) {
>  		written = generic_file_direct_write(iocb, iov, &nr_segs, *ppos,
>  						    ppos, count, ocount);
>  		if (written < 0) {
> @@ -1999,7 +1999,10 @@ relock:
>  			goto out_dio;
>  		}
>  	} else {
> -		written = __generic_file_aio_write(iocb, iov, nr_segs, ppos);
> +		current->backing_dev_info = file->f_mapping->backing_dev_info;
> +		written = generic_file_buffered_write(iocb, iov, nr_segs, *ppos
> +						      ppos, count, 0);
> +		current->backing_dev_info = NULL;
>  	}
> 
>  out_dio:
> 
Joel, any comments on this update?
Br,
Li Dongyang

[Ocfs2-devel] [PATCH] ocfs2: avoid direct write if we fall back to buffered v3

[Joel Becker]

On Sat, Apr 17, 2010 at 05:49:10PM +0800, Li Dongyang wrote:
> 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
> revised with Joel's comments.
> 
> Signed-off-by: Li Dongyang <lidongyang@novell.com>

	This looks good to me.  I'd like Mark's ack before I push it to
merge-window.

Joel

-- 

"Sometimes one pays most for the things one gets for nothing."
        - Albert Einstein

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH] ocfs2: avoid direct write if we fall back to buffered v3

[Mark Fasheh]

On Mon, Apr 26, 2010 at 03:24:23PM -0700, Joel Becker wrote:
> On Sat, Apr 17, 2010 at 05:49:10PM +0800, Li Dongyang wrote:
> > 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
> > revised with Joel's comments.
> > 
> > Signed-off-by: Li Dongyang <lidongyang@novell.com>
> 
> 	This looks good to me.  I'd like Mark's ack before I push it to
> merge-window.

Looks good to me. Nice catch on the backing_dev_info bits.

Acked-by: Mark Fasheh <mfasheh@suse.com>
	--Mark

--
Mark Fasheh

[Ocfs2-devel] [PATCH] ocfs2: avoid direct write if we fall back to buffered v3

[Joel Becker]

On Sat, Apr 17, 2010 at 05:49:10PM +0800, Li Dongyang wrote:
> 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
> revised with Joel's comments.

	Have you tested this version?  I got a compile error:

> @@ -1999,7 +1999,10 @@ relock:
>  			goto out_dio;
>  		}
>  	} else {
> -		written = __generic_file_aio_write(iocb, iov, nr_segs, ppos);
> +		current->backing_dev_info = file->f_mapping->backing_dev_info;
> +		written = generic_file_buffered_write(iocb, iov, nr_segs, *ppos
> +						      ppos, count, 0);
> +		current->backing_dev_info = NULL;
>  	}

- 		written = generic_file_buffered_write(iocb, iov, nr_segs, *ppos
+ 		written = generic_file_buffered_write(iocb, iov, nr_segs, *ppos,

Joel

-- 

"I think it would be a good idea."  
        - Mahatma Ghandi, when asked what he thought of Western
          civilization

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH] ocfs2: avoid direct write if we fall back to buffered v3

[Joel Becker]

On Fri, Apr 30, 2010 at 01:47:05PM -0700, Joel Becker wrote:
> 	Have you tested this version?  I got a compile error:

	I want to be clear - has this patch (v3) been run against the
fsstress that caused the bug report in the first place?  Does the
fsstress now pass?  Let me know so I can send it upstream.

Joel

-- 

Life's Little Instruction Book #314

	"Never underestimate the power of forgiveness."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH] ocfs2: avoid direct write if we fall back to buffered v3

[Li Dongyang]

On Saturday 01 May 2010 04:52:43 Joel Becker wrote:
> On Fri, Apr 30, 2010 at 01:47:05PM -0700, Joel Becker wrote:
> > 	Have you tested this version?  I got a compile error:
> 
> 	I want to be clear - has this patch (v3) been run against the
> fsstress that caused the bug report in the first place?  Does the
> fsstress now pass?  Let me know so I can send it upstream.
> 
> Joel
> 
it's a typo I made on my desktop, I first test it on my 2-node boxes, and it 
had already running fsstress for at least 70hrs(50 hrs with the 
OCFS2_INODE_SKIP_ORPHAN_DIR patches),
then I checked out the ocfs2/master branch on my desktop in case of the 
differences between ocfs2/master and linus tree(like the block check in 
ocfs2_direct_IO_get_blocks). and made changes, then generated the patch, and a 
typo is made here, what a silly mistake!
sorry for the inconvenience here.
Br,
Li Dongyang

[Ocfs2-devel] [PATCH] ocfs2: avoid direct write if we fall back to buffered v3

[Joel Becker]

On Tue, May 04, 2010 at 09:37:48AM +0800, Li Dongyang wrote:
> On Saturday 01 May 2010 04:52:43 Joel Becker wrote:
> > On Fri, Apr 30, 2010 at 01:47:05PM -0700, Joel Becker wrote:
> > > 	Have you tested this version?  I got a compile error:
> > 
> > 	I want to be clear - has this patch (v3) been run against the
> > fsstress that caused the bug report in the first place?  Does the
> > fsstress now pass?  Let me know so I can send it upstream.
> > 
> > Joel
> > 
> it's a typo I made on my desktop, I first test it on my 2-node boxes, and it 
> had already running fsstress for at least 70hrs(50 hrs with the 
> OCFS2_INODE_SKIP_ORPHAN_DIR patches),
> then I checked out the ocfs2/master branch on my desktop in case of the 
> differences between ocfs2/master and linus tree(like the block check in 
> ocfs2_direct_IO_get_blocks). and made changes, then generated the patch, and a 
> typo is made here, what a silly mistake!

	And you are sure that the changes made by-hand on your desktop
are the same as made on your 2-node machines?

Joel

-- 

"I don't even butter my bread; I consider that cooking."
         - Katherine Cebrian

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH] ocfs2: avoid direct write if we fall back to buffered v3

[Li Dongyang]

On Tuesday 04 May 2010 09:47:23 Joel Becker wrote:
> On Tue, May 04, 2010 at 09:37:48AM +0800, Li Dongyang wrote:
> > On Saturday 01 May 2010 04:52:43 Joel Becker wrote:
> > > On Fri, Apr 30, 2010 at 01:47:05PM -0700, Joel Becker wrote:
> > > > 	Have you tested this version?  I got a compile error:
> > >
> > > 	I want to be clear - has this patch (v3) been run against the
> > > fsstress that caused the bug report in the first place?  Does the
> > > fsstress now pass?  Let me know so I can send it upstream.
> > >
> > > Joel
> >
> > it's a typo I made on my desktop, I first test it on my 2-node boxes, and
> > it had already running fsstress for at least 70hrs(50 hrs with the
> > OCFS2_INODE_SKIP_ORPHAN_DIR patches),
> > then I checked out the ocfs2/master branch on my desktop in case of the
> > differences between ocfs2/master and linus tree(like the block check in
> > ocfs2_direct_IO_get_blocks). and made changes, then generated the patch,
> > and a typo is made here, what a silly mistake!
> 
> 	And you are sure that the changes made by-hand on your desktop
> are the same as made on your 2-node machines?
> 
> Joel
> 
I just got a diff on my 2-node boxes, and checked it char to char, the only 
difference is the v3 patch lack the comma you pointed.

Br,
Li Dongyang