2011-11-16 00:46:42

by NeilBrown

[permalink] [raw]
Subject: [PATCH] NFS - fix recent breakage to NFS error handling.

From c6d615d2b97fe305cbf123a8751ced859dca1d5e Mon Sep 17 00:00:00 2001
From: NeilBrown <[email protected]>
Date: Wed, 16 Nov 2011 09:39:05 +1100
Subject: [PATCH] NFS - fix recent breakage to NFS error handling.

commit 02c24a82187d5a628c68edfe71ae60dc135cd178 made a small and
presumably unintended change to write error handling in NFS.

Previously an error from filemap_write_and_wait_range would only be of
interest if nfs_file_fsync did not return an error. After this commit,
an error from filemap_write_and_wait_range would mean that (the rest of)
nfs_file_fsync would not even be called.

This means that:
1/ you are more likely to see EIO than e.g. EDQUOT or ENOSPC.
2/ NFS_CONTEXT_ERROR_WRITE remains set for longer so more writes are
synchronous.

This patch restores previous behaviour.

Cc: [email protected]
Cc: Josef Bacik <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Al Viro <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
fs/nfs/file.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 0a1f831..3099d55 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -312,13 +312,13 @@ nfs_file_fsync(struct file *file, loff_t start, loff_t end, int datasync)
datasync);

ret = filemap_write_and_wait_range(inode->i_mapping, start, end);
- if (ret)
- return ret;
mutex_lock(&inode->i_mutex);

nfs_inc_stats(inode, NFSIOS_VFSFSYNC);
have_error = test_and_clear_bit(NFS_CONTEXT_ERROR_WRITE, &ctx->flags);
status = nfs_commit_inode(inode, FLUSH_SYNC);
+ if (status >= 0 && ret < 0)
+ status = ret;
have_error |= test_bit(NFS_CONTEXT_ERROR_WRITE, &ctx->flags);
if (have_error)
ret = xchg(&ctx->error, 0);
--
1.7.7.3


Attachments:
signature.asc (828.00 B)

2011-11-16 02:16:20

by Peng, Tao

[permalink] [raw]
Subject: RE: [PATCH] NFS - fix recent breakage to NFS error handling.

> -----Original Message-----
> From: [email protected] [mailto:[email protected]]
> On Behalf Of NeilBrown
> Sent: Wednesday, November 16, 2011 8:47 AM
> To: Trond Myklebust
> Cc: NFS; Josef Bacik; Jan Kara; Al Viro
> Subject: [PATCH] NFS - fix recent breakage to NFS error handling.
>
> From c6d615d2b97fe305cbf123a8751ced859dca1d5e Mon Sep 17 00:00:00 2001
> From: NeilBrown <[email protected]>
> Date: Wed, 16 Nov 2011 09:39:05 +1100
> Subject: [PATCH] NFS - fix recent breakage to NFS error handling.
>
> commit 02c24a82187d5a628c68edfe71ae60dc135cd178 made a small and
> presumably unintended change to write error handling in NFS.
>
> Previously an error from filemap_write_and_wait_range would only be of interest if
> nfs_file_fsync did not return an error. After this commit, an error from
> filemap_write_and_wait_range would mean that (the rest of) nfs_file_fsync would
> not even be called.
Do we need more consideration on commit 02c24a82187d5a628c68edfe71ae60dc135cd178?
Before it, vfs_fsync_range() calls f_op->fsync() regardless of filemap_write_and_wait_range() errors. After commit 02c24a82187d5a628c68edfe71ae60dc135cd178, it only calls f_op->fsync() when filemap_write_end_and_wait_range() succeeds. So the change is not really trivial to all file systems.

Thanks,
Tao