[linux-cifs-client] cifs: Detect errors in cifs_writepages when called from pdflush (V2)
diff mbox

Message ID 20090420074221.GA13131@kay
State New, archived
Headers show

Commit Message

Peter Schwenke April 20, 2009, 7:42 a.m. UTC
The return code isn't checked in the case of cifs_writepages being
invoked as a result of pdflush. This results in data integrity
problems when errors such as a network break occur when
CIFSSMBWrite2 is called.  The resulting EAGAIN isn't acted
on.

Signed-off-by: Peter Schwenke <peter@bluetoad.com.au>
---
 fs/cifs/file.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

Comments

Jeff Layton April 20, 2009, 3:52 p.m. UTC | #1
On Mon, 20 Apr 2009 17:42:21 +1000
Peter Schwenke <peter@bluetoad.com.au> wrote:

> The return code isn't checked in the case of cifs_writepages being
> invoked as a result of pdflush. This results in data integrity
> problems when errors such as a network break occur when
> CIFSSMBWrite2 is called.  The resulting EAGAIN isn't acted
> on.
> 
> Signed-off-by: Peter Schwenke <peter@bluetoad.com.au>
> ---
>  fs/cifs/file.c |   15 ++++++++++++++-
>  1 files changed, 14 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 042b122..c00a674 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1200,6 +1200,7 @@ static int cifs_writepages(struct address_space *mapping,
>  	unsigned int bytes_to_write;
>  	unsigned int bytes_written;
>  	struct cifs_sb_info *cifs_sb;
> +	struct cifsInodeInfo *cifs_inode = CIFS_I(mapping->host);
>  	int done = 0;
>  	pgoff_t end;
>  	pgoff_t index;
> @@ -1354,7 +1355,7 @@ retry:
>  			 * CIFSSMBWrite2.  We can't rely on the last handle
>  			 * we used to still be valid
>  			 */
> -			open_file = find_writable_file(CIFS_I(mapping->host));
> +			open_file = find_writable_file(cifs_inode);
>  			if (!open_file) {
>  				cERROR(1, ("No writable handles for inode"));
>  				rc = -EBADF;
> @@ -1374,6 +1375,18 @@ retry:
>  						set_bit(AS_ENOSPC, &mapping->flags);
>  					else
>  						set_bit(AS_EIO, &mapping->flags);
> +					/*
> +					 * The return code isn't checked in the
> +					 * case of cifs_writepages being
> +					 * invoked as a result of pdflush.
> +					 * See generic_sync_sb_inodes()
> +					 */
> +					if (current_is_pdflush()) {
> +						if (rc == -ENOSPC)
> +							cifs_inode->write_behind_rc =  rc;
> +						else
> +							cifs_inode->write_behind_rc =  -EIO;
> +					}
>  				} else {
>  					cifs_stats_bytes_written(cifs_sb->tcon,
>  								 bytes_written);

Looking at this a little more closely...

I'm not sure that current_is_pdflush() the right check to use for this?
Is it possible that we could get similar problem within the context of
another thread? Should we instead check for whether writepages was
called with WB_SYNC_NONE instead?

Patch
diff mbox

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 042b122..c00a674 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1200,6 +1200,7 @@  static int cifs_writepages(struct address_space *mapping,
 	unsigned int bytes_to_write;
 	unsigned int bytes_written;
 	struct cifs_sb_info *cifs_sb;
+	struct cifsInodeInfo *cifs_inode = CIFS_I(mapping->host);
 	int done = 0;
 	pgoff_t end;
 	pgoff_t index;
@@ -1354,7 +1355,7 @@  retry:
 			 * CIFSSMBWrite2.  We can't rely on the last handle
 			 * we used to still be valid
 			 */
-			open_file = find_writable_file(CIFS_I(mapping->host));
+			open_file = find_writable_file(cifs_inode);
 			if (!open_file) {
 				cERROR(1, ("No writable handles for inode"));
 				rc = -EBADF;
@@ -1374,6 +1375,18 @@  retry:
 						set_bit(AS_ENOSPC, &mapping->flags);
 					else
 						set_bit(AS_EIO, &mapping->flags);
+					/*
+					 * The return code isn't checked in the
+					 * case of cifs_writepages being
+					 * invoked as a result of pdflush.
+					 * See generic_sync_sb_inodes()
+					 */
+					if (current_is_pdflush()) {
+						if (rc == -ENOSPC)
+							cifs_inode->write_behind_rc =  rc;
+						else
+							cifs_inode->write_behind_rc =  -EIO;
+					}
 				} else {
 					cifs_stats_bytes_written(cifs_sb->tcon,
 								 bytes_written);