[PATCH] Fix the error handling in direct I/O

[PATCH] Fix the error handling in direct I/O

[Hifumi Hisashi]

	Hello.

	I fixed a bug on error handling in the direct I/O function.
Currenlty, if a file is opened with the O_DIRECT|O_SYNC flag,
  write() syscall cannot receive the EIO error just after
I/O error(SCSI cable is disconnected etc.) occur.

	Return values of other points that call generic_osync_inode()
are treated appropriately.

	With the following patch, this problem was fixed.
	Please apply this patch.

	Thanks,


Signed-off-by: Hisashi Hifumi  <hifumi.hisashi@lab.ntt.co.jp>

diff -Nru linux-2.6.12/mm/filemap.c linux-2.6.12_fix/mm/filemap.c
--- linux-2.6.12/mm/filemap.c	2005-06-22 17:21:21.000000000 +0900
+++ linux-2.6.12_fix/mm/filemap.c	2005-06-22 20:26:34.000000000 +0900
@@ -1927,8 +1927,12 @@
  	 * i_sem is held, which protects generic_osync_inode() from
  	 * livelocking.
  	 */
-	if (written >= 0 && file->f_flags & O_SYNC)
-		generic_osync_inode(inode, mapping, OSYNC_METADATA);
+	if (written >= 0 && ((file->f_flags & O_SYNC) || IS_SYNC(inode))) {
+		int err;
+		err = generic_osync_inode(inode, mapping, OSYNC_METADATA);
+		if (err < 0)
+			written = err;
+	}
  	if (written == count && !is_sync_kiocb(iocb))
  		written = -EIOCBQUEUED;
  	return written; 

Re: [PATCH] Fix the error handling in direct I/O

[Andrew Morton]

Hifumi Hisashi <hifumi.hisashi@lab.ntt.co.jp> wrote:
>
> 	Hello.
> 
> 	I fixed a bug on error handling in the direct I/O function.
> Currenlty, if a file is opened with the O_DIRECT|O_SYNC flag,
>   write() syscall cannot receive the EIO error just after
> I/O error(SCSI cable is disconnected etc.) occur.
> 
> 	Return values of other points that call generic_osync_inode()
> are treated appropriately.
> 
> 	With the following patch, this problem was fixed.
> 	Please apply this patch.
> 
> 	Thanks,
> 
> 
> Signed-off-by: Hisashi Hifumi  <hifumi.hisashi@lab.ntt.co.jp>
> 
> diff -Nru linux-2.6.12/mm/filemap.c linux-2.6.12_fix/mm/filemap.c
> --- linux-2.6.12/mm/filemap.c	2005-06-22 17:21:21.000000000 +0900
> +++ linux-2.6.12_fix/mm/filemap.c	2005-06-22 20:26:34.000000000 +0900
> @@ -1927,8 +1927,12 @@
>   	 * i_sem is held, which protects generic_osync_inode() from
>   	 * livelocking.
>   	 */
> -	if (written >= 0 && file->f_flags & O_SYNC)
> -		generic_osync_inode(inode, mapping, OSYNC_METADATA);
> +	if (written >= 0 && ((file->f_flags & O_SYNC) || IS_SYNC(inode))) {
> +		int err;
> +		err = generic_osync_inode(inode, mapping, OSYNC_METADATA);
> +		if (err < 0)
> +			written = err;
> +	}
>   	if (written == count && !is_sync_kiocb(iocb))
>   		written = -EIOCBQUEUED;
>   	return written; 

Yes, I suppose so.

I note that generic_file_aio_write_nolock() for O_SYNC or IS_SYNC will end
up calling generic_osync_inode() twice.  Once in sync_page_range_nolock()
and once in __generic_file_aio_write_nolock->generic_file_direct_write or
in __generic_file_aio_write_nolock->generic_file_buffered_write

It's all a bit of a mess.  I guess we should sit down and work out where we
actually want to do all this syncing and get it done consistently and
completely, all at the same level.  A filemap.c call graph would be needed :(