| Message ID | 20090420074221.GA13131@kay (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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?
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);
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(-)