Skip to content

Commit

Permalink
cifs: flush data on any setattr
Browse files Browse the repository at this point in the history
We already flush all the dirty pages for an inode before doing
ATTR_SIZE and ATTR_MTIME changes. There's another problem though -- if
we change the mode so that the file becomes read-only then we may not
be able to write data to it after a reconnect.

Fix this by just going back to flushing all the dirty data on any
setattr call. There are probably some cases that can be optimized out,
but I'm not sure they're worthwhile and we need to consider them more
carefully to make sure that we don't cause regressions if we have
to reconnect before writeback occurs.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
  • Loading branch information
Jeff Layton authored and Steve French committed Apr 17, 2009
1 parent 20d9207 commit 0f4d634
Showing 1 changed file with 30 additions and 28 deletions.
58 changes: 30 additions & 28 deletions fs/cifs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -1792,20 +1792,21 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
goto out;
}

if ((attrs->ia_valid & ATTR_MTIME) || (attrs->ia_valid & ATTR_SIZE)) {
/*
Flush data before changing file size or changing the last
write time of the file on the server. If the
flush returns error, store it to report later and continue.
BB: This should be smarter. Why bother flushing pages that
will be truncated anyway? Also, should we error out here if
the flush returns error?
*/
rc = filemap_write_and_wait(inode->i_mapping);
if (rc != 0) {
cifsInode->write_behind_rc = rc;
rc = 0;
}
/*
* Attempt to flush data before changing attributes. We need to do
* this for ATTR_SIZE and ATTR_MTIME for sure, and if we change the
* ownership or mode then we may also need to do this. Here, we take
* the safe way out and just do the flush on all setattr requests. If
* the flush returns error, store it to report later and continue.
*
* BB: This should be smarter. Why bother flushing pages that
* will be truncated anyway? Also, should we error out here if
* the flush returns error?
*/
rc = filemap_write_and_wait(inode->i_mapping);
if (rc != 0) {
cifsInode->write_behind_rc = rc;
rc = 0;
}

if (attrs->ia_valid & ATTR_SIZE) {
Expand Down Expand Up @@ -1903,20 +1904,21 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
return -ENOMEM;
}

if ((attrs->ia_valid & ATTR_MTIME) || (attrs->ia_valid & ATTR_SIZE)) {
/*
Flush data before changing file size or changing the last
write time of the file on the server. If the
flush returns error, store it to report later and continue.
BB: This should be smarter. Why bother flushing pages that
will be truncated anyway? Also, should we error out here if
the flush returns error?
*/
rc = filemap_write_and_wait(inode->i_mapping);
if (rc != 0) {
cifsInode->write_behind_rc = rc;
rc = 0;
}
/*
* Attempt to flush data before changing attributes. We need to do
* this for ATTR_SIZE and ATTR_MTIME for sure, and if we change the
* ownership or mode then we may also need to do this. Here, we take
* the safe way out and just do the flush on all setattr requests. If
* the flush returns error, store it to report later and continue.
*
* BB: This should be smarter. Why bother flushing pages that
* will be truncated anyway? Also, should we error out here if
* the flush returns error?
*/
rc = filemap_write_and_wait(inode->i_mapping);
if (rc != 0) {
cifsInode->write_behind_rc = rc;
rc = 0;
}

if (attrs->ia_valid & ATTR_SIZE) {
Expand Down

0 comments on commit 0f4d634

Please sign in to comment.