[PATCH] Oversight in generic_file_aio_write_nolock

[PATCH] Oversight in generic_file_aio_write_nolock

[Oleg Drokin]

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]

Hello!

   There seems to be an oversight in generic_file_aio_write_nolock where
   when we open inode with O_SYNC and O_APPEND, then after we are done the
   writing, we flush wrong byterange from the file, from original offset for
   number of bytes written, instead from the actual oppended offset.
   This can lead to an unexpected data loss should the crash happen soon after
   such a write was performed.

   Here is a simple fix, though perhaps we can just throw away the pos argument
   from the ->aio_write prototype eventually.

Bye,
    Oleg

[-- Attachment #2: osync_oappend_bug.diff --]
[-- Type: text/plain, Size: 465 bytes --]

 filemap.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
--- mm/filemap.c.orig	2009-04-07 12:06:30.000000000 -0400
+++ mm/filemap.c	2009-04-07 12:06:49.000000000 -0400
@@ -2420,7 +2420,8 @@
 	if (ret > 0 && ((file->f_flags & O_SYNC) || IS_SYNC(inode))) {
 		ssize_t err;
 
-		err = sync_page_range_nolock(inode, mapping, pos, ret);
+		err = sync_page_range_nolock(inode, mapping, iocb->ki_pos - ret,
+					     ret);
 		if (err < 0)
 			ret = err;
 	}

Re: [PATCH] Oversight in generic_file_aio_write_nolock

[Jeff Garzik]

Oleg Drokin wrote:
> Hello!
> 
>    There seems to be an oversight in generic_file_aio_write_nolock where
>    when we open inode with O_SYNC and O_APPEND, then after we are done the
>    writing, we flush wrong byterange from the file, from original offset for
>    number of bytes written, instead from the actual oppended offset.
>    This can lead to an unexpected data loss should the crash happen soon after
>    such a write was performed.
> 
>    Here is a simple fix, though perhaps we can just throw away the pos argument
>    from the ->aio_write prototype eventually.

Could I ask that you CC patches like this to linux-kernel also?

	Jeff